Skip to content
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

Switch to org.postgresql #4031

Merged
merged 4 commits into from
May 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ dependencies {
// VersionEye states that 6.0.5 is the most recent version, but http://dev.mysql.com/downloads/connector/j/ shows that as "Development Release"
compile 'mysql:mysql-connector-java:5.1.46'

compile 'com.impossibl.pgjdbc-ng:pgjdbc-ng:0.7.1'
compile 'org.postgresql:postgresql:42.2.2'

compile 'net.java.dev.glazedlists:glazedlists_java15:1.9.1'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import javax.swing.JComponent;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JOptionPane;
import javax.swing.JPanel;
import javax.swing.JPasswordField;
import javax.swing.JTextField;
Expand Down Expand Up @@ -156,8 +155,9 @@ public void openSharedDatabase() {

return; // setLoadingConnectButtonText(false) should not be reached regularly.
} catch (SQLException | InvalidDBMSConnectionPropertiesException exception) {
JOptionPane.showMessageDialog(ConnectToSharedDatabaseDialog.this, exception.getMessage(),
Localization.lang("Connection error"), JOptionPane.ERROR_MESSAGE);

DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().showErrorDialogAndWait(Localization.lang("Connection error"), exception));

} catch (DatabaseNotSupportedException exception) {
new MigrationHelpDialog(this).setVisible(true);
}
Expand Down Expand Up @@ -187,7 +187,7 @@ public void actionPerformed(ActionEvent e) {

openSharedDatabase();
} catch (JabRefException exception) {
frame.getDialogService().showErrorDialogAndWait(Localization.lang("Warning"), exception);
DefaultTaskExecutor.runInJavaFXThread(() -> frame.getDialogService().showErrorDialogAndWait(Localization.lang("Warning"), exception));

}
}
Expand Down
66 changes: 25 additions & 41 deletions src/main/java/org/jabref/logic/shared/PostgreSQLProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,20 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.logging.Level;
import java.util.logging.Logger;

import org.jabref.logic.shared.listener.PostgresSQLNotificationListener;
import org.jabref.model.database.shared.DatabaseConnection;
import org.jabref.model.entry.BibEntry;

import com.impossibl.postgres.api.jdbc.PGConnection;
import com.impossibl.postgres.jdbc.PGDataSource;
import com.impossibl.postgres.jdbc.ThreadedHousekeeper;
import org.postgresql.PGConnection;

/**
* Processes all incoming or outgoing bib data to PostgreSQL database and manages its structure.
*/
public class PostgreSQLProcessor extends DBMSProcessor {

private PGConnection pgConnection;

private PostgresSQLNotificationListener listener;


public PostgreSQLProcessor(DatabaseConnection connection) {
super(connection);
}
Expand All @@ -37,36 +30,36 @@ public PostgreSQLProcessor(DatabaseConnection connection) {
@Override
public void setUp() throws SQLException {
connection.createStatement().executeUpdate(
"CREATE TABLE IF NOT EXISTS \"ENTRY\" (" +
"\"SHARED_ID\" SERIAL PRIMARY KEY, " +
"\"TYPE\" VARCHAR, " +
"\"VERSION\" INTEGER DEFAULT 1)");
"CREATE TABLE IF NOT EXISTS \"ENTRY\" (" +
"\"SHARED_ID\" SERIAL PRIMARY KEY, " +
"\"TYPE\" VARCHAR, " +
"\"VERSION\" INTEGER DEFAULT 1)");

connection.createStatement().executeUpdate(
"CREATE TABLE IF NOT EXISTS \"FIELD\" (" +
"\"ENTRY_SHARED_ID\" INTEGER REFERENCES \"ENTRY\"(\"SHARED_ID\") ON DELETE CASCADE, " +
"\"NAME\" VARCHAR, " +
"\"VALUE\" TEXT)");
"CREATE TABLE IF NOT EXISTS \"FIELD\" (" +
"\"ENTRY_SHARED_ID\" INTEGER REFERENCES \"ENTRY\"(\"SHARED_ID\") ON DELETE CASCADE, " +
"\"NAME\" VARCHAR, " +
"\"VALUE\" TEXT)");

connection.createStatement().executeUpdate(
"CREATE TABLE IF NOT EXISTS \"METADATA\" ("
+ "\"KEY\" VARCHAR,"
+ "\"VALUE\" TEXT)");
"CREATE TABLE IF NOT EXISTS \"METADATA\" ("
+ "\"KEY\" VARCHAR,"
+ "\"VALUE\" TEXT)");
}

@Override
protected void insertIntoEntryTable(BibEntry bibEntry) {
// Inserting into ENTRY table
StringBuilder insertIntoEntryQuery = new StringBuilder()
.append("INSERT INTO ")
.append(escape("ENTRY"))
.append("(")
.append(escape("TYPE"))
.append(") VALUES(?)");
.append("INSERT INTO ")
.append(escape("ENTRY"))
.append("(")
.append(escape("TYPE"))
.append(") VALUES(?)");

// This is the only method to get generated keys which is accepted by MySQL, PostgreSQL and Oracle.
try (PreparedStatement preparedEntryStatement = connection.prepareStatement(insertIntoEntryQuery.toString(),
Statement.RETURN_GENERATED_KEYS)) {
Statement.RETURN_GENERATED_KEYS)) {

preparedEntryStatement.setString(1, bibEntry.getType());
preparedEntryStatement.executeUpdate();
Expand All @@ -89,23 +82,14 @@ String escape(String expression) {
@Override
public void startNotificationListener(DBMSSynchronizer dbmsSynchronizer) {
// Disable cleanup output of ThreadedHousekeeper
Logger.getLogger(ThreadedHousekeeper.class.getName()).setLevel(Level.SEVERE);

this.listener = new PostgresSQLNotificationListener(dbmsSynchronizer);

PGDataSource dataSource = new PGDataSource();
dataSource.setHost(connectionProperties.getHost());
dataSource.setPort(connectionProperties.getPort());
dataSource.setDatabase(connectionProperties.getDatabase());
dataSource.setUser(connectionProperties.getUser());
dataSource.setPassword(connectionProperties.getPassword());

//Logger.getLogger(ThreadedHousekeeper.class.getName()).setLevel(Level.SEVERE);
try {
pgConnection = (PGConnection) dataSource.getConnection();
pgConnection.createStatement().execute("LISTEN jabrefLiveUpdate");
connection.createStatement().execute("LISTEN jabrefLiveUpdate");
// Do not use `new PostgresSQLNotificationListener(...)` as the object has to exist continuously!
// Otherwise the listener is going to be deleted by GC.
pgConnection.addNotificationListener(listener);
PGConnection pgConnection = connection.unwrap(PGConnection.class);
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the above comment, which contradicts the new code.

Has someone tested the new implementation?

Copy link
Member

Choose a reason for hiding this comment

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

I can try to test it this evening. I think I have a pg sql db somewhere lying around

listener = new PostgresSQLNotificationListener(dbmsSynchronizer, pgConnection);
listener.start();
} catch (SQLException e) {
LOGGER.error("SQL Error: ", e);
}
Expand All @@ -114,7 +98,7 @@ public void startNotificationListener(DBMSSynchronizer dbmsSynchronizer) {
@Override
public void stopNotificationListener() {
try {
pgConnection.close();
connection.close();
} catch (SQLException e) {
LOGGER.error("SQL Error: ", e);
}
Expand All @@ -123,7 +107,7 @@ public void stopNotificationListener() {
@Override
public void notifyClients() {
try {
pgConnection.createStatement().execute("NOTIFY jabrefLiveUpdate, '" + PROCESSOR_ID + "';");
connection.createStatement().execute("NOTIFY jabrefLiveUpdate, '" + PROCESSOR_ID + "';");
} catch (SQLException e) {
LOGGER.error("SQL Error: ", e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,50 @@
package org.jabref.logic.shared.listener;

import java.sql.SQLException;

import org.jabref.logic.shared.DBMSProcessor;
import org.jabref.logic.shared.DBMSSynchronizer;

import com.impossibl.postgres.api.jdbc.PGNotificationListener;
import org.postgresql.PGConnection;
import org.postgresql.PGNotification;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* A listener for PostgreSQL database notifications.
*/
public class PostgresSQLNotificationListener implements PGNotificationListener {
public class PostgresSQLNotificationListener extends Thread {

private final DBMSSynchronizer dbmsSynchronizer;
private static final Logger LOGGER = LoggerFactory.getLogger(PostgresSQLNotificationListener.class);

private final DBMSSynchronizer dbmsSynchronizer;
private final PGConnection pgConnection;

public PostgresSQLNotificationListener(DBMSSynchronizer dbmsSynchronizer) {
public PostgresSQLNotificationListener(DBMSSynchronizer dbmsSynchronizer, PGConnection pgConnection) {
this.dbmsSynchronizer = dbmsSynchronizer;
this.pgConnection = pgConnection;
}

@Override
public void notification(int processId, String channel, String payload) {
if (!payload.equals(DBMSProcessor.PROCESSOR_ID)) {
dbmsSynchronizer.pullChanges();
public void run() {
try {
//noinspection InfiniteLoopStatement
while (true) {
PGNotification notifications[] = pgConnection.getNotifications();

if (notifications != null) {
for (PGNotification notification : notifications) {
if (!notification.getName().equals(DBMSProcessor.PROCESSOR_ID)) {
dbmsSynchronizer.pullChanges();
}
}
}

// Wait a while before checking again for new notifications
Thread.sleep(500);
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't the driver support push notifications?

Copy link
Member

Choose a reason for hiding this comment

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

}
} catch (SQLException | InterruptedException exception) {
LOGGER.error("Error while listening for updates to PostgresSQL", exception);
}
}

}
16 changes: 3 additions & 13 deletions src/main/java/org/jabref/model/database/shared/DBMSType.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,15 @@
*/
public enum DBMSType {

MYSQL(
"MySQL",
"com.mysql.jdbc.Driver",
"jdbc:mysql://%s:%d/%s", 3306),
ORACLE(
"Oracle",
"oracle.jdbc.driver.OracleDriver",
"jdbc:oracle:thin:@%s:%d:%s", 1521),
POSTGRESQL(
"PostgreSQL",
"com.impossibl.postgres.jdbc.PGDriver",
"jdbc:pgsql://%s:%d/%s", 5432);
MYSQL("MySQL", "com.mysql.jdbc.Driver", "jdbc:mysql://%s:%d/%s", 3306),
ORACLE("Oracle", "oracle.jdbc.driver.OracleDriver", "jdbc:oracle:thin:@%s:%d:%s", 1521),
POSTGRESQL("PostgreSQL", "org.postgresql.Driver", "jdbc:postgresql://%s:%d/%s", 5432);

private final String type;
private final String driverPath;
private final String urlPattern;
private final int defaultPort;


private DBMSType(String type, String driverPath, String urlPattern, int defaultPort) {
this.type = type;
this.driverPath = driverPath;
Expand Down