exceptionfactory commented on code in PR #11761:
URL: https://github.com/apache/nifi/pull/11761#discussion_r4368966312
##########
nifi-extension-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/test/java/org/apache/nifi/dbcp/DBCPServiceTest.java:
##########
@@ -63,25 +66,55 @@ public class DBCPServiceTest {
private static final String NON_JDBC_URL =
"http://localhost:3306/database";;
private static final String DRIVER_CLASS = "org.hsqldb.jdbc.JDBCDriver";
- private static final String CONNECTION_URL_FORMAT = "jdbc:hsqldb:file:%s";
+ private static final String CONNECTION_URL_FORMAT =
"jdbc:hsqldb:file:%s/nifi";
+ private static final String USERNAME = String.class.getSimpleName();
+ private static final String PASSWORD = String.class.getName();
private TestRunner runner;
private DBCPConnectionPool service;
+ private String connectionUrl;
+
+ private Path databasePath;
+
@BeforeEach
public void setService(@TempDir final Path tempDir) throws
InitializationException {
service = new DBCPConnectionPool();
runner = TestRunners.newTestRunner(NoOpProcessor.class);
runner.addControllerService(SERVICE_ID, service);
- final String url = CONNECTION_URL_FORMAT.formatted(tempDir);
- runner.setProperty(service, DBCPProperties.DATABASE_URL, url);
- runner.setProperty(service, DBCPProperties.DB_USER,
String.class.getSimpleName());
- runner.setProperty(service, DBCPProperties.DB_PASSWORD,
String.class.getName());
+ databasePath = tempDir.toAbsolutePath();
+ connectionUrl = CONNECTION_URL_FORMAT.formatted(databasePath);
+ runner.setProperty(service, DBCPProperties.DATABASE_URL,
connectionUrl);
+ runner.setProperty(service, DBCPProperties.DB_USER, USERNAME);
+ runner.setProperty(service, DBCPProperties.DB_PASSWORD, PASSWORD);
runner.setProperty(service, DBCPProperties.DB_DRIVERNAME,
DRIVER_CLASS);
}
+ @AfterEach
+ public void shutdownDatabase() throws Exception {
+ runner.shutdown();
+
+ // Property validation tests may never create the database.
+ // Do not create it just to shut it down.
Review Comment:
Recommend changing to:
```suggestion
// Attempt shutdown only when database files found
```
##########
nifi-extension-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/test/java/org/apache/nifi/record/sink/db/DatabaseRecordSinkTest.java:
##########
@@ -90,89 +92,102 @@ public class DatabaseRecordSinkTest {
private static final String SERVICE_ID =
DBCPConnectionPool.class.getName();
private static final String DRIVER_CLASS = "org.hsqldb.jdbc.JDBCDriver";
- private static final String CONNECTION_URL_FORMAT = "jdbc:hsqldb:file:%s";
+ private static final String CONNECTION_URL_FORMAT =
"jdbc:hsqldb:file:%s/nifi";
private String connectionUrl;
private DBCPConnectionPool dbcpService;
+ private TestRunner runner;
@BeforeEach
public void setService(@TempDir final Path tempDir) throws
InitializationException {
dbcpService = new DBCPConnectionPool();
- TestRunner runner = TestRunners.newTestRunner(NoOpProcessor.class);
+ runner = TestRunners.newTestRunner(NoOpProcessor.class);
runner.addControllerService(SERVICE_ID, dbcpService);
- connectionUrl = CONNECTION_URL_FORMAT.formatted(tempDir);
+ connectionUrl =
CONNECTION_URL_FORMAT.formatted(tempDir.toAbsolutePath());
runner.setProperty(dbcpService, DATABASE_URL, connectionUrl);
runner.setProperty(dbcpService, DB_USER, String.class.getSimpleName());
runner.setProperty(dbcpService, DB_PASSWORD, String.class.getName());
runner.setProperty(dbcpService, DB_DRIVERNAME, DRIVER_CLASS);
}
+ @AfterEach
+ public void shutdownDatabase() throws SQLException {
+ runner.shutdown();
+ dbcpService.shutdown();
+
+ try (Connection conn = DriverManager.getConnection(connectionUrl);
Review Comment:
It looks like this should also use the `JDBCDriver` approach instead of
DriverManager
##########
nifi-extension-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/test/java/org/apache/nifi/dbcp/DBCPServiceTest.java:
##########
@@ -63,25 +66,55 @@ public class DBCPServiceTest {
private static final String NON_JDBC_URL =
"http://localhost:3306/database";;
private static final String DRIVER_CLASS = "org.hsqldb.jdbc.JDBCDriver";
- private static final String CONNECTION_URL_FORMAT = "jdbc:hsqldb:file:%s";
+ private static final String CONNECTION_URL_FORMAT =
"jdbc:hsqldb:file:%s/nifi";
+ private static final String USERNAME = String.class.getSimpleName();
+ private static final String PASSWORD = String.class.getName();
private TestRunner runner;
private DBCPConnectionPool service;
+ private String connectionUrl;
+
+ private Path databasePath;
+
@BeforeEach
public void setService(@TempDir final Path tempDir) throws
InitializationException {
service = new DBCPConnectionPool();
runner = TestRunners.newTestRunner(NoOpProcessor.class);
runner.addControllerService(SERVICE_ID, service);
- final String url = CONNECTION_URL_FORMAT.formatted(tempDir);
- runner.setProperty(service, DBCPProperties.DATABASE_URL, url);
- runner.setProperty(service, DBCPProperties.DB_USER,
String.class.getSimpleName());
- runner.setProperty(service, DBCPProperties.DB_PASSWORD,
String.class.getName());
+ databasePath = tempDir.toAbsolutePath();
+ connectionUrl = CONNECTION_URL_FORMAT.formatted(databasePath);
+ runner.setProperty(service, DBCPProperties.DATABASE_URL,
connectionUrl);
+ runner.setProperty(service, DBCPProperties.DB_USER, USERNAME);
+ runner.setProperty(service, DBCPProperties.DB_PASSWORD, PASSWORD);
runner.setProperty(service, DBCPProperties.DB_DRIVERNAME,
DRIVER_CLASS);
}
+ @AfterEach
+ public void shutdownDatabase() throws Exception {
+ runner.shutdown();
+
+ // Property validation tests may never create the database.
+ // Do not create it just to shut it down.
+ final Path propertiesFile = databasePath.resolve("nifi.properties");
+ if (Files.exists(propertiesFile)) {
+ final Properties properties = new Properties();
+ properties.setProperty("user", USERNAME);
+ properties.setProperty("password", PASSWORD);
+
+ // Connect directly through HSQLDB's driver so cleanup still works
+ // when testDeregisterDriver has deregistered it from
DriverManager.
+ try (Connection connection = new
org.hsqldb.jdbc.JDBCDriver().connect(connectionUrl, properties)) {
Review Comment:
The import should be used instead of the full class name
##########
nifi-extension-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/test/java/org/apache/nifi/dbcp/DBCPServiceTest.java:
##########
@@ -63,25 +66,55 @@ public class DBCPServiceTest {
private static final String NON_JDBC_URL =
"http://localhost:3306/database";;
private static final String DRIVER_CLASS = "org.hsqldb.jdbc.JDBCDriver";
- private static final String CONNECTION_URL_FORMAT = "jdbc:hsqldb:file:%s";
+ private static final String CONNECTION_URL_FORMAT =
"jdbc:hsqldb:file:%s/nifi";
+ private static final String USERNAME = String.class.getSimpleName();
+ private static final String PASSWORD = String.class.getName();
private TestRunner runner;
private DBCPConnectionPool service;
+ private String connectionUrl;
+
+ private Path databasePath;
+
@BeforeEach
public void setService(@TempDir final Path tempDir) throws
InitializationException {
service = new DBCPConnectionPool();
runner = TestRunners.newTestRunner(NoOpProcessor.class);
runner.addControllerService(SERVICE_ID, service);
- final String url = CONNECTION_URL_FORMAT.formatted(tempDir);
- runner.setProperty(service, DBCPProperties.DATABASE_URL, url);
- runner.setProperty(service, DBCPProperties.DB_USER,
String.class.getSimpleName());
- runner.setProperty(service, DBCPProperties.DB_PASSWORD,
String.class.getName());
+ databasePath = tempDir.toAbsolutePath();
+ connectionUrl = CONNECTION_URL_FORMAT.formatted(databasePath);
+ runner.setProperty(service, DBCPProperties.DATABASE_URL,
connectionUrl);
+ runner.setProperty(service, DBCPProperties.DB_USER, USERNAME);
+ runner.setProperty(service, DBCPProperties.DB_PASSWORD, PASSWORD);
runner.setProperty(service, DBCPProperties.DB_DRIVERNAME,
DRIVER_CLASS);
}
+ @AfterEach
+ public void shutdownDatabase() throws Exception {
+ runner.shutdown();
+
+ // Property validation tests may never create the database.
+ // Do not create it just to shut it down.
+ final Path propertiesFile = databasePath.resolve("nifi.properties");
+ if (Files.exists(propertiesFile)) {
+ final Properties properties = new Properties();
+ properties.setProperty("user", USERNAME);
+ properties.setProperty("password", PASSWORD);
+
+ // Connect directly through HSQLDB's driver so cleanup still works
+ // when testDeregisterDriver has deregistered it from
DriverManager.
Review Comment:
```suggestion
// Connect with JDBCDriver to avoid DriverManager deregistration
```
##########
nifi-extension-bundles/nifi-extension-utils/nifi-database-utils/src/test/java/org/apache/nifi/util/db/AbstractConnectionTest.java:
##########
@@ -16,17 +16,19 @@
*/
package org.apache.nifi.util.db;
+import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.io.TempDir;
import java.nio.file.Path;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;
+import java.sql.Statement;
abstract class AbstractConnectionTest {
private static final String DRIVER_CLASS = "org.hsqldb.jdbc.JDBCDriver";
- private static final String CONNECTION_URL_FORMAT = "jdbc:hsqldb:file:%s";
+ private static final String CONNECTION_URL_FORMAT =
"jdbc:hsqldb:file:%1$s/nifi";
Review Comment:
```suggestion
private static final String CONNECTION_URL_FORMAT =
"jdbc:hsqldb:file:%s/nifi";
```
##########
nifi-extension-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/test/java/org/apache/nifi/dbcp/DBCPServiceTest.java:
##########
@@ -63,25 +66,55 @@ public class DBCPServiceTest {
private static final String NON_JDBC_URL =
"http://localhost:3306/database";;
private static final String DRIVER_CLASS = "org.hsqldb.jdbc.JDBCDriver";
- private static final String CONNECTION_URL_FORMAT = "jdbc:hsqldb:file:%s";
+ private static final String CONNECTION_URL_FORMAT =
"jdbc:hsqldb:file:%s/nifi";
+ private static final String USERNAME = String.class.getSimpleName();
+ private static final String PASSWORD = String.class.getName();
private TestRunner runner;
private DBCPConnectionPool service;
+ private String connectionUrl;
+
+ private Path databasePath;
+
@BeforeEach
public void setService(@TempDir final Path tempDir) throws
InitializationException {
service = new DBCPConnectionPool();
runner = TestRunners.newTestRunner(NoOpProcessor.class);
runner.addControllerService(SERVICE_ID, service);
- final String url = CONNECTION_URL_FORMAT.formatted(tempDir);
- runner.setProperty(service, DBCPProperties.DATABASE_URL, url);
- runner.setProperty(service, DBCPProperties.DB_USER,
String.class.getSimpleName());
- runner.setProperty(service, DBCPProperties.DB_PASSWORD,
String.class.getName());
+ databasePath = tempDir.toAbsolutePath();
+ connectionUrl = CONNECTION_URL_FORMAT.formatted(databasePath);
+ runner.setProperty(service, DBCPProperties.DATABASE_URL,
connectionUrl);
+ runner.setProperty(service, DBCPProperties.DB_USER, USERNAME);
+ runner.setProperty(service, DBCPProperties.DB_PASSWORD, PASSWORD);
runner.setProperty(service, DBCPProperties.DB_DRIVERNAME,
DRIVER_CLASS);
}
+ @AfterEach
+ public void shutdownDatabase() throws Exception {
+ runner.shutdown();
+
+ // Property validation tests may never create the database.
+ // Do not create it just to shut it down.
+ final Path propertiesFile = databasePath.resolve("nifi.properties");
Review Comment:
Recommend declaring `nifi.properties` next to the `CONNECTION_URL_FORMAT` as
`DATABASE_PROPERTIES` to highlight the naming link
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
