refactors the PR so that we have a separate internal ExtendedTextMapGetter, as per review.

Also changes the return type of the method to Iterator<String> as per review, and renames the method to getAll(). Tests are also adjusted

Author: jamesmoessis

api/all/src/main/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagator.java

@@ -13,10 +13,12 @@
 import io.opentelemetry.api.internal.PercentEscaper;
 import io.opentelemetry.api.internal.StringUtils;
 import io.opentelemetry.context.Context;
+import io.opentelemetry.context.internal.propagation.ExtendedTextMapGetter;
 import io.opentelemetry.context.propagation.TextMapGetter;
 import io.opentelemetry.context.propagation.TextMapPropagator;
 import io.opentelemetry.context.propagation.TextMapSetter;
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.List;
 import javax.annotation.Nullable;
 
@@ -95,14 +97,43 @@ public <C> Context extract(Context context, @Nullable C carrier, TextMapGetter<C
       return context;
     }
 
-    List<String> baggageHeaders = getter.getList(carrier, FIELD);
-    if (baggageHeaders == null || baggageHeaders.isEmpty()) {
+    if (getter instanceof ExtendedTextMapGetter) {
+      return extractMulti(context, carrier, (ExtendedTextMapGetter<C>) getter);
+    }
+    return extractSingle(context, carrier, getter);
+  }
+
+  private static <C> Context extractSingle(
+      Context context, @Nullable C carrier, TextMapGetter<C> getter) {
+    String baggageHeader = getter.get(carrier, FIELD);
+    if (baggageHeader == null) {
+      return context;
+    }
+    if (baggageHeader.isEmpty()) {
+      return context;
+    }
+
+    BaggageBuilder baggageBuilder = Baggage.builder();
+    try {
+      extractEntries(baggageHeader, baggageBuilder);
+    } catch (RuntimeException e) {
+      return context;
+    }
+    return context.with(baggageBuilder.build());
+  }
+
+  private static <C> Context extractMulti(
+      Context context, @Nullable C carrier, ExtendedTextMapGetter<C> getter) {
+    Iterator<String> baggageHeaders = getter.getAll(carrier, FIELD);
+    if (baggageHeaders == null) {
       return context;
     }
 
     boolean extracted = false;
     BaggageBuilder baggageBuilder = Baggage.builder();
-    for (String header : baggageHeaders) {
+
+    while (baggageHeaders.hasNext()) {
+      String header = baggageHeaders.next();
       if (header.isEmpty()) {
         continue;
       }
@@ -114,6 +145,7 @@ public <C> Context extract(Context context, @Nullable C carrier, TextMapGetter<C
         // invalid baggage header, continue
       }
     }
+
     return extracted ? context.with(baggageBuilder.build()) : context;
   }
 

api/all/src/test/java/io/opentelemetry/api/baggage/propagation/W3CBaggagePropagatorTest.java

@@ -13,9 +13,11 @@
 import io.opentelemetry.api.baggage.Baggage;
 import io.opentelemetry.api.baggage.BaggageEntryMetadata;
 import io.opentelemetry.context.Context;
+import io.opentelemetry.context.internal.propagation.ExtendedTextMapGetter;
 import io.opentelemetry.context.propagation.TextMapGetter;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -38,8 +40,8 @@ public String get(Map<String, String> carrier, String key) {
         }
       };
 
-  private static final TextMapGetter<Map<String, List<String>>> multiGetter =
-      new TextMapGetter<Map<String, List<String>>>() {
+  private static final ExtendedTextMapGetter<Map<String, List<String>>> multiGetter =
+      new ExtendedTextMapGetter<Map<String, List<String>>>() {
         @Override
         public Iterable<String> keys(Map<String, List<String>> carrier) {
           return carrier.keySet();
@@ -54,8 +56,9 @@ public String get(Map<String, List<String>> carrier, String key) {
         }
 
         @Override
-        public List<String> getList(Map<String, List<String>> carrier, String key) {
-          return carrier.getOrDefault(key, Collections.emptyList());
+        public Iterator<String> getAll(Map<String, List<String>> carrier, String key) {
+          List<String> values = carrier.get(key);
+          return values == null ? Collections.emptyIterator() : values.iterator();
         }
       };
 

context/src/main/java/io/opentelemetry/context/internal/propagation/ExtendedTextMapGetter.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.context.internal.propagation;
+
+import io.opentelemetry.context.propagation.TextMapGetter;
+import java.util.Collections;
+import java.util.Iterator;
+import javax.annotation.Nullable;
+
+/**
+ * Extends {@link TextMapGetter} to return possibly multiple values for a given key.
+ *
+ * <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
+ * at any time.
+ *
+ * @param <C> carrier of propagation fields, such as an http request.
+ */
+public interface ExtendedTextMapGetter<C> extends TextMapGetter<C> {
+  /**
+   * If implemented, returns all values for a given {@code key} in order, or returns an empty list.
+   *
+   * <p>The default method returns the first value of the given propagation {@code key} as a
+   * singleton list, or returns an empty list.
+   *
+   * <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
+   * at any time.
+   *
+   * @param carrier carrier of propagation fields, such as an http request.
+   * @param key the key of the field.
+   * @return all values for a given {@code key} in order, or returns an empty list. Default method
+   *     wraps {@code get()} as an {@link Iterator}.
+   */
+  default Iterator<String> getAll(@Nullable C carrier, String key) {
+    String first = get(carrier, key);
+    return Collections.singleton(first).iterator();
+  }
+}

context/src/main/java/io/opentelemetry/context/propagation/TextMapGetter.java

@@ -5,8 +5,6 @@
 
 package io.opentelemetry.context.propagation;
 
-import java.util.Collections;
-import java.util.List;
 import javax.annotation.Nullable;
 
 /**
@@ -35,20 +33,4 @@ public interface TextMapGetter<C> {
    */
   @Nullable
   String get(@Nullable C carrier, String key);
-
-  /**
-   * If implemented, returns all values for a given {@code key} in order, or returns an empty list.
-   *
-   * <p>The default method returns the first value of the given propagation {@code key} as a
-   * singleton list, or returns an empty list.
-   *
-   * @param carrier carrier of propagation fields, such as an http request.
-   * @param key the key of the field.
-   * @return the first value of the given propagation {@code key} as a singleton list, or returns an
-   *     empty list.
-   */
-  default List<String> getList(@Nullable C carrier, String key) {
-    String first = get(carrier, key);
-    return first == null ? Collections.emptyList() : Collections.singletonList(first);
-  }
 }