Skip to content

Conversation

@diydriller
Copy link
Contributor

@diydriller diydriller commented Dec 3, 2025

What changes were proposed in this pull request?

This PR fixes the incorrect resource path enumeration logic in the GCS token provider by ensuring that multi-level paths accumulate parent segments correctly and include both non-directory and directory variants where appropriate.
Additionally, the logic that previously concatenated strings to the parent variable using + operations has been replaced with a StringBuilder for improved efficiency, and cases where resourcePath is null are now handled safely.

Why are the changes needed?

The previous implementation did not include the non-directory path segments for intermediate levels.
Additionally, repeatedly concatenating strings using the + operator introduced unnecessary overhead, and the absence of null handling for resourcePath could result in unexpected exceptions.

Fix: #8010

Does this PR introduce any user-facing change?

No new user-facing API is introduced.

How was this patch tested?

Use the test code below

  @Test
  void testGetAllResources() {
    Map<String, List<String>> checkResults =
        ImmutableMap.<String, List<String>>builder()
            .put("a/b", Arrays.asList("a", "a/", "a/b"))
            .put("a/b/", Arrays.asList("a", "a/", "a/b"))
            .put("a/b/c", Arrays.asList("a", "a/", "a/b", "a/b/", "a/b/c"))
            .put("a/b/c/", Arrays.asList("a", "a/", "a/b", "a/b/", "a/b/c"))
            .put("a", Arrays.asList("a"))
            .put("a/", Arrays.asList("a"))
            .put("", Arrays.asList(""))
            .put("/", Arrays.asList(""))
            .build();

    checkResults.forEach(
        (key, value) -> {
          List<String> parentResources = GCSTokenProvider.getAllResources(key);
          Assertions.assertArrayEquals(value.toArray(), parentResources.toArray());
        });
  }

@jerryshao jerryshao requested review from FANNG1, Copilot and mchades and removed request for Copilot December 4, 2025 06:19
Copy link
Member

@justinmclean justinmclean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this

@justinmclean justinmclean merged commit 7d67409 into apache:main Dec 4, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement] TestGCSTokenProvider.java

2 participants