-
Notifications
You must be signed in to change notification settings - Fork 0
Add transaction management to Repository interface #2
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: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Robotv2 <81443374+Robotv2@users.noreply.github.com>
…entations Co-authored-by: Robotv2 <81443374+Robotv2@users.noreply.github.com>
… comprehensive transaction tests Co-authored-by: Robotv2 <81443374+Robotv2@users.noreply.github.com>
…management Co-authored-by: Robotv2 <81443374+Robotv2@users.noreply.github.com>
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.
Pull Request Overview
This pull request adds transaction support to the repository API, enabling atomic operations across multiple database actions. The implementation includes transaction management methods and proper connection handling for pooled database connections.
- Adds transaction management methods (
beginTransaction(),commit(),rollback(),executeInTransaction()) to the Repository interface - Implements transaction support in SQLRepository using ThreadLocal connections
- Updates HikariDatabase to handle transactional and non-transactional connections appropriately
- Adds comprehensive test coverage for transaction scenarios
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| gradlew | Standard Gradle wrapper script added for build automation |
| Anchor-api/src/main/java/fr/robotv2/anchor/api/repository/Repository.java | Defines transaction management methods in the Repository interface |
| Anchor-sql/src/main/java/fr/robotv2/anchor/sql/repository/SQLRepository.java | Implements transaction support with ThreadLocal connection management |
| Anchor-sql/src/main/java/fr/robotv2/anchor/sql/database/HikariDatabase.java | Updates database operations to support transactional connections |
| Anchor-json/src/main/java/fr/robotv2/anchor/json/JsonRepository.java | Throws UnsupportedOperationException for transaction methods |
| Anchor-api/src/testFixtures/java/fr/robotv2/anchor/test/AbstractUserLongTest.java | Adds comprehensive transaction test cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Only close the connection if it's a real transaction connection | ||
| // For pooled connections managed by HikariDatabase, let the pool handle it | ||
| if (!(database instanceof HikariDatabase)) { | ||
| connection.close(); | ||
| } | ||
| } catch (SQLException exception) { | ||
| logger.log(Level.WARNING, "Failed to reset auto-commit", exception); | ||
| } | ||
|
|
||
| // Clear the transaction connection from ThreadLocal | ||
| transactionConnection.remove(); | ||
|
|
||
| // If this is a HikariDatabase, clear the transaction connection and close it | ||
| if (database instanceof HikariDatabase) { | ||
| ((HikariDatabase) database).setTransactionConnection(null); | ||
| try { | ||
| connection.close(); | ||
| } catch (SQLException exception) { | ||
| logger.log(Level.WARNING, "Failed to close connection", exception); | ||
| } |
Copilot
AI
Oct 30, 2025
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.
The connection cleanup logic contains contradictory conditional branches. Lines 207-209 skip closing the connection for HikariDatabase, but lines 218-225 then explicitly close it for HikariDatabase. This creates confusing and potentially inefficient logic. Simplify by removing the first conditional check and always handling HikariDatabase connections in the second block, or consolidate the logic into a single conditional structure.
| // Only close the connection if it's a real transaction connection | |
| // For pooled connections managed by HikariDatabase, let the pool handle it | |
| if (!(database instanceof HikariDatabase)) { | |
| connection.close(); | |
| } | |
| } catch (SQLException exception) { | |
| logger.log(Level.WARNING, "Failed to reset auto-commit", exception); | |
| } | |
| // Clear the transaction connection from ThreadLocal | |
| transactionConnection.remove(); | |
| // If this is a HikariDatabase, clear the transaction connection and close it | |
| if (database instanceof HikariDatabase) { | |
| ((HikariDatabase) database).setTransactionConnection(null); | |
| try { | |
| connection.close(); | |
| } catch (SQLException exception) { | |
| logger.log(Level.WARNING, "Failed to close connection", exception); | |
| } | |
| } catch (SQLException exception) { | |
| logger.log(Level.WARNING, "Failed to reset auto-commit", exception); | |
| } | |
| // Clear the transaction connection from ThreadLocal | |
| transactionConnection.remove(); | |
| try { | |
| if (database instanceof HikariDatabase) { | |
| ((HikariDatabase) database).setTransactionConnection(null); | |
| } | |
| connection.close(); | |
| } catch (SQLException exception) { | |
| logger.log(Level.WARNING, "Failed to close connection", exception); |
|
|
||
| private void cleanupTransaction(Connection connection) { | ||
| try { | ||
| connection.setAutoCommit(true); |
Copilot
AI
Oct 30, 2025
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.
If connection.setAutoCommit(true) fails and throws an exception, the ThreadLocal cleanup (line 215) and HikariDatabase cleanup (lines 218-225) will never execute. This can lead to connection leaks and ThreadLocal memory leaks. Move the transactionConnection.remove() and HikariDatabase cleanup outside the try-catch block or use a nested try-finally structure to ensure cleanup always occurs.
Adds
beginTransaction(),commit(),rollback(), andexecuteInTransaction(Consumer<Repository>)methods to theRepositoryinterface for atomic multi-operation workflows.Changes
Repository interface: Added four transaction methods with comprehensive Javadoc
beginTransaction(),commit(),rollback()executeInTransaction()with automatic commit/rollback handlingSQLRepository: ThreadLocal-based transaction connection management
HikariDatabase: Transaction-aware connection pooling
getConnection()to return transaction connection when activeJsonRepository: Throws
UnsupportedOperationExceptionfor all transaction methodsTests: Added 8 test cases covering commit, rollback, error handling, and isolation
Usage
SQLite already declares
SupportType.TRANSACTIONsupport; JSON databases skip transaction tests via JUnit Assumptions.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
hub.spigotmc.org/usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=384m -XX:+HeapDumpOnOutOfMemoryError -Xms256m -Xmx512m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant -cp /home/REDACTED/.gradle/wrapper/dists/gradle-8.13-bin/5xuhj0ry160q40clulazy9h7d/gradle-8.13/lib/gradle-daemon-main-8.13.jar -javaagent:/home/REDACTED/.gradle/wrapper/dists/gradle-8.13-bin/5xuhj0ry160q40clulazy9h7d/gradle-8.13/lib/agents/gradle-instrumentation-agent-8.13.jar org.gradle.launcher.daemon.bootstrap.GradleDaemon 8.13(dns block)jitpack.io/usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=384m -XX:+HeapDumpOnOutOfMemoryError -Xms256m -Xmx512m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant -cp /home/REDACTED/.gradle/wrapper/dists/gradle-8.13-bin/5xuhj0ry160q40clulazy9h7d/gradle-8.13/lib/gradle-daemon-main-8.13.jar -javaagent:/home/REDACTED/.gradle/wrapper/dists/gradle-8.13-bin/5xuhj0ry160q40clulazy9h7d/gradle-8.13/lib/agents/gradle-instrumentation-agent-8.13.jar org.gradle.launcher.daemon.bootstrap.GradleDaemon 8.13(dns block)s01.oss.sonatype.org/usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -XX:MaxMetaspaceSize=384m -XX:+HeapDumpOnOutOfMemoryError -Xms256m -Xmx512m -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant -cp /home/REDACTED/.gradle/wrapper/dists/gradle-8.13-bin/5xuhj0ry160q40clulazy9h7d/gradle-8.13/lib/gradle-daemon-main-8.13.jar -javaagent:/home/REDACTED/.gradle/wrapper/dists/gradle-8.13-bin/5xuhj0ry160q40clulazy9h7d/gradle-8.13/lib/agents/gradle-instrumentation-agent-8.13.jar org.gradle.launcher.daemon.bootstrap.GradleDaemon 8.13(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.