resolve comments

Author: shivaspeaks

xds/src/main/java/io/grpc/xds/internal/matcher/HeadersWrapper.java

@@ -32,6 +32,8 @@ final class HeadersWrapper extends AbstractMap<String, String> {
   private static final ImmutableSet<String> PSEUDO_HEADERS =
       ImmutableSet.of(":method", ":authority", ":path");
   private final MatchContext context;
+  @Nullable
+  private volatile Set<String> cachedKeySet;
 
   HeadersWrapper(MatchContext context) {
     this.context = context;
@@ -115,21 +117,45 @@ public boolean containsKey(Object key) {
     }
   }
 
+  private boolean isMetadataKeyResolvable(String headerName) {
+    try {
+      if (headerName.endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
+        Metadata.Key.of(headerName, Metadata.BINARY_BYTE_MARSHALLER);
+      } else {
+        Metadata.Key.of(headerName, Metadata.ASCII_STRING_MARSHALLER);
+      }
+      return true;
+    } catch (IllegalArgumentException e) {
+      return false;
+    }
+  }
+
   @Override
   public Set<String> keySet() {
-    ImmutableSet.Builder<String> builder = ImmutableSet.builder();
-    for (String key : context.getMetadata().keys()) {
-      String lowerKey = key.toLowerCase(java.util.Locale.ROOT);
-      // Filter out any keys we provide specialized aliases/values for.
-      if (!lowerKey.equals("te") 
-          && !lowerKey.equals("host") 
-          && !PSEUDO_HEADERS.contains(lowerKey)) {
-        builder.add(key);
+    Set<String> result = cachedKeySet;
+    if (result == null) {
+      synchronized (this) {
+        result = cachedKeySet;
+        if (result == null) {
+          ImmutableSet.Builder<String> builder = ImmutableSet.builder();
+          for (String key : context.getMetadata().keys()) {
+            String lowerKey = key.toLowerCase(java.util.Locale.ROOT);
+            // Filter out any keys we provide specialized aliases/values for.
+            if (!lowerKey.equals("te") 
+                && !lowerKey.equals("host") 
+                && !PSEUDO_HEADERS.contains(lowerKey)
+                && isMetadataKeyResolvable(lowerKey)) {
+              builder.add(key);
+            }
+          }
+          builder.addAll(PSEUDO_HEADERS);
+          builder.add("host");
+          result = builder.build();
+          cachedKeySet = result;
+        }
       }
     }
-    builder.addAll(PSEUDO_HEADERS);
-    builder.add("host");
-    return builder.build();
+    return result;
   }
 
   @Override

xds/src/test/java/io/grpc/xds/internal/matcher/CelEnvironmentTest.java

@@ -290,20 +290,6 @@ public void celEnvironment_runtime_disabledFeatures_throwsException() throws Exc
     }
   }
 
-  @Test
-  public void celEnvironment_method_fallback() {
-    MatchContext context = MatchContext.newBuilder()
-        .setPath("/").setHost("example.com").setMethod("POST").build();
-    
-    GrpcCelEnvironment env = new GrpcCelEnvironment(context);
-    
-    Optional<Object> result = env.find("request");
-    assertThat(result.isPresent()).isTrue();
-    @SuppressWarnings("unchecked")
-    Map<String, Object> requestMap = (Map<String, Object>) result.get();
-    assertThat(requestMap.get("method")).isEqualTo("POST");
-  }
-
   @Test
   public void celEnvironment_resolvesLazyRequestMap() {
     MatchContext context = MatchContext.newBuilder()

xds/src/test/java/io/grpc/xds/internal/matcher/CelStringExtractorTest.java

@@ -152,8 +152,10 @@ public void compile_invalidSyntax_throws() {
     try {
       CelMatcherTestHelper.compileStringExtractor("invalid syntax ???");
       fail("Should throw CelValidationException");
+    } catch (dev.cel.common.CelValidationException e) {
+      assertThat(e).hasMessageThat().contains("mismatched input");
     } catch (Exception e) {
-      // Expected
+      fail("Threw wrong exception type: " + e.getClass().getName());
     }
   }