Merged in feature/add-version-check-to-persistence (pull request #520) by rebazer

Feature/add version check to persistence

Author: rebazer

CHANGELOG.md

@@ -25,6 +25,8 @@ Table of Contents
 
 ### Breaking Changes
 
+* Reports before version 1.7.0 cannot be loaded anymore. Simply re-run your tests with the new recheck version to create them again.
+
 ### Bug Fixes
 
 ### New Features

src/main/java/de/retest/recheck/ioerror/ReTestIOException.java

@@ -12,6 +12,11 @@ public ReTestIOException( final URI location, final Throwable throwable ) {
 		this( location, throwable.getMessage(), throwable );
 	}
 
+	public ReTestIOException( final URI location, final String details ) {
+		super( "Error reading from '" + location + "': " + details );
+		this.location = location;
+	}
+
 	public ReTestIOException( final URI location, final String details, final Throwable throwable ) {
 		super( "Error reading from '" + location + "': " + details, throwable );
 		this.location = location;

src/main/java/de/retest/recheck/ioerror/ReTestLoadException.java

@@ -10,6 +10,10 @@ public ReTestLoadException( final URI location, final Throwable throwable ) {
 		super( location, throwable );
 	}
 
+	public ReTestLoadException( final URI location, final String details ) {
+		super( location, details );
+	}
+
 	public ReTestLoadException( final URI location, final String details, final Throwable throwable ) {
 		super( location, details, throwable );
 	}

src/main/java/de/retest/recheck/persistence/IncompatibleReportVersionException.java

@@ -6,15 +6,24 @@
 
 public class IncompatibleReportVersionException extends ReTestLoadException {
 
+	private static final String MESSAGE =
+			"Incompatible recheck versions: report was written with %s, but read with %s.";
+
 	private static final long serialVersionUID = 1L;
 
 	private final String writerVersion;
 	private final String readerVersion;
 
+	public IncompatibleReportVersionException( final String writerVersion, final String readerVersion,
+			final URI location ) {
+		super( location, String.format( MESSAGE, writerVersion, readerVersion ) );
+		this.writerVersion = writerVersion;
+		this.readerVersion = readerVersion;
+	}
+
 	public IncompatibleReportVersionException( final String writerVersion, final String readerVersion,
 			final URI location, final Throwable throwable ) {
-		super( location, "Incompatible recheck versions: report was written with " + writerVersion + ", but read with "
-				+ readerVersion + ".", throwable );
+		super( location, String.format( MESSAGE, writerVersion, readerVersion ), throwable );
 		this.writerVersion = writerVersion;
 		this.readerVersion = readerVersion;
 	}

src/main/java/de/retest/recheck/persistence/bin/KryoPersistence.java

@@ -8,13 +8,13 @@
 import java.net.URI;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import org.apache.commons.io.FileUtils;
 import org.objenesis.strategy.StdInstantiatorStrategy;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import com.esotericsoftware.kryo.Kryo;
 import com.esotericsoftware.kryo.Registration;
@@ -26,16 +26,28 @@
 import de.retest.recheck.persistence.IncompatibleReportVersionException;
 import de.retest.recheck.persistence.Persistable;
 import de.retest.recheck.persistence.Persistence;
+import de.retest.recheck.report.TestReport;
 import de.retest.recheck.util.FileUtil;
 import de.retest.recheck.util.VersionProvider;
+import lombok.extern.slf4j.Slf4j;
 import net.jpountz.lz4.LZ4FrameInputStream;
 import net.jpountz.lz4.LZ4FrameOutputStream;
 
+@Slf4j
 public class KryoPersistence<T extends Persistable> implements Persistence<T> {
 
 	private static final String OLD_RECHECK_VERSION = "an old recheck version (pre 1.5.0)";
 
-	private static final Logger logger = LoggerFactory.getLogger( KryoPersistence.class );
+	private static final Map<Class<?>, Integer> compatibleVersions = createCompatibleVersions();
+
+	private static Map<Class<?>, Integer> createCompatibleVersions() {
+		final Map<Class<?>, Integer> map = new HashMap<>();
+
+		// Specify the class and lowest readable version here
+		map.put( TestReport.class, TestReport.PERSISTENCE_VERSION );
+
+		return map;
+	}
 
 	private final Kryo kryo;
 	private final String version;
@@ -77,16 +89,15 @@ public void save( final URI identifier, final T element ) throws IOException {
 		FileUtil.ensureFolder( path.toFile() );
 		try ( final Output output = new Output( new LZ4FrameOutputStream( newOutputStream( path ) ) ) ) {
 			output.writeString( version );
-			logger.debug( "Writing {} to {}. Do not write to same identifier or interrupt until done.", element,
+			log.debug( "Writing {} to {}. Do not write to same identifier or interrupt until done.", element,
 					identifier );
 			kryo.writeClassAndObject( output, element );
-			logger.debug( "Done writing {} to {}", element, identifier );
-		} catch ( Error | Exception anything ) {
-			logger.error(
-					"Error writing to file {}. Deleting what has been written to not leave corrupt file behind...",
-					identifier, anything );
+			log.debug( "Done writing {} to {}", element, identifier );
+		} catch ( final Throwable t ) {
+			log.error( "Error writing to file {}. Deleting what has been written to not leave corrupt file behind...",
+					identifier, t );
 			FileUtils.deleteQuietly( file );
-			throw anything;
+			throw t;
 		}
 	}
 
@@ -97,23 +108,34 @@ public T load( final URI identifier ) throws IOException {
 		String writerVersion = null;
 		try ( final Input input = new Input( new LZ4FrameInputStream( newInputStream( path ) ) ) ) {
 			writerVersion = input.readString();
-			return (T) kryo.readClassAndObject( input );
+			final T persistable = (T) kryo.readClassAndObject( input );
+			if ( !isCompatible( persistable ) ) {
+				throw new IncompatibleReportVersionException( writerVersion, version, identifier );
+			}
+			return persistable;
+		} catch ( final IncompatibleReportVersionException e ) {
+			throw e;
 		} catch ( final Exception e ) {
 			if ( version.equals( writerVersion ) ) {
 				throw e;
 			}
 			if ( isUnknownFormat( writerVersion ) ) {
 				writerVersion = OLD_RECHECK_VERSION;
-				// TODO Remove after release of 1.6.0
-				try ( final Input secondInput = new Input( newInputStream( path ) ) ) {
-					return (T) kryo.readClassAndObject( secondInput );
-				}
-				// remove until here
 			}
 			throw new IncompatibleReportVersionException( writerVersion, version, identifier, e );
 		}
 	}
 
+	private boolean isCompatible( final T persistable ) {
+		return isCompatible( persistable.getClass(), persistable.version() );
+	}
+
+	// For testing only
+	boolean isCompatible( final Class<? extends Persistable> clazz, final int version ) {
+		final Integer minVersion = compatibleVersions.get( clazz );
+		return minVersion == null || minVersion <= version;
+	}
+
 	private static final Pattern VERSION_CHARS = Pattern.compile( "[\\w\\.\\{\\}\\$]+" );
 
 	private static boolean isUnknownFormat( final String writerVersion ) {

src/main/java/de/retest/recheck/report/TestReport.java

@@ -24,7 +24,7 @@
 public class TestReport extends Persistable {
 
 	private static final long serialVersionUID = 1L;
-	private static final int PERSISTENCE_VERSION = 20;
+	public static final int PERSISTENCE_VERSION = 21;
 
 	@XmlElement( name = "suite" )
 	private final List<SuiteReplayResult> suiteReplayResults = new ArrayList<>();

src/test/java/de/retest/recheck/persistence/bin/KryoPersistenceTest.java

@@ -22,6 +22,7 @@
 import com.esotericsoftware.kryo.KryoException;
 
 import de.retest.recheck.persistence.IncompatibleReportVersionException;
+import de.retest.recheck.persistence.Persistable;
 import de.retest.recheck.report.TestReport;
 import de.retest.recheck.util.VersionProvider;
 
@@ -67,21 +68,29 @@ void incompatible_version_should_give_persisting_version( @TempDir final Path te
 						+ VersionProvider.RETEST_VERSION + ", but read with old Version." );
 	}
 
+	@Test
+	void load_should_not_be_able_to_load_1_6_0_report_version() throws Exception {
+		final Path report = Paths.get( getClass().getResource( "1.6.0.report" ).toURI() );
+
+		final KryoPersistence<TestReport> cut = new KryoPersistence<>();
+
+		assertThatThrownBy( () -> cut.load( report.toUri() ) ) //
+				.isInstanceOf( IncompatibleReportVersionException.class ) //
+				.hasMessageContaining( "Incompatible recheck versions: report was written with 1.6.0" );
+	}
+
 	@Test
 	void unknown_version_should_give_correct_error() throws IOException {
 		final Path file = Paths.get( "src/test/resources/de/retest/recheck/persistence/bin/old.report" );
 		final URI identifier = file.toUri();
 
 		final KryoPersistence<TestReport> differentKryoPersistence = new KryoPersistence<>();
 
-		//		TODO Reactivate that code after the TODO in KryoPersistence has been adressed (post 1.6.0)
-		//		assertThatThrownBy( () -> differentKryoPersistence.load( identifier ) )
-		//		.isInstanceOf( IncompatibleReportVersionException.class ).hasMessageContaining(
-		//				"Incompatible recheck versions: report was written with an old recheck version (pre 1.5.0), but read with "
-		//						+ VersionProvider.RETEST_VERSION + "." );
-
-		final TestReport report = differentKryoPersistence.load( identifier );
-		assertThat( report ).isNotNull();
+		assertThatThrownBy( () -> differentKryoPersistence.load( identifier ) ) //
+				.isInstanceOf( IncompatibleReportVersionException.class ) //
+				.hasMessageContaining(
+						"Incompatible recheck versions: report was written with an old recheck version (pre 1.5.0), but read with "
+								+ VersionProvider.RETEST_VERSION + "." );
 	}
 
 	@Test
@@ -95,4 +104,26 @@ void on_error_file_should_be_deleted() throws IOException {
 				.isInstanceOf( KryoException.class );
 		assertThat( nonexistent ).doesNotExist();
 	}
+
+	@Test
+	void isCompatible_should_be_compatible_with_equal_or_higher_value() throws Exception {
+		final KryoPersistence<TestReport> cut = new KryoPersistence<>();
+
+		assertThat( cut.isCompatible( TestReport.class, TestReport.PERSISTENCE_VERSION ) ).isTrue();
+		assertThat( cut.isCompatible( TestReport.class, TestReport.PERSISTENCE_VERSION + 1 ) ).isTrue();
+	}
+
+	@Test
+	void isCompatible_should_incompatible_with_lower_value() throws Exception {
+		final KryoPersistence<TestReport> cut = new KryoPersistence<>();
+
+		assertThat( cut.isCompatible( TestReport.class, TestReport.PERSISTENCE_VERSION - 1 ) ).isFalse();
+	}
+
+	@Test
+	void isCompatible_should_compatible_with_not_present_value() throws Exception {
+		final KryoPersistence<Persistable> cut = new KryoPersistence<>();
+
+		assertThat( cut.isCompatible( Persistable.class, 1 ) ).isTrue();
+	}
 }