Skip to content

Commit

Permalink
Address Review Comments:
Browse files Browse the repository at this point in the history
- Refactor Const names in Const.Config
- Update ConfigValidatorUtil log messages
  • Loading branch information
BehnamMozafari committed Jan 23, 2025
1 parent 917b0a1 commit 12fd4b3
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 31 deletions.
6 changes: 3 additions & 3 deletions src/main/java/com/uid2/operator/Const.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ public class Config extends com.uid2.shared.Const.Config {
public static final String MaxInvalidPaths = "logging_limit_max_invalid_paths_per_interval";
public static final String MaxVersionBucketsPerSite = "logging_limit_max_version_buckets_per_site";

public static final String ConfigScanPeriodMs = "config_scan_period_ms";
public static final String IdentityV3 = "identity_v3";
public static final String RemoteConfigFeatureFlag = "remote_config_feature_flag";
public static final String ConfigScanPeriodMsProp = "config_scan_period_ms";
public static final String IdentityV3Prop = "identity_v3";
public static final String RemoteConfigFeatureFlagProp = "remote_config_feature_flag";
}
}
8 changes: 4 additions & 4 deletions src/main/java/com/uid2/operator/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
import java.util.*;
import java.util.function.Supplier;

import static com.uid2.operator.Const.Config.ConfigScanPeriodMs;
import static com.uid2.operator.Const.Config.ConfigScanPeriodMsProp;
import static io.micrometer.core.instrument.Metrics.globalRegistry;

public class Main {
Expand Down Expand Up @@ -282,7 +282,7 @@ private void run() throws Exception {
.put("config", new JsonObject()
.put("path", "conf/feat-flag/feat-flag.json")
.put("format", "json"))
.put(ConfigScanPeriodMs, 60000),
.put(ConfigScanPeriodMsProp, 60000),
""
);

Expand All @@ -296,7 +296,7 @@ private void run() throws Exception {
ConfigService dynamicConfigService = configServiceManagerCompositeFuture.resultAt(0);
ConfigService staticConfigService = configServiceManagerCompositeFuture.resultAt(1);

boolean featureFlag = featureFlagConfigRetriever.getCachedConfig().getJsonObject("remote_config").getBoolean(Const.Config.RemoteConfigFeatureFlag, false);
boolean featureFlag = featureFlagConfigRetriever.getCachedConfig().getJsonObject("remote_config").getBoolean(Const.Config.RemoteConfigFeatureFlagProp, false);

ConfigServiceManager configServiceManager = new ConfigServiceManager(
vertx, dynamicConfigService, staticConfigService, featureFlag);
Expand Down Expand Up @@ -346,7 +346,7 @@ private void run() throws Exception {
private void setupFeatureFlagListener(ConfigServiceManager manager, ConfigRetriever retriever) {
retriever.listen(change -> {
JsonObject newConfig = change.getNewConfiguration();
boolean useDynamicConfig = newConfig.getBoolean(Const.Config.RemoteConfigFeatureFlag, true);
boolean useDynamicConfig = newConfig.getBoolean(Const.Config.RemoteConfigFeatureFlagProp, true);
manager.updateConfigService(useDynamicConfig).onComplete(update -> {
if (update.succeeded()) {
LOGGER.info("Remote config feature flag toggled successfully");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import io.vertx.core.Vertx;
import io.vertx.core.json.JsonObject;

import static com.uid2.operator.Const.Config.ConfigScanPeriodMs;
import static com.uid2.operator.Const.Config.ConfigScanPeriodMsProp;

public class ConfigRetrieverFactory {
public ConfigRetriever create(Vertx vertx, JsonObject bootstrapConfig, String operatorKey) {
Expand All @@ -22,7 +22,7 @@ public ConfigRetriever create(Vertx vertx, JsonObject bootstrapConfig, String op
.setConfig(storeConfig);

ConfigRetrieverOptions retrieverOptions = new ConfigRetrieverOptions()
.setScanPeriod(bootstrapConfig.getLong(ConfigScanPeriodMs))
.setScanPeriod(bootstrapConfig.getLong(ConfigScanPeriodMsProp))
.addStore(storeOptions);

return ConfigRetriever.create(vertx, retrieverOptions);
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/uid2/operator/service/ConfigService.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,14 @@ private JsonObject configValidationHandler(JsonObject config) {
Integer refreshExpiresAfter = config.getInteger(REFRESH_TOKEN_EXPIRES_AFTER_SECONDS);
Integer refreshIdentityAfter = config.getInteger(REFRESH_IDENTITY_TOKEN_AFTER_SECONDS);
Integer maxBidstreamLifetimeSeconds = config.getInteger(Const.Config.MaxBidstreamLifetimeSecondsProp, identityExpiresAfter);
Integer sharingTokenExpiry = config.getInteger(Const.Config.SharingTokenExpiryProp);

isValid &= validateIdentityRefreshTokens(identityExpiresAfter, refreshExpiresAfter, refreshIdentityAfter);

isValid &= validateBidstreamLifetime(maxBidstreamLifetimeSeconds, identityExpiresAfter);

isValid &= validateSharingTokenExpiry(sharingTokenExpiry);

if (!isValid) {
logger.error("Failed to update config");
JsonObject lastConfig = this.getConfig();
Expand Down
23 changes: 16 additions & 7 deletions src/main/java/com/uid2/operator/service/ConfigValidatorUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,41 +4,50 @@
import org.slf4j.LoggerFactory;

import static com.uid2.operator.Const.Config.MaxBidstreamLifetimeSecondsProp;
import static com.uid2.operator.Const.Config.SharingTokenExpiryProp;
import static com.uid2.operator.service.UIDOperatorService.*;

public class ConfigValidatorUtil {
private static final Logger logger = LoggerFactory.getLogger(ConfigValidatorUtil.class);
public static final String VALUES_ARE_NULL = "Required config values are null";
public static final String VALUES_ARE_NULL = "One or more of the following required config values are null: ";

public static Boolean validateIdentityRefreshTokens(Integer identityExpiresAfter, Integer refreshExpiresAfter, Integer refreshIdentityAfter) {
boolean isValid = true;

if (areValuesNull(identityExpiresAfter, refreshExpiresAfter, refreshIdentityAfter)) {
logger.error(VALUES_ARE_NULL);
logger.error(VALUES_ARE_NULL + IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS + ", " + REFRESH_TOKEN_EXPIRES_AFTER_SECONDS + ", " + REFRESH_IDENTITY_TOKEN_AFTER_SECONDS);
return false;
}

if (refreshExpiresAfter < identityExpiresAfter) {
logger.error(REFRESH_TOKEN_EXPIRES_AFTER_SECONDS + " must be >= " + IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS);
logger.error(REFRESH_TOKEN_EXPIRES_AFTER_SECONDS + " ({}) < " + IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS + " ({})", refreshExpiresAfter, identityExpiresAfter);
isValid = false;
}
if (identityExpiresAfter < refreshIdentityAfter) {
logger.error(IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS + " must be >= " + REFRESH_IDENTITY_TOKEN_AFTER_SECONDS);
logger.error(IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS + " ({}) < " + REFRESH_IDENTITY_TOKEN_AFTER_SECONDS + " ({})", identityExpiresAfter, refreshIdentityAfter);
isValid = false;
}
if (refreshExpiresAfter < refreshIdentityAfter) {
logger.error(REFRESH_TOKEN_EXPIRES_AFTER_SECONDS + " must be >= " + REFRESH_IDENTITY_TOKEN_AFTER_SECONDS);
logger.error(REFRESH_TOKEN_EXPIRES_AFTER_SECONDS + " ({}) < " + REFRESH_IDENTITY_TOKEN_AFTER_SECONDS + " ({})", refreshExpiresAfter, refreshIdentityAfter);
}
return isValid;
}

public static Boolean validateBidstreamLifetime(Integer maxBidstreamLifetimeSeconds, Integer identityTokenExpiresAfterSeconds) {
if (areValuesNull(maxBidstreamLifetimeSeconds, identityTokenExpiresAfterSeconds)) {
logger.error(VALUES_ARE_NULL);
logger.error(VALUES_ARE_NULL + MaxBidstreamLifetimeSecondsProp + ", " + IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS);
return false;
}
if (maxBidstreamLifetimeSeconds < identityTokenExpiresAfterSeconds) {
logger.error(MaxBidstreamLifetimeSecondsProp + " must be >= " + IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS);
logger.error(MaxBidstreamLifetimeSecondsProp + " ({}) < " + IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS + " ({})", maxBidstreamLifetimeSeconds, identityTokenExpiresAfterSeconds);
return false;
}
return true;
}

public static Boolean validateSharingTokenExpiry(Integer sharingTokenExpiry) {
if (areValuesNull(sharingTokenExpiry)) {
logger.error(VALUES_ARE_NULL + SharingTokenExpiryProp);
return false;
}
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public UIDOperatorVerticle(IConfigService configService,
this.optOutStatusApiEnabled = config.getBoolean(Const.Config.OptOutStatusApiEnabled, true);
this.optOutStatusMaxRequestSize = config.getInteger(Const.Config.OptOutStatusMaxRequestSize, 5000);
this.allowLegacyAPI = config.getBoolean(Const.Config.AllowLegacyAPIProp, true);
this.identityV3Enabled = config.getBoolean(IdentityV3, false);
this.identityV3Enabled = config.getBoolean(IdentityV3Prop, false);
}

@Override
Expand Down
9 changes: 5 additions & 4 deletions src/test/java/com/uid2/operator/ConfigServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@ void setUp() {
.put("host", "localhost")
.put("port", 8088)
.put("path", "/operator/config"))
.put(ConfigScanPeriodMs, 300000);
.put(ConfigScanPeriodMsProp, 300000);

runtimeConfig = new JsonObject()
.put(IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS, 3600)
.put(REFRESH_TOKEN_EXPIRES_AFTER_SECONDS, 7200)
.put(REFRESH_IDENTITY_TOKEN_AFTER_SECONDS, 1800)
.put(MaxBidstreamLifetimeSecondsProp, 7200);
.put(MaxBidstreamLifetimeSecondsProp, 7200)
.put(SharingTokenExpiryProp, 3600);

configRetrieverFactory = new ConfigRetrieverFactory();

Expand Down Expand Up @@ -108,7 +109,7 @@ void testInvalidConfigRevertsToPrevious(VertxTestContext testContext) {
JsonObject jsonBootstrapConfig = new JsonObject()
.put("type", "json")
.put("config", invalidConfig)
.put(ConfigScanPeriodMs, -1);
.put(ConfigScanPeriodMsProp, -1);
ConfigRetriever spyRetriever = spy(configRetrieverFactory.create(vertx, jsonBootstrapConfig, ""));
when(spyRetriever.getCachedConfig()).thenReturn(lastConfig);
ConfigService.create(spyRetriever)
Expand All @@ -128,7 +129,7 @@ void testFirstInvalidConfigThrowsRuntimeException(VertxTestContext testContext)
JsonObject jsonBootstrapConfig = new JsonObject()
.put("type", "json")
.put("config", invalidConfig)
.put(ConfigScanPeriodMs, -1);
.put(ConfigScanPeriodMsProp, -1);
ConfigRetriever configRetriever = configRetrieverFactory.create(vertx, jsonBootstrapConfig, "");
ConfigService.create(configRetriever)
.onComplete(testContext.failing(throwable -> {
Expand Down
14 changes: 7 additions & 7 deletions src/test/java/com/uid2/operator/UIDOperatorServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static com.uid2.operator.Const.Config.IdentityV3;
import static com.uid2.operator.Const.Config.IdentityV3Prop;
import static org.junit.jupiter.api.Assertions.*;

import org.junit.jupiter.params.ParameterizedTest;
Expand Down Expand Up @@ -91,7 +91,7 @@ void setup() throws Exception {
uid2Config.put(UIDOperatorService.IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS, IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS);
uid2Config.put(UIDOperatorService.REFRESH_TOKEN_EXPIRES_AFTER_SECONDS, REFRESH_TOKEN_EXPIRES_AFTER_SECONDS);
uid2Config.put(UIDOperatorService.REFRESH_IDENTITY_TOKEN_AFTER_SECONDS, REFRESH_IDENTITY_TOKEN_AFTER_SECONDS);
uid2Config.put(IdentityV3, false);
uid2Config.put(IdentityV3Prop, false);

uid2Service = new ExtendedUIDOperatorService(
optOutStore,
Expand All @@ -100,14 +100,14 @@ void setup() throws Exception {
this.clock,
IdentityScope.UID2,
this.shutdownHandler::handleSaltRetrievalResponse,
uid2Config.getBoolean(IdentityV3)
uid2Config.getBoolean(IdentityV3Prop)
);

euidConfig = new JsonObject();
euidConfig.put(UIDOperatorService.IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS, IDENTITY_TOKEN_EXPIRES_AFTER_SECONDS);
euidConfig.put(UIDOperatorService.REFRESH_TOKEN_EXPIRES_AFTER_SECONDS, REFRESH_TOKEN_EXPIRES_AFTER_SECONDS);
euidConfig.put(UIDOperatorService.REFRESH_IDENTITY_TOKEN_AFTER_SECONDS, REFRESH_IDENTITY_TOKEN_AFTER_SECONDS);
euidConfig.put(IdentityV3, true);
euidConfig.put(IdentityV3Prop, true);

euidService = new ExtendedUIDOperatorService(
optOutStore,
Expand All @@ -116,7 +116,7 @@ void setup() throws Exception {
this.clock,
IdentityScope.EUID,
this.shutdownHandler::handleSaltRetrievalResponse,
euidConfig.getBoolean(IdentityV3)
euidConfig.getBoolean(IdentityV3Prop)
);
}

Expand Down Expand Up @@ -783,7 +783,7 @@ void testExpiredSaltsNotifiesShutdownHandler(TestIdentityInputType type, String
this.clock,
IdentityScope.UID2,
this.shutdownHandler::handleSaltRetrievalResponse,
uid2Config.getBoolean(IdentityV3)
uid2Config.getBoolean(IdentityV3Prop)
);

UIDOperatorService euidService = new UIDOperatorService(
Expand All @@ -793,7 +793,7 @@ void testExpiredSaltsNotifiesShutdownHandler(TestIdentityInputType type, String
this.clock,
IdentityScope.EUID,
this.shutdownHandler::handleSaltRetrievalResponse,
euidConfig.getBoolean(IdentityV3)
euidConfig.getBoolean(IdentityV3Prop)
);

when(this.optOutStore.getLatestEntry(any())).thenReturn(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ private void setupConfig(JsonObject config) {
config.put(Const.Config.SharingTokenExpiryProp, 60 * 60 * 24 * 30);

config.put("identity_scope", getIdentityScope().toString());
config.put(Const.Config.IdentityV3, useRawUidV3());
config.put(Const.Config.IdentityV3Prop, useRawUidV3());
config.put("client_side_token_generate", true);
config.put("key_sharing_endpoint_provide_app_names", true);
config.put("client_side_token_generate_log_invalid_http_origins", true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
import java.util.List;
import java.util.Random;

import static com.uid2.operator.Const.Config.IdentityV3;
import static com.uid2.operator.Const.Config.IdentityV3Prop;

public class BenchmarkCommon {

Expand Down Expand Up @@ -77,7 +77,7 @@ static IUIDOperatorService createUidOperatorService() throws Exception {
Clock.systemUTC(),
IdentityScope.UID2,
null,
config.getBoolean(IdentityV3)
config.getBoolean(IdentityV3Prop)
);
}

Expand Down

0 comments on commit 12fd4b3

Please sign in to comment.