Skip to content

Commit

Permalink
chore: various improvements as suggested by sonar (#674)
Browse files Browse the repository at this point in the history
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
  • Loading branch information
Kavindu-Dodan authored Feb 15, 2024
1 parent 7e029a2 commit 07eb45a
Show file tree
Hide file tree
Showing 13 changed files with 82 additions and 180 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
* gRPC channel builder helper.
*/
public class ChannelBuilder {

private ChannelBuilder() {
}

/**
* This method is a helper to build a {@link ManagedChannel} from provided {@link FlagdOptions}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
* Utils for flagd resolvers.
*/
public class Util {

private Util() {
}

/**
* A helper to block the caller for given conditions.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ private void handleProviderReadyEvent() {

private void handleEndOfStream() {
synchronized (this.sync) {
this.sync.notify();
this.sync.notifyAll();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ public class GrpcConnector {
private final Object sync = new Object();
private final AtomicBoolean connected = new AtomicBoolean(false);
private final Random random = new Random();
private final int maxJitter = 100;

private final ServiceGrpc.ServiceBlockingStub serviceBlockingStub;
private final ServiceGrpc.ServiceStub serviceStub;
Expand Down Expand Up @@ -129,19 +128,19 @@ private void observeEventStream() {
// Interruptions are considered end calls for this observer, hence log and return
// Note - this is the most common interruption when shutdown, hence the log level debug
log.debug("interruption while waiting for condition", e);
return;
Thread.currentThread().interrupt();
}

this.eventStreamAttempt++;
// backoff with a jitter
this.eventStreamRetryBackoff = 2 * this.eventStreamRetryBackoff + random.nextInt(maxJitter);
this.eventStreamRetryBackoff = 2 * this.eventStreamRetryBackoff + random.nextInt(100);

try {
Thread.sleep(this.eventStreamRetryBackoff);
} catch (InterruptedException e) {
// Interruptions are considered end calls for this observer, hence log and return
log.warn("interrupted while restarting gRPC Event Stream");
return;
Thread.currentThread().interrupt();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/
@Slf4j
public class Cache {
private Map<String, ProviderEvaluation<? extends Object>> store;
private Map<String, ProviderEvaluation<?>> store;

@Getter
private final Boolean enabled;
Expand All @@ -39,11 +39,11 @@ public Cache(final String forType, int maxCacheSize) {
}
}

public void put(String key, ProviderEvaluation<? extends Object> value) {
public void put(String key, ProviderEvaluation<?> value) {
this.store.put(key, value);
}

public ProviderEvaluation<? extends Object> get(String key) {
public ProviderEvaluation<?> get(String key) {
return this.store.get(key);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
* Factory to create a ResolveStrategy.
*/
public final class ResolveFactory {

private ResolveFactory() {
}

/**
* Factory method to initialize the resolving strategy.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public void init() throws Exception {
}
} catch (InterruptedException e) {
log.warn("Storage state watcher interrupted", e);
Thread.currentThread().interrupt();
}
});
stateWatcher.setDaemon(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@ public class FlagParser {
private static final String SCHEMA_RESOURCE = "flagd-definitions.json";

private static final ObjectMapper MAPPER = new ObjectMapper();

private static final Map<String, Pattern> PATTERN_MAP = new HashMap<>();
private static final Pattern REG_BRACKETS = Pattern.compile("^[^{]*\\{|}[^}]*$");

private static JsonSchema SCHEMA_VALIDATOR;

private FlagParser() {
}

static {
try (InputStream schema = FlagParser.class.getClassLoader().getResourceAsStream(SCHEMA_RESOURCE)) {
if (schema == null) {
Expand Down Expand Up @@ -110,8 +111,9 @@ private static String transposeEvaluators(final String configuration) throws IOE

while (evalFields.hasNext()) {
final String evalName = evalFields.next();
// first replace brackets
final String replacer = REG_BRACKETS.matcher(evaluators.get(evalName).toString()).replaceAll("");
// first replace outmost brackets
final String evaluator = evaluators.get(evalName).toString();
final String replacer = evaluator.substring(1, evaluator.length() - 1);

final String replacePattern = String.format(REPLACER_FORMAT, evalName);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public void init() throws Exception {
streamerListener(connector);
} catch (InterruptedException e) {
log.warn("connection listener failed", e);
Thread.currentThread().interrupt();
}
});
streamer.setDaemon(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
public class FileConnector implements Connector {

private static final int POLL_INTERVAL_MS = 5000;
private static final String OFFER_WARN = "Unable to offer file content to queue: queue is full";

private final String flagSourcePath;
private final BlockingQueue<StreamPayload> queue = new LinkedBlockingQueue<>(1);
Expand All @@ -45,7 +46,7 @@ public void init() throws IOException {
// initial read
String flagData = new String(Files.readAllBytes(filePath), StandardCharsets.UTF_8);
if (!queue.offer(new StreamPayload(StreamPayloadType.DATA, flagData))) {
log.warn("Unable to offer file content to queue: queue is full");
log.warn(OFFER_WARN);
}

long lastTS = Files.getLastModifiedTime(filePath).toMillis();
Expand All @@ -58,18 +59,21 @@ public void init() throws IOException {
lastTS = currentTS;
flagData = new String(Files.readAllBytes(filePath), StandardCharsets.UTF_8);
if (!queue.offer(new StreamPayload(StreamPayloadType.DATA, flagData))) {
log.warn("Unable to offer file content to queue: queue is full");
log.warn(OFFER_WARN);
}
}

Thread.sleep(POLL_INTERVAL_MS);
}

log.info("Shutting down file connector.");
} catch (InterruptedException ex) {
log.error("Interrupted while waiting for polling", ex);
Thread.currentThread().interrupt();
} catch (Throwable t) {
log.error("Error from file connector. File connector will exit", t);
if (!queue.offer(new StreamPayload(StreamPayloadType.ERROR, t.toString()))) {
log.warn("Unable to offer file content to queue: queue is full");
log.warn(OFFER_WARN);
}
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public void init() {
observeEventStream(blockingQueue, shutdown, serviceStub, requestBuilder.build());
} catch (InterruptedException e) {
log.warn("gRPC event stream interrupted, flag configurations are stale", e);
Thread.currentThread().interrupt();
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ public class FlagdOptionsTest {
public void TestDefaults() {
final FlagdOptions builder = FlagdOptions.builder().build();

assertEquals(builder.getHost(), DEFAULT_HOST);
assertEquals(Integer.toString(builder.getPort()), DEFAULT_PORT);
assertEquals(DEFAULT_HOST, builder.getHost());
assertEquals(DEFAULT_PORT, Integer.toString(builder.getPort()));
assertFalse(builder.isTls());
assertNull(builder.getCertPath());
assertNull(builder.getSocketPath());
assertEquals(builder.getCacheType(), DEFAULT_CACHE);
assertEquals(builder.getMaxCacheSize(), DEFAULT_MAX_CACHE_SIZE);
assertEquals(builder.getMaxEventStreamRetries(), DEFAULT_MAX_EVENT_STREAM_RETRIES);
assertEquals(DEFAULT_CACHE, builder.getCacheType());
assertEquals(DEFAULT_MAX_CACHE_SIZE, builder.getMaxCacheSize());
assertEquals(DEFAULT_MAX_EVENT_STREAM_RETRIES, builder.getMaxEventStreamRetries());
assertNull(builder.getSelector());
assertNull(builder.getOpenTelemetry());
assertNull(builder.getOfflineFlagSourcePath());
Expand All @@ -50,15 +50,15 @@ public void TestBuilderOptions() {
.openTelemetry(openTelemetry)
.build();

assertEquals(flagdOptions.getHost(), "https://hosted-flagd");
assertEquals(flagdOptions.getPort(), 80);
assertEquals("https://hosted-flagd", flagdOptions.getHost());
assertEquals(80, flagdOptions.getPort());
assertTrue(flagdOptions.isTls());
assertEquals(flagdOptions.getCertPath(), "etc/cert/ca.crt");
assertEquals(flagdOptions.getCacheType(), "lru");
assertEquals(flagdOptions.getMaxCacheSize(), 100);
assertEquals(flagdOptions.getMaxEventStreamRetries(), 1);
assertEquals(flagdOptions.getSelector(), "app=weatherApp");
assertEquals("etc/cert/ca.crt", flagdOptions.getCertPath());
assertEquals("lru", flagdOptions.getCacheType());
assertEquals(100, flagdOptions.getMaxCacheSize());
assertEquals(1, flagdOptions.getMaxEventStreamRetries());
assertEquals("app=weatherApp", flagdOptions.getSelector());
assertEquals("some-path", flagdOptions.getOfflineFlagSourcePath());
assertEquals(flagdOptions.getOpenTelemetry(), openTelemetry);
assertEquals(openTelemetry, flagdOptions.getOpenTelemetry());
}
}
Loading

0 comments on commit 07eb45a

Please sign in to comment.