Skip to content

refactor: make it easier to initialize mapper & cloner in subclasses #1403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,40 @@
import io.javaoperatorsdk.operator.ReconcilerUtils;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;

import com.fasterxml.jackson.databind.ObjectMapper;

@SuppressWarnings("rawtypes")
public class AbstractConfigurationService implements ConfigurationService {
private final Map<String, ControllerConfiguration> configurations = new ConcurrentHashMap<>();
private final Version version;
private Cloner cloner;
private ObjectMapper mapper;

public AbstractConfigurationService(Version version) {
this(version, null, null);
}

public AbstractConfigurationService(Version version, Cloner cloner) {
this(version, cloner, null);
}

public AbstractConfigurationService(Version version, Cloner cloner, ObjectMapper mapper) {
this.version = version;
init(cloner, mapper);
}

/**
* Subclasses can call this method to more easily initialize the {@link Cloner} and
* {@link ObjectMapper} associated with this ConfigurationService implementation. This is useful
* in situations where the cloner depends on a mapper that might require additional configuration
* steps before it's ready to be used.
*
* @param cloner the {@link Cloner} instance to be used
* @param mapper the {@link ObjectMapper} instance to be used
*/
protected void init(Cloner cloner, ObjectMapper mapper) {
this.cloner = cloner != null ? cloner : ConfigurationService.super.getResourceCloner();
this.mapper = mapper != null ? mapper : ConfigurationService.super.getObjectMapper();
}

protected <R extends HasMetadata> void register(ControllerConfiguration<R> config) {
Expand Down Expand Up @@ -77,10 +104,12 @@ protected <R extends HasMetadata> String keyFor(Reconciler<R> reconciler) {
return ReconcilerUtils.getNameFor(reconciler);
}

@SuppressWarnings("unused")
protected ControllerConfiguration getFor(String reconcilerName) {
return configurations.get(reconcilerName);
}

@SuppressWarnings("unused")
protected Stream<ControllerConfiguration> controllerConfigurations() {
return configurations.values().stream();
}
Expand All @@ -94,4 +123,14 @@ public Set<String> getKnownReconcilerNames() {
public Version getVersion() {
return version;
}

@Override
public Cloner getResourceCloner() {
return cloner;
}

@Override
public ObjectMapper getObjectMapper() {
return mapper;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;

import com.fasterxml.jackson.databind.ObjectMapper;

public class BaseConfigurationService extends AbstractConfigurationService {

private static final String LOGGER_NAME = "Default ConfigurationService implementation";
Expand All @@ -15,6 +17,10 @@ public BaseConfigurationService(Version version) {
super(version);
}

public BaseConfigurationService(Version version, Cloner cloner, ObjectMapper mapper) {
super(version, cloner, mapper);
}

public BaseConfigurationService() {
this(Utils.loadFromProperties());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.client.utils.Serialization;
import io.javaoperatorsdk.operator.api.monitoring.Metrics;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory;
Expand All @@ -17,20 +18,6 @@
/** An interface from which to retrieve configuration information. */
public interface ConfigurationService {

ObjectMapper OBJECT_MAPPER = new ObjectMapper();

Cloner DEFAULT_CLONER = new Cloner() {
@SuppressWarnings("unchecked")
@Override
public HasMetadata clone(HasMetadata object) {
try {
return OBJECT_MAPPER.readValue(OBJECT_MAPPER.writeValueAsString(object), object.getClass());
} catch (JsonProcessingException e) {
throw new IllegalStateException(e);
}
}
};

/**
* Retrieves the configuration associated with the specified reconciler
*
Expand Down Expand Up @@ -93,12 +80,25 @@ default int concurrentReconciliationThreads() {
}

/**
* Used to clone custom resources.
* Used to clone custom resources. It is strongly suggested that implementors override this method
* since the default implementation creates a new {@link Cloner} instance each time this method is
* called.
*
* @return the ObjectMapper to use
* @return the configured {@link Cloner}
*/
default Cloner getResourceCloner() {
return DEFAULT_CLONER;
return new Cloner() {
@SuppressWarnings("unchecked")
@Override
public HasMetadata clone(HasMetadata object) {
try {
final var mapper = getObjectMapper();
return mapper.readValue(mapper.writeValueAsString(object), object.getClass());
} catch (JsonProcessingException e) {
throw new IllegalStateException(e);
}
}
};
}

int DEFAULT_TERMINATION_TIMEOUT_SECONDS = 10;
Expand Down Expand Up @@ -126,7 +126,7 @@ default boolean closeClientOnStop() {
}

default ObjectMapper getObjectMapper() {
return OBJECT_MAPPER;
return Serialization.jsonMapper();
}

default DependentResourceFactory dependentResourceFactory() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public ConfigurationServiceOverrider withObjectMapper(ObjectMapper objectMapper)
}

public ConfigurationService build() {
return new BaseConfigurationService(original.getVersion()) {
return new BaseConfigurationService(original.getVersion(), cloner, objectMapper) {
@Override
public Set<String> getKnownReconcilerNames() {
return original.getKnownReconcilerNames();
Expand All @@ -102,11 +102,6 @@ public int concurrentReconciliationThreads() {
return threadNumber;
}

@Override
public Cloner getResourceCloner() {
return cloner;
}

@Override
public int getTerminationTimeoutSeconds() {
return timeoutSeconds;
Expand All @@ -130,11 +125,6 @@ public ExecutorService getExecutorService() {
return super.getExecutorService();
}
}

@Override
public ObjectMapper getObjectMapper() {
return objectMapper;
}
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.apps.Deployment;
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
import io.javaoperatorsdk.operator.sample.standalonedependent.StandaloneDependentTestCustomResource;
import io.javaoperatorsdk.operator.sample.standalonedependent.StandaloneDependentTestCustomResourceSpec;
Expand Down Expand Up @@ -52,7 +52,7 @@ void executeUpdateForTestingCacheUpdateForGetResource() {

awaitForDeploymentReadyReplicas(1);

var clonedCr = ConfigurationService.DEFAULT_CLONER.clone(createdCR);
var clonedCr = ConfigurationServiceProvider.instance().getResourceCloner().clone(createdCR);
clonedCr.getSpec().setReplicaCount(2);
operator.replace(StandaloneDependentTestCustomResource.class, clonedCr);

Expand Down