Skip to content

Commit

Permalink
Merge pull request #34641 from brunobat/no-remote-config-by-default
Browse files Browse the repository at this point in the history 
Don't load remote build analytics config if user has not accepted
  • Loading branch information
@geoand
geoand authored Jul 14, 2023
2 parents 564e9dc + a3d661a commit 1a9a20d
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 38 deletions.
6 changes: 6 additions & 0 deletions independent-projects/tools/analytics-common/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@
<version>${system-stubs-jupiter.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.awaitility</groupId>
<artifactId>awaitility</artifactId>
<version>${awaitility.version}</version>
<scope>test</scope>
</dependency>
</dependencies>

</project>
80 changes: 58 additions & 22 deletions ...ent-projects/tools/analytics-common/src/main/java/io/quarkus/analytics/ConfigService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ public ConfigService(final ConfigClient client, final AnonymousUserId userId, fi
this.lastRefreshTime = initLastRefreshTime(fileLocations.getRemoteConfigFile());
this.remoteConfigFile = fileLocations.getRemoteConfigFile();
this.localConfigFile = fileLocations.getLocalConfigFile();
loadConfig(RemoteConfig.class, remoteConfigFile)
.ifPresentOrElse(c -> this.config = c, this::loadConfigFromInternet);
}

public void userAcceptance(Function<String, String> analyticsEnabledSupplier) {
Expand Down Expand Up @@ -118,19 +116,41 @@ public void userAcceptance(Function<String, String> analyticsEnabledSupplier) {
* @return true if active
*/
public boolean isActive() {
if (!isLocalConfigActive()) {
if (log.isDebugEnabled()) {
log.debug("[Quarkus build analytics] Local config is not active. Skipping analytics.");
}
return false;
}
AnalyticsRemoteConfig analyticsRemoteConfig = getRemoteConfig();
if (!analyticsRemoteConfig.isActive()) {
if (log.isDebugEnabled()) {
log.debug("[Quarkus build analytics] Remote config is not active. Skipping analytics.");
}
return false;
}
if (!isUserEnabled(analyticsRemoteConfig, userId.getUuid())) {
if (log.isDebugEnabled()) {
log.debug("[Quarkus build analytics] Remote config is not active for anonymous user. " +
"Skipping analytics.");
}
return false;
}
return true;
}

boolean isLocalConfigActive() {
if (getProperty(QUARKUS_ANALYTICS_DISABLED_LOCAL_PROP, false)) {
return false; // disabled by local property
}
if (!Files.exists(localConfigFile)) {
return false; // disabled because user has not decided yet
} else if (!loadConfig(LocalConfig.class, localConfigFile)
.map(localConfig -> !localConfig.isDisabled())
.orElse(true)) {
return false; // disabled by the user and recorded on the local config
}

if (getProperty(QUARKUS_ANALYTICS_DISABLED_LOCAL_PROP, false)) {
return false; // disabled by local property
}
AnalyticsRemoteConfig analyticsRemoteConfig = getRemoteConfig();
return analyticsRemoteConfig.isActive() && isUserEnabled(analyticsRemoteConfig, userId.getUuid());
return true;
}

/**
Expand All @@ -152,6 +172,28 @@ boolean isUserEnabled(final AnalyticsRemoteConfig analyticsRemoteConfig, final S
.noneMatch(uId -> uId.equals(user));
}

AnalyticsRemoteConfig getRemoteConfig() {
try {
if (!isLocalConfigActive()) {
return checkAgainConfig(); // disabled. Will check again in a few hours.
}

if (this.config == null || shouldRefreshRemoteConfig(this.config)) {
this.config = loadConfig(RemoteConfig.class, remoteConfigFile)
.filter(remoteConfig -> !shouldRefreshRemoteConfig(remoteConfig))
.orElseGet(() -> (RemoteConfig) loadConfigFromInternet());
}
return this.config;
} catch (Exception e) {
if (log.isDebugEnabled()) {
log.debug("[Quarkus build analytics] Failed to load remote config. Will check again latter. " +
"Exception: " + e.getMessage());
}
this.config = checkAgainConfig();
return this.config;
}
}

private boolean validInput(String input) {
String[] allowedValues = { "n", "nn", "no", "y", "yy", "yes" };
for (String allowedValue : allowedValues) {
Expand All @@ -162,18 +204,12 @@ private boolean validInput(String input) {
return false;
}

private AnalyticsRemoteConfig getRemoteConfig() {
if (shouldRefresh()) {
loadConfigFromInternet();
}
return this.config;
}

private boolean shouldRefresh() {
return lastRefreshTime == null || Duration.between(
lastRefreshTime,
Instant.now()).compareTo(
this.config.getRefreshInterval()) > 0;
private boolean shouldRefreshRemoteConfig(final AnalyticsRemoteConfig remoteConfig) {
return lastRefreshTime == null ||
Duration.between(
lastRefreshTime,
Instant.now()).compareTo(
remoteConfig.getRefreshInterval()) > 0;
}

private <T> Optional<T> loadConfig(Class<T> clazz, Path file) {
Expand All @@ -188,10 +224,10 @@ private <T> Optional<T> loadConfig(Class<T> clazz, Path file) {
}
}

private void loadConfigFromInternet() {
private AnalyticsRemoteConfig loadConfigFromInternet() {
AnalyticsRemoteConfig analyticsRemoteConfig = this.client.getConfig().orElse(checkAgainConfig());
this.lastRefreshTime = Instant.now();
this.config = storeRemoteConfigOnDisk(analyticsRemoteConfig);
return storeRemoteConfigOnDisk(analyticsRemoteConfig);
}

private AnalyticsRemoteConfig storeRemoteConfigOnDisk(AnalyticsRemoteConfig config) {
Expand Down
8 changes: 5 additions & 3 deletions ...t-projects/tools/analytics-common/src/main/java/io/quarkus/analytics/rest/RestClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,11 @@ Optional<AnalyticsRemoteConfig> getConfig(final URI uri) {
}
return Optional.empty();
} catch (IOException | InterruptedException | ExecutionException | TimeoutException e) {
log.warn("[Quarkus build analytics] Analytics remote config not received. " +
e.getClass().getName() + ": " +
(e.getMessage() == null ? "(no message)" : e.getMessage()));
if (log.isDebugEnabled()) {
log.debug("[Quarkus build analytics] Analytics remote config not received. " +
e.getClass().getName() + ": " +
(e.getMessage() == null ? "(no message)" : e.getMessage()));
}
}
return Optional.empty();
}
Expand Down
6 changes: 6 additions & 0 deletions ...jects/tools/analytics-common/src/test/java/io/quarkus/analytics/AnalyticsServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import static io.quarkus.analytics.dto.segment.ContextBuilder.CommonSystemProperties.GRAALVM_VERSION_VERSION;
import static io.quarkus.analytics.dto.segment.ContextBuilder.CommonSystemProperties.GRADLE_VERSION;
import static io.quarkus.analytics.dto.segment.ContextBuilder.CommonSystemProperties.MAVEN_VERSION;
import static org.awaitility.Awaitility.await;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
Expand All @@ -37,6 +38,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

import org.junit.jupiter.api.AfterAll;
Expand Down Expand Up @@ -153,6 +155,10 @@ void sendAnalyticsTest() throws IOException {
Map.of(),
new File(FILE_LOCATIONS.getFolder().toUri()));
service.close();

await().atMost(5, TimeUnit.SECONDS)
.untilAsserted(() -> wireMockServer.verify(1, postRequestedFor(urlEqualTo("/v1/track"))));

wireMockServer.verify(postRequestedFor(urlEqualTo("/v1/track"))
.withRequestBody(notMatching("null")));
assertTrue(new File(FILE_LOCATIONS.getFolder().toString() + "/" + FILE_LOCATIONS.lastTrackFileName()).exists());
Expand Down
61 changes: 52 additions & 9 deletions ...projects/tools/analytics-common/src/test/java/io/quarkus/analytics/ConfigServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,18 @@ void tearDown() throws IOException {

@Test
void activeWithRemoteConfig() throws IOException {
ConfigService configService = createConfigService();

RemoteConfig remoteConfig = RemoteConfig.builder()
.active(true)
.denyQuarkusVersions(Collections.emptyList())
.denyUserIds(Collections.emptyList())
.refreshInterval(Duration.ofHours(12)).build();
FileUtils.write(remoteConfig, fileLocations.getRemoteConfigFile());
long lastModified = fileLocations.getRemoteConfigFile().toFile().lastModified();

ConfigService configService = createConfigService(remoteConfig);

assertNotNull(configService);
assertTrue(configService.isActive()); // if remote config not found, it will be downloaded (it shouldn't)
assertTrue(configService.isActive(), "Remote config is there and active");
assertEquals(lastModified, fileLocations.getRemoteConfigFile().toFile().lastModified(), "File must not change");
}

Expand Down Expand Up @@ -180,8 +186,10 @@ void userAcceptance_yes() throws IOException {
assertEquals(ACCEPTANCE_PROMPT, s);
return "y";
});
assertTrue(Files.exists(fileLocations.getLocalConfigFile()), "Local config file must be present");

assertTrue(configService.isActive());
assertTrue(Files.exists(fileLocations.getLocalConfigFile()), "Local config file must be present");
assertTrue(Files.exists(fileLocations.getRemoteConfigFile()), "remote config file was not created");
}

@Test
Expand All @@ -194,21 +202,55 @@ void userAcceptance_no() throws IOException {
assertEquals(ACCEPTANCE_PROMPT, s);
return "n";
});
assertTrue(Files.exists(fileLocations.getLocalConfigFile()), "Local config file must be present");
assertFalse(configService.isActive());
assertTrue(Files.exists(fileLocations.getLocalConfigFile()), "Local config file must be present");
assertFalse(Files.exists(fileLocations.getRemoteConfigFile()), "remote config file cannot be present");
}

@Test
void userAcceptance_fail() throws IOException {
void userAcceptance_invalidInput() throws IOException {
deleteLocalConfigFile();

ConfigService configService = createConfigService();

configService.userAcceptance(s -> {
throw new RuntimeException("User input failed");
assertEquals(ACCEPTANCE_PROMPT, s);
return "not valid input";
});
assertFalse(configService.isActive());
assertFalse(Files.exists(fileLocations.getLocalConfigFile()), "Local config file cannot be present");
assertFalse(Files.exists(fileLocations.getRemoteConfigFile()), "remote config file cannot be present");
}

@Test
void noRemoteConfigOnInit() throws IOException {
deleteLocalConfigFile();
assertFalse(Files.exists(fileLocations.getLocalConfigFile()), "Local config file cannot be present");
assertFalse(Files.exists(fileLocations.getRemoteConfigFile()), "remote config file cannot be present");
ConfigService configService = new ConfigService(new TestRestClient(NoopRemoteConfig.INSTANCE),
AnonymousUserId.getInstance(fileLocations, MessageWriter.info()),
fileLocations,
MessageWriter.info());
assertFalse(configService.isActive());
assertFalse(Files.exists(fileLocations.getLocalConfigFile()), "Local config file cannot be present on 1st init");
assertFalse(Files.exists(fileLocations.getRemoteConfigFile()), "remote config file cannot be present on 1st init");
}

@Test
void noRemoteConfigUserDisabledByProp() throws IOException {
deleteLocalConfigFile();
System.setProperty("quarkus.analytics.disabled", "true");
ConfigService configService = createConfigService();

configService.userAcceptance(s -> {
fail("User should not be asked");
return null;
});

assertFalse(configService.isActive());
assertFalse(Files.exists(fileLocations.getLocalConfigFile()), "Local config is present");
assertFalse(Files.exists(fileLocations.getRemoteConfigFile()), "remote config file cannot be present on 1st init");
System.clearProperty("quarkus.analytics.disabled");
}

private void deleteLocalConfigFile() {
Expand All @@ -221,9 +263,10 @@ private ConfigService createConfigService() throws IOException {
.denyQuarkusVersions(Collections.emptyList())
.denyUserIds(Collections.emptyList())
.refreshInterval(Duration.ofHours(12)).build();
return createConfigService(remoteConfig);
}

FileUtils.write(remoteConfig, fileLocations.getRemoteConfigFile());

private ConfigService createConfigService(final RemoteConfig remoteConfig) throws IOException {
ConfigService configService = new ConfigService(new TestRestClient(remoteConfig),
AnonymousUserId.getInstance(fileLocations, MessageWriter.info()),
fileLocations,
Expand Down
21 changes: 17 additions & 4 deletions ...ts/tools/analytics-common/src/test/java/io/quarkus/analytics/rest/RestClientFailTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,18 @@
import static io.quarkus.analytics.common.ContextTestData.createContext;
import static io.quarkus.analytics.rest.RestClient.IDENTITY_ENDPOINT;
import static io.quarkus.analytics.util.StringUtils.getObjectMapper;
import static org.awaitility.Awaitility.await;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.fail;

import java.net.URI;
import java.net.URISyntaxException;
import java.net.http.HttpResponse;
import java.time.Instant;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

Expand Down Expand Up @@ -65,14 +67,25 @@ static void stop() {
}

@Test
void postIdentityServerTTLExceeded()
throws URISyntaxException, JsonProcessingException {
void postIdentityServerTTLExceeded() throws URISyntaxException, JsonProcessingException {
RestClient restClient = new RestClient();
Identity identity = createIdentity();
CompletableFuture<HttpResponse<String>> post = restClient.post(
identity,
new URI("http://localhost:" + MOCK_SERVER_PORT + "/" + IDENTITY_ENDPOINT));
assertThrows(TimeoutException.class, () -> post.get(100, TimeUnit.MILLISECONDS).statusCode());

try {
post.get(100, TimeUnit.MILLISECONDS).statusCode();
fail("Should have thrown an TimeoutException or ExecutionException");
} catch (ExecutionException | TimeoutException e) {
// ok
} catch (Exception e) {
fail("Should have thrown an TimeoutException or ExecutionException");
}

await().atMost(5, TimeUnit.SECONDS)
.untilAsserted(() -> wireMockServer.verify(1, postRequestedFor(urlEqualTo("/" + IDENTITY_ENDPOINT))));

wireMockServer.verify(postRequestedFor(urlEqualTo("/" + IDENTITY_ENDPOINT))
.withRequestBody(equalToJson(getObjectMapper().writeValueAsString(identity))));
}
Expand Down
5 changes: 5 additions & 0 deletions ...ojects/tools/analytics-common/src/test/java/io/quarkus/analytics/rest/RestClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import static io.quarkus.analytics.rest.RestClient.IDENTITY_ENDPOINT;
import static io.quarkus.analytics.rest.RestClient.TRACK_ENDPOINT;
import static io.quarkus.analytics.util.StringUtils.getObjectMapper;
import static org.awaitility.Awaitility.await;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

Expand Down Expand Up @@ -99,6 +100,10 @@ void postIdentity()
CompletableFuture<HttpResponse<String>> post = restClient.post(identity,
new URI("http://localhost:" + MOCK_SERVER_PORT + "/" + IDENTITY_ENDPOINT));
assertEquals(201, post.get(1, TimeUnit.SECONDS).statusCode());

await().atMost(5, TimeUnit.SECONDS)
.untilAsserted(() -> wireMockServer.verify(1, postRequestedFor(urlEqualTo("/" + IDENTITY_ENDPOINT))));

wireMockServer.verify(postRequestedFor(urlEqualTo("/" + IDENTITY_ENDPOINT))
.withRequestBody(equalToJson(getObjectMapper().writeValueAsString(identity))));
}
Expand Down
1 change: 1 addition & 0 deletions independent-projects/tools/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
<maven-model-helper.version>25</maven-model-helper.version>
<jandex.version>3.1.2</jandex.version>
<system-stubs-jupiter.version>2.0.2</system-stubs-jupiter.version>
<awaitility.version>4.2.0</awaitility.version>
</properties>
<modules>
<module>registry-client</module>
Expand Down

0 comments on commit 1a9a20d

Please sign in to comment.