Skip to content

Commit

Permalink
Refactor internals to replace large arg lists with "options" class (#301
Browse files Browse the repository at this point in the history
)

Note specifically that NO public APIs are affected.

* Introduce ErrorContextOptions class
* Refactor code to use the options class instead of large argument lists
* Add Checker Nullable annotation to methods in ErrorContextUtilities

Closes #300
  • Loading branch information
sleberknight authored Oct 25, 2023
1 parent a55175c commit 81a5f4b
Show file tree
Hide file tree
Showing 10 changed files with 251 additions and 142 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -335,18 +335,7 @@ public ErrorContext buildWithJdbi3(Jdbi jdbi) {
}

private Jdbi3ErrorContext newJdbi3ErrorContext(Jdbi jdbi) {
return new Jdbi3ErrorContext(
environment,
serviceDetails,
jdbi,
dataStoreType,
addErrorsResource,
addGotErrorsResource,
addHealthCheck,
timeWindowValue,
timeWindowUnit,
addCleanupJob,
cleanupConfig);
return new Jdbi3ErrorContext(environment, serviceDetails, jdbi, buildOptions());
}

/**
Expand Down Expand Up @@ -381,17 +370,19 @@ public ErrorContext buildWithConcurrentMapDao() {
* @return a new {@link ErrorContext} instance
*/
public ErrorContext buildWithDao(ApplicationErrorDao errorDao) {
return new SimpleErrorContext(
environment,
serviceDetails,
errorDao,
dataStoreType,
addErrorsResource,
addGotErrorsResource,
addHealthCheck,
timeWindowValue,
timeWindowUnit,
addCleanupJob,
cleanupConfig);
return new SimpleErrorContext(environment, serviceDetails, errorDao, buildOptions());
}

private ErrorContextOptions buildOptions() {
return ErrorContextOptions.builder()
.dataStoreType(dataStoreType)
.addErrorsResource(addErrorsResource)
.addGotErrorsResource(addGotErrorsResource)
.addHealthCheck(addHealthCheck)
.timeWindowValue(timeWindowValue)
.timeWindowValue(timeWindowValue)
.addCleanupJob(addCleanupJob)
.cleanupConfig(cleanupConfig)
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package org.kiwiproject.dropwizard.error;

import lombok.Builder;
import lombok.Getter;
import lombok.NonNull;

import org.kiwiproject.dropwizard.error.config.CleanupConfig;
import org.kiwiproject.dropwizard.error.health.TimeWindow;
import org.kiwiproject.dropwizard.error.model.DataStoreType;

import java.time.temporal.ChronoUnit;
import java.time.temporal.TemporalUnit;

/**
* Contains options common to different types of {@link ErrorContext}.
*
* @implNote This is not public and is subject to change.
*/
@Builder
@Getter
class ErrorContextOptions {

@NonNull
@Builder.Default
private DataStoreType dataStoreType = DataStoreType.SHARED;

@Builder.Default
private boolean addErrorsResource = true;

@Builder.Default
private boolean addGotErrorsResource = true;

@Builder.Default
private boolean addHealthCheck = true;

@Builder.Default
private long timeWindowValue = TimeWindow.DEFAULT_TIME_WINDOW_MINUTES;

@NonNull
@Builder.Default
private TemporalUnit timeWindowUnit = ChronoUnit.MINUTES;

@Builder.Default
private boolean addCleanupJob = true;

@Builder.Default
private CleanupConfig cleanupConfig = new CleanupConfig();
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package org.kiwiproject.dropwizard.error;

import static com.google.common.base.Preconditions.checkArgument;
import static java.util.Objects.requireNonNull;
import static org.kiwiproject.base.KiwiPreconditions.checkArgumentNotNull;

import io.dropwizard.core.setup.Environment;
import lombok.experimental.UtilityClass;
import org.kiwiproject.dropwizard.error.config.CleanupConfig;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.kiwiproject.dropwizard.error.dao.ApplicationErrorDao;
import org.kiwiproject.dropwizard.error.health.RecentErrorsHealthCheck;
import org.kiwiproject.dropwizard.error.job.CleanupApplicationErrorsJob;
Expand All @@ -27,6 +28,18 @@
@UtilityClass
class ErrorContextUtilities {

static void checkCommonArguments(Environment environment,
ServiceDetails serviceDetails,
ErrorContextOptions options) {

checkArgumentNotNull(options, "options cannot be null");
checkCommonArguments(environment,
serviceDetails,
options.getDataStoreType(),
options.getTimeWindowValue(),
options.getTimeWindowUnit());
}

static void checkCommonArguments(Environment environment,
ServiceDetails serviceDetails,
DataStoreType dataStoreType,
Expand All @@ -53,44 +66,50 @@ static PersistentHostInformation setPersistentHostInformationFrom(ServiceDetails

static void registerResources(Environment environment,
ApplicationErrorDao errorDao,
DataStoreType dataStoreType,
boolean addErrorsResource,
boolean addGotErrorsResource) {
ErrorContextOptions options) {

checkArgumentNotNull(environment);
checkArgumentNotNull(errorDao);
checkArgumentNotNull(options);

var dataStoreType = options.getDataStoreType();
checkArgumentNotNull(dataStoreType);

if (addErrorsResource) {
if (options.isAddErrorsResource()) {
environment.jersey().register(new ApplicationErrorResource(errorDao));
}

if (addGotErrorsResource) {
if (options.isAddGotErrorsResource()) {
environment.jersey().register(new GotErrorsResource(dataStoreType));
}
}

static RecentErrorsHealthCheck registerRecentErrorsHealthCheckOrNull(boolean addHealthCheck,
Environment environment,
ApplicationErrorDao errorDao,
@Nullable
static RecentErrorsHealthCheck registerRecentErrorsHealthCheckOrNull(Environment environment,
ServiceDetails serviceDetails,
long timeWindowValue,
TemporalUnit timeWindowUnit) {
ApplicationErrorDao errorDao,
ErrorContextOptions options) {

if (addHealthCheck) {
var healthCheck = new RecentErrorsHealthCheck(errorDao, serviceDetails, timeWindowValue, timeWindowUnit);
if (options.isAddHealthCheck()) {
var healthCheck = new RecentErrorsHealthCheck(errorDao,
serviceDetails,
options.getTimeWindowValue(),
options.getTimeWindowUnit());
environment.healthChecks().register("recentApplicationErrors", healthCheck);
return healthCheck;
}

return null;
}

static CleanupApplicationErrorsJob registerCleanupJobOrNull(boolean addCleanupJob,
Environment environment,
@Nullable
static CleanupApplicationErrorsJob registerCleanupJobOrNull(Environment environment,
ApplicationErrorDao errorDao,
CleanupConfig cleanupConfig) {
if (addCleanupJob) {
ErrorContextOptions options) {

if (options.isAddCleanupJob()) {
var cleanupConfig = requireNonNull(options.getCleanupConfig(),
"cleanupConfig cannot be null when addCleanupJob=true");
var executor = environment.lifecycle()
.scheduledExecutorService(cleanupConfig.getCleanupJobName(), true)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,12 @@
import io.dropwizard.core.setup.Environment;
import org.jdbi.v3.core.Jdbi;
import org.jdbi.v3.core.extension.NoSuchExtensionException;
import org.kiwiproject.dropwizard.error.config.CleanupConfig;
import org.kiwiproject.dropwizard.error.dao.ApplicationErrorDao;
import org.kiwiproject.dropwizard.error.dao.jdbi3.Jdbi3ApplicationErrorDao;
import org.kiwiproject.dropwizard.error.health.RecentErrorsHealthCheck;
import org.kiwiproject.dropwizard.error.model.DataStoreType;
import org.kiwiproject.dropwizard.error.model.ServiceDetails;

import java.time.temporal.TemporalUnit;
import java.util.Optional;

/**
Expand All @@ -43,26 +41,18 @@ class Jdbi3ErrorContext implements ErrorContext {
Jdbi3ErrorContext(Environment environment,
ServiceDetails serviceDetails,
Jdbi jdbi,
DataStoreType dataStoreType,
boolean addErrorsResource,
boolean addGotErrorsResource,
boolean addHealthCheck,
long timeWindowValue,
TemporalUnit timeWindowUnit,
boolean addCleanupJob,
CleanupConfig cleanupConfig) {
ErrorContextOptions options) {

checkCommonArguments(environment, serviceDetails, dataStoreType, timeWindowValue, timeWindowUnit);
checkCommonArguments(environment, serviceDetails, options);
checkArgumentNotNull(jdbi, "Jdbi (version 3) instance cannot be null");
setPersistentHostInformationFrom(serviceDetails);

this.dataStoreType = dataStoreType;
this.dataStoreType = options.getDataStoreType();
this.errorDao = getOnDemandErrorDao(jdbi);
this.healthCheck = registerRecentErrorsHealthCheckOrNull(
addHealthCheck, environment, errorDao, serviceDetails, timeWindowValue, timeWindowUnit);
this.healthCheck = registerRecentErrorsHealthCheckOrNull(environment, serviceDetails, errorDao, options);

registerCleanupJobOrNull(addCleanupJob, environment, errorDao, cleanupConfig);
registerResources(environment, errorDao, dataStoreType, addErrorsResource, addGotErrorsResource);
registerCleanupJobOrNull(environment, errorDao, options);
registerResources(environment, errorDao, options);
}

private static Jdbi3ApplicationErrorDao getOnDemandErrorDao(Jdbi jdbi) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@
import static org.kiwiproject.dropwizard.error.ErrorContextUtilities.setPersistentHostInformationFrom;

import io.dropwizard.core.setup.Environment;
import org.kiwiproject.dropwizard.error.config.CleanupConfig;
import org.kiwiproject.dropwizard.error.dao.ApplicationErrorDao;
import org.kiwiproject.dropwizard.error.health.RecentErrorsHealthCheck;
import org.kiwiproject.dropwizard.error.model.DataStoreType;
import org.kiwiproject.dropwizard.error.model.ServiceDetails;

import java.time.temporal.TemporalUnit;
import java.util.Optional;

/**
Expand All @@ -28,29 +26,21 @@ class SimpleErrorContext implements ErrorContext {
private final DataStoreType dataStoreType;
private final RecentErrorsHealthCheck healthCheck;

public SimpleErrorContext(Environment environment,
ServiceDetails serviceDetails,
ApplicationErrorDao errorDao,
DataStoreType dataStoreType,
boolean addErrorsResource,
boolean addGotErrorsResource,
boolean addHealthCheck,
long timeWindowValue,
TemporalUnit timeWindowUnit,
boolean addCleanupJob,
CleanupConfig cleanupConfig) {
SimpleErrorContext(Environment environment,
ServiceDetails serviceDetails,
ApplicationErrorDao errorDao,
ErrorContextOptions options) {

checkCommonArguments(environment, serviceDetails, dataStoreType, timeWindowValue, timeWindowUnit);
checkCommonArguments(environment, serviceDetails, options);
checkArgumentNotNull(errorDao, "ApplicationErrorDao must not be null");
setPersistentHostInformationFrom(serviceDetails);

this.errorDao = errorDao;
this.dataStoreType = dataStoreType;
this.healthCheck = registerRecentErrorsHealthCheckOrNull(
addHealthCheck, environment, errorDao, serviceDetails, timeWindowValue, timeWindowUnit);
this.dataStoreType = options.getDataStoreType();
this.healthCheck = registerRecentErrorsHealthCheckOrNull(environment, serviceDetails, errorDao, options);

registerCleanupJobOrNull(addCleanupJob, environment, errorDao, cleanupConfig);
registerResources(environment, errorDao, dataStoreType, addErrorsResource, addGotErrorsResource);
registerCleanupJobOrNull(environment, errorDao, options);
registerResources(environment, errorDao, options);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,7 @@ void shouldAllowSkippingResourceRegistration() {
builder.buildWithConcurrentMapDao();

var jersey = environment.jersey();
verify(jersey, never()).register(isA(ApplicationErrorResource.class));
verify(jersey, never()).register(isA(GotErrorsResource.class));
verifyNoMoreInteractions(jersey);
verifyNoInteractions(jersey);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package org.kiwiproject.dropwizard.error;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatNullPointerException;
import static org.junit.jupiter.api.Assertions.assertAll;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.kiwiproject.dropwizard.error.config.CleanupConfig;
import org.kiwiproject.dropwizard.error.health.TimeWindow;
import org.kiwiproject.dropwizard.error.model.DataStoreType;

import java.time.temporal.ChronoUnit;

@DisplayName("ErrorContextOptions")
class ErrorContextOptionsTest {

@Test
void shouldBuildWithDefaultValues() {
var options = ErrorContextOptions.builder().build();

assertAll(
() -> assertThat(options.getDataStoreType()).isEqualTo(DataStoreType.SHARED),
() -> assertThat(options.isAddErrorsResource()).isTrue(),
() -> assertThat(options.isAddGotErrorsResource()).isTrue(),
() -> assertThat(options.isAddHealthCheck()).isTrue(),
() -> assertThat(options.getTimeWindowValue()).isEqualTo(TimeWindow.DEFAULT_TIME_WINDOW_MINUTES),
() -> assertThat(options.getTimeWindowUnit()).isEqualTo(ChronoUnit.MINUTES),
() -> assertThat(options.isAddCleanupJob()).isTrue(),
() -> assertThat(options.getCleanupConfig()).usingRecursiveComparison().isEqualTo(new CleanupConfig())
);
}

@Test
void shouldRequireDataStoreType() {
assertThatNullPointerException()
.isThrownBy(() -> ErrorContextOptions.builder().dataStoreType(null).build());
}

@Test
void shouldRequireTimeWindowUnit() {
assertThatNullPointerException()
.isThrownBy(() -> ErrorContextOptions.builder().timeWindowUnit(null).build());
}
}
Loading

0 comments on commit 81a5f4b

Please sign in to comment.