Skip to content

Improve Pattern Management in ReplicaTopologyProvider with InfoPatterns Enum #3263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ori0o0p opened this issue Apr 18, 2025 · 0 comments · May be fixed by #3264
Open

Improve Pattern Management in ReplicaTopologyProvider with InfoPatterns Enum #3263

ori0o0p opened this issue Apr 18, 2025 · 0 comments · May be fixed by #3264

Comments

@ori0o0p
Copy link
Contributor

ori0o0p commented Apr 18, 2025

Feature Request

Is your feature request related to a problem? Please describe

The current implementation of ReplicaTopologyProvider defines regular expression patterns as individual static final Pattern constants at the class level (e.g., ROLE_PATTERN, SLAVE_PATTERN, etc.). This approach scatters related patterns across the codebase, making it harder to manage, maintain, and extend them. For example, adding a new pattern requires declaring a new constant and ensuring it aligns with existing patterns, which can lead to inconsistencies or errors. Additionally, the getNested method directly accepts Pattern objects, which reduces type safety and makes the code less intuitive when referencing patterns.

Describe the solution you'd like

We propose refactoring the pattern management in ReplicaTopologyProvider by introducing an InfoPatterns enum to encapsulate all regular expression patterns. The enum will group related patterns (e.g., ROLE, SLAVE, MASTER_HOST, etc.) in a single, cohesive structure, improving code organization and readability. Each enum constant will hold a compiled Pattern and provide a matcher method to streamline pattern usage.

The getNested method will be updated to accept an InfoPatterns enum instead of a raw Pattern, enhancing type safety and making the code more self-documenting. For example:

enum InfoPatterns {
    ROLE(Pattern.compile("^role\\:([a-z]+)$", Pattern.MULTILINE)),
    SLAVE(Pattern.compile("^slave(\\d+)\\:([a-zA-Z\\,\\=\\d\\.\\:\\-]+)$", Pattern.MULTILINE)),
    // ...

    private final Pattern pattern;

    InfoPatterns(Pattern pattern) {
        this.pattern = pattern;
    }

    public Pattern getPattern() {
        return pattern;
    }

    public Matcher matcher(String input) {
        return pattern.matcher(input);
    }
}

Benefits:

  • Improved organization: Patterns are grouped in a single enum, making it easier to locate and manage them.
  • Type safety: Using InfoPatterns instead of raw Pattern objects reduces the risk of passing incorrect patterns.
  • Readability: Referencing patterns like InfoPatterns.ROLE is more intuitive than ROLE_PATTERN.
  • Extensibility: Adding new patterns is as simple as adding a new enum constant.

Drawbacks:

  • Minor increase in code complexity due to the introduction of an enum.
  • Negligible performance overhead from enum initialization, which is mitigated since patterns are compiled only once.

Describe alternatives you've considered

  1. Keep individual Pattern constants: Continue using static final Pattern constants as in the current implementation. This avoids introducing new constructs but retains the issues of scattered pattern definitions and lower type safety.
  2. Use a Map or class to group patterns: Instead of an enum, patterns could be stored in a Map<String, Pattern> or a dedicated class. However, this approach is less type-safe than an enum and requires additional boilerplate for pattern access.
  3. Inline patterns: Define patterns directly in methods where they are used. This would eliminate constants but make the code harder to maintain and less reusable.

The enum approach was chosen for its balance of type safety, readability, and maintainability.

Teachability, Documentation, Adoption, Migration Strategy

Usage:
Developers using ReplicaTopologyProvider will not notice functional changes, as the public API (getNodes and getNodesAsync) remains unchanged. Internally, the class now uses InfoPatterns for pattern management, which simplifies maintenance for contributors.

Migration Strategy:

  • The change is backward-compatible, as it only affects the internal implementation of ReplicaTopologyProvider.
  • Replace all static final Pattern constants (ROLE_PATTERN, SLAVE_PATTERN, etc.) with the InfoPatterns enum.
  • Update the getNested method to accept InfoPatterns instead of Pattern.
  • Update all pattern references in the code to use InfoPatterns (e.g., InfoPatterns.ROLE.matcher(info) instead of ROLE_PATTERN.matcher(info)).
  • No user-facing changes are required, and existing tests should pass without modification.

Testing:
Existing tests for ReplicaTopologyProvider should cover the functionality, as the logic remains unchanged. Add new unit tests to verify that InfoPatterns correctly compiles and matches patterns for various INFO REPLICATION outputs.


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 a pull request may close this issue.

1 participant