From be480eb321cbf0304a270034c852cb6e3ee8b71a Mon Sep 17 00:00:00 2001 From: Andre Dietisheim Date: Thu, 20 Jun 2024 13:01:23 +0200 Subject: [PATCH] support dailyLimit Signed-off-by: Andre Dietisheim --- .../configuration/limits/EventCounts.java | 22 ++- .../configuration/limits/EventLimits.java | 41 ++++- .../core/configuration/limits/Filter.java | 17 +- .../configuration/limits/PluginLimits.java | 11 +- .../limits/PluginLimitsDeserialization.java | 21 ++- .../configuration/limits/EventLimitsTest.java | 8 +- .../limits/EventNameFilterTest.java | 8 +- .../core/configuration/limits/Mocks.java | 60 +++++-- .../limits/PluginLimitsDailyLimitTest.java | 66 ++++++++ .../PluginLimitsDeserializationTest.java | 37 ++++- ...ioTest.java => PluginLimitsRatioTest.java} | 22 ++- ...inLimitTest.java => PluginLimitsTest.java} | 152 ++++++++---------- 12 files changed, 319 insertions(+), 146 deletions(-) create mode 100644 src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsDailyLimitTest.java rename src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/{PluginLimitRatioTest.java => PluginLimitsRatioTest.java} (88%) rename src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/{PluginLimitTest.java => PluginLimitsTest.java} (71%) diff --git a/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventCounts.java b/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventCounts.java index 7fe9350c..ed7c0ff7 100644 --- a/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventCounts.java +++ b/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventCounts.java @@ -20,9 +20,7 @@ import org.apache.commons.lang.StringUtils; import org.jetbrains.annotations.NotNull; -import java.time.LocalDate; import java.time.LocalDateTime; -import java.time.LocalTime; import java.time.ZonedDateTime; import java.util.HashMap; import java.util.Map; @@ -72,11 +70,19 @@ public Count get(Event event) { } public void put(Event event) { - Count count = newOrUpdateExisting(event); + Count count = newOrUpdateCount(event); put(event, count); } - private Count newOrUpdateExisting(Event event) { + EventCounts put(Event event, Count count) { + if (event == null) { + return null; + } + counts.put(event.getName(), toString(count)); + return this; + } + + private Count newOrUpdateCount(Event event) { Count count = get(event); if (count != null) { // update existing @@ -115,14 +121,6 @@ private int toTotal(String value) { } } - EventCounts put(Event event, Count count) { - if (event == null) { - return null; - } - counts.put(event.getName(), toString(count)); - return this; - } - String toString(@NotNull Count count) { long epochSecond = count.lastOccurrence.toEpochSecond(ZonedDateTime.now().getOffset()); return epochSecond + COUNT_VALUES_SEPARATOR + count.total; diff --git a/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventLimits.java b/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventLimits.java index 185da837..b052bd8f 100644 --- a/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventLimits.java +++ b/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventLimits.java @@ -18,11 +18,13 @@ import java.io.IOException; import java.nio.file.attribute.FileTime; import java.time.Duration; +import java.time.LocalDate; import java.time.LocalDateTime; import java.time.ZoneId; -import java.util.Collections; import java.util.List; +import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.EventCounts.*; + public class EventLimits implements IEventLimits { static final Duration DEFAULT_REFRESH_PERIOD = Duration.ofHours(6); @@ -40,7 +42,11 @@ public EventLimits(String pluginId) { this(pluginId, null, PluginLimitsDeserialization::create, new Configurations(), new EventCounts()); } - EventLimits(String pluginId, List limits, PluginLimitsFactory factory, Configurations configuration, EventCounts counts) { + EventLimits(String pluginId, + List limits, + PluginLimitsFactory factory, + Configurations configuration, + EventCounts counts) { this.pluginId = pluginId; this.limits = limits; this.factory = factory; @@ -50,15 +56,36 @@ public EventLimits(String pluginId) { public boolean canSend(Event event) { List all = getAllLimits(); - return canSend(event, getPluginLimits(pluginId, all)) - && canSend(event, getDefaultLimits(all)); + return canSend(event, counts, getPluginLimits(pluginId, all)) + && canSend(event, counts, getDefaultLimits(all)); + } + + public void wasSent(Event event) { + counts.put(event); + } + + private boolean canSend(Event event, EventCounts counts, PluginLimits limits) { + if (limits == null) { + return true; + } + return limits.canSend(event, getApplicableTotal(counts.get(event))); + } + + private int getApplicableTotal(Count count) { + if (occurredToday(count)) { + return count.getTotal(); + } else { + return 0; + } } - private boolean canSend(Event event, PluginLimits limits) { - return limits == null - || limits.canSend(event); + private static boolean occurredToday(Count count) { + return count != null + && count.getLastOccurrence() != null + && LocalDate.now().atStartOfDay().isBefore(count.getLastOccurrence()); } + /* for testing purposes */ List getAllLimits() { PluginLimits defaults = getDefaultLimits(limits); diff --git a/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/Filter.java b/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/Filter.java index 88a32e9e..8dabff55 100644 --- a/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/Filter.java +++ b/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/Filter.java @@ -21,6 +21,8 @@ public interface Filter { boolean isExcludedByRatio(float percentile); + boolean isWithinDailyLimit(int total); + class EventPropertyFilter implements Filter { private final String name; private final BasicGlobPattern glob; @@ -46,14 +48,18 @@ public boolean isExcludedByRatio(float percentile) { return false; } + @Override + public boolean isWithinDailyLimit(int total) { + return true; + } } class EventNameFilter implements Filter { private final BasicGlobPattern name; private final float ratio; - private final String dailyLimit; + private final int dailyLimit; - EventNameFilter(String name, float ratio, String dailyLimit) { + EventNameFilter(String name, float ratio, int dailyLimit) { this.name = BasicGlobPattern.compile(name); this.ratio = ratio; this.dailyLimit = dailyLimit; @@ -63,7 +69,7 @@ public float getRatio() { return ratio; } - public String getDailyLimit() { + public int getDailyLimit() { return dailyLimit; } @@ -82,5 +88,10 @@ public boolean isIncludedByRatio(float percentile) { public boolean isExcludedByRatio(float percentile) { return 1 - ratio < percentile; } + + @Override + public boolean isWithinDailyLimit(int total) { + return total < dailyLimit; // at least 1 more to go + } } } \ No newline at end of file diff --git a/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimits.java b/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimits.java index 25a61d25..138be142 100644 --- a/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimits.java +++ b/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimits.java @@ -58,7 +58,7 @@ float getRatio() { return ratio; } - public boolean canSend(Event event) { + public boolean canSend(Event event, int currentTotal) { if (event == null) { return false; } @@ -71,7 +71,7 @@ public boolean canSend(Event event) { return false; } - return isIncluded(event) + return isIncluded(event, currentTotal) && !isExcluded(event); } @@ -99,13 +99,14 @@ List getIncludes() { return includes; } - boolean isIncluded(Event event) { + boolean isIncluded(Event event, int currentTotal) { Filter matching = includes.stream() .filter(filter -> filter.isMatching(event)) .findAny() .orElse(null); - return matching == null - || matching.isIncludedByRatio(userId.getPercentile()); + return matching == null || + (matching.isIncludedByRatio(userId.getPercentile()) + && matching.isWithinDailyLimit(currentTotal)); } boolean isExcluded(Event event) { diff --git a/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsDeserialization.java b/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsDeserialization.java index 13f5bd24..d85c0c61 100644 --- a/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsDeserialization.java +++ b/src/main/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsDeserialization.java @@ -17,9 +17,11 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.deser.std.StdDeserializer; import com.fasterxml.jackson.databind.module.SimpleModule; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.text.StringUtil; import com.redhat.devtools.intellij.telemetry.core.configuration.limits.Filter.EventNameFilter; import com.redhat.devtools.intellij.telemetry.core.configuration.limits.Filter.EventPropertyFilter; +import com.redhat.devtools.intellij.telemetry.core.service.TelemetryService; import org.jetbrains.annotations.NotNull; import java.io.IOException; @@ -34,6 +36,8 @@ class PluginLimitsDeserialization extends StdDeserializer> { + private static final Logger LOGGER = Logger.getInstance(PluginLimitsDeserialization.class); + public static final String FIELDNAME_ENABLED = "enabled"; public static final String FIELDNAME_REFRESH = "refresh"; public static final String FIELDNAME_RATIO = "ratio"; @@ -44,6 +48,8 @@ class PluginLimitsDeserialization extends StdDeserializer> { public static final String FIELDNAME_DAILY_LIMIT = "dailyLimit"; public static final String FIELDNAME_NAME = "name"; + public static final int DEFAULT_NUMERIC_VALUE = -1; + public static List create(String json) throws JsonProcessingException { ObjectMapper mapper = new ObjectMapper(); SimpleModule module = new SimpleModule(); @@ -88,14 +94,14 @@ private Enabled getEnabled(JsonNode node) { } private int getRefresh(JsonNode node) { - int numeric = -1; + int numeric = DEFAULT_NUMERIC_VALUE; if (node != null) { String refresh = getNumericPortion(node.asText().toCharArray()); if (!StringUtil.isEmptyOrSpaces(refresh)) { try { numeric = Integer.parseInt(refresh); } catch (NumberFormatException e) { - // swallow + LOGGER.warn("Could not convert " + FIELDNAME_REFRESH + " to integer value: " + refresh); } } } @@ -128,7 +134,7 @@ private Filter createMessageLimitFilter(JsonNode node) { private EventNameFilter createEventNameFilter(JsonNode node) { String name = getStringValue(FIELDNAME_NAME, node); float ratio = getRatio(node.get(FIELDNAME_RATIO)); - String dailyLimit = getStringValue(FIELDNAME_DAILY_LIMIT, node); + int dailyLimit = getIntValue(FIELDNAME_DAILY_LIMIT, node); return new EventNameFilter(name, ratio, dailyLimit); } @@ -158,6 +164,15 @@ private static String getStringValue(String name, JsonNode node) { return node.get(name).asText(); } + private static int getIntValue(String name, JsonNode node) { + int numeric = DEFAULT_NUMERIC_VALUE; + if (node != null + && node.get(name) == null) { + numeric = node.get(name).asInt(DEFAULT_NUMERIC_VALUE); + } + return numeric; + } + private static String getNumericPortion(char[] characters) { StringBuilder builder = new StringBuilder(); for (int i = 0; i < characters.length && Character.isDigit(characters[i]); i++) { diff --git a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventLimitsTest.java b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventLimitsTest.java index 8eb871ad..09d86090 100644 --- a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventLimitsTest.java +++ b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventLimitsTest.java @@ -50,7 +50,7 @@ public void getAllLimits_should_return_null_if_deserialization_throws() throws I @Test public void getAllLimits_should_download_remote_if_local_file_has_no_modification_timestamp() { // given - List allLimits = List.of(mockDefaultPluginLimitsWithRefresh(Integer.MAX_VALUE)); + List allLimits = List.of(createDefaultPluginLimits(Integer.MAX_VALUE)); Configurations configurations = mock(Configurations.class); doReturn(null) // no modification timestamp, file does not exist .when(configurations).getLocalLastModified(); @@ -82,7 +82,7 @@ public void getAllLimits_should_download_remote_if_local_file_was_modified_7h_ag @Test public void getAllLimits_should_download_remote_if_local_file_was_modified_7h_ago_and_default_limits_has_no_refresh() { // given - List allLimits = List.of(mockDefaultPluginLimitsWithRefresh(-1)); + List allLimits = List.of(createDefaultPluginLimits(-1)); PluginLimitsFactory factory = mock(PluginLimitsFactory.class); Configurations configurations = mock(Configurations.class); // default refresh (with plugin limits without refresh) is 6h @@ -99,7 +99,7 @@ public void getAllLimits_should_download_remote_if_local_file_was_modified_7h_ag @Test public void getAllLimits_should_NOT_download_remote_if_local_file_was_modified_within_specified_refresh_period() { // given - List allLimits = List.of(mockDefaultPluginLimitsWithRefresh(2)); + List allLimits = List.of(createDefaultPluginLimits(2)); PluginLimitsFactory factory = mock(PluginLimitsFactory.class); Configurations configurations = mock(Configurations.class); doReturn(createFileTime(1)) // 1h ago @@ -132,7 +132,7 @@ private static FileTime createFileTime(int createdHoursAgo) { Instant.now().minus(createdHoursAgo, ChronoUnit.HOURS)); } - private static PluginLimits mockDefaultPluginLimitsWithRefresh(int refresh) { + private static PluginLimits createDefaultPluginLimits(int refresh) { PluginLimits pluginLimit = mock(PluginLimits.class); doReturn(refresh) .when(pluginLimit).getRefresh(); diff --git a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventNameFilterTest.java b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventNameFilterTest.java index f91890f4..0f6b9704 100644 --- a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventNameFilterTest.java +++ b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/EventNameFilterTest.java @@ -24,7 +24,7 @@ public class EventNameFilterTest { @Test public void isMatching_should_match_event_name() { // given - Filter filter = new EventNameFilter("yoda", 0.42f, "42"); + Filter filter = new EventNameFilter("yoda", 0.42f, 42); Event event = new Event(Event.Type.USER, "yoda"); // when boolean matching = filter.isMatching(event); @@ -35,7 +35,7 @@ public void isMatching_should_match_event_name() { @Test public void isMatching_should_NOT_match_event_name_that_is_different() { // given - Filter filter = new EventNameFilter("yoda", 0.42f, "42"); + Filter filter = new EventNameFilter("yoda", 0.42f, 42); Event event = new Event(Event.Type.USER, "darthvader"); // when boolean matching = filter.isMatching(event); @@ -46,7 +46,7 @@ public void isMatching_should_NOT_match_event_name_that_is_different() { @Test public void isMatching_should_match_event_name_when_pattern_is_wildcard() { // given - Filter filter = new EventNameFilter("*", 0.42f, "42"); + Filter filter = new EventNameFilter("*", 0.42f, 42); Event event = new Event(Event.Type.USER, "skywalker"); // when boolean matching = filter.isMatching(event); @@ -57,7 +57,7 @@ public void isMatching_should_match_event_name_when_pattern_is_wildcard() { @Test public void isMatching_should_match_event_name_when_pattern_has_name_with_wildcards() { // given - Filter filter = new EventNameFilter("*walk*", 0.42f, "42"); + Filter filter = new EventNameFilter("*walk*", 0.42f, 42); Event event = new Event(Event.Type.USER, "skywalker"); // when boolean matching = filter.isMatching(event); diff --git a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/Mocks.java b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/Mocks.java index 7a2e6bb4..cd5e15aa 100644 --- a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/Mocks.java +++ b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/Mocks.java @@ -10,26 +10,27 @@ ******************************************************************************/ package com.redhat.devtools.intellij.telemetry.core.configuration.limits; -import com.redhat.devtools.intellij.telemetry.core.configuration.limits.Filter; import com.redhat.devtools.intellij.telemetry.core.service.Event; import com.redhat.devtools.intellij.telemetry.core.service.UserId; import java.util.HashMap; +import java.util.List; import java.util.Map; +import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Filter.*; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; public class Mocks { - public static Event event(Map properties) { - return new Event(null, null, properties); - } - public static Event event() { return event(new HashMap<>()); } + public static Event event(Map properties) { + return new Event(null, null, properties); + } + public static UserId userId(float percentile) { UserId userId = mock(UserId.class); doReturn(percentile) @@ -37,8 +38,30 @@ public static UserId userId(float percentile) { return userId; } - public static Filter.EventNameFilter eventNameFilterFake(final boolean isMatching, boolean isIncludedRatio, boolean isExcludedRatio) { - return new Filter.EventNameFilter( null, -1f, null) { + public static PluginLimits pluginLimitsWithIncludesExcludes(List includes, List excludes) { + return pluginLimitsWithIncludesExcludesWithPercentile( + includes, + excludes, + userId(0)); + } + + public static PluginLimits pluginLimitsWithIncludesExcludesWithPercentile(List includes, List excludes, UserId userId) { + return new PluginLimits( + "jedis", + Enabled.ALL, // ignore + -1, // ignore + 1f, // ratio 100% + includes, + excludes, + userId); + } + + public static EventNameFilter eventNameFilter(final boolean isMatching, boolean isIncludedRatio, boolean isExcludedRatio) { + return eventNameFilter(isMatching, isIncludedRatio, isExcludedRatio, Integer.MAX_VALUE); // no daily limit + } + + public static EventNameFilter eventNameFilter(final boolean isMatching, boolean isIncludedRatio, boolean isExcludedRatio, int dailyLimit) { + return new EventNameFilter( null, -1f, dailyLimit) { @Override public boolean isMatching(Event event) { return isMatching; @@ -56,12 +79,31 @@ public boolean isIncludedByRatio(float percentile) { }; } - public static Filter.EventNameFilter eventNameFilterFakeMatchingWithRatio(float ratio) { - return new Filter.EventNameFilter( null, ratio, null) { + public static EventNameFilter eventNameFilterFakeWithRatio(float ratio) { + return new EventNameFilter( null, ratio, Integer.MAX_VALUE) { @Override public boolean isMatching(Event event) { return true; } }; } + + public static EventNameFilter eventNameWithDailyLimit(int dailyLimit) { + return new EventNameFilter( null, 1, dailyLimit) { + @Override + public boolean isMatching(Event event) { + return true; + } + }; + } + + public static EventPropertyFilter eventProperty() { + return new EventPropertyFilter( null, null) { + @Override + public boolean isMatching(Event event) { + return true; + } + }; + } + } diff --git a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsDailyLimitTest.java b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsDailyLimitTest.java new file mode 100644 index 00000000..6e11b8d8 --- /dev/null +++ b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsDailyLimitTest.java @@ -0,0 +1,66 @@ +/******************************************************************************* + * Copyright (c) 2024 Red Hat, Inc. + * Distributed under license by Red Hat, Inc. All rights reserved. + * This program is made available under the terms of the + * Eclipse Public License v2.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v20.html + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + ******************************************************************************/ +package com.redhat.devtools.intellij.telemetry.core.configuration.limits; + +import com.redhat.devtools.intellij.telemetry.core.service.Event; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.List; + +import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.event; +import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.eventNameWithDailyLimit; +import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.eventProperty; +import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.pluginLimitsWithIncludesExcludes; +import static org.assertj.core.api.Assertions.assertThat; + +public class PluginLimitsDailyLimitTest { + + @Test + public void canSend_returns_true_if_dailyLimit_is_not_reached() { + // given + PluginLimits limits = pluginLimitsWithIncludesExcludes( + List.of(eventNameWithDailyLimit(Integer.MAX_VALUE)), + Collections.emptyList()); + Event event = event(); + // when + boolean canSend = limits.canSend(event,0); + // then + assertThat(canSend).isEqualTo(true); + } + + @Test + public void canSend_returns_false_if_dailyLimit_is_reached() { + // given + PluginLimits limits = pluginLimitsWithIncludesExcludes( + List.of(eventNameWithDailyLimit(1)), + Collections.emptyList()); + Event event = event(); + // when + boolean canSend = limits.canSend(event,1); + // then + assertThat(canSend).isEqualTo(false); + } + + @Test + public void canSend_returns_true_for_property_filter_event_though_dailyLimit_is_reached() { + // given + PluginLimits limits = pluginLimitsWithIncludesExcludes( + List.of(eventProperty()), + Collections.emptyList()); + Event event = event(); + // when + boolean canSend = limits.canSend(event,Integer.MAX_VALUE); + // then + assertThat(canSend).isEqualTo(true); + } + +} diff --git a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsDeserializationTest.java b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsDeserializationTest.java index 5438e8b8..dca9a0c4 100644 --- a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsDeserializationTest.java +++ b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsDeserializationTest.java @@ -267,7 +267,7 @@ public void getIncludes_should_return_1_event_name_filter() throws JsonProcessin } @Test - public void getIncludes_should_return_1_event_name_filter_with_ratio_and_daily_limit() throws JsonProcessingException { + public void getIncludes_should_return_event_name_filter_with_negative_daily_limit_if_daily_limit_is_not_numeric() throws JsonProcessingException { // given String config = "{\n" + @@ -275,8 +275,7 @@ public void getIncludes_should_return_1_event_name_filter_with_ratio_and_daily_l " \"includes\": [\n" + " {\n" + " \"name\" : \"yoda\",\n" + - " \"ratio\" : \"0.544\",\n" + - " \"dailyLimit\" : \"42\"\n" + + " \"dailyLimit\" : \"bogus\"\n" + // not numeric " }\n" + " ]\n" + " }" + @@ -289,11 +288,39 @@ public void getIncludes_should_return_1_event_name_filter_with_ratio_and_daily_l assertThat(filter).isExactlyInstanceOf(Filter.EventNameFilter.class); Filter.EventNameFilter nameFilter = (Filter.EventNameFilter) filter; // when + int dailyLimit = nameFilter.getDailyLimit(); + // then + assertThat(dailyLimit).isEqualTo(-1); + } + + @Test + public void getIncludes_should_return_1_event_name_filter_with_ratio_and_daily_limit() throws JsonProcessingException { + // given + String config = + "{\n" + + " \"*\": {\n" + + " \"includes\": [\n" + + " {\n" + + " \"name\" : \"yoda\",\n" + + " \"ratio\" : \"0.544\",\n" + + " \"dailyLimit\" : \"42\"\n" + + " }\n" + + " ]\n" + + " }" + + "}"; + List limits = PluginLimitsDeserialization.create(config); + PluginLimits limit = limits.get(0); // * + List includes = limit.getIncludes(); + assertThat(includes).hasSize(1); + Filter filter = includes.get(0); + assertThat(filter).isExactlyInstanceOf(Filter.EventNameFilter.class); + Filter.EventNameFilter nameFilter = (Filter.EventNameFilter) filter; + // when float ratio = nameFilter.getRatio(); - String dailyLimit = nameFilter.getDailyLimit(); + int dailyLimit = nameFilter.getDailyLimit(); // then assertThat(ratio).isEqualTo(0.544f); - assertThat(dailyLimit).isEqualTo("42"); + assertThat(dailyLimit).isEqualTo(42); } @Test diff --git a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitRatioTest.java b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsRatioTest.java similarity index 88% rename from src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitRatioTest.java rename to src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsRatioTest.java index ebe7538a..5c970c4a 100644 --- a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitRatioTest.java +++ b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsRatioTest.java @@ -20,11 +20,12 @@ import java.util.stream.Stream; import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.event; -import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.eventNameFilterFakeMatchingWithRatio; +import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.eventNameFilterFakeWithRatio; +import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.pluginLimitsWithIncludesExcludesWithPercentile; import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.userId; import static org.assertj.core.api.Assertions.assertThat; -public class PluginLimitRatioTest { +public class PluginLimitsRatioTest { @ParameterizedTest @MethodSource("canSend_for_include_ratio_and_percentile") @@ -40,7 +41,7 @@ public void canSend_for_given_ratio_in_limits_and_user_percentile(float limitsRa userId(percentile)); Event event = event(); // when - boolean canSend = limits.canSend(event); + boolean canSend = limits.canSend(event, 0); // then assertThat(canSend).isEqualTo(shouldSend); } @@ -55,13 +56,13 @@ public void canSend_for_given_ratio_in_include_filter_and_user_percentile(float -1, // ignore 1f, // ratio 100% List.of( - eventNameFilterFakeMatchingWithRatio(filterRatio) + eventNameFilterFakeWithRatio(filterRatio) ), Collections.emptyList(), userId(percentile)); Event event = event(); // when - boolean canSend = limits.canSend(event); + boolean canSend = limits.canSend(event,0); // then assertThat(canSend).isEqualTo(shouldSend); } @@ -83,19 +84,15 @@ private static Stream canSend_for_include_ratio_and_percentile() { @MethodSource("canSend_for_exclude_ratio_and_percentile") public void canSend_for_given_ratio_in_exclude_filter_and_user_percentile(float filterRatio, float percentile, boolean shouldSend) { // given - PluginLimits limits = new PluginLimits( - "jedis", - Enabled.ALL, // ignore - -1, // ignore - 1f, // ratio 100% + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( Collections.emptyList(), List.of( - eventNameFilterFakeMatchingWithRatio(filterRatio) + eventNameFilterFakeWithRatio(filterRatio) ), userId(percentile)); Event event = event(); // when - boolean canSend = limits.canSend(event); + boolean canSend = limits.canSend(event,0); // then assertThat(canSend).isEqualTo(shouldSend); } @@ -112,5 +109,4 @@ private static Stream canSend_for_exclude_ratio_and_percentile() { Arguments.of(1f, 1f, false) // exclude ratio: 1, percentile: 1 -> false ); } - } diff --git a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitTest.java b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsTest.java similarity index 71% rename from src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitTest.java rename to src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsTest.java index c591b15e..2f6f4833 100644 --- a/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitTest.java +++ b/src/test/java/com/redhat/devtools/intellij/telemetry/core/configuration/limits/PluginLimitsTest.java @@ -19,13 +19,13 @@ import java.util.Map; import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.event; -import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.eventNameFilterFake; -import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.eventNameFilterFakeMatchingWithRatio; +import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.eventNameFilter; +import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.pluginLimitsWithIncludesExcludesWithPercentile; import static com.redhat.devtools.intellij.telemetry.core.configuration.limits.Mocks.userId; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; -public class PluginLimitTest { +public class PluginLimitsTest { @Test public void canSend_should_return_false_if_enabled_is_OFF() { @@ -39,7 +39,7 @@ public void canSend_should_return_false_if_enabled_is_OFF() { Collections.emptyList(), null); // no ratio check // when - boolean canSend = limits.canSend(null); + boolean canSend = limits.canSend(null, Integer.MAX_VALUE); // then assertThat(canSend).isFalse(); } @@ -56,7 +56,7 @@ public void canSend_should_return_false_if_enabled_is_null() { Collections.emptyList(), null); // no ratio check; // when - boolean canSend = limits.canSend(null); + boolean canSend = limits.canSend(null, Integer.MAX_VALUE); // then assertThat(canSend).isFalse(); } @@ -75,7 +75,7 @@ public void canSend_should_return_true_if_enabled_is_CRASH_and_event_has_error() Event error = event( Map.of("error", "anakin turned to the dark side")); // error // when - boolean canSend = limits.canSend(error); + boolean canSend = limits.canSend(error, 0); // then assertThat(canSend).isTrue(); } @@ -93,7 +93,7 @@ public void canSend_should_return_false_if_enabled_is_CRASH_and_event_has_no_err null); // no ratio check Event error = event(); // no error // when - boolean canSend = limits.canSend(error); + boolean canSend = limits.canSend(error, 0); // then assertThat(canSend).isFalse(); } @@ -112,7 +112,7 @@ public void canSend_should_return_true_if_enabled_is_ERROR_and_event_has_error() Event error = event( Map.of("error", "anakin turned to the dark side")); // error // when - boolean canSend = limits.canSend(error); + boolean canSend = limits.canSend(error, 0); // then assertThat(canSend).isTrue(); } @@ -130,7 +130,7 @@ public void canSend_should_return_false_if_enabled_is_ERROR_and_event_has_no_err null); // no ratio check Event event = event(); // no error // when - boolean canSend = limits.canSend(event); + boolean canSend = limits.canSend(event, 0); // then assertThat(canSend).isFalse(); } @@ -139,20 +139,16 @@ public void canSend_should_return_false_if_enabled_is_ERROR_and_event_has_no_err public void canSend_should_return_true_if_event_is_matched_by_inclusion_filter_with_ratio() { // given UserId userId = userId(1f); - PluginLimits limits = new PluginLimits( - "yoda is included", - Enabled.ALL, // all enabled - -1, // ignore - 1f, // ignore + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( List.of( // matching & ratio - eventNameFilterFake(true,true, false) + eventNameFilter(true,true, false) ), Collections.emptyList(), userId); Event event = event(); // no error // when - boolean canSend = limits.canSend(event); + boolean canSend = limits.canSend(event, 0); // then assertThat(canSend).isTrue(); } @@ -161,19 +157,15 @@ public void canSend_should_return_true_if_event_is_matched_by_inclusion_filter_w public void canSend_should_return_false_if_event_is_matched_by_exclusion_filter_with_ratio() { // given UserId userId = userId(1f); - PluginLimits limits = new PluginLimits( - "yoda is excluded", - Enabled.ALL, // all enabled - -1, // ignore - 1f, // ignore + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( Collections.emptyList(), List.of( - eventNameFilterFake(true,false, true) + eventNameFilter(true,false, true) ), userId); Event event = event(); // when - boolean canSend = limits.canSend(event); + boolean canSend = limits.canSend(event, 0); // then assertThat(canSend).isFalse(); } @@ -182,21 +174,17 @@ public void canSend_should_return_false_if_event_is_matched_by_exclusion_filter_ public void canSend_should_return_false_if_event_is_matched_by_inclusion_and_exclusion_filter() { // given UserId userId = userId(1f); - PluginLimits limits = new PluginLimits( - "yoda cannot send", - Enabled.ALL, // all enabled - -1, // ignore - 1f, // ignore + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( List.of( - eventNameFilterFake(true,true, false) + eventNameFilter(true,true, false) ), List.of( - eventNameFilterFake(true,false, true) + eventNameFilter(true,false, true) ), userId); Event event = event(); // when - boolean canSend = limits.canSend(event); + boolean canSend = limits.canSend(event, 0); // then assertThat(canSend).isFalse(); } @@ -204,17 +192,13 @@ public void canSend_should_return_false_if_event_is_matched_by_inclusion_and_exc @Test public void isIncluded_should_return_true_if_there_is_no_include_filter() { // given - PluginLimits limits = new PluginLimits( - null, // ignore - Enabled.ALL, // ignore - -1, // ignore - 1f, // ignore + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( Collections.emptyList(), Collections.emptyList(), mock(UserId.class)); Event event = event(); // when - boolean isIncluded = limits.isIncluded(event); + boolean isIncluded = limits.isIncluded(event, 0); // then assertThat(isIncluded).isTrue(); } @@ -222,20 +206,16 @@ public void isIncluded_should_return_true_if_there_is_no_include_filter() { @Test public void isIncluded_should_return_true_if_there_is_no_matching_include_filter() { // given - PluginLimits limits = new PluginLimits( - null, // ignore - Enabled.ALL, // ignore - -1, // ignore - 1f, // ignore + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( List.of( // is NOT matching - eventNameFilterFake(false,true, false) + eventNameFilter(false,true, false) ), Collections.emptyList(), mock(UserId.class)); Event event = event(); // when - boolean isIncluded = limits.isIncluded(event); + boolean isIncluded = limits.isIncluded(event, 0); // then assertThat(isIncluded).isTrue(); } @@ -243,20 +223,16 @@ public void isIncluded_should_return_true_if_there_is_no_matching_include_filter @Test public void isIncluded_should_return_true_if_event_is_matching_filter_and_is_included() { // given - PluginLimits limits = new PluginLimits( - null, // ignore - Enabled.ALL, // ignore - -1, // ignore - 1f, // ignore + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( List.of( // is matching & is excluded in ratio - eventNameFilterFake(true,true, false) + eventNameFilter(true,true, false) ), Collections.emptyList(), mock(UserId.class)); Event event = event(); // when - boolean isIncluded = limits.isIncluded(event); + boolean isIncluded = limits.isIncluded(event, 0); // then assertThat(isIncluded).isTrue(); } @@ -264,20 +240,50 @@ public void isIncluded_should_return_true_if_event_is_matching_filter_and_is_inc @Test public void isIncluded_should_return_false_if_event_is_matching_filter_but_isnt_included() { // given - PluginLimits limits = new PluginLimits( - null, // ignore - Enabled.ALL, // ignore - -1, // ignore - 1f, // ignore + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( List.of( // is matching & is NOT included in ratio - eventNameFilterFake(true,false, false) + eventNameFilter(true,false, false) ), Collections.emptyList(), mock(UserId.class)); Event event = event(); // when - boolean isIncluded = limits.isIncluded(event); + boolean isIncluded = limits.isIncluded(event, 0); + // then + assertThat(isIncluded).isFalse(); + } + + @Test + public void isIncluded_should_return_true_if_event_is_within_daily_limit() { + // given + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( + List.of( + // is matching & is NOT included in ratio + eventNameFilter(true,true, false, 1) // max 1 daily + ), + Collections.emptyList(), + mock(UserId.class)); + Event event = event(); + // when + boolean isIncluded = limits.isIncluded(event, 0); // 0 events so far + // then + assertThat(isIncluded).isTrue(); + } + + @Test + public void isIncluded_should_return_false_if_daily_limit_reached_already() { + // given + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( + List.of( + // is matching & is NOT included in ratio + eventNameFilter(true,true, false, 1) // max 1 daily + ), + Collections.emptyList(), + mock(UserId.class)); + Event event = event(); + // when + boolean isIncluded = limits.isIncluded(event, 1); // 1 event so far, no room left // then assertThat(isIncluded).isFalse(); } @@ -285,11 +291,7 @@ public void isIncluded_should_return_false_if_event_is_matching_filter_but_isnt_ @Test public void isExcluded_should_return_false_if_there_is_no_filter() { // given - PluginLimits limits = new PluginLimits( - null, // ignore - Enabled.ALL, // ignore - -1, // ignore - 1f, // ignore + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( Collections.emptyList(), Collections.emptyList(), mock(UserId.class)); @@ -303,14 +305,10 @@ public void isExcluded_should_return_false_if_there_is_no_filter() { @Test public void isExcluded_should_return_false_if_there_is_no_matching_filter() { // given - PluginLimits limits = new PluginLimits( - null, // ignore - Enabled.ALL, // ignore - -1, // ignore - 1f, // ignore + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( List.of( // is NOT matching - eventNameFilterFake(false,true, true) + eventNameFilter(false,true, true) ), Collections.emptyList(), mock(UserId.class)); @@ -324,15 +322,11 @@ public void isExcluded_should_return_false_if_there_is_no_matching_filter() { @Test public void isExcluded_should_return_true_if_event_is_matching_filter_and_is_excluded_in_ratio() { // given - PluginLimits limits = new PluginLimits( - null, // ignore - Enabled.ALL, // ignore - -1, // ignore - 1f, // ignore + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( Collections.emptyList(), List.of( // is matching & is excluded in ratio - eventNameFilterFake(true,true, true) + eventNameFilter(true,true, true) ), mock(UserId.class)); Event event = event(); @@ -345,15 +339,11 @@ public void isExcluded_should_return_true_if_event_is_matching_filter_and_is_exc @Test public void isExcluded_should_return_false_if_event_is_matching_filter_and_is_NOT_excluded_in_ratio() { // given - PluginLimits limits = new PluginLimits( - null, // ignore - Enabled.ALL, // ignore - -1, // ignore - 1f, // ignore + PluginLimits limits = pluginLimitsWithIncludesExcludesWithPercentile( Collections.emptyList(), List.of( // is matching & is NOT excluded in ratio - eventNameFilterFake(true,true, false) + eventNameFilter(true,true, false) ), mock(UserId.class)); Event event = event();