refactor(resource): Refactor StaticSelector.match to avoid throwing exceptions

[ceekay47]

Summary:
The addVariableValues method in StaticSelector previously iterated over all variableNames and caught IllegalArgumentException for each name not present as a named group in the pattern. This is inefficient and relies on exception handling for control flow.

This change pre-computes the named groups for each regex pattern at construction time into an ImmutableMap<Pattern, ImmutableSet<String>>. The addVariableValues method now looks up the pre-computed groups for the given pattern and only iterates over matching group names, avoiding the need to catch exceptions entirely.

Differential Revision: D93437091

Summary by Sourcery

Refactor StaticSelector variable extraction to precompute regex named groups and avoid exception-based control flow during matching.

Enhancements:

• Precompute named groups for user, source, and principal regex patterns and cache them per pattern for reuse.
• Update variable value extraction to iterate only over applicable named groups, eliminating reliance on IllegalArgumentException when a group is absent.

== RELEASE NOTES ==

Resource Groups Changes
* Improve StaticSelector.addVariableValues to avoid using exceptions as control flow.

[sourcery-ai]

Reviewer's Guide

StaticSelector now precomputes named capture groups per regex pattern at construction time and reuses them in addVariableValues, eliminating per-variable IllegalArgumentException-based control flow and reducing repeated regex work.

Sequence diagram for updated addVariableValues flow

sequenceDiagram
    participant SS as StaticSelector
    participant P as Pattern
    participant C as CandidateString
    participant M as MappingMap
    participant G as PatternNamedGroups
    participant Ma as Matcher

    SS->>G: getOrDefault(P, emptySet)
    G-->>SS: groups
    SS->>SS: check groups isEmpty
    alt groups empty
        SS-->>M: return without changes
    else groups not empty
        SS->>P: matcher(C)
        P-->>Ma: new Matcher
        SS->>Ma: find()
        alt match found
            loop for each key in groups
                SS->>Ma: group(key)
                Ma-->>SS: value
                SS->>M: put(key, value) if value not null
            end
        else no match
            SS-->>M: return without changes
        end
    end

Loading

Class diagram for updated StaticSelector precomputed named groups

classDiagram
    class StaticSelector {
        - Optional userRegex
        - Optional sourceRegex
        - Optional principalRegex
        - ResourceGroupIdTemplate group
        - Set variableNames
        - ImmutableMap patternNamedGroups
        + StaticSelector(Optional userRegex, Optional sourceRegex, Optional principalRegex, ResourceGroupIdTemplate group)
        - static void addNamedGroups(Pattern pattern, HashSet variables)
        - static ImmutableSet extractNamedGroups(Pattern pattern)
        - void addVariableValues(Pattern pattern, String candidate, Map mapping)
    }

    StaticSelector --> Pattern : uses
    StaticSelector --> ImmutableMap : stores
    StaticSelector --> ImmutableSet : stores
    StaticSelector --> Matcher : uses
    StaticSelector --> ResourceGroupIdTemplate : uses
    StaticSelector --> Set : uses
    StaticSelector --> Map : uses

Loading

File-Level Changes

Change	Details	Files
Precompute named groups for each optional regex pattern and store them on the selector instance.	Introduce a new ImmutableMap field mapping Pattern instances to their corresponding named group sets. In the constructor, build the pattern-to-named-groups map by extracting named groups from user, source, and principal regex patterns when present. Reuse the existing variableNames collection logic but separate it from the new pattern-to-groups map.	presto-resource-group-managers/src/main/java/com/facebook/presto/resourceGroups/StaticSelector.java
Refactor addVariableValues to iterate only over precomputed named groups for the specific pattern instead of all variableNames and avoid exception-based control flow.	Lookup the set of named groups for the given pattern from the precomputed map, returning early if none are present. Create a single Matcher per addVariableValues call instead of per variable, and only proceed if the pattern matches the candidate string. Iterate over the pattern’s named groups and populate the mapping with non-null group values without catching IllegalArgumentException.	presto-resource-group-managers/src/main/java/com/facebook/presto/resourceGroups/StaticSelector.java

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `presto-resource-group-managers/src/main/java/com/facebook/presto/resourceGroups/StaticSelector.java:178-185` </location>
<code_context>
         }
     }

+    private static ImmutableSet<String> extractNamedGroups(Pattern pattern)
+    {
+        ImmutableSet.Builder<String> groups = ImmutableSet.builder();
+        Matcher matcher = NAMED_GROUPS_PATTERN.matcher(pattern.pattern());
+        while (matcher.find()) {
+            groups.add(matcher.group(1));
+        }
+        return groups.build();
+    }
+
</code_context>

<issue_to_address>
**suggestion:** Named-group extraction logic is duplicated and could be consolidated.

This duplicates the existing `NAMED_GROUPS_PATTERN` scan in `addNamedGroups`, differing only in how the result is stored. To avoid maintaining the parsing logic in two places, you could extract a single helper that returns a `Set<String>` of group names, then have `addNamedGroups` and `extractNamedGroups` both delegate to it (e.g., `addAll` on the returned set or wrap it in an `ImmutableSet`).

Suggested implementation:

```java
    private static ImmutableSet<String> extractNamedGroups(Pattern pattern)
    {
        return ImmutableSet.copyOf(parseNamedGroups(pattern));
    }

    private static Set<String> parseNamedGroups(Pattern pattern)
    {
        Set<String> groups = new HashSet<>();
        Matcher matcher = NAMED_GROUPS_PATTERN.matcher(pattern.pattern());
        while (matcher.find()) {
            groups.add(matcher.group(1));
        }
        return groups;
    }

```

```java
    private static void addNamedGroups(Pattern pattern, Set<String> variableNames)
    {
        variableNames.addAll(parseNamedGroups(pattern));
    }

```

1. If `HashSet` is not yet imported in this file, add `import java.util.HashSet;` near the other `java.util` imports.
2. If the existing `addNamedGroups` method signature differs (e.g., different parameter names or modifiers), adjust the `SEARCH` and `REPLACE` blocks to match the exact current signature and style.
3. If there is an existing `parseNamedGroups`-like helper, merge with it instead of introducing a duplicate.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion: Named-group extraction logic is duplicated and could be consolidated.

This duplicates the existing NAMED_GROUPS_PATTERN scan in addNamedGroups, differing only in how the result is stored. To avoid maintaining the parsing logic in two places, you could extract a single helper that returns a Set<String> of group names, then have addNamedGroups and extractNamedGroups both delegate to it (e.g., addAll on the returned set or wrap it in an ImmutableSet).

Suggested implementation:

    private static ImmutableSet<String> extractNamedGroups(Pattern pattern)
    {
        return ImmutableSet.copyOf(parseNamedGroups(pattern));
    }

    private static Set<String> parseNamedGroups(Pattern pattern)
    {
        Set<String> groups = new HashSet<>();
        Matcher matcher = NAMED_GROUPS_PATTERN.matcher(pattern.pattern());
        while (matcher.find()) {
            groups.add(matcher.group(1));
        }
        return groups;
    }

    private static void addNamedGroups(Pattern pattern, Set<String> variableNames)
    {
        variableNames.addAll(parseNamedGroups(pattern));
    }

1. If HashSet is not yet imported in this file, add import java.util.HashSet; near the other java.util imports.
2. If the existing addNamedGroups method signature differs (e.g., different parameter names or modifiers), adjust the SEARCH and REPLACE blocks to match the exact current signature and style.
3. If there is an existing parseNamedGroups-like helper, merge with it instead of introducing a duplicate.