-
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?
Changes from 2 commits
908c075
e5a6d16
7ad6f52
4f12471
6964e82
5012e2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| /******************************************************************************* | ||
| * Copyright (c) 2025 Eclipse RDF4J contributors. | ||
| * | ||
| * All rights reserved. This program and the accompanying materials | ||
| * are made available under the terms of the Eclipse Distribution License v1.0 | ||
| * which accompanies this distribution, and is available at | ||
| * http://www.eclipse.org/org/documents/edl-v10.php. | ||
| * | ||
| * SPDX-License-Identifier: BSD-3-Clause | ||
| *******************************************************************************/ | ||
| package org.eclipse.rdf4j.rio.turtle; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; | ||
|
|
||
| import java.io.StringReader; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| import org.eclipse.rdf4j.model.Statement; | ||
| import org.eclipse.rdf4j.rio.RDFHandler; | ||
| import org.eclipse.rdf4j.rio.RDFParser; | ||
| import org.eclipse.rdf4j.rio.helpers.StatementCollector; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| /** | ||
| * Tests for duplicate prefix declarations in Turtle parser. | ||
| */ | ||
| public class TurtlePrefixDuplicateTest { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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. |
||
|
|
||
| private RDFParser parser; | ||
| private List<Statement> statements; | ||
| private RDFHandler handler; | ||
|
|
||
| @BeforeEach | ||
| public void setUp() { | ||
| parser = new TurtleParser(); | ||
| statements = new ArrayList<>(); | ||
| handler = new StatementCollector(statements); | ||
| parser.setRDFHandler(handler); | ||
| } | ||
|
|
||
| @Test | ||
| public void testDuplicatePrefixDeclarations_SameNamespace_ShouldPass() throws Exception { | ||
| String turtle = "@prefix foaf: <http://xmlns.com/foaf/0.1/> .\n" + | ||
| "@prefix foaf: <http://xmlns.com/foaf/0.1/> .\n" + | ||
| "\n" + | ||
| "<http://example.org/person> foaf:name \"John Doe\" .\n"; | ||
|
|
||
| // Should not throw an exception when same prefix maps to same namespace | ||
| assertDoesNotThrow(() -> parser.parse(new StringReader(turtle), "http://example.org/")); | ||
|
|
||
| // Should produce the expected statement | ||
| assertThat(statements).hasSize(1); | ||
| assertThat(statements.get(0).getPredicate().toString()).isEqualTo("http://xmlns.com/foaf/0.1/name"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testDuplicatePrefixDeclarations_DifferentNamespace_LastOneWins() throws Exception { | ||
| String turtle = "@prefix foaf: <http://xmlns.com/foaf/0.1/> .\n" + | ||
| "@prefix foaf: <http://example.org/different/> .\n" + | ||
| "\n" + | ||
| "<http://example.org/person> foaf:name \"John Doe\" .\n"; | ||
|
|
||
| // Should not throw an exception - Turtle parsers typically allow redefinition | ||
| assertDoesNotThrow(() -> parser.parse(new StringReader(turtle), "http://example.org/")); | ||
|
|
||
| // The last prefix declaration should win | ||
| assertThat(statements).hasSize(1); | ||
| assertThat(statements.get(0).getPredicate().toString()).isEqualTo("http://example.org/different/name"); | ||
| } | ||
|
|
||
| @Test | ||
| public void testDuplicateDefaultPrefixDeclarations_SameNamespace_ShouldPass() throws Exception { | ||
| String turtle = "@prefix : <http://example.org/ns#> .\n" + | ||
| "@prefix : <http://example.org/ns#> .\n" + | ||
| "\n" + | ||
| "<http://example.org/person> :name \"John Doe\" .\n"; | ||
|
|
||
| // Should not throw an exception when same default prefix maps to same namespace | ||
| assertDoesNotThrow(() -> parser.parse(new StringReader(turtle), "http://example.org/")); | ||
|
|
||
| // Should produce the expected statement | ||
| assertThat(statements).hasSize(1); | ||
| assertThat(statements.get(0).getPredicate().toString()).isEqualTo("http://example.org/ns#name"); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| ------------------------------------------------------------------------------- | ||
|
||
| Test set: org.eclipse.rdf4j.query.parser.sparql.SPARQLParserTest | ||
| ------------------------------------------------------------------------------- | ||
| Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.101 s <<< FAILURE! -- in org.eclipse.rdf4j.query.parser.sparql.SPARQLParserTest | ||
| org.eclipse.rdf4j.query.parser.sparql.SPARQLParserTest.testDuplicatePrefixDeclarations_SameNamespace_ShouldPass -- Time elapsed: 0.070 s <<< FAILURE! | ||
| org.opentest4j.AssertionFailedError: Unexpected exception thrown: org.eclipse.rdf4j.query.MalformedQueryException: Multiple prefix declarations for prefix 'foaf' | ||
| at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:152) | ||
| at org.junit.jupiter.api.AssertDoesNotThrow.createAssertionFailedError(AssertDoesNotThrow.java:84) | ||
| at org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:75) | ||
| at org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:58) | ||
| at org.junit.jupiter.api.Assertions.assertDoesNotThrow(Assertions.java:3196) | ||
| at org.eclipse.rdf4j.query.parser.sparql.SPARQLParserTest.testDuplicatePrefixDeclarations_SameNamespace_ShouldPass(SPARQLParserTest.java:1059) | ||
| at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) | ||
| at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) | ||
| at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) | ||
| at java.base/java.lang.reflect.Method.invoke(Method.java:566) | ||
| at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:727) | ||
| at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60) | ||
| at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131) | ||
| at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156) | ||
| at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147) | ||
| at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:86) | ||
| at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(InterceptingExecutableInvoker.java:103) | ||
| at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.lambda$invoke$0(InterceptingExecutableInvoker.java:93) | ||
| at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106) | ||
| at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64) | ||
| at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45) | ||
| at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37) | ||
| at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:92) | ||
| at org.junit.jupiter.engine.execution.InterceptingExecutableInvoker.invoke(InterceptingExecutableInvoker.java:86) | ||
| at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:217) | ||
| at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) | ||
| at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:213) | ||
| at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:138) | ||
| at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:68) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151) | ||
| at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) | ||
| at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) | ||
| at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) | ||
| at java.base/java.util.ArrayList.forEach(ArrayList.java:1541) | ||
| at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155) | ||
| at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) | ||
| at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) | ||
| at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) | ||
| at java.base/java.util.ArrayList.forEach(ArrayList.java:1541) | ||
| at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155) | ||
| at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141) | ||
| at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139) | ||
| at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138) | ||
| at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95) | ||
| at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35) | ||
| at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57) | ||
| at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54) | ||
| at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:147) | ||
| at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:127) | ||
| at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:90) | ||
| at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:55) | ||
| at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:102) | ||
| at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:54) | ||
| at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114) | ||
| at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86) | ||
| at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86) | ||
| at org.apache.maven.surefire.junitplatform.LauncherAdapter.executeWithoutCancellationToken(LauncherAdapter.java:60) | ||
| at org.apache.maven.surefire.junitplatform.LauncherAdapter.execute(LauncherAdapter.java:52) | ||
| at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.execute(JUnitPlatformProvider.java:203) | ||
| at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invokeAllTests(JUnitPlatformProvider.java:168) | ||
| at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:136) | ||
| at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385) | ||
| at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162) | ||
| at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507) | ||
| at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495) | ||
| Caused by: org.eclipse.rdf4j.query.MalformedQueryException: Multiple prefix declarations for prefix 'foaf' | ||
| at org.eclipse.rdf4j.query.parser.sparql.PrefixDeclProcessor.process(PrefixDeclProcessor.java:72) | ||
| at org.eclipse.rdf4j.query.parser.sparql.SPARQLParser.parseQuery(SPARQLParser.java:198) | ||
| at org.eclipse.rdf4j.query.parser.sparql.SPARQLParserTest.lambda$testDuplicatePrefixDeclarations_SameNamespace_ShouldPass$5(SPARQLParserTest.java:1059) | ||
| at org.junit.jupiter.api.AssertDoesNotThrow.assertDoesNotThrow(AssertDoesNotThrow.java:71) | ||
| ... 75 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.
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.