KEYCLOAK-18414 Remove unnecessary id parameter from update operation

model/map/src/main/java/org/keycloak/models/map/common/MapStorageUtils.java

@@ -37,7 +37,7 @@ public class MapStorageUtils {
      */
     public static <K, V extends AbstractEntity<K>> V registerEntityForChanges(MapKeycloakTransaction<K, V, ?> tx, V origEntity) {
         final V res = tx.read(origEntity.getId(), id -> Serialization.from(origEntity));
-        tx.updateIfChanged(origEntity.getId(), res, AbstractEntity<K>::isUpdated);
+        tx.updateIfChanged(res, AbstractEntity<K>::isUpdated);
         return res;
     }
 }

model/map/src/main/java/org/keycloak/models/map/storage/MapKeycloakTransaction.java

@@ -74,13 +74,12 @@ public interface MapKeycloakTransaction<K, V extends AbstractEntity<K>, M> exten
     long getCount(ModelCriteriaBuilder<M> mcb);
 
     /**
-     * Instructs this transaction to force-update the {@code value} associated with the identifier {@code key} in the
+     * Instructs this transaction to force-update the {@code value} associated with the identifier {@code value.getId()} in the
      * underlying store on commit.
      *
-     * @param key identifier of the {@code value}
      * @param value updated version of the value
      */
-    void update(K key, V value);
+    void update(V value);
 
     /**
      * Returns an updated version of the {@code orig} object as updated in this transaction.
@@ -95,14 +94,14 @@ public interface MapKeycloakTransaction<K, V extends AbstractEntity<K>, M> exten
     }
 
     /**
-     * Instructs this transaction to update the {@code value} associated with the identifier {@code key} in the
+     * Instructs this transaction to update the {@code value} associated with the identifier {@code value.getId()} in the
      * underlying store on commit, if by the time of {@code commit} the {@code shouldPut} predicate returns {@code true}
      *
-     * @param key identifier of the {@code value}
+     * @param value new version of the value. Must not alter the {@code id} of the entity
-     * @param value new version of the value
      * @param shouldPut predicate to check in commit phase
+     * @see AbstractEntity#getId()
      */
-    void updateIfChanged(K key, V value, Predicate<V> shouldPut);
+    void updateIfChanged(V value, Predicate<V> shouldPut);
 
     /**
      * Instructs this transaction to delete a value associated with the identifier {@code key} from the underlying store

model/map/src/main/java/org/keycloak/models/map/storage/MapStorage.java

@@ -83,12 +83,13 @@ public interface MapStorage<K, V extends AbstractEntity<K>, M> {
     long getCount(ModelCriteriaBuilder<M> criteria);
 
     /**
-     * Updates the object with the given {@code id} in the storage if it already exists.
+     * Updates the object with the key of the {@code value}'s ID in the storage if it already exists.
      * @param key Primary key of the object to update
      * @param value Updated value
-     * @throws NullPointerException if object or its {@code id} is {@code null}
+     * @throws NullPointerException if the object or its {@code id} is {@code null}
+     * @see AbstractEntity#getId()
      */
-    V update(K key, V value);
+    V update(V value);
 
     /**
      * Deletes object with the given {@code key} from the storage, if exists, no-op otherwise.

model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapKeycloakTransaction.java

@@ -175,18 +175,20 @@ public class ConcurrentHashMapKeycloakTransaction<K, V extends AbstractEntity<K>
     }
 
     @Override
-    public void update(K key, V value) {
+    public void update(V value) {
-        addTask(key, new UpdateOperation(key, value));
+        K key = value.getId();
+        addTask(key, new UpdateOperation(value));
     }
 
     @Override
     public void create(V value) {
         K key = value.getId();
-        addTask(key, new CreateOperation(key, value));
+        addTask(key, new CreateOperation(value));
     }
 
     @Override
-    public void updateIfChanged(K key, V value, Predicate<V> shouldPut) {
+    public void updateIfChanged(V value, Predicate<V> shouldPut) {
+        K key = value.getId();
         log.tracef("Adding operation UPDATE_IF_CHANGED for %s @ %08x", key, System.identityHashCode(value));
 
         K taskKey = key;
@@ -194,7 +196,7 @@ public class ConcurrentHashMapKeycloakTransaction<K, V extends AbstractEntity<K>
             @Override
             public void execute() {
                 if (shouldPut.test(getValue())) {
-                    map.update(key, getValue());
+                    map.update(getValue());
                 }
             }
             @Override public MapOperation getOperation() { return MapOperation.UPDATE; }
@@ -323,11 +325,8 @@ public class ConcurrentHashMapKeycloakTransaction<K, V extends AbstractEntity<K>
     }
 
     private class CreateOperation extends MapTaskWithValue {
-        private final K key;
+        public CreateOperation(V value) {
-
-        public CreateOperation(K key, V value) {
             super(value);
-            this.key = key;
         }
 
         @Override public void execute() { map.create(getValue()); }
@@ -335,14 +334,11 @@ public class ConcurrentHashMapKeycloakTransaction<K, V extends AbstractEntity<K>
     }
 
     private class UpdateOperation extends MapTaskWithValue {
-        private final K key;
+        public UpdateOperation(V value) {
-
-        public UpdateOperation(K key, V value) {
             super(value);
-            this.key = key;
         }
 
-        @Override public void execute() { map.update(key, getValue()); }
+        @Override public void execute() { map.update(getValue()); }
         @Override public MapOperation getOperation() { return MapOperation.UPDATE; }
     }
 

model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorage.java

@@ -54,6 +54,7 @@ public class ConcurrentHashMapStorage<K, V extends AbstractEntity<K>, M> impleme
 
     @Override
     public V create(V value) {
+        K key = value.getId();
         return store.putIfAbsent(key, value);
     }
 
@@ -64,7 +65,8 @@ public class ConcurrentHashMapStorage<K, V extends AbstractEntity<K>, M> impleme
     }
 
     @Override
-    public V update(K key, V value) {
+    public V update(V value) {
+        K key = value.getId();
         return store.replace(key, value);
     }