acrites commented on code in PR #40750:
URL: https://github.com/apache/beam/pull/40750#discussion_r4391873031


##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/windmill/state/WindmillMapViaMultimap.java:
##########
@@ -62,19 +62,26 @@ public void put(KeyT key, ValueT value) {
     multimap.put(key, value);
   }
 
+  @SuppressWarnings("nullness")
   @Override
   public ReadableState<ValueT> computeIfAbsent(
       KeyT key, Function<? super KeyT, ? extends ValueT> mappingFunction) {
     // Note that computeIfAbsent comments indicate that the read is lazy but 
this matches the
-    // existing eager
-    // behavior of WindmillMap.
+    // existing eager behavior of WindmillMap.
     Iterable<ValueT> existingValues = multimap.get(key).read();
-    if (Iterables.isEmpty(existingValues)) {
+    Iterator<ValueT> iterator = existingValues.iterator();
+    if (!iterator.hasNext()) {
       ValueT inserted = mappingFunction.apply(key);
       multimap.put(key, inserted);
       return ReadableStates.immediate(inserted);
     } else {
-      return 
ReadableStates.immediate(Iterables.getOnlyElement(existingValues));
+      ValueT existing = Iterators.getOnlyElement(iterator);
+      if (existing == null) {

Review Comment:
   In all of these cases the behavior seems to be "if the value is currently 
mapped to null, apply the mapping function and overwrite it with whatever the 
mapping function returns". I think they all return the previous value that was 
stored in the map. So it seems like `WindmillMapViaMultimap` is the only class 
not implementing `computeIfAbsent` based on the original comments.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to