feat: extend PURL identifier support for query string parameters #180
Conversation
WalkthroughThe pull request introduces modifications to the Changes
Sequence DiagramsequenceDiagram
participant Parser
participant Lexer
participant Query
Parser->>Lexer: Parse input with extended Ident rule
Lexer-->>Parser: Return tokens with new symbol support
Parser->>Query: Create query with optional NodeName
Query-->>Parser: Return query object
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This commit enhances the package URL (PURL) parsing capabilities to support query string parameters in identifiers. The changes include: - Updated the Ident lexer rule to accept additional characters (?=&+) commonly found in PURL query strings - Added test cases for PURLs containing qualifiers (e.g., ?type=jar) - Included a real-world example using Maven artifacts with query parameters - Improved test documentation and structure The enhancement allows for proper handling of package URLs that include qualifiers, such as: pkg:maven/org.apache.camel.quarkus/camel-quarkus-cassandraql-deployment@3.18.0-SNAPSHOT?type=jar This change is particularly important for Maven artifacts and other package types that commonly use query parameters in their identifiers. #178 Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com>
naveensrinivasan
force-pushed
the
naveen/fix/special-character
branch
from
42ad018 to
1ace367
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/graph/parser.go (1)
39-40: Expand character set and consider using keyed struct fields
Allowing?,=,&, and+in theIdentrule improves PURL query parameter parsing. As per the static analysis hint, you might consider using keyed struct fields in these literals to avoid accidental misordering in the future:-{"Operator", `\b(?:and|or|xor)\b`}, -{"Ident", `[a-zA-Z][a-zA-Z0-9:/._@?=&+\-]*`}, +{Type: "Operator", Pattern: `\b(?:and|or|xor)\b`}, +{Type: "Ident", Pattern: `[a-zA-Z][a-zA-Z0-9:/._@?=&+\-]*`},🧰 Tools
🪛 golangci-lint (1.62.2)
39-39: composites: github.com/alecthomas/participle/v2/lexer.SimpleRule struct literal uses unkeyed fields
(govet)
40-40: composites: github.com/alecthomas/participle/v2/lexer.SimpleRule struct literal uses unkeyed fields
(govet)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/graph/parser.go(1 hunks)pkg/graph/parser_test.go(7 hunks)
🧰 Additional context used
🪛 golangci-lint (1.62.2)
pkg/graph/parser.go
39-39: composites: github.com/alecthomas/participle/v2/lexer.SimpleRule struct literal uses unkeyed fields
(govet)
40-40: composites: github.com/alecthomas/participle/v2/lexer.SimpleRule struct literal uses unkeyed fields
(govet)
🔇 Additional comments (11)
pkg/graph/parser_test.go (11)
9-10: Nice documentation update for qualifier coverage
These comments clearly explain the addition of tests involving query parameters, making it easier to understand the context and purpose of these tests.
32-36: Good introduction of a PURL with a query-string
Creating nodeCamel with ?type=jar helps demonstrate real-world use cases of qualifiers in PURLs.
38-39: Clearer explanation of dependency structure
Documenting the dependency chain helps readers quickly grasp the purpose of the subsequent test assertions.
53-57: Good test coverage for qualified PURLs
Making nodeCamel depend on node4 confirms that PURLs with query parameters can be processed in the same manner as regular PURLs.
74-74: No issues with test assertion
This verifies basic dependents functionality for node1, node2, and node3.
80-80: No issues with test assertion
Ensures the correct chain of dependencies for node1, including node3 and node4.
92-92: No issues with combined queries
Validates logical operators in queries involving multiple nodes and ensures correct result sets.
102-104: Solid test for default node name
This scenario checks fallback behavior when no node name is specified, improving coverage of edge cases.
108-114: Comprehensive test for dependencies with qualifiers
Verifies that queries with ?type=jar are accurately recognized and processed.
115-120: Dependents query for qualified PURL
Useful to confirm that qualifiers function correctly in both dependents and dependencies queries.
121-126: Proper handling of invalid qualified queries
Ensures erroneous queries with qualifiers are caught, maintaining robust validation.
This commit enhances the package URL (PURL) parsing capabilities to support query string parameters in identifiers. The changes include:
The enhancement allows for proper handling of package URLs that include qualifiers, such as:
pkg:maven/org.apache.camel.quarkus/camel-quarkus-cassandraql-deployment@3.18.0-SNAPSHOT?type=jar
This change is particularly important for Maven artifacts and other package types commonly using query parameters in their identifiers.
#178
Summary by CodeRabbit
New Features
?,=,&,+).NodeNamefield in queries is now optional.Bug Fixes
Tests