Skip to content

Refactor DriverResolver to create temporary directory for driver file management #433

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

Closed
TharmiganK wants to merge 1 commit into from
Closed

Refactor DriverResolver to create temporary directory for driver file management #433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 2 additions & 4 deletions persist-core/src/main/java/io/ballerina/persist/introspect/Introspector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ public PersistConfiguration getPersistConfiguration() {
* or mapping database structures to Ballerina types
*/
public Module introspectDatabase() throws BalException {
DriverResolver driverResolver = new DriverResolver(this.persistConfiguration.getSourcePath(),
this.persistConfiguration.getProvider());
DriverResolver driverResolver = new DriverResolver(this.persistConfiguration.getProvider());
try {
Project driverProject = driverResolver.resolveDriverDependencies();
try (Connection connection = prepareDatabaseConnection(driverProject)) {
Expand All @@ -146,8 +145,7 @@ public Module introspectDatabase() throws BalException {
* reading table information
*/
public String[] getAvailableTables() throws BalException {
DriverResolver driverResolver = new DriverResolver(this.persistConfiguration.getSourcePath(),
this.persistConfiguration.getProvider());
DriverResolver driverResolver = new DriverResolver(this.persistConfiguration.getProvider());
try {
Project driverProject = driverResolver.resolveDriverDependencies();
try (Connection connection = prepareDatabaseConnection(driverProject)) {
Expand Down
29 changes: 19 additions & 10 deletions persist-core/src/main/java/io/ballerina/persist/nodegenerator/DriverResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,23 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

public class DriverResolver {

private final Path driverImportFile;
private final String datastore;
private final Path tempDirectory;

public DriverResolver(String sourcePath, String datastore) {
driverImportFile = Paths.get(sourcePath, "persist/driver.bal");
this.datastore = datastore;
public DriverResolver(String datastore) throws BalException {
try {
// Create a temporary directory along with some prefix
this.tempDirectory = Files.createTempDirectory("persist-driver-test-");
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

The prefix "persist-driver-test-" suggests this is for testing purposes, but this class appears to be production code. Consider using a more appropriate prefix like "persist-driver-" instead.

Suggested change
this.tempDirectory = Files.createTempDirectory("persist-driver-test-");
this.tempDirectory = Files.createTempDirectory("persist-driver-");

Copilot uses AI. Check for mistakes.
// Set the driver file path in the temp directory
this.driverImportFile = this.tempDirectory.resolve("driver.bal");
this.datastore = datastore;
} catch (IOException e) {
throw new BalException("failed to create temporary directory: " + e.getMessage());
}
}
Comment on lines +38 to 48
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

The refactored constructor introduces new behavior (temporary directory creation and error handling) that should be tested. Consider adding unit tests to verify: 1) successful temp directory creation, 2) proper error handling when directory creation fails, 3) cleanup behavior in deleteDriverFile() including edge cases where the directory is not empty or deletion fails.

Copilot uses AI. Check for mistakes.
Comment on lines +38 to 48
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

Consider implementing AutoCloseable for DriverResolver to ensure proper cleanup of the temporary directory. The current design relies on the caller to explicitly invoke deleteDriverFile(), which could lead to resource leaks if an exception occurs before cleanup or if callers forget to call the cleanup method. Using try-with-resources would provide more robust resource management.

Copilot uses AI. Check for mistakes.

public Project resolveDriverDependencies() throws BalException {
Expand All @@ -48,25 +55,27 @@ public Project resolveDriverDependencies() throws BalException {
private void createDriverImportFile() throws BalException {
DbModelGenSyntaxTree dbModelGenSyntaxTree = new DbModelGenSyntaxTree();
try {
writeOutputFile
(Formatter.format(dbModelGenSyntaxTree.createInitialDriverImportFile(datastore).toSourceCode()),
driverImportFile);
writeOutputFile(Formatter.format(
dbModelGenSyntaxTree.createInitialDriverImportFile(datastore).toSourceCode()));
} catch (Exception e) {
throw new BalException("failed to create driver import file: " + e.getMessage());
}
}

private void writeOutputFile(String syntaxTree, Path outPath) throws IOException {
try (PrintWriter writer = new PrintWriter(outPath.toString(), StandardCharsets.UTF_8)) {
private void writeOutputFile(String syntaxTree) throws IOException {
try (PrintWriter writer = new PrintWriter(this.driverImportFile.toString(), StandardCharsets.UTF_8)) {
writer.println(syntaxTree);
}
}

public void deleteDriverFile() throws BalException {
try {
Files.deleteIfExists(driverImportFile);
if (Files.exists(tempDirectory)) {
Files.delete(tempDirectory);
Comment on lines +74 to +75
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

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

The Files.delete(tempDirectory) call will fail if the directory is not empty. Files.delete() only works for empty directories or files. Since buildDriverFile() calls SingleFileProject.load() which may create additional cache or build artifacts in the temp directory, the deletion may fail. Consider checking if the directory is empty first, or using a recursive deletion approach to ensure all contents are removed before deleting the directory itself.

Copilot uses AI. Check for mistakes.
}
} catch (IOException e) {
throw new BalException("failed to delete driver import file: " + e.getMessage());
throw new BalException("failed to delete driver import file and temp directory: " + e.getMessage());
}
}

Expand Down
Loading