Skip to content

Conversation

hmottestad
Copy link
Contributor

GitHub issue resolved: #5403

Briefly describe the changes proposed in this PR:


PR Author Checklist (see the contributor guidelines for more details):

  • my pull request is self-contained
  • I've added tests for the changes I made
  • I've applied code formatting (you can use mvn process-resources to format from the command line)
  • I've squashed my commits where necessary
  • every commit message starts with the issue number (GH-xxxx) followed by a meaningful description of the change

@hmottestad hmottestad requested a review from Copilot September 29, 2025 17:43
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR strengthens regression testing for base URI handling in transactions by adding comprehensive test coverage and fixing a bug in base URI parameter encoding. The changes ensure that base URIs are passed as plain strings rather than being incorrectly encoded as RDF values.

  • Added integration test to verify base URI resolution works correctly end-to-end
  • Added unit test to verify base URI parameter is passed correctly in HTTP requests
  • Fixed bug where base URI was being encoded as an RDF IRI instead of passed as a plain string

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
RemoteRepositoryBaseUriIT.java New integration test verifying base URI resolution with RDF/XML data
RDF4JProtocolSessionTest.java New unit test verifying base URI parameter handling in HTTP requests
RDF4JProtocolSession.java Bug fix to pass base URI as plain string instead of encoded RDF value

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +1072 to 1077
if (baseURI != null) {
String trimmedBaseURI = baseURI.trim();
if (!trimmedBaseURI.isEmpty()) {
url.setParameter(Protocol.BASEURI_PARAM_NAME, trimmedBaseURI);
}
}
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider extracting the trimmed base URI validation into a separate method or using a guard clause to reduce nesting. The current nested if structure could be flattened for better readability.

Suggested change
if (baseURI != null) {
String trimmedBaseURI = baseURI.trim();
if (!trimmedBaseURI.isEmpty()) {
url.setParameter(Protocol.BASEURI_PARAM_NAME, trimmedBaseURI);
}
}
setBaseURIParameter(url, baseURI);

Copilot uses AI. Check for mistakes.

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 this pull request may close these issues.

Remote transaction sends base URI with < and > delimiters, causing invalid IRIs to be created on remote end

1 participant