Skip to content

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.

Sign up for GitHub

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
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Allow duplicate prefix declarations when parsing RDF or SPARQL with identical namespaces #5478

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
908c075
Initial plan
Copilot Oct 5, 2025
e5a6d16
Implement duplicate prefix support for SPARQL and add comprehensive t…
Copilot Oct 5, 2025
7ad6f52
Remove initial test evidence file as no longer needed
Copilot Oct 5, 2025
4f12471
Add comprehensive @base tests for duplicate relative prefix declarations
Copilot Oct 5, 2025
6964e82
Remove unnecessary wrapper methods in AbstractRDFParserTest
Copilot Oct 6, 2025
5012e2f
Add .m2_repo to gitignore and remove accidentally committed Maven dep…
Copilot Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions ...arser/sparql/src/main/java/org/eclipse/rdf4j/query/parser/sparql/PrefixDeclProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,15 @@ public static Map<String, String> process(ASTOperationContainer qc, Map<String,
String iri = prefixDecl.getIRI().getValue();

if (prefixMap.containsKey(prefix)) {
throw new MalformedQueryException("Multiple prefix declarations for prefix '" + prefix + "'");
String existingIri = prefixMap.get(prefix);
if (!existingIri.equals(iri)) {
throw new MalformedQueryException("Multiple prefix declarations for prefix '" + prefix
+ "' with different namespaces: '" + existingIri + "' and '" + iri + "'");
}
// If the IRI is the same, allow the duplicate (no-op)
} else {
prefixMap.put(prefix, iri);
}

prefixMap.put(prefix, iri);
}

int preDefaultPrefixes = 0;
Expand Down
55 changes: 55 additions & 0 deletions ...ryparser/sparql/src/test/java/org/eclipse/rdf4j/query/parser/sparql/SPARQLParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1047,4 +1047,59 @@ private Object bytesToObject(byte[] str) {
throw new RuntimeException(e);
}
}

@Test
public void testDuplicatePrefixDeclarations_SameNamespace_ShouldPass() {
// Test that duplicate prefix declarations with the same namespace are allowed
String query = "PREFIX foaf: <http://xmlns.com/foaf/0.1/>\n" +
"PREFIX foaf: <http://xmlns.com/foaf/0.1/>\n" +
"SELECT ?name WHERE { ?person foaf:name ?name }";

// This should not throw an exception
assertDoesNotThrow(() -> parser.parseQuery(query, null));

ParsedQuery parsed = parser.parseQuery(query, null);
assertNotNull(parsed);
}

@Test
public void testDuplicatePrefixDeclarations_DifferentNamespace_ShouldFail() {
// Test that duplicate prefix declarations with different namespaces are rejected
String query = "PREFIX foaf: <http://xmlns.com/foaf/0.1/>\n" +
"PREFIX foaf: <http://example.org/different/>\n" +
"SELECT ?name WHERE { ?person foaf:name ?name }";

// This should throw a MalformedQueryException
assertThatExceptionOfType(MalformedQueryException.class)
.isThrownBy(() -> parser.parseQuery(query, null))
.withMessageContaining("Multiple prefix declarations")
.withMessageContaining("foaf");
}

@Test
public void testDuplicatePrefixDeclarations_EmptyPrefix_SameNamespace_ShouldPass() {
// Test that duplicate default prefix declarations with the same namespace are allowed
String query = "PREFIX : <http://example.org/ns#>\n" +
"PREFIX : <http://example.org/ns#>\n" +
"SELECT ?name WHERE { :person :name ?name }";

// This should not throw an exception
assertDoesNotThrow(() -> parser.parseQuery(query, null));

ParsedQuery parsed = parser.parseQuery(query, null);
assertNotNull(parsed);
}

@Test
public void testDuplicatePrefixDeclarations_EmptyPrefix_DifferentNamespace_ShouldFail() {
// Test that duplicate default prefix declarations with different namespaces are rejected
String query = "PREFIX : <http://example.org/ns#>\n" +
"PREFIX : <http://example.org/different/>\n" +
"SELECT ?name WHERE { :person :name ?name }";

// This should throw a MalformedQueryException
assertThatExceptionOfType(MalformedQueryException.class)
.isThrownBy(() -> parser.parseQuery(query, null))
.withMessageContaining("Multiple prefix declarations");
}
}
30 changes: 30 additions & 0 deletions core/rio/api/src/test/java/org/eclipse/rdf4j/rio/helpers/AbstractRDFParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ public Resource getBNode() {
public Resource getBNode(String id) {
return createNode(id);
}

public void setNamespace(String prefix, String namespace) {
Copy link
Contributor

@JervenBolleman JervenBolleman Oct 6, 2025

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.

Copy link
Contributor

@hmottestad hmottestad Oct 6, 2025

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.

Copy link
Contributor

@hmottestad hmottestad Oct 6, 2025

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?

Copy link
Author

Copilot AI Oct 6, 2025

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.

super.setNamespace(prefix, namespace);
}

public String getNamespace(String prefix) throws RDFParseException {
return super.getNamespace(prefix);
}
}

@BeforeEach
Expand Down Expand Up @@ -89,4 +97,26 @@ public void testNodeIdHashing() {
assertThat(parser.createNode(longNodeId).stringValue())
.endsWith("2A372A91878F0980C8F53341D2D8A944");
}

@Test
public void testSetNamespace_DuplicateWithSameNamespace() {
// Test that setting the same namespace twice is allowed (last one wins)
parser.setNamespace("foaf", "http://xmlns.com/foaf/0.1/");
parser.setNamespace("foaf", "http://xmlns.com/foaf/0.1/");

// Should not throw an exception - namespace should be available
String namespace = parser.getNamespace("foaf");
assertThat(namespace).isEqualTo("http://xmlns.com/foaf/0.1/");
}

@Test
public void testSetNamespace_DuplicateWithDifferentNamespace() {
// Test that setting different namespaces for same prefix overwrites (existing behavior)
parser.setNamespace("foaf", "http://xmlns.com/foaf/0.1/");
parser.setNamespace("foaf", "http://example.org/different/");

// Should overwrite - current behavior of AbstractRDFParser
String namespace = parser.getNamespace("foaf");
assertThat(namespace).isEqualTo("http://example.org/different/");
}
}
Loading
Loading