From eb21561fa4727154a10d9ebf0560d81ba0f3d1ab Mon Sep 17 00:00:00 2001 From: Ben Pennell Date: Tue, 14 Mar 2023 11:53:12 -0400 Subject: [PATCH] Removed unused value property. Pass provided action down to the event being sent. Make some constants static. Add test coverage --- .../webapp/WEB-INF/access-fedora-context.xml | 1 + .../common/utils/AnalyticsTrackerUtil.java | 36 ++-- .../utils/AnalyticsTrackerUtilTest.java | 160 ++++++++++++++++++ .../main/webapp/WEB-INF/service-context.xml | 1 + 4 files changed, 181 insertions(+), 17 deletions(-) create mode 100644 web-common/src/test/java/edu/unc/lib/boxc/web/common/utils/AnalyticsTrackerUtilTest.java diff --git a/web-access-app/src/main/webapp/WEB-INF/access-fedora-context.xml b/web-access-app/src/main/webapp/WEB-INF/access-fedora-context.xml index 9ffb4bb5a3..e47f0585ed 100644 --- a/web-access-app/src/main/webapp/WEB-INF/access-fedora-context.xml +++ b/web-access-app/src/main/webapp/WEB-INF/access-fedora-context.xml @@ -25,6 +25,7 @@ + diff --git a/web-common/src/main/java/edu/unc/lib/boxc/web/common/utils/AnalyticsTrackerUtil.java b/web-common/src/main/java/edu/unc/lib/boxc/web/common/utils/AnalyticsTrackerUtil.java index c92dd890df..973cf0f2fa 100644 --- a/web-common/src/main/java/edu/unc/lib/boxc/web/common/utils/AnalyticsTrackerUtil.java +++ b/web-common/src/main/java/edu/unc/lib/boxc/web/common/utils/AnalyticsTrackerUtil.java @@ -37,15 +37,16 @@ public class AnalyticsTrackerUtil { private static final Logger log = LoggerFactory.getLogger(AnalyticsTrackerUtil.class); // Made up CID to use if the request does not include one, such as from a API request - private static final String DEFAULT_CID = "35009a79-1a05-49d7-b876-2b884d0f825b"; + protected static final String DEFAULT_CID = "35009a79-1a05-49d7-b876-2b884d0f825b"; // Google analytics measurement API url - private final String GA_URL = "https://www.google-analytics.com/collect"; + private static final String GA_URL = "https://www.google-analytics.com/collect"; // Google analytics tracking id private String gaTrackingID; private HttpClientConnectionManager httpClientConnectionManager; private CloseableHttpClient httpClient; + private String repositoryHost; private SolrSearchService solrSearchService; @@ -62,6 +63,10 @@ public void setHttpClientConnectionManager(HttpClientConnectionManager manager) .build(); } + public void setHttpClient(CloseableHttpClient httpClient) { + this.httpClient = httpClient; + } + public void setGaTrackingID(String trackingID) { this.gaTrackingID = trackingID; } @@ -83,14 +88,14 @@ public void trackEvent(HttpServletRequest request, String action, PID pid, Acces "(no collection)" : briefObject.getParentCollectionName(); String viewedObjectLabel = briefObject.getTitle() + "|" + pid; - trackEvent(userData, parentCollection, "download", viewedObjectLabel, null); + trackEvent(userData, parentCollection, action, viewedObjectLabel); } catch (Exception e) { // Prevent analytics exceptions from impacting user log.warn("An exception occurred while recording {} event on {}", action, pid, e); } } - public void trackEvent(AnalyticsUserData userData, String category, String action, String label, Integer value) { + private void trackEvent(AnalyticsUserData userData, String category, String action, String label) { // Use a default customer ID if none was provided, since it is required if (userData == null) { @@ -98,7 +103,7 @@ public void trackEvent(AnalyticsUserData userData, String category, String actio } // Perform the analytics tracking event asynchronously - Thread trackerThread = new Thread(new EventTrackerRunnable(userData, category, action, label, value)); + Thread trackerThread = new Thread(new EventTrackerRunnable(userData, category, action, label)); trackerThread.start(); } @@ -109,6 +114,10 @@ public void setSolrSearchService(SolrSearchService solrSearchService) { this.solrSearchService = solrSearchService; } + public void setRepositoryHost(String repositoryHost) { + this.repositoryHost = repositoryHost; + } + public static class AnalyticsUserData { // ip of client public String uip; @@ -161,29 +170,24 @@ public AnalyticsUserData(HttpServletRequest request) { } } - private class EventTrackerRunnable implements Runnable { + protected class EventTrackerRunnable implements Runnable { private final AnalyticsUserData userData; private final String category; private final String action; private final String label; - private final Integer value; - public EventTrackerRunnable(AnalyticsUserData userData, String category, String action, String label, - Integer value) { + public EventTrackerRunnable(AnalyticsUserData userData, String category, String action, String label) { this.category = category; this.action = action; this.label = label; - this.value = value; this.userData = userData; } @Override public void run() { - if (log.isDebugEnabled()) { - log.debug("Tracking user {} with event {} in category {} with label {}", - userData.cid, action, category, label); - } + log.debug("Tracking user {} with event {} in category {} with label {}", + userData.cid, action, category, label); URIBuilder builder; try { @@ -201,6 +205,7 @@ public void run() { params.add(new BasicNameValuePair("t", "event")); params.add(new BasicNameValuePair("uip", userData.uip)); params.add(new BasicNameValuePair("ua", userData.userAgent)); + params.add(new BasicNameValuePair("dh", repositoryHost)); params.add(new BasicNameValuePair("an", "cdr")); params.add(new BasicNameValuePair("de", "UTF-8")); params.add(new BasicNameValuePair("ul", "en-us")); @@ -217,9 +222,6 @@ public void run() { if (label != null) { params.add(new BasicNameValuePair("el", label)); } - if (value != null) { - params.add(new BasicNameValuePair("ev", value.toString())); - } builder.addParameters(params); diff --git a/web-common/src/test/java/edu/unc/lib/boxc/web/common/utils/AnalyticsTrackerUtilTest.java b/web-common/src/test/java/edu/unc/lib/boxc/web/common/utils/AnalyticsTrackerUtilTest.java new file mode 100644 index 0000000000..8d9c01f0ec --- /dev/null +++ b/web-common/src/test/java/edu/unc/lib/boxc/web/common/utils/AnalyticsTrackerUtilTest.java @@ -0,0 +1,160 @@ +package edu.unc.lib.boxc.web.common.utils; + +import edu.unc.lib.boxc.auth.api.models.AccessGroupSet; +import edu.unc.lib.boxc.auth.fcrepo.models.AccessGroupSetImpl; +import edu.unc.lib.boxc.model.fcrepo.ids.PIDs; +import edu.unc.lib.boxc.search.api.models.ContentObjectRecord; +import edu.unc.lib.boxc.search.solr.services.SolrSearchService; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.conn.HttpClientConnectionManager; +import org.apache.http.impl.client.CloseableHttpClient; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * @author bbpennel + */ +public class AnalyticsTrackerUtilTest { + private final static String PID_UUID = "03114533-0017-4c83-b9d9-567b08fb2429"; + @Mock + private HttpClientConnectionManager httpClientConnectionManager; + @Mock + private CloseableHttpClient httpClient; + @Mock + private SolrSearchService solrSearchService; + @Mock + private HttpServletRequest request; + @Mock + private ContentObjectRecord contentObjectRecord; + @Captor + private ArgumentCaptor httpRequestCaptor; + private AccessGroupSet principals; + + private AnalyticsTrackerUtil analyticsTrackerUtil; + private String repositoryHost = "boxy.example.com"; + private String trackingId = "trackme"; + + @BeforeEach + public void setup() { + MockitoAnnotations.openMocks(this); + analyticsTrackerUtil = new AnalyticsTrackerUtil(); + analyticsTrackerUtil.setRepositoryHost(repositoryHost); + analyticsTrackerUtil.setGaTrackingID(trackingId); + analyticsTrackerUtil.setSolrSearchService(solrSearchService); + analyticsTrackerUtil.setHttpClient(httpClient); + principals = new AccessGroupSetImpl("some_group"); + } + + @Test + public void testTrackEventInCollection() throws Exception { + when(request.getHeader("Proxy-Client-IP")).thenReturn("0.0.0.0"); + var cidCookie = mock(Cookie.class); + when(cidCookie.getName()).thenReturn("_ga"); + when(cidCookie.getValue()).thenReturn("ga.1.123456789.1234567890"); + when(request.getCookies()).thenReturn(new Cookie[]{ cidCookie }); + when(request.getHeader("User-Agent")).thenReturn("boxy-client"); + var pid = PIDs.get(PID_UUID); + when(solrSearchService.getObjectById(any())).thenReturn(contentObjectRecord); + when(contentObjectRecord.getTitle()).thenReturn("Test Work"); + when(contentObjectRecord.getParentCollection()).thenReturn("parent_coll"); + when(contentObjectRecord.getParentCollectionName()).thenReturn("Parent Collection"); + + analyticsTrackerUtil.trackEvent(request, "testAction", pid, principals); + + verify(httpClient, timeout(1000).times(1)).execute(httpRequestCaptor.capture()); + + var gaRequest = httpRequestCaptor.getValue(); + var gaUri = gaRequest.getURI(); + assertEquals("www.google-analytics.com", gaUri.getHost()); + assertEquals("/collect", gaUri.getPath()); + var gaQuery = gaUri.getQuery(); + assertTrue(gaQuery.contains("&cid=123456789.1234567890")); + assertTrue(gaQuery.contains("&t=event")); + assertTrue(gaQuery.contains("&uip=0.0.0.0")); + assertTrue(gaQuery.contains("&ua=boxy-client")); + assertTrue(gaQuery.contains("&dh=boxy.example.com")); + assertTrue(gaQuery.contains("&an=cdr")); + assertTrue(gaQuery.contains("&de=UTF-8")); + assertTrue(gaQuery.contains("&ec=Parent+Collection")); + assertTrue(gaQuery.contains("&ea=testAction")); + assertTrue(gaQuery.contains("&el=Test+Work|http://example.com/rest/content/03/11/45/33/03114533-0017-4c83-b9d9-567b08fb2429")); + } + + @Test + public void testTrackEventNotInCollection() throws Exception { + when(request.getHeader("X-Forwarded-For")).thenReturn("1.1.1.1"); + var pid = PIDs.get(PID_UUID); + when(solrSearchService.getObjectById(any())).thenReturn(contentObjectRecord); + when(contentObjectRecord.getTitle()).thenReturn("Test Work2"); + when(contentObjectRecord.getParentCollection()).thenReturn(null); + + analyticsTrackerUtil.trackEvent(request, "testTraction", pid, principals); + + verify(httpClient, timeout(1000).times(1)).execute(httpRequestCaptor.capture()); + + var gaRequest = httpRequestCaptor.getValue(); + var gaUri = gaRequest.getURI(); + assertEquals("www.google-analytics.com", gaUri.getHost()); + assertEquals("/collect", gaUri.getPath()); + var gaQuery = gaUri.getQuery(); + assertTrue(gaQuery.contains("&cid=" + AnalyticsTrackerUtil.DEFAULT_CID)); + assertTrue(gaQuery.contains("&t=event")); + assertTrue(gaQuery.contains("&uip=1.1.1.1")); + assertTrue(gaQuery.contains("&ua=")); + assertTrue(gaQuery.contains("&dh=boxy.example.com")); + assertTrue(gaQuery.contains("&an=cdr")); + assertTrue(gaQuery.contains("&de=UTF-8")); + assertTrue(gaQuery.contains("&ec=(no+collection)")); + assertTrue(gaQuery.contains("&ea=testTraction")); + assertTrue(gaQuery.contains("&el=Test+Work2|http://example.com/rest/content/03/11/45/33/03114533-0017-4c83-b9d9-567b08fb2429")); + } + + @Test + public void testAnalyticsUserDataWlProxyClientIP() throws Exception { + when(request.getHeader("WL-Proxy-Client-IP")).thenReturn("1.0.0.0"); + var userData = new AnalyticsTrackerUtil.AnalyticsUserData(request); + assertEquals("1.0.0.0", userData.uip); + } + + @Test + public void testAnalyticsUserDataHttpClientIp() throws Exception { + when(request.getHeader("HTTP_CLIENT_IP")).thenReturn("1.1.0.0"); + var userData = new AnalyticsTrackerUtil.AnalyticsUserData(request); + assertEquals("1.1.0.0", userData.uip); + } + + @Test + public void testAnalyticsUserDataHttpXForwardedFor() throws Exception { + when(request.getHeader("HTTP_X_FORWARDED_FOR")).thenReturn("1.1.1.0"); + var userData = new AnalyticsTrackerUtil.AnalyticsUserData(request); + assertEquals("1.1.1.0", userData.uip); + } + + @Test + public void testAnalyticsUserDataRemoteAddr() throws Exception { + when(request.getRemoteAddr()).thenReturn("1.1.1.1"); + var userData = new AnalyticsTrackerUtil.AnalyticsUserData(request); + assertEquals("1.1.1.1", userData.uip); + } + + @Test + public void testSetHttpClientConnectionManager() throws Exception { + // Just verifying the setter doesn't error + analyticsTrackerUtil.setHttpClientConnectionManager(httpClientConnectionManager); + } +} diff --git a/web-services-app/src/main/webapp/WEB-INF/service-context.xml b/web-services-app/src/main/webapp/WEB-INF/service-context.xml index 7c2c6ddde3..654a7692f9 100644 --- a/web-services-app/src/main/webapp/WEB-INF/service-context.xml +++ b/web-services-app/src/main/webapp/WEB-INF/service-context.xml @@ -184,6 +184,7 @@ +