REPO-2627 Refactor code as per code review

* Add new collector AuthoritiesDataCollector
* Remove unused data from RepositoryDataCollector
* Remove unused imports

src/main/java/org/alfresco/heartbeat/AuthoritiesDataCollector.java

@@ -0,0 +1,85 @@
+/*
+ * #%L
+ * Alfresco Repository
+ * %%
+ * Copyright (C) 2005 - 2017 Alfresco Software Limited
+ * %%
+ * This file is part of the Alfresco software.
+ * If the software was purchased under a paid Alfresco license, the terms of
+ * the paid license agreement will prevail.  Otherwise, the software is
+ * provided under the following open source license terms:
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * #L%
+ */
+package org.alfresco.heartbeat;
+
+import org.alfresco.heartbeat.datasender.HBData;
+import org.alfresco.repo.descriptor.DescriptorDAO;
+import org.alfresco.service.cmr.security.AuthorityService;
+import org.alfresco.service.cmr.security.AuthorityType;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import java.util.*;
+
+/**
+ *
+ * @author eknizat
+ */
+public class AuthoritiesDataCollector extends HBBaseDataCollector
+{
+
+    /** The logger. */
+    private static final Log logger = LogFactory.getLog(AuthoritiesDataCollector.class);
+
+    /** DAO for current repository descriptor. */
+    private DescriptorDAO currentRepoDescriptorDAO;
+
+    /** The authority service. */
+    private AuthorityService authorityService;
+
+    public void setCurrentRepoDescriptorDAO(DescriptorDAO currentRepoDescriptorDAO)
+    {
+        this.currentRepoDescriptorDAO = currentRepoDescriptorDAO;
+    }
+
+    public void setAuthorityService(AuthorityService authorityService)
+    {
+        this.authorityService = authorityService;
+    }
+
+    @Override
+    public List<HBData> collectData()
+    {
+
+        List<HBData> collectedData = new LinkedList<>();
+
+        // Collect repository usage (authorities) data
+        this.logger.debug("Preparing repository usage (authorities) data...");
+        Map<String, Object> authoritiesUsageValues = new HashMap<>();
+        authoritiesUsageValues.put("numUsers", new Integer(this.authorityService.getAllAuthoritiesInZone(
+                AuthorityService.ZONE_APP_DEFAULT, AuthorityType.USER).size()));
+        authoritiesUsageValues.put("numGroups", new Integer(this.authorityService.getAllAuthoritiesInZone(
+                AuthorityService.ZONE_APP_DEFAULT, AuthorityType.GROUP).size()));
+        HBData authoritiesUsageData = new HBData(
+                this.currentRepoDescriptorDAO.getDescriptor().getId(),
+                "acs.repository.usage.authorities",
+                "1.0",
+                new Date(),
+                authoritiesUsageValues);
+        collectedData.add(authoritiesUsageData);
+
+        return collectedData;
+    }
+}

src/main/java/org/alfresco/heartbeat/HBBaseDataCollector.java

@@ -30,6 +30,10 @@ import java.util.List;
 import org.alfresco.heartbeat.datasender.HBData;
 import org.alfresco.service.cmr.repository.HBDataCollectorService;
 
+/**
+ *
+ * @author eknizat
+ */
 public abstract class HBBaseDataCollector
 {
     private HBDataCollectorService hbDataCollectorService;

src/main/java/org/alfresco/heartbeat/RepositoryDataCollector.java

@@ -26,11 +26,8 @@
 package org.alfresco.heartbeat;
 
 import java.io.IOException;
-import java.net.InetAddress;
-import java.net.NetworkInterface;
 import java.security.GeneralSecurityException;
 import java.util.Date;
-import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
@@ -40,16 +37,16 @@ import org.alfresco.heartbeat.datasender.HBData;
 import org.alfresco.repo.descriptor.DescriptorDAO;
 import org.alfresco.repo.dictionary.CustomModelsInfo;
 import org.alfresco.repo.transaction.RetryingTransactionHelper;
-import org.alfresco.repo.usage.RepoUsageComponent;
-import org.alfresco.service.cmr.admin.RepoUsage;
 import org.alfresco.service.cmr.dictionary.CustomModelService;
-import org.alfresco.service.cmr.security.AuthorityService;
-import org.alfresco.service.cmr.security.AuthorityType;
 import org.alfresco.service.descriptor.Descriptor;
 import org.alfresco.service.transaction.TransactionService;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+/**
+ *
+ * @author eknizat
+ */
 public class RepositoryDataCollector extends HBBaseDataCollector
 {
 
@@ -71,11 +68,6 @@ public class RepositoryDataCollector extends HBBaseDataCollector
     /** DAO for current descriptor. */
     private DescriptorDAO serverDescriptorDAO;
 
-    /** The authority service. */
-    private AuthorityService authorityService;
-
-    private RepoUsageComponent repoUsageComponent;
-
     /** Provides information about custom models */
     private CustomModelService customModelService;
 
@@ -89,16 +81,6 @@ public class RepositoryDataCollector extends HBBaseDataCollector
         this.serverDescriptorDAO = serverDescriptorDAO;
     }
 
-    public void setAuthorityService(AuthorityService authorityService) 
-    {
-        this.authorityService = authorityService;
-    }
-
-    public void setRepoUsageComponent(RepoUsageComponent repoUsageComponent) 
-    {
-        this.repoUsageComponent = repoUsageComponent;
-    }
-
     public void setTransactionService(TransactionService transactionService) 
     {
         this.transactionService = transactionService;
@@ -127,7 +109,7 @@ public class RepositoryDataCollector extends HBBaseDataCollector
 
         // collect repository info data
         logger.debug("Preparing repository info data...");
-        Map<String, Object> infoValues = new HashMap<String, Object>();
+        Map<String, Object> infoValues = new HashMap<>();
         infoValues.put("repoName", this.staticParameters.get("repoName"));
         infoValues.put("edition", this.staticParameters.get("edition"));
         infoValues.put("versionMajor", this.staticParameters.get("versionMajor"));
@@ -144,7 +126,7 @@ public class RepositoryDataCollector extends HBBaseDataCollector
         // collect repository usage (system) data
         logger.debug("Preparing repository usage (system) data...");
         Runtime runtime = Runtime.getRuntime();
-        Map<String, Object> systemUsageValues = new HashMap<String, Object>();
+        Map<String, Object> systemUsageValues = new HashMap<>();
         systemUsageValues.put("memFree", runtime.freeMemory());
         systemUsageValues.put("memMax", runtime.maxMemory());
         systemUsageValues.put("memTotal", runtime.totalMemory());
@@ -167,7 +149,7 @@ public class RepositoryDataCollector extends HBBaseDataCollector
                     }
                 }, true);
 
-        Map<String, Object> modelUsageValues = new HashMap<String, Object>();
+        Map<String, Object> modelUsageValues = new HashMap<>();
         modelUsageValues.put("numOfActiveModels", new Integer(customModelsInfo.getNumberOfActiveModels()));
         modelUsageValues.put("numOfActiveTypes", new Integer(customModelsInfo.getNumberOfActiveTypes()));
         modelUsageValues.put("numOfActiveAspects", new Integer(customModelsInfo.getNumberOfActiveAspects()));
@@ -196,86 +178,13 @@ public class RepositoryDataCollector extends HBBaseDataCollector
             this.staticParameters = new TreeMap<String, Object>();
 
             // Load up the static parameters
-            final String ip = getLocalIps();
-            this.staticParameters.put("ip", ip);
-            final String uid;
-            final Descriptor currentRepoDescriptor = this.currentRepoDescriptorDAO.getDescriptor();
-            if (currentRepoDescriptor != null)
-            {
-                uid = currentRepoDescriptor.getId();
-                this.staticParameters.put("uid", uid);
-            }
-            else
-            {
-                uid = "Unknown";
-            }
-
             final Descriptor serverDescriptor = this.serverDescriptorDAO.getDescriptor();
             this.staticParameters.put("repoName", serverDescriptor.getName());
             this.staticParameters.put("edition", serverDescriptor.getEdition());
             this.staticParameters.put("versionMajor", serverDescriptor.getVersionMajor());
             this.staticParameters.put("versionMinor", serverDescriptor.getVersionMinor());
             this.staticParameters.put("schema", new Integer(serverDescriptor.getSchema()));
-            this.staticParameters.put("numUsers", new Integer(this.authorityService.getAllAuthoritiesInZone(
-                    AuthorityService.ZONE_APP_DEFAULT, AuthorityType.USER).size()));
-            this.staticParameters.put("numGroups", new Integer(this.authorityService.getAllAuthoritiesInZone(
-                    AuthorityService.ZONE_APP_DEFAULT, AuthorityType.GROUP).size()));
-
-            if(repoUsageComponent != null)
-            {
-                RepoUsage usage = repoUsageComponent.getUsage();
-
-                if (usage.getUsers() != null)
-                {
-                    this.staticParameters.put("licenseUsers", new Long((usage.getUsers())));
-                }
-            }
 
         }
     }
-
-    /**
-     * Attempts to get all the local IP addresses of this machine in order to distinguish it from other nodes in the
-     * same network. The machine may use a static IP address in conjunction with a loopback adapter (e.g. to support
-     * Oracle on Windows), so the IP of the default network interface may not be enough to uniquely identify this
-     * machine.
-     *
-     * @return the local IP addresses, separated by the '/' character
-     */
-    private String getLocalIps()
-    {
-        final StringBuilder ip = new StringBuilder(1024);
-        boolean first = true;
-        try
-        {
-            final Enumeration<NetworkInterface> i = NetworkInterface.getNetworkInterfaces();
-            while (i.hasMoreElements())
-            {
-                final NetworkInterface n = i.nextElement();
-                final Enumeration<InetAddress> j = n.getInetAddresses();
-                while (j.hasMoreElements())
-                {
-                    InetAddress a = j.nextElement();
-                    if (a.isLoopbackAddress())
-                    {
-                        continue;
-                    }
-                    if (first)
-                    {
-                        first = false;
-                    }
-                    else
-                    {
-                        ip.append('/');
-                    }
-                    ip.append(a.getHostAddress());
-                }
-            }
-        }
-        catch (final Exception e)
-        {
-            // Ignore
-        }
-        return first ? "127.0.0.1" : ip.toString();
-    }
 }

src/main/resources/alfresco/heartbeat/heartbeat-repo-collector-context.xml

@@ -7,10 +7,14 @@
     <bean id="repositoryDataCollector" class="org.alfresco.heartbeat.RepositoryDataCollector" parent="hbBaseDataCollector" init-method="register">
         <property name="currentRepoDescriptorDAO" ref="currentRepoDescriptorDAO"/>
         <property name="serverDescriptorDAO" ref="serverDescriptorDAO"/>
-        <property name="authorityService" ref="authorityService"/>
-        <property name="repoUsageComponent" ref="repoUsageComponent"/>
         <property name="transactionService" ref="transactionService"/>
         <property name="customModelService" ref="customModelService"/>
     </bean>
 
+    <!-- HeartBeat authority data collector -->
+    <bean id="authorityDataCollector" class="org.alfresco.heartbeat.AuthoritiesDataCollector" parent="hbBaseDataCollector" init-method="register">
+        <property name="currentRepoDescriptorDAO" ref="currentRepoDescriptorDAO"/>
+        <property name="authorityService" ref="authorityService"/>
+    </bean>
+
 </beans>

src/test/java/org/alfresco/AllUnitTestsSuite.java

@@ -188,7 +188,7 @@ import org.junit.runners.Suite;
     org.alfresco.repo.workflow.WorkflowSuiteContextShutdownTest.class,
     org.alfresco.repo.search.impl.lucene.analysis.PathTokenFilterTest.class,
     org.alfresco.heartbeat.HBDataCollectorServiceImplTest.class,
-    org.alfresco.heartbeat.HBDataCollectorServiceImplTest.class
+    org.alfresco.heartbeat.AuthoritiesDataCollectorTest.class
 })
 public class AllUnitTestsSuite
 {

src/test/java/org/alfresco/heartbeat/AuthoritiesDataCollectorTest.java

@@ -0,0 +1,106 @@
+/*
+ * #%L
+ * Alfresco Repository
+ * %%
+ * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * %%
+ * This file is part of the Alfresco software.
+ * If the software was purchased under a paid Alfresco license, the terms of
+ * the paid license agreement will prevail.  Otherwise, the software is
+ * provided under the following open source license terms:
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * #L%
+ */
+package org.alfresco.heartbeat;
+
+import org.alfresco.heartbeat.datasender.HBData;
+import org.alfresco.repo.descriptor.DescriptorDAO;
+import org.alfresco.service.cmr.repository.HBDataCollectorService;
+import org.alfresco.service.cmr.security.AuthorityService;
+import org.alfresco.service.descriptor.Descriptor;
+import org.junit.Before;
+import org.junit.Test;
+import java.util.List;
+import java.util.Map;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * @author eknizat
+ */
+public class AuthoritiesDataCollectorTest
+{
+
+    private AuthoritiesDataCollector authorityDataCollector;
+    private List<HBData> collectedData;
+
+    @Before
+    public void setUp()
+    {
+        HBDataCollectorService mockCollectorService = mock(HBDataCollectorService.class);
+        AuthorityService authorityService = mock(AuthorityService.class);
+
+        Descriptor mockDescriptor = mock(Descriptor.class);
+        when(mockDescriptor.getId()).thenReturn("mock_id");
+        DescriptorDAO descriptorDAO = mock(DescriptorDAO.class);
+        when(descriptorDAO.getDescriptor()).thenReturn(mockDescriptor);
+
+        authorityDataCollector = new AuthoritiesDataCollector();
+        authorityDataCollector.setAuthorityService(authorityService);
+        authorityDataCollector.setCurrentRepoDescriptorDAO(descriptorDAO);
+        authorityDataCollector.setHbDataCollectorService(mockCollectorService);
+        collectedData = authorityDataCollector.collectData();
+    }
+
+    @Test
+    public void testHBDataFields()
+    {
+        for(HBData data : this.collectedData)
+        {
+            assertNotNull(data.getCollectorId());
+            assertNotNull(data.getCollectorVersion());
+            assertNotNull(data.getSchemaVersion());
+            assertNotNull(data.getSystemId());
+            assertNotNull(data.getTimestamp());
+        }
+
+    }
+
+    @Test
+    public void testInfoDataIsCollected()
+    {
+        HBData authorityInfo = grabDataByCollectorId("acs.repository.usage.authorities");
+        assertNotNull("Authority info data missing.", authorityInfo);
+
+        Map<String,Object> data = authorityInfo.getData();
+        assertTrue(data.containsKey("numUsers"));
+        assertTrue(data.containsKey("numGroups"));
+    }
+
+
+    private HBData grabDataByCollectorId(String collectorId)
+    {
+        for (HBData d : this.collectedData)
+        {
+            if(d.getCollectorId()!=null && d.getCollectorId().equals(collectorId))
+            {
+                return d;
+            }
+        }
+        return null;
+    }
+}

src/test/java/org/alfresco/heartbeat/HeartBeatTest.java

@@ -25,21 +25,18 @@
  */
 package org.alfresco.heartbeat;
 
-import org.alfresco.heartbeat.datasender.HBDataSenderService;
 import org.alfresco.service.cmr.repository.HBDataCollectorService;
 import org.alfresco.service.license.LicenseDescriptor;
 import org.alfresco.service.license.LicenseService;
 import org.junit.Before;
 import org.junit.Test;
 import org.quartz.Scheduler;
-import org.quartz.SchedulerException;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ConfigurableApplicationContext;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 import java.util.Arrays;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -56,7 +53,6 @@ public class HeartBeatTest
     private ApplicationContext context;
 
     LicenseService mockLicenseService;
-    HBDataSenderService mockDataSenderService;
     HBDataCollectorService mockDataCollectorService;
 
     @Before

src/test/java/org/alfresco/heartbeat/RepositoryDataCollectorTest.java

@@ -28,11 +28,8 @@ package org.alfresco.heartbeat;
 import org.alfresco.heartbeat.datasender.HBData;
 import org.alfresco.repo.descriptor.DescriptorDAO;
 import org.alfresco.repo.dictionary.CustomModelsInfo;
-import org.alfresco.repo.usage.RepoUsageComponent;
-import org.alfresco.service.cmr.admin.RepoUsage;
 import org.alfresco.service.cmr.dictionary.CustomModelService;
 import org.alfresco.service.cmr.repository.HBDataCollectorService;
-import org.alfresco.service.cmr.security.AuthorityService;
 import org.alfresco.service.descriptor.Descriptor;
 import org.alfresco.service.transaction.TransactionService;
 import org.alfresco.util.ApplicationContextHelper;
@@ -62,26 +59,19 @@ public class RepositoryDataCollectorTest
 
         TransactionService transactionService = (TransactionService) context.getBean("transactionService");
         HBDataCollectorService mockCollectorService = mock(HBDataCollectorService.class);
-        AuthorityService authorityService = mock(AuthorityService.class);
 
         Descriptor mockDescriptor = mock(Descriptor.class);
         when(mockDescriptor.getId()).thenReturn("mock_id");
         DescriptorDAO descriptorDAO = mock(DescriptorDAO.class);
         when(descriptorDAO.getDescriptor()).thenReturn(mockDescriptor);
 
-        RepoUsage mockRepoUsage = mock(RepoUsage.class);
-        RepoUsageComponent repoUsageComponent = mock(RepoUsageComponent.class);
-        when(repoUsageComponent.getUsage()).thenReturn(mockRepoUsage);
-
         CustomModelsInfo mockCustomModelsInfo = mock(CustomModelsInfo.class);
         CustomModelService customModelService = mock(CustomModelService.class);
         when(customModelService.getCustomModelsInfo()).thenReturn(mockCustomModelsInfo);
 
         repoCollector = new RepositoryDataCollector();
-        repoCollector.setAuthorityService(authorityService);
         repoCollector.setCurrentRepoDescriptorDAO(descriptorDAO);
         repoCollector.setCustomModelService(customModelService);
-        repoCollector.setRepoUsageComponent(repoUsageComponent);
         repoCollector.setServerDescriptorDAO(descriptorDAO);
         repoCollector.setTransactionService(transactionService);
         repoCollector.setHbDataCollectorService(mockCollectorService);
@@ -99,7 +89,6 @@ public class RepositoryDataCollectorTest
             assertNotNull(data.getSystemId());
             assertNotNull(data.getTimestamp());
         }
-
     }
 
     @Test