Skip to content

Conversation

Copy link

Copilot AI commented Oct 5, 2025

Fixed issue where RepositoryConnection#getIsolationLevel() was always returning null even when an isolation level was set via begin(IsolationLevel).

Root Cause: SailRepositoryConnection overrides begin(IsolationLevel) and begin(TransactionSetting...) but doesn't call setIsolationLevel() to update the AbstractRepositoryConnection's isolationLevel field that is returned by getIsolationLevel().

Solution: Modified both methods in SailRepositoryConnection to call setIsolationLevel() before starting the transaction.

Changes Made

  • Modified SailRepositoryConnection.begin(IsolationLevel level) to call setIsolationLevel(level)
  • Modified SailRepositoryConnection.begin(TransactionSetting... settings) to call setIsolationLevel() for isolation level settings
  • Added comprehensive unit tests that verify the fix works for both method variants:
    • testGetIsolationLevel_shouldReturnSetLevel - Tests explicit isolation level setting via begin(IsolationLevel)
    • testGetIsolationLevel_shouldReturnSetLevelFromTransactionSettings - Tests isolation level via begin(TransactionSetting...)
    • testGetIsolationLevel_shouldReturnDefaultLevelWhenNotExplicitlySet - Tests that default isolation level is returned when begin() is called without parameters (currently failing, demonstrates additional issue)

Testing

  • ✅ Created failing test that reproduces the original issue
  • ✅ Verified that test fails before fix and passes after fix
  • ✅ All existing tests continue to pass
  • ✅ Code formatted and follows project conventions
  • ✅ Added test for default isolation level behavior (exposes additional issue where begin() without parameters doesn't properly set isolation level)

The fix is minimal and surgical - it only adds the missing setIsolationLevel() call that should have been there. This ensures that getIsolationLevel() properly returns the isolation level that was set when beginning a transaction. The additional test case reveals that there's still an issue with the default isolation level behavior that may need to be addressed separately.

Original prompt

This section details on the original issue you should resolve

<issue_title>Isolation level is always null, if queried from the RepositoryConnection</issue_title>
<issue_description>### Discussed in #4798

Originally posted by domkun October 6, 2023
Hi there,

I had a problem with concurrent transactions, so I wanted to log the isolation level of the current connection using RepositoryConnection#getIsolationLevel method. However it always returns null. The underlying sailConnection property of the SailRepositoryConnection shows the set isolation level. Is this intended?

I am a bit confused about this. Here is a short reproducer:

import org.eclipse.rdf4j.common.transaction.IsolationLevels;
import org.eclipse.rdf4j.repository.Repository;
import org.eclipse.rdf4j.repository.RepositoryConnection;
import org.eclipse.rdf4j.repository.sail.SailRepository;
import org.eclipse.rdf4j.sail.memory.MemoryStore;

class Scratch {
    public static void main(String[] args) {
        Repository sailRepository = new SailRepository(new MemoryStore());

        try (RepositoryConnection connection = sailRepository.getConnection()) {
            connection.begin(IsolationLevels.SERIALIZABLE);
            System.out.println(connection.getIsolationLevel());
            connection.commit();
        }
    }
}

Edit: Screenshot of the debugger window attached

image

Comments on the Issue (you are @copilot in this section)

Fixes #4802

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI changed the title [WIP] Isolation level is always null, if queried from the RepositoryConnection Fix RepositoryConnection.getIsolationLevel() returning null when isolation level is set Oct 5, 2025
Copilot AI requested a review from hmottestad October 5, 2025 20:32
Copilot finished work on behalf of hmottestad October 5, 2025 20:32
@hmottestad
Copy link
Contributor

@copilot remove initial-evidence.txt

Copy link
Author

Copilot AI commented Oct 5, 2025

@copilot remove initial-evidence.txt

Removed initial-evidence.txt file as requested in commit c14c3a9.

Copilot finished work on behalf of hmottestad October 5, 2025 20:49
@hmottestad
Copy link
Contributor

@copilot add a test where the isolation level is not explicitly specified by the user, which means that the default isolation level is used. Only add the test!

Copy link
Author

Copilot AI commented Oct 6, 2025

@copilot add a test where the isolation level is not explicitly specified by the user, which means that the default isolation level is used. Only add the test!

Added test testGetIsolationLevel_shouldReturnDefaultLevelWhenNotExplicitlySet that verifies default isolation level behavior when begin() is called without parameters. The test currently fails as expected, showing that getIsolationLevel() returns null instead of the default READ_COMMITTED level. Commit: 6f7e9d5

Copilot finished work on behalf of hmottestad October 6, 2025 14:19
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.

Isolation level is always null, if queried from the RepositoryConnection

2 participants