From 22e209322c61bc1c7f940a1b686b87d90b5fd433 Mon Sep 17 00:00:00 2001 From: Nona Luypaert Date: Thu, 17 Aug 2023 16:43:21 +0200 Subject: [PATCH] 103837: Refactor GA config to list bundles --- .../google/GoogleAsyncEventListener.java | 30 +++++----- .../google/GoogleAsyncEventListenerIT.java | 56 ++++++++++++------- dspace/config/dspace.cfg | 12 ++-- 3 files changed, 60 insertions(+), 38 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/google/GoogleAsyncEventListener.java b/dspace-api/src/main/java/org/dspace/google/GoogleAsyncEventListener.java index e84d9f8591f..cb9a120fd08 100644 --- a/dspace-api/src/main/java/org/dspace/google/GoogleAsyncEventListener.java +++ b/dspace-api/src/main/java/org/dspace/google/GoogleAsyncEventListener.java @@ -23,6 +23,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.dspace.content.Bitstream; +import org.dspace.content.Bundle; import org.dspace.content.factory.ContentServiceFactory; import org.dspace.core.Constants; import org.dspace.core.Context; @@ -176,26 +177,27 @@ private String getDocumentPath(HttpServletRequest request) { * Verifies if the usage event is a content bitstream view event, by checking if: - * This last one can be skipped if 'google-analytics.exclude-non-content-bitstreams' is set to false. - * This will make it so the bundle name is completely ignored when sending events. + *
  • the bitstream belongs to one of the configured bundles (fallback: ORIGINAL bundle)
    • */ private boolean isContentBitstream(UsageEvent usageEvent) { // check if event is a VIEW event and object is a Bitstream if (usageEvent.getAction() == UsageEvent.Action.VIEW || usageEvent.getObject().getType() == Constants.BITSTREAM) { - // check if config is set to true - if (configurationService.getBooleanProperty("google-analytics.exclude-non-content-bitstreams")) { - try { - // check if bitstream belongs to the ORIGINAL bundle - return ((Bitstream) usageEvent.getObject()) - .getBundles().stream() - .anyMatch(bundle -> bundle.getName().equals(Constants.CONTENT_BUNDLE_NAME)); - } catch (SQLException e) { - throw new RuntimeException(e.getMessage(), e); - } + // check if bitstream belongs to a configured bundle + List allowedBundles = List.of(configurationService + .getArrayProperty("google-analytics.bundles", new String[]{Constants.CONTENT_BUNDLE_NAME})); + if (allowedBundles.contains("none")) { + // GA events for bitstream views were turned off in config + return false; } - return true; + List bitstreamBundles; + try { + bitstreamBundles = ((Bitstream) usageEvent.getObject()) + .getBundles().stream().map(Bundle::getName).collect(Collectors.toList()); + } catch (SQLException e) { + throw new RuntimeException(e.getMessage(), e); + } + return allowedBundles.stream().anyMatch(bitstreamBundles::contains); } return false; } diff --git a/dspace-server-webapp/src/test/java/org/dspace/google/GoogleAsyncEventListenerIT.java b/dspace-server-webapp/src/test/java/org/dspace/google/GoogleAsyncEventListenerIT.java index e43e9fd8203..17df839ebf1 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/google/GoogleAsyncEventListenerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/google/GoogleAsyncEventListenerIT.java @@ -245,17 +245,12 @@ public void testOnBitstreamContentDownloadWithTooManyEvents() throws Exception { } @Test - public void testOnBitstreamContentDownloadExcludeNonContentBitstreams() throws Exception { - configurationService.setProperty("google-analytics.exclude-non-content-bitstreams", true); - + public void testOnBitstreamContentDownloadDefaultBundleConfig() throws Exception { context.turnOffAuthorisationSystem(); Bundle licenseBundle = BundleBuilder.createBundle(context, item) .withName(Constants.LICENSE_BUNDLE_NAME).build(); Bitstream license = BitstreamBuilder.createBitstream(context, licenseBundle, toInputStream("License", defaultCharset())).build(); - Bundle thumbnailBundle = BundleBuilder.createBundle(context, item).withName("THUMBNAIL").build(); - Bitstream thumbnail = BitstreamBuilder.createBitstream(context, thumbnailBundle, - toInputStream("Thumbnail", defaultCharset())).build(); context.restoreAuthSystemState(); assertThat(getStoredEventsAsList(), empty()); @@ -264,14 +259,14 @@ public void testOnBitstreamContentDownloadExcludeNonContentBitstreams() throws E downloadBitstreamContent("Postman", "123456", "REF"); downloadContent("Chrome", "ABCDEFG", "REF-1", license); - downloadContent("Chrome", "987654", "REF-2", thumbnail); assertThat(getStoredEventsAsList(), hasSize(1)); List storedEvents = getStoredEventsAsList(); assertThat(storedEvents, contains( - event("123456", "127.0.0.1", "Postman", "REF", bitstreamUrl, "Test item"))); + event("123456", "127.0.0.1", "Postman", "REF", bitstreamUrl, "Test item")) + ); googleAsyncEventListener.sendCollectedEvents(); @@ -284,14 +279,14 @@ public void testOnBitstreamContentDownloadExcludeNonContentBitstreams() throws E } @Test - public void testOnBitstreamContentDownloadIncludeNonContentBitstreams() throws Exception { - configurationService.setProperty("google-analytics.exclude-non-content-bitstreams", false); + public void testOnBitstreamContentDownloadMultipleBundleConfig() throws Exception { + configurationService.setProperty("google-analytics.bundles", + List.of(Constants.DEFAULT_BUNDLE_NAME, "CONTENT")); context.turnOffAuthorisationSystem(); - Bundle licenseBundle = BundleBuilder.createBundle(context, item) - .withName(Constants.LICENSE_BUNDLE_NAME).build(); - Bitstream license = BitstreamBuilder.createBitstream(context, licenseBundle, - toInputStream("License", defaultCharset())).build(); + Bundle contentBundle = BundleBuilder.createBundle(context, item).withName("CONTENT").build(); + Bitstream content = BitstreamBuilder.createBitstream(context, contentBundle, + toInputStream("Test Content", defaultCharset())).build(); Bundle thumbnailBundle = BundleBuilder.createBundle(context, item).withName("THUMBNAIL").build(); Bitstream thumbnail = BitstreamBuilder.createBitstream(context, thumbnailBundle, toInputStream("Thumbnail", defaultCharset())).build(); @@ -300,21 +295,20 @@ public void testOnBitstreamContentDownloadIncludeNonContentBitstreams() throws E assertThat(getStoredEventsAsList(), empty()); String bitstreamUrl = "/api/core/bitstreams/" + bitstream.getID() + "/content"; - String licenseUrl = "/api/core/bitstreams/" + license.getID() + "/content"; - String thumbnailUrl = "/api/core/bitstreams/" + thumbnail.getID() + "/content"; + String contentUrl = "/api/core/bitstreams/" + content.getID() + "/content"; downloadBitstreamContent("Postman", "123456", "REF"); - downloadContent("Chrome", "ABCDEFG", "REF-1", license); + downloadContent("Chrome", "ABCDEFG", "REF-1", content); downloadContent("Chrome", "987654", "REF-2", thumbnail); - assertThat(getStoredEventsAsList(), hasSize(3)); + assertThat(getStoredEventsAsList(), hasSize(2)); List storedEvents = getStoredEventsAsList(); assertThat(storedEvents, contains( event("123456", "127.0.0.1", "Postman", "REF", bitstreamUrl, "Test item"), - event("ABCDEFG", "127.0.0.1", "Chrome", "REF-1", licenseUrl, "Test item"), - event("987654", "127.0.0.1", "Chrome", "REF-2", thumbnailUrl, "Test item"))); + event("ABCDEFG", "127.0.0.1", "Chrome", "REF-1", contentUrl, "Test item") + )); googleAsyncEventListener.sendCollectedEvents(); @@ -326,6 +320,28 @@ public void testOnBitstreamContentDownloadIncludeNonContentBitstreams() throws E verifyNoMoreInteractions(firstGaClientMock, secondGaClientMock); } + @Test + public void testOnBitstreamContentDownloadNoneBundleConfig() throws Exception { + configurationService.setProperty("google-analytics.bundles", "none"); + + context.turnOffAuthorisationSystem(); + Bundle contentBundle = BundleBuilder.createBundle(context, item).withName("CONTENT").build(); + Bitstream content = BitstreamBuilder.createBitstream(context, contentBundle, + toInputStream("Test Content", defaultCharset())).build(); + Bundle thumbnailBundle = BundleBuilder.createBundle(context, item).withName("THUMBNAIL").build(); + Bitstream thumbnail = BitstreamBuilder.createBitstream(context, thumbnailBundle, + toInputStream("Thumbnail", defaultCharset())).build(); + context.restoreAuthSystemState(); + + assertThat(getStoredEventsAsList(), empty()); + + downloadBitstreamContent("Postman", "123456", "REF"); + downloadContent("Chrome", "ABCDEFG", "REF-1", content); + downloadContent("Chrome", "987654", "REF-2", thumbnail); + + assertThat(getStoredEventsAsList(), empty()); + } + @SuppressWarnings("unchecked") private List getStoredEventsAsList() { List events = new ArrayList<>(); diff --git a/dspace/config/dspace.cfg b/dspace/config/dspace.cfg index 89c8da92558..be11cbf0330 100644 --- a/dspace/config/dspace.cfg +++ b/dspace/config/dspace.cfg @@ -1537,10 +1537,14 @@ log.report.dir = ${dspace.dir}/log # For more details see https://developers.google.com/analytics/devguides/collection/protocol/ga4 # google.analytics.api-secret = -# Ensures only views of bitstreams in the 'ORIGINAL' bundle result in a GA4 event. -# Setting this to false may cause inflated bitstream view numbers, since requesting -# bitstreams in the 'THUMBNAIL' and 'LICENSE' bundles, will also result in GA4 events. -google-analytics.exclude-non-content-bitstreams=true +# Ensures only views of bitstreams in configured bundles result in a GA4 event. +# Config can contain multiple bundles for which the bitstream views will result in GA4 events, eg: +# google-analytics.bundles = ORIGINAL, CONTENT +# If config is not set or empty, the default fallback is Constants#CONTENT_BUNDLE_NAME bundle ('ORIGINAL'). +# If config contains 'LICENSE' or 'THUMBNAIL' bundles, it may cause inflated bitstream view numbers. +# Set config to 'none' to disable GA4 bitstream events, eg: +# google-analytics.bundles = none +google-analytics.bundles = ORIGINAL #################################################################### #---------------------------------------------------------------#