fix: Update the heuristic logic per latest discussion.

Author: blakeli0

sdk-platform-java/api-common-java/src/main/java/com/google/api/pathtemplate/PathTemplate.java

@@ -332,124 +332,87 @@ public String getCanonicalResourceName(Set<String> knownResources) {
     if (knownResources == null) {
       return "";
     }
-    StringBuilder canonical = new StringBuilder();
-    StringBuilder currentSequence = new StringBuilder();
+
+    int firstBindingIndex = -1;
+    for (int i = 0; i < segments.size(); i++) {
+      if (segments.get(i).kind() == SegmentKind.BINDING) {
+        firstBindingIndex = i;
+        break;
+      }
+    }
+
+    if (firstBindingIndex == -1) {
+      return "";
+    }
+
+    int startIndex = 0;
+    for (int i = firstBindingIndex - 1; i >= 0; i--) {
+      Segment seg = segments.get(i);
+      if (seg.kind() == SegmentKind.LITERAL) {
+        String value = seg.value();
+        if (value.matches("^v\\d+.*") || value.matches("^u\\d+.*")) {
+          startIndex = i + 1;
+          break;
+        }
+      }
+    }
+
+    int lastValidEndBindingIndex = -1;
     boolean inBinding = false;
-    String currentBindingName = "";
-    boolean keepBinding = true;
-    List<Segment> bindingSegments = new ArrayList<>();
-    boolean afterKeptNamedBinding = false;
+    int literalCountInBinding = 0;
+    int currentBindingStartIndex = -1;
 
     for (int i = 0; i < segments.size(); i++) {
       Segment seg = segments.get(i);
       if (seg.kind() == SegmentKind.BINDING) {
         inBinding = true;
-        currentBindingName = seg.value();
-        bindingSegments.clear();
-        keepBinding = true;
+        literalCountInBinding = 0;
+        currentBindingStartIndex = i;
       } else if (seg.kind() == SegmentKind.END_BINDING) {
         inBinding = false;
-        StringBuilder innerPattern = new StringBuilder();
-        int literalCount = 0;
-        for (Segment innerSeg : bindingSegments) {
-          if (innerSeg.kind() == SegmentKind.LITERAL) {
-            String value = innerSeg.value();
-            if (value.matches("^v\\d+.*") || value.matches("^u\\d+.*")) {
-              continue;
-            }
-            literalCount++;
-            if (innerPattern.length() > 0) {
-              innerPattern.append("/");
-            }
-            innerPattern.append(value);
-          } else if (innerSeg.kind() == SegmentKind.WILDCARD) {
-            if (innerPattern.length() > 0) {
-              innerPattern.append("/");
-            }
-            innerPattern.append("*");
-          }
-        }
+        boolean isValidPair = false;
 
-        boolean extractInner = false;
-        if (canonical.length() == 0 && currentSequence.length() == 0) {
-          if (i + 1 < segments.size()) {
-            Segment nextSeg = segments.get(i + 1);
-            if (nextSeg.kind() == SegmentKind.LITERAL) {
-              String nextValue = nextSeg.value();
-              if (knownResources.contains(nextValue)) {
-                extractInner = true;
+        if (literalCountInBinding > 1) {
+          // Named bindings are unconditionally considered pairs
+          isValidPair = true;
+        } else {
+          // Check inner literals
+          for (int j = currentBindingStartIndex + 1; j < i; j++) {
+            if (segments.get(j).kind() == SegmentKind.LITERAL) {
+              if (knownResources.contains(segments.get(j).value())) {
+                isValidPair = true;
+                break;
               }
             }
           }
-        }
-
-        if (extractInner) {
-          if (innerPattern.length() > 0) {
-            if (canonical.length() > 0) {
-              canonical.append("/");
+          // If not valid yet, check preceding literal
+          if (!isValidPair && currentBindingStartIndex > 0) {
+            Segment prevSeg = segments.get(currentBindingStartIndex - 1);
+            if (prevSeg.kind() == SegmentKind.LITERAL && knownResources.contains(prevSeg.value())) {
+              isValidPair = true;
             }
-            canonical.append(innerPattern);
-          }
-        } else {
-          if (currentSequence.length() > 0) {
-            if (canonical.length() > 0) {
-              canonical.append("/");
-            }
-            canonical.append(currentSequence);
-            currentSequence.setLength(0);
-          }
-          if (canonical.length() > 0) {
-            canonical.append("/");
           }
+        }
 
-          if (literalCount <= 1 || innerPattern.toString().equals("*")) {
-            canonical.append("{").append(currentBindingName).append("}");
-          } else {
-            canonical
-                .append("{")
-                .append(currentBindingName)
-                .append("=")
-                .append(innerPattern)
-                .append("}");
-            afterKeptNamedBinding = true;
-          }
+        if (isValidPair) {
+          lastValidEndBindingIndex = i;
         }
       } else if (seg.kind() == SegmentKind.LITERAL) {
-        String value = seg.value();
-        if (value.matches("^v\\d+.*") || value.matches("^u\\d+.*")) {
-          continue;
-        }
         if (inBinding) {
-          bindingSegments.add(seg);
-          if (!knownResources.contains(value)) {
-            keepBinding = false;
-          }
-        } else {
-          if (knownResources.contains(value)) {
-            if (currentSequence.length() > 0) {
-              currentSequence.append("/");
-            }
-            currentSequence.append(value);
-          } else {
-            if (afterKeptNamedBinding) {
-              if (currentSequence.length() > 0) {
-                currentSequence.append("/");
-              }
-              currentSequence.append(value);
-            } else {
-              if (canonical.length() > 0 || currentSequence.length() > 0) {
-                break;
-              }
-            }
+          String value = seg.value();
+          if (!value.matches("^v\\d+.*") && !value.matches("^u\\d+.*")) {
+            literalCountInBinding++;
           }
         }
-      } else if (seg.kind() == SegmentKind.WILDCARD) {
-        if (inBinding) {
-          bindingSegments.add(seg);
-        }
       }
     }
-    return canonical.toString();
+
+    if (lastValidEndBindingIndex == -1 || lastValidEndBindingIndex < startIndex) {
+      return "";
+    }
+
+    List<Segment> canonicalSegments = segments.subList(startIndex, lastValidEndBindingIndex + 1);
+    return toSyntax(canonicalSegments, true);
   }
 
   /**
@@ -1278,7 +1241,7 @@ private String decodeUrl(String url) {
   // the list iterator in its state.
   private static boolean peek(ListIterator<Segment> segments, SegmentKind... kinds) {
     int start = segments.nextIndex();
-    boolean success = false;
+    boolean success = true;
     for (SegmentKind kind : kinds) {
       if (!segments.hasNext() || segments.next().kind() != kind) {
         success = false;

sdk-platform-java/api-common-java/src/test/java/com/google/api/pathtemplate/PathTemplateTest.java

@@ -966,7 +966,7 @@ void testGetCanonicalResourceName_regexVariables() {
     PathTemplate template =
         PathTemplate.create("v1/projects/{project=projects/*}/instances/{instance_id=instances/*}");
     Truth.assertThat(template.getCanonicalResourceName(knownResources))
-        .isEqualTo("projects/{project}/instances/{instance_id}");
+        .isEqualTo("projects/{project=projects/*}/instances/{instance_id=instances/*}");
   }
 
   @Test
@@ -1004,40 +1004,38 @@ void testGetCanonicalResourceName_customVerb() {
   }
 
   @Test
-  void testGetCanonicalResourceName_nameBinding() {
+  void testGetCanonicalResourceName_nameBindingMixedWithSimpleBinding() {
     Set<String> knownResources = ImmutableSet.of("projects", "locations", "instances", "widgets");
-    PathTemplate template = PathTemplate.create("v1/{field=projects/*/instances/*}");
+    PathTemplate template =
+        PathTemplate.create("v1/{field=projects/*/instances/*}/actions/{action}");
     Truth.assertThat(template.getCanonicalResourceName(knownResources))
         .isEqualTo("{field=projects/*/instances/*}");
   }
 
   @Test
-  void testGetCanonicalResourceName_nameBindingMixedWithSimpleBinding() {
-    Set<String> knownResources = ImmutableSet.of("projects", "locations", "instances", "widgets");
-    PathTemplate template =
-        PathTemplate.create("v1/{field=projects/*/instances/*}/actions/{action}");
+  void testGetCanonicalResourceName_multipleLiteralsWithSimpleBinding() {
+    Set<String> knownResources = ImmutableSet.of("actions");
+    PathTemplate template = PathTemplate.create("v1/locations/global/actions/{action}");
     Truth.assertThat(template.getCanonicalResourceName(knownResources))
-        .isEqualTo("{field=projects/*/instances/*}/actions/{action}");
+        .isEqualTo("locations/global/actions/{action}");
   }
 
   @Test
-  void testGetCanonicalResourceName_nameBindingWithUnknownLiterals() {
+  void testGetCanonicalResourceName_multipleLiteralsWithMultipleBindings() {
+    Set<String> knownResources = ImmutableSet.of("instances", "actions");
     PathTemplate template =
-        PathTemplate.create(
-            "/compute/v1/projects/{project}/zones/{zone}/{parent_name=reservations/*/reservationBlocks/*/reservationSubBlocks/*}/reservationSlots/{reservation_slot}");
-    String canonical = template.getCanonicalResourceName(template.getResourceLiterals());
-    Truth.assertThat(canonical)
-        .isEqualTo(
-            "projects/{project}/zones/{zone}/{parent_name=reservations/*/reservationBlocks/*/reservationSubBlocks/*}/reservationSlots/{reservation_slot}");
+        PathTemplate.create("v1/locations/global/instances/{instance}/actions/{action}");
+    Truth.assertThat(template.getCanonicalResourceName(knownResources))
+        .isEqualTo("locations/global/instances/{instance}/actions/{action}");
   }
 
   @Test
-  void testGetCanonicalResourceName_nameBindingMixedWithSimpleBinding_moreKnownResources() {
-    Set<String> moreKnownResources = ImmutableSet.of("projects", "instances", "actions");
+  void testGetCanonicalResourceName_multipleLiteralsBetweenMultipleBindings() {
+    Set<String> knownResources = ImmutableSet.of("instances", "actions");
     PathTemplate template =
-        PathTemplate.create("v1/{name=projects/*/instances/*}/actions/{action}");
-    Truth.assertThat(template.getCanonicalResourceName(moreKnownResources))
-        .isEqualTo("projects/*/instances/*/actions/{action}");
+        PathTemplate.create("v1/instances/{instance}/locations/global/actions/{action}");
+    Truth.assertThat(template.getCanonicalResourceName(knownResources))
+        .isEqualTo("instances/{instance}/locations/global/actions/{action}");
   }
 
   @Test