Merge pull request #16 from prdoyle/driver-validation

ValidatingDriver

bosk-core/src/main/java/works/bosk/Bosk.java

@@ -29,6 +29,7 @@
 import works.bosk.exceptions.InvalidTypeException;
 import works.bosk.exceptions.NoReadContextException;
 import works.bosk.exceptions.NonexistentReferenceException;
+import works.bosk.exceptions.NotYetImplementedException;
 import works.bosk.exceptions.ReferenceBindingException;
 import works.bosk.util.Classes;
 
@@ -79,6 +80,7 @@ public class Bosk<R extends StateTreeNode> implements BoskInfo<R> {
 	@Getter private final Identifier instanceID = Identifier.from(randomUUID().toString());
 	@Getter private final BoskDriver<R> driver;
 	@Getter private final BoskDiagnosticContext diagnosticContext = new BoskDiagnosticContext();
+	private final BoskDriver<R> userSuppliedDriver;
 	private final LocalDriver localDriver;
 	private final RootRef rootRef;
 	private final ThreadLocal<R> rootSnapshot = new ThreadLocal<>();
@@ -97,7 +99,7 @@ public class Bosk<R extends StateTreeNode> implements BoskInfo<R> {
 	 *    other state.
 	 * @param driverFactory Will be applied to this Bosk's local driver during
 	 * the Bosk's constructor, and the resulting {@link BoskDriver} will be the
-	 * one returned by {@link #driver}.
+	 * one returned by {@link #getDriver}.
 	 *
 	 * @see DriverStack
 	 */
@@ -124,7 +126,8 @@ public Bosk(String name, Type rootType, DefaultRootFunction<R> defaultRootFuncti
 		// to do such things as create References, so it needs the rest of the
 		// initialization to have completed already.
 		//
-		this.driver = driverFactory.build(boskInfo, this.localDriver);
+		this.userSuppliedDriver = driverFactory.build(boskInfo, this.localDriver);
+		this.driver = new ValidatingDriver(userSuppliedDriver);
 
 		try {
 			this.currentRoot = requireNonNull(driver.initialRoot(rootType));
@@ -168,6 +171,90 @@ public static <RR extends StateTreeNode> BoskDriver<RR> simpleDriver(@SuppressWa
 		return downstream;
 	}
 
+	/**
+	 * <strong>Evolution note</strong>: we need better handling of the driver stack.
+	 * For now, we just provide access to the topmost driver, but code should be able
+	 * to look up any driver on the stack. We need to think carefully about how we
+	 * want this to work.
+	 *
+	 * @return the driver from the driver stack having the given type.
+	 * @throws IllegalArgumentException if there is no unique driver of the given type
+	 */
+	@SuppressWarnings("unchecked")
+	public <D extends BoskDriver<R>> D getDriver(Class<? super D> driverType) {
+		if (driverType.isInstance(userSuppliedDriver)) {
+			return (D)driverType.cast(userSuppliedDriver);
+		} else {
+			throw new NotYetImplementedException("Can't look up driver of type " + driverType);
+		}
+	}
+
+	/**
+	 * We wrap the user-supplied driver with one of these to ensure the error-checking
+	 * requirements of the {@link BoskDriver} are enforced.
+	 */
+	@RequiredArgsConstructor
+	final class ValidatingDriver implements BoskDriver<R> {
+		final BoskDriver<R> downstream;
+
+		@Override
+		public <T> void submitReplacement(Reference<T> target, T newValue) {
+			assertCorrectBosk(target);
+			downstream.submitReplacement(target, newValue);
+		}
+
+		@Override
+		public <T> void submitConditionalReplacement(Reference<T> target, T newValue, Reference<Identifier> precondition, Identifier requiredValue) {
+			assertCorrectBosk(target);
+			assertCorrectBosk(precondition);
+			downstream.submitConditionalReplacement(target, newValue, precondition, requiredValue);
+		}
+
+		@Override
+		public <T> void submitInitialization(Reference<T> target, T newValue) {
+			assertCorrectBosk(target);
+			downstream.submitInitialization(target, newValue);
+		}
+
+		@Override
+		public <T> void submitDeletion(Reference<T> target) {
+			if (target.path().isEmpty()) {
+				// TODO: Augment dereferencer so it can tell us this for all references, not just the root
+				throw new IllegalArgumentException("Cannot delete root object");
+			}
+			assertCorrectBosk(target);
+			downstream.submitDeletion(target);
+		}
+
+		@Override
+		public <T> void submitConditionalDeletion(Reference<T> target, Reference<Identifier> precondition, Identifier requiredValue) {
+			assertCorrectBosk(target);
+			assertCorrectBosk(precondition);
+			downstream.submitConditionalDeletion(target, precondition, requiredValue);
+		}
+
+		@Override
+		public R initialRoot(Type rootType) throws InvalidTypeException, IOException, InterruptedException {
+			return downstream.initialRoot(rootType);
+		}
+
+		@Override
+		public void flush() throws IOException, InterruptedException {
+			downstream.flush();
+		}
+
+		private <T> void assertCorrectBosk(Reference<T> target) {
+			// TODO: Do we need to be this strict?
+			// On the one hand, we could write conditional updates in a way that don't require the
+			// reference to point to the right bosk.
+			// On the other hand, there's a certain symmetry to requiring the references to have the right
+			// bosk for both reads and writes, and forcing this discipline on users might help them avoid
+			// some pretty confusing mistakes.
+			assert ((Bosk<?>.RootRef)target.root()).bosk() == Bosk.this: "Reference supplied to driver operation must refer to the correct bosk";
+		}
+
+	}
+
 	/**
 	 * {@link BoskDriver} that writes directly to this {@link Bosk}.
 	 *
@@ -210,7 +297,6 @@ public R initialRoot(Type rootType) throws InvalidTypeException {
 
 		@Override
 		public <T> void submitReplacement(Reference<T> target, T newValue) {
-			assertCorrectBosk(target);
 			synchronized (this) {
 				R priorRoot = currentRoot;
 				if (!tryGraftReplacement(target, newValue)) {
@@ -223,7 +309,6 @@ public <T> void submitReplacement(Reference<T> target, T newValue) {
 
 		@Override
 		public <T> void submitInitialization(Reference<T> target, T newValue) {
-			assertCorrectBosk(target);
 			synchronized (this) {
 				boolean preconditionsSatisfied;
 				try (@SuppressWarnings("unused") ReadContext executionContext = supersedingReadContext()) {
@@ -242,7 +327,6 @@ public <T> void submitInitialization(Reference<T> target, T newValue) {
 
 		@Override
 		public <T> void submitDeletion(Reference<T> target) {
-			assertCorrectBosk(target);
 			synchronized (this) {
 				R priorRoot = currentRoot;
 				if (!tryGraftDeletion(target)) {
@@ -261,8 +345,6 @@ public void flush() {
 
 		@Override
 		public <T> void submitConditionalReplacement(Reference<T> target, T newValue, Reference<Identifier> precondition, Identifier requiredValue) {
-			assertCorrectBosk(target);
-			assertCorrectBosk(precondition);
 			synchronized (this) {
 				boolean preconditionsSatisfied;
 				try (@SuppressWarnings("unused") ReadContext executionContext = supersedingReadContext()) {
@@ -281,8 +363,6 @@ public <T> void submitConditionalReplacement(Reference<T> target, T newValue, Re
 
 		@Override
 		public <T> void submitConditionalDeletion(Reference<T> target, Reference<Identifier> precondition, Identifier requiredValue) {
-			assertCorrectBosk(target);
-			assertCorrectBosk(precondition);
 			synchronized (this) {
 				boolean preconditionsSatisfied;
 				try (@SuppressWarnings("unused") ReadContext executionContext = supersedingReadContext()) {
@@ -309,16 +389,6 @@ void triggerEverywhere(HookRegistration<?> reg) {
 			drainQueueIfAllowed();
 		}
 
-		private <T> void assertCorrectBosk(Reference<T> target) {
-			// TODO: Do we need to be this strict?
-			// On the one hand, we could write conditional updates in a way that don't require the
-			// reference to point to the right bosk.
-			// On the other hand, there's a certain symmetry to requiring the references to have the right
-			// bosk for both reads and writes, and forcing this discipline on users might help them avoid
-			// some pretty confusing mistakes.
-			assert ((Bosk<?>.RootRef)target.root()).bosk() == Bosk.this: "Reference supplied to driver operation must refer to the correct bosk";
-		}
-
 		/**
 		 * @return false if the update was ignored
 		 */
@@ -348,9 +418,7 @@ private synchronized <T> boolean tryGraftReplacement(Reference<T> target, T newV
 		 */
 		private synchronized <T> boolean tryGraftDeletion(Reference<T> target) {
 			Path targetPath = target.path();
-			if (targetPath.isEmpty()) {
-				throw new IllegalArgumentException("Cannot delete root object");
-			}
+			assert !targetPath.isEmpty();
 			Dereferencer dereferencer = dereferencerFor(target);
 			try {
 				LOGGER.debug("Applying deletion at {}", target);

bosk-core/src/test/java/works/bosk/BoskConstructorTest.java

@@ -48,7 +48,7 @@ void basicProperties_correctValues() {
 
 		// The driver object and root node should be exactly the same object passed in
 
-		assertSame(driver.get(), bosk.driver());
+		assertSame(driver.get(), bosk.getDriver(ForwardingDriver.class));
 
 		try (val __ = bosk.readContext()) {
 			assertSame(root, bosk.rootReference().value());

bosk-core/src/test/java/works/bosk/BoskLocalReferenceTest.java

@@ -8,7 +8,6 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
-import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.UnaryOperator;
 import lombok.AccessLevel;
 import lombok.EqualsAndHashCode;
@@ -277,18 +276,6 @@ public InvalidRoot(Identifier id, Catalog<TestEntity> entities, String str) {
 		assertThrows(IllegalArgumentException.class, () -> new Bosk<>(boskName(), String.class, new InvalidRoot(Identifier.unique("yucky"), Catalog.empty(), "hello"), Bosk::simpleDriver));
 	}
 
-	@Test
-	void testDriver() {
-		// This doesn't test the operation of the driver; merely that the right driver is returned
-		AtomicReference<BoskDriver<Root>> driver = new AtomicReference<>();
-		Bosk<Root> myBosk = new Bosk<>(boskName(), Root.class, new Root(123, Catalog.empty()), (b,d) -> {
-			BoskDriver<Root> bd = new ProxyDriver(d);
-			driver.set(bd);
-			return bd;
-		});
-		assertSame(driver.get(), myBosk.driver());
-	}
-
 	@RequiredArgsConstructor
 	private static final class ProxyDriver implements BoskDriver<Root> {
 		@Delegate final BoskDriver<Root> delegate;

bosk-mongo/src/test/java/works/bosk/drivers/mongo/MongoDriverRecoveryTest.java

@@ -252,12 +252,17 @@ private void setRevision(long revisionNumber) {
 
 	private TestEntity initializeDatabase(String distinctiveString) {
 		try {
+			AtomicReference<MongoDriver<TestEntity>> driverRef = new AtomicReference<>();
 			Bosk<TestEntity> prepBosk = new Bosk<TestEntity>(
 				boskName("Prep " + getClass().getSimpleName()),
 				TestEntity.class,
 				bosk -> initialRoot(bosk).withString(distinctiveString),
-				driverFactory);
-			MongoDriver<TestEntity> driver = (MongoDriver<TestEntity>) prepBosk.driver();
+				(b,d) -> {
+					var mongoDriver = (MongoDriver<TestEntity>) driverFactory.build(b, d);
+					driverRef.set(mongoDriver);
+					return mongoDriver;
+				});
+			var driver = driverRef.get();
 			waitFor(driver);
 			driver.close();
 

bosk-mongo/src/test/java/works/bosk/drivers/mongo/MongoDriverSpecialTest.java

@@ -563,7 +563,7 @@ void refurbish_createsField() throws IOException, InterruptedException {
 		assertEquals(Optional.empty(), before); // Not there yet
 
 		LOGGER.debug("Call refurbish");
-		((MongoDriver<?>)upgradeableBosk.driver()).refurbish();
+		upgradeableBosk.getDriver(MongoDriver.class).refurbish();
 		originalBosk.driver().flush(); // Not the bosk that did refurbish!
 
 		LOGGER.debug("Check state after");
@@ -621,7 +621,7 @@ void refurbish_fixesMetadata() throws IOException, InterruptedException {
 		);
 
 		// (Close this so it doesn't crash when we delete the "path" field)
-		((MongoDriver<TestEntity>)initialBosk.driver()).close();
+		initialBosk.getDriver(MongoDriver.class).close();
 
 		// Delete some metadata fields
 		MongoCollection<Document> collection = mongoService.client()
@@ -655,7 +655,7 @@ void refurbish_fixesMetadata() throws IOException, InterruptedException {
 		}
 
 		// Refurbish
-		((MongoDriver<?>)bosk.driver()).refurbish();
+		bosk.getDriver(MongoDriver.class).refurbish();
 
 		// Verify the fields are all now there
 		try (MongoCursor<Document> cursor = collection.find(filterDoc).cursor()) {

bosk-mongo/src/test/java/works/bosk/drivers/mongo/SchemaEvolutionTest.java

@@ -91,7 +91,8 @@ void pairwise_readCompatible() throws Exception {
 		}
 
 		LOGGER.debug("Refurbish");
-		((MongoDriver<?>)toBosk.driver()).refurbish();
+		MongoDriver<TestEntity> driver = toBosk.getDriver(MongoDriver.class);
+		driver.refurbish();
 
 		LOGGER.debug("Perform fromBosk read");
 		try (var __ = fromBosk.readContext()) {
@@ -117,7 +118,8 @@ void pairwise_writeCompatible() throws Exception {
 		Refs toRefs = toBosk.buildReferences(Refs.class);
 
 		LOGGER.debug("Refurbish toBosk ({})", toBosk.name());
-		((MongoDriver<?>)toBosk.driver()).refurbish();
+		MongoDriver<TestEntity> driver = toBosk.getDriver(MongoDriver.class);
+		driver.refurbish();
 
 		flushIfLiveRefurbishIsNotSupported(fromBosk, fromHelper, toHelper);