Skip to content

Commit 67168ad

Browse files
committed
fix: simplify logics to get the start of the binding.
1 parent e5142f1 commit 67168ad

File tree

2 files changed

+43
-35
lines changed

2 files changed

+43
-35
lines changed

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

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -354,51 +354,51 @@ public String getCanonicalResourceName(Set<String> knownResources) {
354354
}
355355

356356
int lastValidEndBindingIndex = -1;
357-
boolean inBinding = false;
358-
int literalCountInBinding = 0;
359-
int currentBindingStartIndex = -1;
360-
361-
for (int i = 0; i < segments.size(); i++) {
357+
// Iterate from the end of the segments to find the last valid resource binding.
358+
// Searching backwards allows us to stop immediately once the last valid pair is found.
359+
for (int i = segments.size() - 1; i >= 0; i--) {
362360
Segment seg = segments.get(i);
363-
if (seg.kind() == SegmentKind.BINDING) {
364-
inBinding = true;
365-
literalCountInBinding = 0;
366-
currentBindingStartIndex = i;
367-
} else if (seg.kind() == SegmentKind.END_BINDING) {
368-
inBinding = false;
361+
362+
// We are looking for the end of a binding (e.g., "}" in "{project}" or "{name=projects/*}")
363+
if (seg.kind() == SegmentKind.END_BINDING) {
364+
int bindingStartIndex = -1;
365+
int literalCountInBinding = 0;
369366
boolean isValidPair = false;
370367

371-
if (literalCountInBinding > 1) {
372-
// Named bindings are unconditionally considered pairs
373-
isValidPair = true;
374-
} else {
375-
// Check inner literals
376-
for (int j = currentBindingStartIndex + 1; j < i; j++) {
377-
if (segments.get(j).kind() == SegmentKind.LITERAL) {
378-
if (knownResources.contains(segments.get(j).value())) {
379-
isValidPair = true;
380-
break;
381-
}
382-
}
368+
// Traverse backwards to find the start of this specific binding
369+
// and count the literals captured inside it.
370+
for (int j = i - 1; j >= 0; j--) {
371+
Segment innerSeg = segments.get(j);
372+
if (innerSeg.kind() == SegmentKind.BINDING) {
373+
bindingStartIndex = j;
374+
break;
375+
} else if (innerSeg.kind() == SegmentKind.LITERAL) {
376+
literalCountInBinding++;
383377
}
384-
// If not valid yet, check preceding literal
385-
if (!isValidPair && currentBindingStartIndex > 0) {
386-
Segment prevSeg = segments.get(currentBindingStartIndex - 1);
378+
}
379+
380+
if (bindingStartIndex != -1) {
381+
// 1. If the binding contains any literals, it is considered a valid named resource binding.
382+
if (literalCountInBinding > 0) {
383+
isValidPair = true;
384+
} else if (bindingStartIndex > 0) {
385+
// 2. For simple bindings like "{project}", the binding itself has no inner literal resources.
386+
// Instead, we check if the literal segment immediately preceding it (e.g., "projects/")
387+
// is a known resource.
388+
Segment prevSeg = segments.get(bindingStartIndex - 1);
387389
if (prevSeg.kind() == SegmentKind.LITERAL && knownResources.contains(prevSeg.value())) {
388390
isValidPair = true;
389391
}
390392
}
391-
}
392393

393-
if (isValidPair) {
394-
lastValidEndBindingIndex = i;
395-
}
396-
} else if (seg.kind() == SegmentKind.LITERAL) {
397-
if (inBinding) {
398-
String value = seg.value();
399-
if (!value.matches("^v\\d+[a-zA-Z0-9]*$")) {
400-
literalCountInBinding++;
394+
if (isValidPair) {
395+
// We successfully found the last valid binding! Record its end index and terminate the search.
396+
lastValidEndBindingIndex = i;
397+
break;
401398
}
399+
// The current binding wasn't a valid resource pair.
400+
// Skip over all inner segments of this invalid binding so we don't evaluate them again.
401+
i = bindingStartIndex;
402402
}
403403
}
404404
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -951,6 +951,14 @@ void testGetCanonicalResourceName_namedBindingsSimple() {
951951
.isEqualTo("{bar=projects/*/locations/*/bars/*}");
952952
}
953953

954+
@Test
955+
void testGetCanonicalResourceName_namedBindingsWithUnknownResource() {
956+
Set<String> knownResources = ImmutableSet.of();
957+
PathTemplate template = PathTemplate.create("/v1/{bar=projects/*/locations/*/unknown/*}");
958+
Truth.assertThat(template.getCanonicalResourceName(knownResources))
959+
.isEqualTo("{bar=projects/*/locations/*/unknown/*}");
960+
}
961+
954962
@Test
955963
void testGetCanonicalResourceName_simplePath() {
956964
Set<String> knownResources = ImmutableSet.of("projects", "locations", "instances", "widgets");

0 commit comments

Comments
 (0)