-
Notifications
You must be signed in to change notification settings - Fork 175
Allow duplicate prefix declarations when parsing RDF or SPARQL with identical namespaces #5478
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
base: main
Are you sure you want to change the base?
Conversation
…ests Co-authored-by: hmottestad <[email protected]>
initial-evidence.txt
Outdated
@@ -0,0 +1,91 @@ | |||
------------------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot thank you for proving that your test failed. You can remove the proof now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the initial test evidence file. It was only needed to demonstrate the initial test failure before implementing the fix.
Co-authored-by: hmottestad <[email protected]>
/** | ||
* Tests for duplicate prefix declarations in Turtle parser. | ||
*/ | ||
public class TurtlePrefixDuplicateTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added 12 comprehensive @base tests showing how different placement of base changes duplicate relative prefix declarations. Tests cover same base (allowed), base changes between declarations (last wins), external vs internal base handling, complex relative paths, missing base errors, and absolute/relative transitions. All 15 turtle tests pass.
Co-authored-by: hmottestad <[email protected]>
return createNode(id); | ||
} | ||
|
||
public void setNamespace(String prefix, String namespace) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This addition of the set and getNamespace methods seems unnecessary, could you please remove it. Directly invoking a super method is redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I'm currently experimenting with copilot in Github to see if it can be useful for fixing small bugs. It's mostly fine, but often makes small changes like these that don't make much sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Can you fix this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the unnecessary setNamespace and getNamespace wrapper methods and their associated tests. These were redundant since they only called the super methods without adding functionality.
Co-authored-by: hmottestad <[email protected]>
…endencies Co-authored-by: hmottestad <[email protected]>
✅ COMPLETED: Allow duplicate prefix declarations when parsing RDF or SPARQL
Goal: Allow duplicate prefix declarations when both the namespace and prefix are exactly the same, but prevent duplicates when they map to different namespaces.
Scope:
✅ IMPLEMENTATION COMPLETED:
📋 SUMMARY OF CHANGES:
1. PrefixDeclProcessor.java (SPARQL)
2. RDF Parsers (AbstractRDFParser, Turtle, etc.)
3. SHACL Prefix Parser
🧪 TEST COVERAGE ADDED:
SPARQL Parser Tests (4 new tests)
RDF Parser Tests (15 comprehensive tests)
🔍 REGRESSION TESTING:
✅ REQUIREMENTS FULFILLED:
The implementation is complete and thoroughly tested. All requirements are met with minimal, surgical changes that maintain backward compatibility while enabling the desired functionality. The comprehensive @base tests demonstrate how relative prefix declarations are resolved in various scenarios, ensuring robust handling of edge cases where base URIs change between duplicate prefix declarations.
Original prompt
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.