Skip to content
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

Method signature mismatched with javadoc and implementation #2587

Merged
merged 3 commits into from
Nov 20, 2024
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 @@ -3,11 +3,16 @@
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult;
import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileResult;

@SuppressWarnings("rawtypes")
public class DefaultManagedDependentResourceContext implements ManagedDependentResourceContext {
private static final Logger log =
LoggerFactory.getLogger(DefaultManagedDependentResourceContext.class);
public static final Object RECONCILE_RESULT_KEY = new Object();
public static final Object CLEANUP_RESULT_KEY = new Object();
private final ConcurrentHashMap attributes = new ConcurrentHashMap();
Expand All @@ -22,10 +27,26 @@ public <T> Optional<T> get(Object key, Class<T> expectedType) {
@Override
@SuppressWarnings("unchecked")
public <T> T put(Object key, T value) {
Object previous;
if (value == null) {
return (T) Optional.ofNullable(attributes.remove(key));
return (T) attributes.remove(key);
} else {
previous = attributes.put(key, value);
}

if (previous != null && !previous.getClass().isAssignableFrom(value.getClass())) {
logWarning("Previous value (" + previous +
") for key (" + key +
") was not of type " + value.getClass() +
". This might indicate an issue in your code. If not, use put(" + key +
", null) first to remove the previous value.");
}
return (T) Optional.ofNullable(attributes.put(key, value));
return (T) previous;
}

// only for testing purposes
void logWarning(String message) {
log.warn(message);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,23 @@ public interface ManagedDependentResourceContext {
* the semantics of this operation is defined as removing the mapping associated with the
* specified key.
*
* <p>
* Note that, while implementations shouldn't throw a {@link ClassCastException} when the new
* value type differs from the type of the existing value, calling sites might encounter such
* exceptions if they bind the return value to a specific type. Users are either expected to
* disregard the return value (most common case) or "reset" the value type associated with the
* specified key by first calling {@code put(key, null)} if they want to ensure some level of type
* safety in their code (where attempting to store values of different types under the same key
* might be indicative of an issue).
* </p>
*
* @param <T> object type
* @param key the key identifying which contextual object to add or remove from the context
* @param value the value to add to the context or {@code null} to remove an existing entry
* associated with the specified key
* @return an Optional containing the previous value associated with the key or
* {@link Optional#empty()} if none existed
* @return the previous value if one was associated with the specified key, {@code null}
* otherwise.
*/
@SuppressWarnings("unchecked")
<T> T put(Object key, T value);

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package io.javaoperatorsdk.operator.api.reconciler.dependent.managed;

import java.util.List;
import java.util.Map;
import java.util.Optional;

import org.junit.jupiter.api.Test;

import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileResult;

import static io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext.RECONCILE_RESULT_KEY;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

class DefaultManagedDependentResourceContextTest {

private final ManagedDependentResourceContext context =
new DefaultManagedDependentResourceContext();

@Test
void getWhenEmpty() {
Optional<String> actual = context.get("key", String.class);
assertThat(actual).isEmpty();
}

@Test
void get() {
context.put("key", "value");
Optional<String> actual = context.get("key", String.class);
assertThat(actual).contains("value");
}

@Test
void putNewValueOverwrites() {
context.put("key", "value");
context.put("key", "valueB");
Optional<String> actual = context.get("key", String.class);
assertThat(actual).contains("valueB");
}

@Test
void putNewValueReturnsPriorValue() {
final var prior = "value";
context.put("key", prior);
String actual = context.put("key", "valueB");
assertThat(actual).isEqualTo(prior);
}

@Test
void putNewValueLogsWarningIfTypesDiffer() {
// to check that we properly log things without setting up a complex fixture
final String[] messages = new String[1];
var context = new DefaultManagedDependentResourceContext() {
@Override
void logWarning(String message) {
messages[0] = message;
}
};
final var prior = "value";
final var key = "key";
context.put(key, prior);
context.put(key, 10);
assertThat(messages[0]).contains(key).contains(prior).contains("put(" + key + ", null)");
}

@Test
void putNullRemoves() {
context.put("key", "value");
context.put("key", null);
Optional<String> actual = context.get("key", String.class);
assertThat(actual).isEmpty();
}

@Test
void putNullReturnsPriorValue() {
context.put("key", "value");
String actual = context.put("key", null);
assertThat(actual).contains("value");
}

@Test
void getMandatory() {
context.put("key", "value");
String actual = context.getMandatory("key", String.class);
assertThat(actual).isEqualTo("value");
}

@Test
void getMandatoryWhenEmpty() {
assertThatThrownBy(() -> {
context.getMandatory("key", String.class);
}).isInstanceOf(IllegalStateException.class)
.hasMessage(
"Mandatory attribute (key: key, type: java.lang.String) is missing or not of the expected type");
}

@Test
void getWorkflowReconcileResult() {
WorkflowReconcileResult result =
new WorkflowReconcileResult(List.of(), List.of(), Map.of(), Map.of());
context.put(RECONCILE_RESULT_KEY, result);
Optional<WorkflowReconcileResult> actual = context.getWorkflowReconcileResult();
assertThat(actual).containsSame(result);
}
}
Loading