Skip to content

Avoid NPE if get() is never called before close() #35

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

Merged
garricko merged 7 commits into from
Sep 10, 2020
Merged

Avoid NPE if get() is never called before close() #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9c5c25a
Avoid NPE if get() is never called before close()
garricko Sep 9, 2020
af89c54
Remove IntelliJ module files
garricko Sep 9, 2020
661dc12
Revert "commitAndClose/rollbackAndClose should close even if there is…
garricko Sep 9, 2020
bc012d2
Remove a redundant flag
garricko Sep 10, 2020
4a30256
Ignore IntelliJ module files
garricko Sep 10, 2020
af272cb
Add a test case for an NPE in close()
garricko Sep 10, 2020
b030eb3
Update Vertx provider for changes to regular one
garricko Sep 10, 2020
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ build
out
classes
.idea
*.iml
local.properties
.vagrant
.DS_Store
Expand Down
44 changes: 0 additions & 44 deletions database.iml
View file
Open in desktop

This file was deleted.

51 changes: 0 additions & 51 deletions demo/database-demo.iml
View file
Open in desktop

This file was deleted.

21 changes: 10 additions & 11 deletions src/main/java/com/github/susom/database/DatabaseProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ public final class DatabaseProvider implements Supplier<Database> {
private static final AtomicInteger poolNameCounter = new AtomicInteger(1);
private DatabaseProvider delegateTo = null;
private Supplier<Connection> connectionProvider;
private boolean txStarted = false;
private Connection connection = null;
private Database database = null;
private final Options options;
Expand Down Expand Up @@ -954,7 +953,6 @@ public Database get() {
Metric metric = new Metric(log.isDebugEnabled());
try {
connection = connectionProvider.get();
txStarted = true;
metric.checkpoint("getConn");
try {
// Generally check autocommit before setting because databases like
Expand Down Expand Up @@ -1060,14 +1058,14 @@ public void commitAndClose() {
return;
}

if (txStarted) {
if (connection != null) {
try {
connection.commit();
} catch (Exception e) {
throw new DatabaseException("Unable to commit the transaction", e);
}
close();
}
close();
}

public void rollbackAndClose() {
Expand All @@ -1076,25 +1074,26 @@ public void rollbackAndClose() {
return;
}

if (txStarted) {
if (connection != null) {
try {
connection.rollback();
} catch (Exception e) {
log.error("Unable to rollback the transaction", e);
}
close();
}
close();
}

private void close() {
try {
connection.close();
} catch (Exception e) {
log.error("Unable to close the database connection", e);
if (connection != null) {
try {
connection.close();
} catch (Exception e) {
log.error("Unable to close the database connection", e);
}
}
connection = null;
database = null;
txStarted = false;
connectionProvider = null;
}

Expand Down
17 changes: 8 additions & 9 deletions src/main/java/com/github/susom/database/DatabaseProviderVertx.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ public final class DatabaseProviderVertx implements Supplier<Database> {
private WorkerExecutor executor;
private DatabaseProviderVertx delegateTo = null;
private Supplier<Connection> connectionProvider;
private boolean txStarted = false;
private Connection connection = null;
private Database database = null;
private final Options options;
Expand Down Expand Up @@ -536,7 +535,6 @@ public Database get() {
Metric metric = new Metric(log.isDebugEnabled());
try {
connection = connectionProvider.get();
txStarted = true;
metric.checkpoint("getConn");
try {
// Generally check autocommit before setting because databases like
Expand Down Expand Up @@ -652,7 +650,7 @@ public void commitAndClose() {
return;
}

if (txStarted) {
if (connection != null) {
try {
connection.commit();
} catch (Exception e) {
Expand All @@ -668,7 +666,7 @@ public void rollbackAndClose() {
return;
}

if (txStarted) {
if (connection != null) {
try {
connection.rollback();
} catch (Exception e) {
Expand All @@ -679,14 +677,15 @@ public void rollbackAndClose() {
}

private void close() {
try {
connection.close();
} catch (Exception e) {
log.error("Unable to close the database connection", e);
if (connection != null) {
try {
connection.close();
} catch (Exception e) {
log.error("Unable to close the database connection", e);
}
}
connection = null;
database = null;
txStarted = false;
connectionProvider = null;
}
}
19 changes: 19 additions & 0 deletions src/test/java/com/github/susom/database/test/HsqldbTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,25 @@ protected DatabaseProvider createDatabaseProvider(OptionsOverride options) throw
.withOptions(options).create();
}

@Test
public void noDatabaseAccess() throws Exception {
DatabaseProvider provider = createDatabaseProvider(new OptionsOverride());
provider.transact(dbp -> {
// Do nothing, just making sure no exception is thrown
});
provider.transact((dbp, tx) -> {
// Do nothing, just making sure no exception is thrown
});
provider.transact((dbp, tx) -> {
tx.setRollbackOnError(true);
// Do nothing, just making sure no exception is thrown
});
provider.transact((dbp, tx) -> {
tx.setRollbackOnly(true);
// Do nothing, just making sure no exception is thrown
});
}

@Ignore("LocalDate implementations should be TimeZone agnostic, but HSQLDB implementation has a bug.")
@Test
public void argLocalDateTimeZones() {
Expand Down