OAK-9370 : Deprecate LoginModuleStats

git-svn-id: https://svn.apache.org/repos/asf/jackrabbit/oak/trunk@1887139 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/oak-core/pom.xml b/oak-core/pom.xml
index 6943768..bec8c9c 100644
--- a/oak-core/pom.xml
+++ b/oak-core/pom.xml
@@ -156,6 +156,7 @@
                                 <element>PACKAGE</element>
                                 <includes>
                                     <include>org.apache.jackrabbit.oak.security.authentication</include>
+                                    <include>org.apache.jackrabbit.oak.security.authentication.monitor</include>
                                     <include>org.apache.jackrabbit.oak.security.authentication.user</include>
                                     <include>org.apache.jackrabbit.oak.security.authorization</include>
                                     <include>org.apache.jackrabbit.oak.security.internal</include>
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/AuthenticationConfigurationImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/AuthenticationConfigurationImpl.java
index f95466f..416cc65 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/AuthenticationConfigurationImpl.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/AuthenticationConfigurationImpl.java
@@ -17,6 +17,7 @@
 package org.apache.jackrabbit.oak.security.authentication;
 
 import org.apache.jackrabbit.oak.api.ContentRepository;
+import org.apache.jackrabbit.oak.security.authentication.monitor.LoginModuleMonitorImpl;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationBase;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
 import org.apache.jackrabbit.oak.spi.security.SecurityConfiguration;
@@ -24,7 +25,6 @@
 import org.apache.jackrabbit.oak.spi.security.authentication.AuthenticationConfiguration;
 import org.apache.jackrabbit.oak.spi.security.authentication.LoginContextProvider;
 import org.apache.jackrabbit.oak.spi.security.authentication.LoginModuleMonitor;
-import org.apache.jackrabbit.oak.spi.security.authentication.LoginModuleStats;
 import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard;
 import org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardAware;
 import org.apache.jackrabbit.oak.stats.Monitor;
@@ -97,9 +97,9 @@
 
     /**
      * Constructor for non-OSGi
-     * @param securityProvider
+     * @param securityProvider The {@code SecurityProvider} this configuration belongs to.
      */
-    public AuthenticationConfigurationImpl(SecurityProvider securityProvider) {
+    public AuthenticationConfigurationImpl(@NotNull SecurityProvider securityProvider) {
         super(securityProvider, securityProvider.getParameters(NAME));
     }
 
@@ -113,7 +113,7 @@
     @NotNull
     @Override
     public Iterable<Monitor<?>> getMonitors(@NotNull StatisticsProvider statisticsProvider) {
-        lmMonitor = new LoginModuleStats(statisticsProvider);
+        lmMonitor = new LoginModuleMonitorImpl(statisticsProvider);
         return Collections.singleton(lmMonitor);
     }
 
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/monitor/LoginModuleMonitorImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/monitor/LoginModuleMonitorImpl.java
new file mode 100644
index 0000000..17a8971
--- /dev/null
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/monitor/LoginModuleMonitorImpl.java
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.security.authentication.monitor;
+
+import org.apache.jackrabbit.api.stats.TimeSeries;
+import org.apache.jackrabbit.oak.spi.security.authentication.LoginModuleMBean;
+import org.apache.jackrabbit.oak.spi.security.authentication.LoginModuleMonitor;
+import org.apache.jackrabbit.oak.stats.MeterStats;
+import org.apache.jackrabbit.oak.stats.StatisticsProvider;
+import org.apache.jackrabbit.oak.stats.StatsOptions;
+import org.apache.jackrabbit.stats.TimeSeriesStatsUtil;
+import org.jetbrains.annotations.NotNull;
+
+import javax.management.openmbean.CompositeData;
+
+public class LoginModuleMonitorImpl implements LoginModuleMBean, LoginModuleMonitor {
+
+    private final StatisticsProvider statisticsProvider;
+
+    static final String LOGIN_ERRORS = "LOGIN_ERRORS";
+
+    private final MeterStats loginErrors;
+
+    public LoginModuleMonitorImpl(StatisticsProvider statisticsProvider) {
+        this.statisticsProvider = statisticsProvider;
+        loginErrors = statisticsProvider.getMeter(LOGIN_ERRORS, StatsOptions.DEFAULT);
+    }
+
+    //------------------------------------------------------- < LoginModuleMonitor >---
+
+    @Override
+    public void loginError() {
+        loginErrors.mark();
+    }
+
+    //----------------------------------------------------------< LoginModuleMBean >---
+
+    @Override
+    public long getLoginErrors() {
+        return loginErrors.getCount();
+    }
+
+    @Override
+    public CompositeData getLoginErrorsHistory() {
+        return getTimeSeriesData(LOGIN_ERRORS, "Number of login errors.");
+    }
+
+    //-----------------------------------------------------------------< internal >---
+
+    @NotNull
+    private CompositeData getTimeSeriesData(@NotNull String name, @NotNull String desc) {
+        return TimeSeriesStatsUtil.asCompositeData(getTimeSeries(name), desc);
+    }
+
+    @NotNull
+    private TimeSeries getTimeSeries(@NotNull String name) {
+        return statisticsProvider.getStats().getTimeSeries(name, true);
+    }
+}
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authentication/AuthenticationConfigurationImplTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authentication/AuthenticationConfigurationImplTest.java
index 7ff98cb..92afe7a 100644
--- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authentication/AuthenticationConfigurationImplTest.java
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authentication/AuthenticationConfigurationImplTest.java
@@ -18,12 +18,12 @@
 
 import com.google.common.collect.Iterables;
 import org.apache.jackrabbit.oak.api.ContentRepository;
+import org.apache.jackrabbit.oak.security.authentication.monitor.LoginModuleMonitorImpl;
 import org.apache.jackrabbit.oak.security.internal.SecurityProviderBuilder;
 import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
 import org.apache.jackrabbit.oak.spi.security.authentication.AuthenticationConfiguration;
 import org.apache.jackrabbit.oak.spi.security.authentication.LoginContextProvider;
 import org.apache.jackrabbit.oak.spi.security.authentication.LoginModuleMonitor;
-import org.apache.jackrabbit.oak.spi.security.authentication.LoginModuleStats;
 import org.apache.jackrabbit.oak.spi.whiteboard.DefaultWhiteboard;
 import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard;
 import org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardAware;
@@ -103,7 +103,7 @@
         assertEquals(1, Iterables.size(monitors));
 
         Monitor<?> m = monitors.iterator().next();
-        assertTrue(m instanceof LoginModuleStats);
-        assertTrue(f.get(authConfiguration) instanceof LoginModuleStats);
+        assertTrue(m instanceof LoginModuleMonitorImpl);
+        assertTrue(f.get(authConfiguration) instanceof LoginModuleMonitorImpl);
     }
 }
\ No newline at end of file
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authentication/monitor/LoginModuleMonitorImplTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authentication/monitor/LoginModuleMonitorImplTest.java
new file mode 100644
index 0000000..a7a8a68
--- /dev/null
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authentication/monitor/LoginModuleMonitorImplTest.java
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.security.authentication.monitor;
+
+import org.apache.jackrabbit.oak.stats.DefaultStatisticsProvider;
+import org.apache.jackrabbit.oak.stats.MeterStats;
+import org.apache.jackrabbit.oak.stats.StatisticsProvider;
+import org.apache.jackrabbit.oak.stats.StatsOptions;
+import org.apache.jackrabbit.oak.stats.TimerStats;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.clearInvocations;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class LoginModuleMonitorImplTest {
+
+    private final MeterStats meter = mock(MeterStats.class);
+    private final TimerStats timer = mock(TimerStats.class);
+    private StatisticsProvider sp;
+
+    @Before
+    public void before() {
+        sp = mock(StatisticsProvider.class);
+        when(sp.getMeter(anyString(), any(StatsOptions.class))).thenReturn(meter);
+        when(sp.getTimer(anyString(), any(StatsOptions.class))).thenReturn(timer);
+    }
+
+    @After
+    public void after() {
+        clearInvocations(meter, timer, sp);
+    }
+
+    @Test
+    public void testLoginError() {
+        ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
+        StatisticsProvider sp = new DefaultStatisticsProvider(executor);
+
+        LoginModuleMonitorImpl s = new LoginModuleMonitorImpl(sp);
+        s.loginError();
+        assertEquals(1, s.getLoginErrors());
+        assertNotNull(s.getLoginErrorsHistory());
+    }
+
+    @Test
+    public void testConstructor() {
+        LoginModuleMonitorImpl s = new LoginModuleMonitorImpl(sp);
+        verify(sp, times(1)).getMeter(anyString(), any(StatsOptions.class));
+        verify(sp, times(0)).getTimer(anyString(), any(StatsOptions.class));
+    }
+}
\ No newline at end of file
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistrationTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistrationTest.java
index 89e0cde..7a78134 100644
--- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistrationTest.java
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/internal/SecurityProviderRegistrationTest.java
@@ -32,6 +32,7 @@
 import org.apache.jackrabbit.oak.plugins.tree.TreeLocation;
 import org.apache.jackrabbit.oak.plugins.tree.TreeProvider;
 import org.apache.jackrabbit.oak.security.authentication.AuthenticationConfigurationImpl;
+import org.apache.jackrabbit.oak.security.authentication.monitor.LoginModuleMonitorImpl;
 import org.apache.jackrabbit.oak.security.authorization.AuthorizationConfigurationImpl;
 import org.apache.jackrabbit.oak.security.authorization.composite.CompositeAuthorizationConfiguration;
 import org.apache.jackrabbit.oak.security.authorization.restriction.RestrictionProviderImpl;
@@ -330,7 +331,7 @@
         
         // register AuthenticationConfiguration to trigger MBean registration 
         AuthenticationConfigurationImpl mockAc = mock(AuthenticationConfigurationImpl.class);
-        when(mockAc.getMonitors(any(StatisticsProvider.class))).thenReturn(Collections.singleton(new LoginModuleStats(StatisticsProvider.NOOP)));
+        when(mockAc.getMonitors(any(StatisticsProvider.class))).thenReturn(Collections.singleton(new LoginModuleMonitorImpl(StatisticsProvider.NOOP)));
         registration.bindAuthenticationConfiguration(mockAc);
 
         // register required service
diff --git a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/AbstractLoginModule.java b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/AbstractLoginModule.java
index acb0036..9961d4b 100644
--- a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/AbstractLoginModule.java
+++ b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/AbstractLoginModule.java
@@ -592,6 +592,7 @@
         subject.getPublicCredentials().add(authInfo);
     }
 
+    @NotNull
     protected LoginModuleMonitor getLoginModuleMonitor() {
         if (loginModuleMonitor == null && callbackHandler != null) {
             RepositoryCallback rcb = new RepositoryCallback();
@@ -602,13 +603,13 @@
                 log.error(e.getMessage(), e);
             }
         }
+        if (loginModuleMonitor == null) {
+            loginModuleMonitor = LoginModuleMonitor.NOOP;
+        }
         return loginModuleMonitor;
     }
 
     protected void onError() {
-        LoginModuleMonitor lmm = getLoginModuleMonitor();
-        if (lmm != null) {
-            lmm.loginError();
-        }
+        getLoginModuleMonitor().loginError();
     }
 }
diff --git a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/LoginModuleStats.java b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/LoginModuleStats.java
index cee6003..713144d 100644
--- a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/LoginModuleStats.java
+++ b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/LoginModuleStats.java
@@ -16,14 +16,18 @@
  */
 package org.apache.jackrabbit.oak.spi.security.authentication;
 
-import javax.management.openmbean.CompositeData;
-
 import org.apache.jackrabbit.api.stats.TimeSeries;
 import org.apache.jackrabbit.oak.stats.MeterStats;
 import org.apache.jackrabbit.oak.stats.StatisticsProvider;
 import org.apache.jackrabbit.oak.stats.StatsOptions;
 import org.apache.jackrabbit.stats.TimeSeriesStatsUtil;
 
+import javax.management.openmbean.CompositeData;
+
+/**
+ * @deprecated Since Oak 1.40.0. A full implementation of {@code LoginModuleMonitor} and {@link LoginModuleMBean} has been added to oak-core.
+ */
+@Deprecated
 public class LoginModuleStats implements LoginModuleMBean, LoginModuleMonitor {
 
     private final StatisticsProvider statisticsProvider;
diff --git a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/package-info.java b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/package-info.java
index ef32682..04332cb 100644
--- a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/package-info.java
+++ b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/package-info.java
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-@Version("1.5.0")
+@Version("1.5.1")
 package org.apache.jackrabbit.oak.spi.security.authentication;
 
 import org.osgi.annotation.versioning.Version;
diff --git a/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeConfigurationTest.java b/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeConfigurationTest.java
index 6e3d304..3308e6c 100644
--- a/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeConfigurationTest.java
+++ b/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/CompositeConfigurationTest.java
@@ -30,7 +30,6 @@
 import org.apache.jackrabbit.oak.spi.lifecycle.RepositoryInitializer;
 import org.apache.jackrabbit.oak.spi.lifecycle.WorkspaceInitializer;
 import org.apache.jackrabbit.oak.spi.security.authentication.LoginModuleMonitor;
-import org.apache.jackrabbit.oak.spi.security.authentication.LoginModuleStats;
 import org.apache.jackrabbit.oak.spi.xml.ProtectedItemImporter;
 import org.apache.jackrabbit.oak.stats.Monitor;
 import org.apache.jackrabbit.oak.stats.StatisticsProvider;
@@ -38,7 +37,6 @@
 import org.jetbrains.annotations.Nullable;
 import org.junit.Before;
 import org.junit.Test;
-import org.mockito.Mockito;
 import org.osgi.framework.Constants;
 
 import java.security.Principal;
@@ -51,6 +49,7 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
 
 public class CompositeConfigurationTest extends AbstractCompositeConfigurationTest {
 
@@ -204,7 +203,7 @@
     public void testSetSecurityProvider() {
         CompositeConfiguration cc = new CompositeConfiguration("name") {};
 
-        SecurityProvider securityProvider = Mockito.mock(SecurityProvider.class);
+        SecurityProvider securityProvider = mock(SecurityProvider.class);
         cc.setSecurityProvider(securityProvider);
 
         assertSame(securityProvider, cc.getSecurityProvider());
@@ -220,7 +219,7 @@
     public void testSetRootProvider() {
         CompositeConfiguration cc = new CompositeConfiguration("name") {};
 
-        RootProvider rootProvider = Mockito.mock(RootProvider.class);
+        RootProvider rootProvider = mock(RootProvider.class);
         cc.setRootProvider(rootProvider);
 
         assertSame(rootProvider, cc.getRootProvider());
@@ -236,7 +235,7 @@
     public void testSetTreeProvider() {
         CompositeConfiguration cc = new CompositeConfiguration("name") {};
 
-        TreeProvider treeProvider = Mockito.mock(TreeProvider.class);
+        TreeProvider treeProvider = mock(TreeProvider.class);
         cc.setTreeProvider(treeProvider);
 
         assertSame(treeProvider, cc.getTreeProvider());
@@ -253,7 +252,7 @@
             @NotNull
             @Override
             public List<ProtectedItemImporter> getProtectedItemImporters() {
-                return ImmutableList.of(Mockito.mock(ProtectedItemImporter.class));
+                return ImmutableList.of(mock(ProtectedItemImporter.class));
             }
         };
         addConfiguration(withImporter);
@@ -272,7 +271,7 @@
             @NotNull
             @Override
             public List<ThreeWayConflictHandler> getConflictHandlers() {
-                return ImmutableList.of(Mockito.mock(ThreeWayConflictHandler.class));
+                return ImmutableList.of(mock(ThreeWayConflictHandler.class));
             }
         };
         addConfiguration(withConflictHandler);
@@ -291,7 +290,7 @@
             @NotNull
             @Override
             public List<? extends CommitHook> getCommitHooks(@NotNull String workspaceName) {
-                return ImmutableList.of(Mockito.mock(CommitHook.class));
+                return ImmutableList.of(mock(CommitHook.class));
             }
         };
         addConfiguration(withCommitHook);
@@ -310,7 +309,7 @@
             @NotNull
             @Override
             public List<? extends ValidatorProvider> getValidators(@NotNull String workspaceName, @NotNull Set<Principal> principals, @NotNull MoveTracker moveTracker) {
-                return ImmutableList.of(Mockito.mock(ValidatorProvider.class));
+                return ImmutableList.of(mock(ValidatorProvider.class));
             }
         };
         addConfiguration(withValidator);
@@ -329,7 +328,7 @@
             @NotNull
             @Override
             public WorkspaceInitializer getWorkspaceInitializer() {
-                return Mockito.mock(WorkspaceInitializer.class);
+                return mock(WorkspaceInitializer.class);
             }
         };
         addConfiguration(withWorkspaceInitializer);
@@ -348,7 +347,7 @@
             @NotNull
             @Override
             public RepositoryInitializer getRepositoryInitializer() {
-                return Mockito.mock(RepositoryInitializer.class);
+                return mock(RepositoryInitializer.class);
             }
         };
         addConfiguration(withRepositoryInitializer);
@@ -399,7 +398,7 @@
         addConfiguration(new SecurityConfiguration.Default());
         assertTrue(Iterables.isEmpty(compositeConfiguration.getMonitors(statisticsProvider)));
 
-        Monitor<LoginModuleMonitor> monitor = new LoginModuleStats(statisticsProvider);
+        Monitor<LoginModuleMonitor> monitor = mock(LoginModuleMonitor.class);
         SecurityConfiguration withMonitors = new SecurityConfiguration.Default() {
             @NotNull
             @Override
diff --git a/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/AbstractLoginModuleTest.java b/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/AbstractLoginModuleTest.java
index d4c5a99..fd14c60 100644
--- a/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/AbstractLoginModuleTest.java
+++ b/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/AbstractLoginModuleTest.java
@@ -16,27 +16,6 @@
  */
 package org.apache.jackrabbit.oak.spi.security.authentication;
 
-import java.io.IOException;
-import java.security.Principal;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-
-import javax.jcr.Credentials;
-import javax.jcr.GuestCredentials;
-import javax.jcr.SimpleCredentials;
-import javax.security.auth.DestroyFailedException;
-import javax.security.auth.Destroyable;
-import javax.security.auth.Subject;
-import javax.security.auth.callback.Callback;
-import javax.security.auth.callback.CallbackHandler;
-import javax.security.auth.callback.UnsupportedCallbackException;
-import javax.security.auth.login.LoginException;
-
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import org.apache.jackrabbit.api.security.authentication.token.TokenCredentials;
@@ -63,12 +42,28 @@
 import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
 import org.apache.jackrabbit.oak.spi.whiteboard.DefaultWhiteboard;
 import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard;
-import org.apache.jackrabbit.oak.stats.DefaultStatisticsProvider;
-import org.apache.jackrabbit.oak.stats.StatisticsProvider;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.junit.Test;
 
+import javax.jcr.Credentials;
+import javax.jcr.GuestCredentials;
+import javax.jcr.SimpleCredentials;
+import javax.security.auth.DestroyFailedException;
+import javax.security.auth.Destroyable;
+import javax.security.auth.Subject;
+import javax.security.auth.callback.Callback;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.callback.UnsupportedCallbackException;
+import javax.security.auth.login.LoginException;
+import java.io.IOException;
+import java.security.Principal;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
 import static org.apache.jackrabbit.oak.spi.security.authentication.AbstractLoginModule.SHARED_KEY_CREDENTIALS;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -81,11 +76,15 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.withSettings;
 
 public class AbstractLoginModuleTest {
 
+    private final LoginModuleMonitor monitor = mock(LoginModuleMonitor.class);
+
     private static AbstractLoginModule initLoginModule(@NotNull Class<?> supportedCredentials, @NotNull  Map<String, ?> sharedState) {
         AbstractLoginModule lm = new TestLoginModule(supportedCredentials);
         initialize(lm, new Subject(), null, sharedState);
@@ -124,7 +123,7 @@
     @Test
     public void testInitializeWithOptions() {
         AbstractLoginModule lm = new TestLoginModule(TestCredentials.class);
-        Map options = ImmutableMap.of("key", "value");
+        Map<String, String> options = ImmutableMap.of("key", "value");
         lm.initialize(new Subject(), null, Collections.emptyMap(), options);
 
         assertNotSame(options, lm.options);
@@ -184,7 +183,7 @@
 
     @Test
     public void testLogoutSubjectWithoutPrincipals() throws Exception {
-        Subject subject = new Subject(false, ImmutableSet.<Principal>of(), ImmutableSet.of(new TestCredentials()), ImmutableSet.of());
+        Subject subject = new Subject(false, ImmutableSet.of(), ImmutableSet.of(new TestCredentials()), ImmutableSet.of());
         AbstractLoginModule loginModule = initLoginModule(subject);
         loginModule.logout();
 
@@ -202,6 +201,7 @@
     public void testLogoutCPSuccess() throws LoginException {
         PrincipalProvider pp = new TestPrincipalProvider("user");
         Principal p = pp.getPrincipal("user");
+        assertNotNull(p);
         Principal foreignPrincipal = TestPrincipalProvider.UNKNOWN;
 
         String userId = TestPrincipalProvider.getIDFromPrincipal(p);
@@ -222,7 +222,7 @@
 
         assertTrue(loginModule.logout(ImmutableSet.of(authInfo), principals));
 
-        Set publicCreds = subject.getPublicCredentials();
+        Set<Object> publicCreds = subject.getPublicCredentials();
         assertFalse(publicCreds.contains(authInfo));
         assertTrue(publicCreds.contains(foreign1));
         assertTrue(publicCreds.contains(foreign2));
@@ -292,6 +292,7 @@
         Principal unknownPrincipal = TestPrincipalProvider.UNKNOWN;
         TestPrincipalProvider pp = new TestPrincipalProvider("principal");
         Principal p = pp.getPrincipal("principal");
+        assertNotNull(p);
 
         Subject subject = new Subject(false, ImmutableSet.of(unknownPrincipal, p), ImmutableSet.of(creds), ImmutableSet.of());
 
@@ -363,13 +364,13 @@
         ContentRepository cr = mockContentRepository(cs);
         doThrow(IOException.class).when(cs).close();
 
-        LoginModuleStats stats = newLoginModuleStats();
         CallbackHandler cbh = new TestCallbackHandler(cr, mock(SecurityProvider.class));
 
-        AbstractLoginModule loginModule = initLoginModule(cbh, stats);
+        AbstractLoginModule loginModule = initLoginModule(cbh, monitor);
         loginModule.getRoot();
         loginModule.clearState();
-        assertEquals(1, stats.getLoginErrors());
+        verify(monitor).loginError();
+        verifyNoMoreInteractions(monitor);
         verify(cs, times(1)).close();
     }
 
@@ -451,7 +452,7 @@
         subject.getPublicCredentials().add(new TestCredentials());
 
         AbstractLoginModule lm = new TestLoginModule(TestCredentials.class);
-        lm.initialize(subject, null, ImmutableMap.<String, Object>of(), null);
+        lm.initialize(subject, null, ImmutableMap.of(), null);
 
         assertTrue(lm.getCredentials() instanceof TestCredentials);
     }
@@ -487,18 +488,18 @@
 
     @Test
     public void testGetCredentialsIOException() {
-        LoginModuleMonitor monitor = mock(LoginModuleMonitor.class);
         AbstractLoginModule lm = initLoginModule(new ThrowingCallbackHandler(true), monitor);
         assertNull(lm.getCredentials());
         verify(monitor, times(1)).loginError();
+        verifyNoMoreInteractions(monitor);
     }
 
     @Test
     public void testGetCredentialsUnsupportedCallbackException() {
-        LoginModuleMonitor monitor = mock(LoginModuleMonitor.class);
         AbstractLoginModule lm = initLoginModule(new ThrowingCallbackHandler(false), monitor);
         assertNull(lm.getCredentials());
         verify(monitor, times(1)).loginError();
+        verifyNoMoreInteractions(monitor);
     }
 
     @Test
@@ -559,26 +560,20 @@
         assertSame(root, loginModule.getRoot());
     }
 
-    private static LoginModuleStats newLoginModuleStats() {
-        ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor();
-        StatisticsProvider sp = new DefaultStatisticsProvider(executor);
-        return new LoginModuleStats(sp);
-    }
-
     @Test
     public void testGetRootIOException() {
-        LoginModuleStats stats = newLoginModuleStats();
-        AbstractLoginModule lm = initLoginModule(new ThrowingCallbackHandler(true), stats);
+        AbstractLoginModule lm = initLoginModule(new ThrowingCallbackHandler(true), monitor);
         assertNull(lm.getRoot());
-        assertEquals(1, stats.getLoginErrors());
+        verify(monitor).loginError();
+        verifyNoMoreInteractions(monitor);
     }
 
     @Test
     public void testGetRootUnsupportedCallbackException() {
-        LoginModuleStats stats = newLoginModuleStats();
-        AbstractLoginModule lm = initLoginModule(new ThrowingCallbackHandler(false), stats);
+        AbstractLoginModule lm = initLoginModule(new ThrowingCallbackHandler(false), monitor);
         assertNull(lm.getRoot());
-        assertEquals(1, stats.getLoginErrors());
+        verify(monitor).loginError();
+        verifyNoMoreInteractions(monitor);
     }
 
     @Test
@@ -871,38 +866,36 @@
 
     @Test
     public void testOnError() {
-        LoginModuleStats stats = newLoginModuleStats();
         CallbackHandler cbh = callbacks -> {
             for (Callback cb : callbacks) {
                 if (cb instanceof RepositoryCallback) {
-                    ((RepositoryCallback) cb).setLoginModuleMonitor(stats);
+                    ((RepositoryCallback) cb).setLoginModuleMonitor(monitor);
                 }
             }
         };
 
         AbstractLoginModule lm = initLoginModule(cbh);
-        assertTrue(lm.getLoginModuleMonitor() instanceof LoginModuleStats);
+        assertSame(monitor, lm.getLoginModuleMonitor());
         lm.onError();
-        assertEquals(1, stats.getLoginErrors());
+        verify(monitor).loginError();
+        verifyNoMoreInteractions(monitor);
     }
 
     @Test
-    public void testNullLoginModuleMonitor() {
-        LoginModuleStats stats = newLoginModuleStats();
+    public void testLoginModuleMonitorMissingCallback() {
         AbstractLoginModule lm = initLoginModule((CallbackHandler) null);
-        assertNull(lm.getLoginModuleMonitor());
+        assertSame(LoginModuleMonitor.NOOP, lm.getLoginModuleMonitor());
         lm.onError();
-        assertEquals(0, stats.getLoginErrors());
+        verifyNoInteractions(monitor);
     }
 
     @Test
     public void testErrorOnGetLoginModuleMonitor() {
-        LoginModuleStats stats = newLoginModuleStats();
         AbstractLoginModule lm = initLoginModule(new ThrowingCallbackHandler(true));
 
-        assertNull(lm.getLoginModuleMonitor());
+        assertSame(LoginModuleMonitor.NOOP, lm.getLoginModuleMonitor());
         lm.onError();
-        assertEquals(0, stats.getLoginErrors());
+        verifyNoInteractions(monitor);
     }
 
     //--------------------------------------------------------------------------
@@ -911,14 +904,14 @@
 
     private static final class TestLoginModule extends AbstractLoginModule {
 
-        private final Class supportedCredentialsClass;
+        private final Class<?> supportedCredentialsClass;
         private final LoginModuleMonitor mon;
 
-        private TestLoginModule(@NotNull Class supportedCredentialsClass) {
+        private TestLoginModule(@NotNull Class<?> supportedCredentialsClass) {
             this(supportedCredentialsClass, null);
         }
 
-        private TestLoginModule(@NotNull Class supportedCredentialsClass, @Nullable LoginModuleMonitor mon) {
+        private TestLoginModule(@NotNull Class<?> supportedCredentialsClass, @Nullable LoginModuleMonitor mon) {
             this.supportedCredentialsClass = supportedCredentialsClass;
             this.mon = mon;
         }
@@ -940,7 +933,7 @@
         }
 
         @Override
-        protected LoginModuleMonitor getLoginModuleMonitor() {
+        protected @NotNull LoginModuleMonitor getLoginModuleMonitor() {
             if (mon != null) {
                 return mon;
             } else {