APPENG-656: change twContext.put() to always update values

CHANGELOG.md

@@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres
 to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.0.0] - 2023-09-22
+
+### Changed
+* When using `twContext.put()` it now always stores the new value to attributes. It no longer checks if old value is not equal to the new one.
+  This is needed to support use cases where the objects might be equal (like an empty map), but we want to create a new separate object anyway in the sub contexts.
+* As a result also change the `TwContextAttributeChangeListener` to `TwContextAttributePutListener`.
+  `TwContextAttributePutListener` is now always called when the `twContext.put()` is called regardless if the value changed or not. This also enables wider use of the interface.
+
 ## [0.12.2] - 2023-08-03
 
 ### Bumped

docs/index.md

@@ -12,7 +12,7 @@ Many applications have multiple owners - on endpoints, jobs and other units of w
 
 It is useful to correlate Rollbar errors, logs and even metrics by specific owners.
 
-`com.transferwise.common.context.TwContextAttributeChangeListenerTest.ownerCanBeSetWhenNameIsChanged` describes how
+`com.transferwise.common.context.TwContextAttributePutListenerTest.ownerCanBeSetWhenNameIsChanged` describes how
 to make the application set the owner field.
 
 But for services, to set an owner it is recommended to use `tw-context-ownership-starter` module instead.

gradle.properties

@@ -1 +1 @@
-version=0.12.2
+version=1.0.0

tw-context-ownership-starter/src/main/java/com/transferwise/common/context/ownership/EntryPointOwnerAttributesChangeListener.java → tw-context-ownership-starter/src/main/java/com/transferwise/common/context/ownership/EntryPointOwnerAttributesPutListener.java

@@ -1,17 +1,18 @@
 package com.transferwise.common.context.ownership;
 
 import com.transferwise.common.context.TwContext;
-import com.transferwise.common.context.TwContextAttributeChangeListener;
+import com.transferwise.common.context.TwContextAttributePutListener;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.tuple.Pair;
 import org.springframework.beans.factory.annotation.Autowired;
 
 @Slf4j
-public class EntryPointOwnerAttributesChangeListener implements TwContextAttributeChangeListener {
+public class EntryPointOwnerAttributesPutListener implements TwContextAttributePutListener {
 
   @Autowired
   private EntryPointOwnerProviderRegistry entryPointOwnerProviderRegistry;
@@ -31,7 +32,10 @@ public boolean removeEldestEntry(Map.Entry<Pair<String, String>, Boolean> eldest
   private final Lock defaultOwnersLock = new ReentrantLock();
 
   @Override
-  public void attributeChanged(TwContext context, String key, Object oldValue, Object newValue) {
+  public void attributePut(TwContext context, String key, Object oldValue, Object newValue) {
+    if (Objects.equals(oldValue, newValue)) {
+      return;
+    }
     if (TwContext.NAME_KEY.equals(key)) {
       String epGroup = context.getGroup();
       String epName = context.getName();

tw-context-ownership-starter/src/main/java/com/transferwise/common/context/ownership/TwContextOwnershipAutoConfiguration.java

@@ -1,7 +1,7 @@
 package com.transferwise.common.context.ownership;
 
 import com.transferwise.common.context.TwContext;
-import com.transferwise.common.context.TwContextAttributeChangeListener;
+import com.transferwise.common.context.TwContextAttributePutListener;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.context.properties.ConfigurationProperties;
@@ -18,9 +18,9 @@ public EntryPointOwnerProviderRegistry entryPointOwnerProviderRegistry() {
   }
 
   @Bean
-  public TwContextAttributeChangeListener twContextOwnershipAttributesChangeListener() {
-    EntryPointOwnerAttributesChangeListener listener = new EntryPointOwnerAttributesChangeListener();
-    TwContext.addAttributeChangeListener(listener);
+  public TwContextAttributePutListener twContextOwnershipAttributesPutListener() {
+    EntryPointOwnerAttributesPutListener listener = new EntryPointOwnerAttributesPutListener();
+    TwContext.addAttributePutListener(listener);
     return listener;
   }
 

tw-context-ownership-starter/src/test/java/com/transferwise/common/context/ownership/OwnershipIntTest.java

@@ -17,7 +17,7 @@
 public class OwnershipIntTest {
 
   @Autowired
-  private EntryPointOwnerAttributesChangeListener entryPointOwnerAttributesChangeListener;
+  private EntryPointOwnerAttributesPutListener entryPointOwnerAttributesPutListener;
 
   @Test
   public void ownerShipIsMappedByConfiguration() {
@@ -33,11 +33,11 @@ public void ownerShipIsMappedByConfiguration() {
 
   @Test
   void entrypointsWithoutOwnerShouldBeLoggedOnce() {
-    entryPointOwnerAttributesChangeListener.clearDefaultOwners();
+    entryPointOwnerAttributesPutListener.clearDefaultOwners();
 
     TwContext twContext = TwContext.current().createSubContext().asEntryPoint("Jobs", "testJob1");
 
-    Logger logger = (Logger) LoggerFactory.getLogger(EntryPointOwnerAttributesChangeListener.class);
+    Logger logger = (Logger) LoggerFactory.getLogger(EntryPointOwnerAttributesPutListener.class);
     ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
     listAppender.start();
     logger.addAppender(listAppender);

tw-context/src/main/java/com/transferwise/common/context/TwContext.java

@@ -7,7 +7,6 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Function;
@@ -35,7 +34,7 @@ public class TwContext {
 
   private static final ThreadLocal<Optional<TwContext>> contextTl = new ThreadLocal<>();
   private static final List<TwContextExecutionInterceptor> interceptors = new CopyOnWriteArrayList<>();
-  private static final List<TwContextAttributeChangeListener> attributeChangeListeners = new CopyOnWriteArrayList<>();
+  private static final List<TwContextAttributePutListener> attributePutListeners = new CopyOnWriteArrayList<>();
   private static final TwContext ROOT_CONTEXT = new TwContext(null, true);
   private static final RateLimiter throwableLoggingRateLimiter = RateLimiter.create(2);
 
@@ -57,17 +56,17 @@ public static List<TwContextExecutionInterceptor> getExecutionInterceptors() {
     return interceptors;
   }
 
-  public static void addAttributeChangeListener(@NonNull TwContextAttributeChangeListener listener) {
-    attributeChangeListeners.add(listener);
+  public static void addAttributePutListener(@NonNull TwContextAttributePutListener listener) {
+    attributePutListeners.add(listener);
   }
 
-  public static boolean removeAttributeChangeListener(TwContextAttributeChangeListener listener) {
-    return attributeChangeListeners.remove(listener);
+  public static boolean removeAttributePutListener(TwContextAttributePutListener listener) {
+    return attributePutListeners.remove(listener);
   }
 
   @SuppressFBWarnings(value = "MS", justification = "Performance")
-  public static List<TwContextAttributeChangeListener> getAttributeChangeListeners() {
-    return attributeChangeListeners;
+  public static List<TwContextAttributePutListener> getAttributePutListeners() {
+    return attributePutListeners;
   }
 
   public static void putCurrentMdc(@NonNull String key, String value) {
@@ -204,17 +203,16 @@ public TwContext put(String key, Object newValue) {
     }
     Object oldValue = attributes.get(key);
 
-    if (!Objects.equals(oldValue, newValue)) {
-      if (newValue == null) {
-        attributes.remove(key);
-        newAttributes.remove(key);
-      } else {
-        attributes.put(key, newValue);
-        newAttributes.put(key, newValue);
-      }
-      fireAttributeChangeEvent(key, oldValue, newValue);
+    if (newValue == null) {
+      attributes.remove(key);
+      newAttributes.remove(key);
+    } else {
+      attributes.put(key, newValue);
+      newAttributes.put(key, newValue);
     }
 
+    fireAttributePutEvent(key, oldValue, newValue);
+
     return this;
   }
 
@@ -306,15 +304,15 @@ private <T> T executeWithInterceptors(Supplier<T> supplier, Iterator<TwContextEx
     return supplier.get();
   }
 
-  private void fireAttributeChangeEvent(String key, Object oldValue, Object newValue) {
-    if (attributeChangeListeners != null) {
+  private void fireAttributePutEvent(String key, Object oldValue, Object newValue) {
+    if (attributePutListeners != null) {
       try {
-        attributeChangeListeners.forEach((l) -> l.attributeChanged(this, key, oldValue, newValue));
+        attributePutListeners.forEach((l) -> l.attributePut(this, key, oldValue, newValue));
       } catch (Throwable t) {
         // This is just a safety net, every listener needs to be bullet-proof by themselves.
         if (throwableLoggingRateLimiter.tryAcquire()) {
           // Don't log value, could be PII.
-          log.error("Attribute change listener failed for key '" + key + "'.", t);
+          log.error("Attribute put listener failed for key '" + key + "'.", t);
         }
       }
     }

tw-context/src/main/java/com/transferwise/common/context/TwContextAttributePutListener.java

@@ -0,0 +1,9 @@
+package com.transferwise.common.context;
+
+import javax.annotation.concurrent.NotThreadSafe;
+
+@NotThreadSafe
+public interface TwContextAttributePutListener {
+
+  void attributePut(TwContext context, String key, Object oldValue, Object newValue);
+}

tw-context/src/test/java/com/transferwise/common/context/TwContextAttributeChangeListenerTest.java → tw-context/src/test/java/com/transferwise/common/context/TwContextAttributePutListenerTest.java

@@ -6,13 +6,13 @@
 import org.junit.jupiter.api.Test;
 import org.slf4j.MDC;
 
-public class TwContextAttributeChangeListenerTest {
+public class TwContextAttributePutListenerTest {
 
   @Test
   void nameAttributeIsChangedAfterGroupAttribute() {
     MutableObject<Boolean> groupChanged = new MutableObject<>(false);
     MutableObject<Boolean> nameChanged = new MutableObject<>(false);
-    TwContextAttributeChangeListener listener = (context, key, oldValue, newValue) -> {
+    TwContextAttributePutListener listener = (context, key, oldValue, newValue) -> {
       if (TwContext.GROUP_KEY.equals(key)) {
         groupChanged.setValue(true);
       } else if (TwContext.NAME_KEY.equals(key)) {
@@ -22,20 +22,20 @@ void nameAttributeIsChangedAfterGroupAttribute() {
         nameChanged.setValue(true);
       }
     };
-    TwContext.addAttributeChangeListener(listener);
+    TwContext.addAttributePutListener(listener);
     try {
       TwContext.current().createSubContext().asEntryPoint("SRE", "Task123");
 
       assertThat(nameChanged.getValue()).isTrue();
       assertThat(groupChanged.getValue()).isTrue();
     } finally {
-      TwContext.removeAttributeChangeListener(listener);
+      TwContext.removeAttributePutListener(listener);
     }
   }
 
   @Test
   void ownerCanBeSetWhenNameIsChanged() {
-    TwContextAttributeChangeListener listener = (context, key, oldValue, newValue) -> {
+    TwContextAttributePutListener listener = (context, key, oldValue, newValue) -> {
       // Here we are making an assumption (covered by test in TwContext), that the name key is always changed after group key.
       // We could remove that assumption by deciding owner both on group key change and on name key change, but that would incur double work.
       if (TwContext.NAME_KEY.equals(key)) {
@@ -47,7 +47,7 @@ void ownerCanBeSetWhenNameIsChanged() {
       }
     };
 
-    TwContext.addAttributeChangeListener(listener);
+    TwContext.addAttributePutListener(listener);
     try {
       TwContext twContext = TwContext.current().createSubContext().asEntryPoint("Test", "/ep1");
       assertThat(twContext.getOwner()).isEqualTo("Kristo");
@@ -59,7 +59,7 @@ void ownerCanBeSetWhenNameIsChanged() {
         assertThat(MDC.get(TwContext.MDC_KEY_EP_OWNER)).isEqualTo("Yurii");
       });
     } finally {
-      TwContext.removeAttributeChangeListener(listener);
+      TwContext.removeAttributePutListener(listener);
     }
   }
 }