Database refactoring to fix injection problems.

This commit is contained in:
akwizgran
2011-09-23 18:52:56 +01:00
parent 4b0e91f52c
commit 98ab523092
10 changed files with 88 additions and 34 deletions

View File

@@ -0,0 +1,15 @@
package net.sf.briar.api.db;
import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import com.google.inject.BindingAnnotation;
/** Annotation for injecting the directory where the database is stored. */
@BindingAnnotation
@Target({ PARAMETER })
@Retention(RUNTIME)
public @interface DatabaseDirectory {}

View File

@@ -0,0 +1,15 @@
package net.sf.briar.api.db;
import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import com.google.inject.BindingAnnotation;
/** Annotation for injecting the maximum size in bytes of the database. */
@BindingAnnotation
@Target({ PARAMETER })
@Retention(RUNTIME)
public @interface DatabaseMaxSize {}

View File

@@ -6,9 +6,10 @@ interface DatabaseCleaner {
/** /**
* Starts a new thread to monitor the amount of free storage space * Starts a new thread to monitor the amount of free storage space
* available to the database and expire old messages as necessary. * available to the database and expire old messages as necessary. The
* cleaner will pause for the given number of milliseconds between sweeps.
*/ */
void startCleaning(); void startCleaning(Callback callback, long msBetweenSweeps);
/** Tells the cleaner thread to exit and returns when it has done so. */ /** Tells the cleaner thread to exit and returns when it has done so. */
void stopCleaning(); void stopCleaning();
@@ -18,9 +19,9 @@ interface DatabaseCleaner {
/** /**
* Checks how much free storage space is available to the database, and * Checks how much free storage space is available to the database, and
* if necessary expires old messages until the free space is at least * if necessary expires old messages until the free space is at least
* MIN_FREE_SPACE. While the free space is less than * DatabaseConstants.MIN_FREE_SPACE. While the free space is less than
* CRITICAL_FREE_SPACE, operations that attempt to store messages in * DatabaseConstants.CRITICAL_FREE_SPACE, operations that attempt to
* the database will block. * store messages in the database will block.
*/ */
void checkFreeSpaceAndClean() throws DbException; void checkFreeSpaceAndClean() throws DbException;

View File

@@ -2,22 +2,17 @@ package net.sf.briar.db;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
import com.google.inject.Inject;
class DatabaseCleanerImpl implements DatabaseCleaner, Runnable { class DatabaseCleanerImpl implements DatabaseCleaner, Runnable {
private final Callback db;
private final int msBetweenSweeps;
private final AtomicBoolean stopped = new AtomicBoolean(false); private final AtomicBoolean stopped = new AtomicBoolean(false);
private final Thread cleanerThread = new Thread(this); private final Thread cleanerThread = new Thread(this);
@Inject private volatile Callback callback;
DatabaseCleanerImpl(Callback db, int msBetweenSweeps) { private volatile long msBetweenSweeps;
this.db = db;
this.msBetweenSweeps = msBetweenSweeps;
}
public void startCleaning() { public void startCleaning(Callback callback, long msBetweenSweeps) {
this.callback = callback;
this.msBetweenSweeps = msBetweenSweeps;
cleanerThread.start(); cleanerThread.start();
} }
@@ -35,8 +30,8 @@ class DatabaseCleanerImpl implements DatabaseCleaner, Runnable {
public void run() { public void run() {
try { try {
while(!stopped.get()) { while(!stopped.get()) {
if(db.shouldCheckFreeSpace()) { if(callback.shouldCheckFreeSpace()) {
db.checkFreeSpaceAndClean(); callback.checkFreeSpaceAndClean();
} else { } else {
synchronized(stopped) { synchronized(stopped) {
try { try {

View File

@@ -99,7 +99,7 @@ DatabaseCleaner.Callback {
public void open(boolean resume) throws DbException { public void open(boolean resume) throws DbException {
db.open(resume); db.open(resume);
cleaner.startCleaning(); cleaner.startCleaning(this, MAX_MS_BETWEEN_SPACE_CHECKS);
} }
public void close() throws DbException { public void close() throws DbException {

View File

@@ -2,13 +2,15 @@ package net.sf.briar.db;
interface DatabaseConstants { interface DatabaseConstants {
static final int MEGABYTE = 1024 * 1024;
// FIXME: These should be configurable // FIXME: These should be configurable
static final long MIN_FREE_SPACE = 300L * MEGABYTE; static final long MIN_FREE_SPACE = 300 * 1024 * 1024; // 300 MiB
static final long CRITICAL_FREE_SPACE = 100L * MEGABYTE; static final long CRITICAL_FREE_SPACE = 100 * 1024 * 1024; // 100 MiB
static final int MAX_BYTES_BETWEEN_SPACE_CHECKS = 5 * MEGABYTE;
static final int MAX_BYTES_BETWEEN_SPACE_CHECKS = 5 * 1024 * 1024; // 5 MiB
static final long MAX_MS_BETWEEN_SPACE_CHECKS = 60L * 1000L; // 1 min static final long MAX_MS_BETWEEN_SPACE_CHECKS = 60L * 1000L; // 1 min
static final int BYTES_PER_SWEEP = 5 * MEGABYTE;
static final long MS_PER_SWEEP = 10L * 1000L; // 10 sec
static final int BYTES_PER_SWEEP = 5 * 1024 * 1024; // 5 MiB
static final long EXPIRY_MODULUS = 60L * 60L * 1000L; // 1 hour static final long EXPIRY_MODULUS = 60L * 60L * 1000L; // 1 hour
} }

View File

@@ -1,16 +1,39 @@
package net.sf.briar.db; package net.sf.briar.db;
import java.io.File;
import java.sql.Connection;
import net.sf.briar.api.crypto.Password;
import net.sf.briar.api.db.DatabaseComponent; import net.sf.briar.api.db.DatabaseComponent;
import net.sf.briar.api.db.DatabaseDirectory;
import net.sf.briar.api.db.DatabaseMaxSize;
import net.sf.briar.api.db.DatabasePassword;
import net.sf.briar.api.protocol.GroupFactory;
import net.sf.briar.api.transport.ConnectionWindowFactory;
import com.google.inject.AbstractModule; import com.google.inject.AbstractModule;
import com.google.inject.Provides;
import com.google.inject.Singleton; import com.google.inject.Singleton;
public class DatabaseModule extends AbstractModule { public class DatabaseModule extends AbstractModule {
@Override @Override
protected void configure() { protected void configure() {
bind(Database.class).to(H2Database.class); bind(DatabaseCleaner.class).to(DatabaseCleanerImpl.class);
bind(DatabaseComponent.class).to(DatabaseComponentImpl.class).in( }
Singleton.class);
@Provides
Database<Connection> getDatabase(@DatabaseDirectory File dir,
@DatabasePassword Password password, @DatabaseMaxSize long maxSize,
ConnectionWindowFactory connectionWindowFactory,
GroupFactory groupFactory) {
return new H2Database(dir, password, maxSize, connectionWindowFactory,
groupFactory);
}
@Provides @Singleton
DatabaseComponent getDatabaseComponent(Database<Connection> db,
DatabaseCleaner cleaner) {
return new DatabaseComponentImpl<Connection>(db, cleaner);
} }
} }

View File

@@ -11,6 +11,8 @@ import java.util.logging.Level;
import java.util.logging.Logger; import java.util.logging.Logger;
import net.sf.briar.api.crypto.Password; import net.sf.briar.api.crypto.Password;
import net.sf.briar.api.db.DatabaseDirectory;
import net.sf.briar.api.db.DatabaseMaxSize;
import net.sf.briar.api.db.DatabasePassword; import net.sf.briar.api.db.DatabasePassword;
import net.sf.briar.api.db.DbException; import net.sf.briar.api.db.DbException;
import net.sf.briar.api.protocol.GroupFactory; import net.sf.briar.api.protocol.GroupFactory;
@@ -32,7 +34,8 @@ class H2Database extends JdbcDatabase {
private final long maxSize; private final long maxSize;
@Inject @Inject
H2Database(File dir, @DatabasePassword Password password, long maxSize, H2Database(@DatabaseDirectory File dir, @DatabasePassword Password password,
@DatabaseMaxSize long maxSize,
ConnectionWindowFactory connectionWindowFactory, ConnectionWindowFactory connectionWindowFactory,
GroupFactory groupFactory) { GroupFactory groupFactory) {
super(connectionWindowFactory, groupFactory, "BINARY(32)", "BINARY"); super(connectionWindowFactory, groupFactory, "BINARY(32)", "BINARY");

View File

@@ -25,17 +25,15 @@ public class DatabaseCleanerImplTest extends TestCase {
return false; return false;
} }
}; };
// Configure the cleaner to wait for 30 seconds between sweeps DatabaseCleanerImpl cleaner = new DatabaseCleanerImpl();
DatabaseCleanerImpl cleaner =
new DatabaseCleanerImpl(callback, 30 * 1000);
long start = System.currentTimeMillis(); long start = System.currentTimeMillis();
// Start the cleaner and check that shouldCheckFreeSpace() is called // Start the cleaner and check that shouldCheckFreeSpace() is called
cleaner.startCleaning(); cleaner.startCleaning(callback, 30L * 1000L);
assertTrue(latch.await(5, TimeUnit.SECONDS)); assertTrue(latch.await(5, TimeUnit.SECONDS));
// Stop the cleaner (it should be waiting between sweeps) // Stop the cleaner (it should be waiting between sweeps)
cleaner.stopCleaning(); cleaner.stopCleaning();
long end = System.currentTimeMillis(); long end = System.currentTimeMillis();
// Check that much less than 30 seconds expired // Check that much less than 30 seconds expired
assertTrue(end - start < 10 * 1000); assertTrue(end - start < 10L * 1000L);
} }
} }

View File

@@ -96,7 +96,9 @@ public abstract class DatabaseComponentTest extends TestCase {
allowing(database).commitTransaction(txn); allowing(database).commitTransaction(txn);
// open(false) // open(false)
oneOf(database).open(false); oneOf(database).open(false);
oneOf(cleaner).startCleaning(); oneOf(cleaner).startCleaning(
with(any(DatabaseCleaner.Callback.class)),
with(any(long.class)));
// getRating(authorId) // getRating(authorId)
oneOf(database).getRating(txn, authorId); oneOf(database).getRating(txn, authorId);
will(returnValue(Rating.UNRATED)); will(returnValue(Rating.UNRATED));