Refactor: Add state management to CityObjectCaches

CityDoctorParent/CityDoctorModel/src/main/java/de/hft/stuttgart/citydoctor2/database/CacheManager.java

 package de.hft.stuttgart.citydoctor2.database;
 
-import de.hft.stuttgart.citydoctor2.datastructure.GmlId;
-
-
-import java.util.List;
 
 /**
- * Utility class for accessing the FeatureCache instance. Enables access to the cache in contexts without reference or
- * pass through of the {@link de.hft.stuttgart.citydoctor2.datastructure.CityDoctorModel CityDoctorModel}.
+ * Utility class for accessing and managing the FeatureCache instance. Enables access to the cache in contexts without
+ * reference or pass through of the {@link de.hft.stuttgart.citydoctor2.datastructure.CityDoctorModel CityDoctorModel}.
  */
 public class CacheManager {
 
-    private static FeatureCache cache = null;
+
+    private static final FeatureCache cache = new FeatureCache();
 
     private CacheManager() {}
 
@@ -19,31 +16,9 @@ public class CacheManager {
      * Returns the FeatureCache instance
      */
     public static FeatureCache getFeatureCache() {
-
-        if (cache == null){
-            cache = new FeatureCache();
-        }
         return cache;
     }
 
-    /**
-     * Signals the cache that parsing has finished
-     */
-    public static void finishParsing(){
-        if (cache != null){
-            cache.finishParsing();
-        }
-    }
-
-    /**
-     * Signals the cache to synchronize its content with the database.
-     */
-    public static void synchronizeCache(){
-        if (cache != null){
-            cache.synchronize();
-        }
-    }
-
     /**
      * Returns a new UnconnectedCache Object.
      * @return

CityDoctorParent/CityDoctorModel/src/main/java/de/hft/stuttgart/citydoctor2/database/CityObjectCache.java

@@ -9,7 +9,7 @@ import java.util.stream.Stream;
 public interface CityObjectCache {
 
     /**
-     * Adds a CityObject to this cache, overwriting existing CityObjects with the same GmlID
+     * Inserts a CityObject into this cache. Overwrites any existing CityObject with the same GmlID.
      * @param cityObject
      */
     void put(CityObject cityObject);
@@ -54,4 +54,16 @@ public interface CityObjectCache {
      */
     List<GmlId> getErrorFeatureIds();
 
+    /**
+     * Sets this cache into parsing mode, returning an AutoClosable representing the scoped-state of parsing operation.
+     * @return an AutoClosable representing the scoped-state for parsing
+     */
+    AutoCloseable startParsingMode();
+
+    /**
+     * Sets this cache into checking mode, returning an AutoClosable representing the scoped-state of checking operation.
+     * @return an AutoClosable representing the scoped-state for checking
+     */
+    AutoCloseable startCheckingMode();
+
 }

CityDoctorParent/CityDoctorModel/src/main/java/de/hft/stuttgart/citydoctor2/database/FeatureCache.java

@@ -11,20 +11,21 @@ import org.apache.logging.log4j.Logger;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.Stream;
 
 
 /**
- * A managed cache for CityObjects. This cache is connected to the embedded database and will automatically load missing
- * CityObjects from it. Has a hard limit on concurrently held CityObjects and will write them back to the database upon
- * eviction.
+ * A managed cache for top-level CityObjects (Features). This cache is connected to the embedded database and will
+ * automatically load missing CityObjects from it. This cache is size limited and will evict entries using the
+ * Window Tiny LFU policy.
  * <p>
- * This cache may only have a single instance, use {@link CacheManager} for instantiation.
+ * This cache may only have a single instance, use {@link CacheManager} for instantiation and access.
  * <p/>
  * <p>
  * Avoid holding long-time strong references to CityObjects returned by the cache, as eviction will sever the
- * connection between reference and cache, which can lead to conflicting Object states.
+ * connection between reference and cache, which will lead to conflicting Object states.
  * <p/>
  */
 public class FeatureCache implements CityObjectCache{
@@ -35,32 +36,50 @@ public class FeatureCache implements CityObjectCache{
     private final LoadingCache<GmlId, CityObject> cache;
     private final EmbeddedDatabaseHandler handler;
     private final AtomicInteger putsCounter = new AtomicInteger(0);
-    private boolean parsingFinished = false;
+    private CacheState state = CacheState.IDLE;
+    private final ConcurrentHashMap<Thread, GmlId> evictionLocks = new ConcurrentHashMap<>();
 
     FeatureCache() {
         handler = new EmbeddedDatabaseHandler();
         cache = Caffeine.newBuilder().maximumSize(3000).removalListener((GmlId key, CityObject value, RemovalCause cause) -> {
-                    if (value != null && cause.wasEvicted()) {
+                    if (value != null && cause.wasEvicted() && isFeatureMarshallable(value)){
                             handler.marshallCityObject(value);
                         }
         }).build(handler::unmarshallCityObject);
     }
 
+    /**
+     * Checks if the CityObject is currently being checked by a Checker-thread to prevent concurrent modifications by the
+     * Marshaller on eviction.
+     * @param co the CityObject
+     * @return true if the CityObject can be marshalled, false otherwise
+     */
+    private boolean isFeatureMarshallable(CityObject co) {
+        return switch (state){
+            case IDLE, PARSING -> true;
+            case CHECKING -> ! (evictionLocks.containsValue(co.getGmlId()));
+        };
+    }
+
     @Override
     public void put(CityObject cityObject) {
         cache.put(cityObject.getGmlId(), cityObject);
-        /* Batch marshall during parsing to improve performance
-         */
-        if (!parsingFinished){
+        switch (state){
+            case IDLE:
+                break;
+            case PARSING:
                 int count = putsCounter.incrementAndGet();
                 if (count >= 2500) {
                     batchMarshall();
                 }
+                break;
+            case CHECKING:
+                evictionLocks.remove(Thread.currentThread());
+                break;
         }
     }
 
 
-
     private void batchMarshall(){
         logger.trace("Setting up batch Marshaller");
         Map<GmlId, CityObject> cacheContent= new HashMap<>(cache.asMap());
@@ -84,6 +103,9 @@ public class FeatureCache implements CityObjectCache{
 
     @Override
     public CityObject get(GmlId id) {
+        if (state == CacheState.CHECKING){
+            evictionLocks.put(Thread.currentThread(), id);
+        }
         return cache.get(id);
     }
 
@@ -92,7 +114,7 @@ public class FeatureCache implements CityObjectCache{
     @Override
     public void replace(GmlId id, CityObject cityObject) {
         cache.invalidate(id);
-        put(cityObject);
+        cache.put(cityObject.getGmlId(), cityObject);
     }
 
     @Override
@@ -110,12 +132,12 @@ public class FeatureCache implements CityObjectCache{
         logger.trace("Resetting cache");
         cache.invalidateAll();
         putsCounter.set(0);
-        parsingFinished = false;
     }
 
     @Override
     public List<GmlId> getErrorFeatureIds() {
         return handler.getIdsOfAllCityObjectsWithErrors();
+
     }
 
     /**
@@ -132,15 +154,173 @@ public class FeatureCache implements CityObjectCache{
         }
     }
 
+    /**
+     * Signals the cache that parsing is beginning.
+     */
+    private boolean beginParsing(){
+        return state.changeState(this, CacheState.PARSING);
+    }
+
     /**
      * Signals the cache that parsing has finished.
      */
-    public void finishParsing() {
-        if (!parsingFinished) {
-            synchronize();
-            parsingFinished = true;
+    private void finishParsing() {
+        if (state == CacheState.PARSING && state.changeState(this, CacheState.IDLE)){
+            return;
+        }
+        throw new IllegalStateException("Cache failed to exit parsing mode");
+    }
+
+    /**
+     * Signals the cache that checking is beginning.
+     */
+    private boolean beginChecking(){
+        return state.changeState(this, CacheState.CHECKING);
+    }
+
+    /**
+     * Signals the cache that checking has finished.
+     */
+    private void finishChecking() {
+        if (state == CacheState.CHECKING && state.changeState(this, CacheState.IDLE)){
+               return;
+        }
+        throw new IllegalStateException("Cache failed to exit checking mode");
+    }
+
+    @Override
+    public AutoCloseable startParsingMode(){
+        if (this.beginParsing()){
+            return this::finishParsing;
+        } else throw new IllegalStateException("Cache is not in IDLE state, cannot start parsing mode");
+    }
+
+    @Override
+    public AutoCloseable startCheckingMode(){
+        if (this.beginChecking()){
+            return this::finishChecking;
+        } else throw new IllegalStateException("Cache is not in IDLE state, cannot start checking mode");
+    }
+
+    protected void setState(CacheState state) {
+        this.state = state;
+    }
+
+    public CacheState getState() {
+        return state;
+    }
+
+    public enum CacheState {
+        IDLE {
+            @Override
+            protected boolean changeState(FeatureCache cache, CacheState nextState) {
+                if (cache.getState() != this){
+                    logger.debug("FeatureCache invoked changeState() of IDLE state while cache state is {}", cache.getState());
+                    return false;
+                }
+                this.onExit(cache);
+                cache.setState(nextState);
+                nextState.onEntry(cache);
+                return true;
+            }
+
+            @Override
+            public void onEntry(FeatureCache cache) {
+                logger.trace("FeatureCache: Entering IDLE state");
+                // Idle state does nothing on entry
+            }
+
+            @Override
+            protected void onExit(FeatureCache cache) {
+                logger.trace("FeatureCache: Leaving IDLE state");
+                // Idle state does nothing on exit
+            }
+
+        }, PARSING {
+            @Override
+            protected boolean changeState(FeatureCache cache, CacheState nextState) {
+                if (cache.getState() != this){
+                    logger.debug("FeatureCache invoked changeState() of PARSING state while cache state is {}", cache.getState());
+                    return false;
+                }
+                boolean success = false;
+                switch (nextState){
+                    case IDLE:
+                        this.onExit(cache);
+                        cache.setState(CacheState.IDLE);
+                        success = true;
+                        break;
+                    case PARSING:
+                        logger.debug("Tried to set cache to PARSING state while already parsing");
+                        break;
+                    case CHECKING:
+                        logger.debug("Tried to set cache to CHECKING state while parsing");
+                        break;
                 }
+                return success;
+            }
+
+            @Override
+            public void onEntry(FeatureCache cache) {
+                logger.trace("FeatureCache: Entering PARSING state");
+                cache.resetCache();
+            }
+
+            @Override
+            protected void onExit(FeatureCache cache) {
+                logger.trace("FeatureCache: Leaving PARSING state");
+                cache.synchronize();
             }
 
+        }, CHECKING {
+            @Override
+            protected boolean changeState(FeatureCache cache, CacheState nextState) {
+                if (cache.getState() != this){
+                    logger.debug("FeatureCache invoked changeState() of CHECKING state while cache state is {}", cache.getState());
+                    return false;
+                }
+                boolean success = false;
+                switch (nextState){
+                    case IDLE:
+                        this.onExit(cache);
+                        cache.setState(CacheState.IDLE);
+                        success = true;
+                        break;
+                    case PARSING:
+                        logger.debug("Tried to set cache to PARSING state while checking");
+                        break;
+                    case CHECKING:
+                        logger.debug("Tried to set cache to CHECKING state while already checking");
+                        break;
+                }
+                return success;
+            }
+
+            @Override
+            public void onEntry(FeatureCache cache) {
+                logger.trace("FeatureCache: Entering CHECKING state");
+
+            }
+
+            @Override
+            protected void onExit(FeatureCache cache) {
+                logger.trace("FeatureCache: Leaving CHECKING state");
+                cache.synchronize();
+            }
+
+
+        };
+
+        /**
+         * Signals the statemachine to change the state of the FeatureCache. Returns true if the state successfully
+         * changed, false otherwise with the FeatureCache remaining in its current state.
+         * @param cache the FeatureCache instance
+         * @param nextState the requested next state
+         * @return true if the cache successfully changed state, false otherwise
+         */
+        protected abstract boolean changeState(FeatureCache cache, CacheState nextState);
+        protected abstract void onEntry(FeatureCache cache);
+        protected abstract void onExit(FeatureCache cache);
+    }
 }
 

CityDoctorParent/CityDoctorModel/src/main/java/de/hft/stuttgart/citydoctor2/database/UnconnectedCache.java

 package de.hft.stuttgart.citydoctor2.database;
 
+import de.hft.stuttgart.citydoctor2.datastructure.CityDoctorModel;
 import de.hft.stuttgart.citydoctor2.datastructure.CityObject;
 import de.hft.stuttgart.citydoctor2.datastructure.GmlId;
 
@@ -10,13 +11,19 @@ import java.util.Map;
 import java.util.stream.Stream;
 
 /**
- * A lightweight, unmanaged CityObjectCache that is not connected with the embedded Database.
+ * An adapter implementation for CityObjectCache, wrapping the Interface around a simple HashMap.
+ * <p>
+ * This class is intended to allow parsing or checking {@link CityDoctorModel CityDoctorModels} in specific cases where the model
+ * must not use the {@link FeatureCache}, like resolving of {@link de.hft.stuttgart.citydoctor2.datastructure.LibraryObject LibraryObject}
+ * files or mocking of the Interface in unit tests.
+ *
  */
 public class UnconnectedCache implements CityObjectCache{
 
     private final Map<GmlId, CityObject> cache;
+    private boolean idle = true;
 
-    UnconnectedCache() {
+    public UnconnectedCache() {
         cache = new HashMap<>();
     }
 
@@ -32,7 +39,7 @@ public class UnconnectedCache implements CityObjectCache{
 
     @Override
     public void replace(GmlId id, CityObject cityObject) {
-        // Not needed
+        cache.put(id, cityObject);
     }
 
     @Override
@@ -46,7 +53,7 @@ public class UnconnectedCache implements CityObjectCache{
     }
 
     public void resetCache(){
-        //
+        cache.clear();
     }
 
     @Override
@@ -59,4 +66,20 @@ public class UnconnectedCache implements CityObjectCache{
         });
         return errorFeatureIds;
     }
+
+    @Override
+    public AutoCloseable startParsingMode() {
+        if (idle){
+            idle = false;
+            return ()-> idle=true;
+        } else throw new IllegalStateException("Cache is not idle, could not start parsing mode");
+    }
+
+    @Override
+    public AutoCloseable startCheckingMode() {
+        if (idle){
+            idle = false;
+            return ()-> idle=true;
+        } else throw new IllegalStateException("Cache is not idle, could not start checking mode");
+    }
 }

CityDoctorParent/CityDoctorModel/src/main/java/de/hft/stuttgart/citydoctor2/datastructure/CityDoctorModel.java

@@ -92,6 +92,7 @@ public class CityDoctorModel {
         genericObjects = new ArrayList<>();
         globalErrors = new ArrayList<>();
         this.cache = cache;
+        cache.resetCache();
     }
 
     public boolean isValidated() {

CityDoctorParent/CityDoctorModel/src/main/java/de/hft/stuttgart/citydoctor2/mapper/citygml3/Citygml3FeatureMapper.java

@@ -25,6 +25,7 @@ import java.util.List;
 import java.util.Map;
 
 import de.hft.stuttgart.citydoctor2.database.CacheManager;
+import de.hft.stuttgart.citydoctor2.database.UnconnectedCache;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.citygml4j.core.model.CityGMLVersion;
@@ -172,11 +173,12 @@ public class Citygml3FeatureMapper extends ObjectWalker {
 	private List<ResolvableReference> references = new ArrayList<>();
 	private Map<Vertex, Vertex> vertexMap = new HashMap<>();
 
-    public Citygml3FeatureMapper(ParserConfiguration config, Path path, boolean nested) {
+    public Citygml3FeatureMapper(ParserConfiguration config, Path path, boolean unconnected) {
         this.config = config;
         this.directory = path.getParent();
-		if (nested){
-			model = new CityDoctorModel(config, path.toFile(), CacheManager.getUnconnectedCache());
+
+		if (unconnected){
+			model = new CityDoctorModel(config, path.toFile(), new UnconnectedCache());
 		} else {
 			model = new CityDoctorModel(config, path.toFile());
 		}

CityDoctorParent/CityDoctorModel/src/main/java/de/hft/stuttgart/citydoctor2/parser/CityGmlParser.java

@@ -44,7 +44,6 @@ import javax.xml.validation.Schema;
 import javax.xml.validation.SchemaFactory;
 import javax.xml.validation.Validator;
 
-import de.hft.stuttgart.citydoctor2.database.CacheManager;
 import de.hft.stuttgart.citydoctor2.datastructure.GmlId;
 import de.hft.stuttgart.citydoctor2.datastructure.ImplicitGeometryHolder;
 import org.apache.logging.log4j.Level;
@@ -254,13 +253,14 @@ public class CityGmlParser {
      * @param l Listener hook for tracking of parsing progress
      * @param handler Validation handler for GML-file validation
      * @param verbose If verbosity is enabled the parser will write info logs to the logger
-     * @param nested If nested is enabled the parser will use an {@link de.hft.stuttgart.citydoctor2.database.UnconnectedCache UnconnectedCache}
+     * @param unconnected If true the parser will use an {@link de.hft.stuttgart.citydoctor2.database.UnconnectedCache UnconnectedCache}
+     *                    to collect the parsed CityObjects
      * @return A {@link CityDoctorModel} object with the parsed Features
      * @throws CityGmlParseException if parsing of the CityGML-file failed
      * @throws InvalidGmlFileException if the GMLValidationHandler found an error
      */
     private static CityDoctorModel parseCityGmlFile(String filePath, ParserConfiguration config, ProgressListener l,
-                                                   GMLValidationHandler handler, boolean verbose, boolean nested)
+                                                   GMLValidationHandler handler, boolean verbose, boolean unconnected)
             throws CityGmlParseException, InvalidGmlFileException {
         CityGMLContext context = getContext();
         Path file = Paths.get(filePath);
@@ -279,7 +279,7 @@ public class CityGmlParser {
                 if (l != null) {
                     ois.addListener(l::updateProgress);
                 }
-                return readAndKeepFeatures(config, file, in, ois, verbose, nested);
+                return readAndKeepFeatures(config, file, in, ois, verbose, unconnected);
             }
         } catch (CityGMLReadException | IOException e) {
             throw new CityGmlParseException("Failed to read CityGML file", e);
@@ -431,8 +431,8 @@ public class CityGmlParser {
 
     private static CityDoctorModel readAndKeepFeatures(ParserConfiguration config, Path file,
                                                        CityGMLInputFactory inputFactory, ObservedInputStream ois,
-                                                       boolean verbose, boolean nested) throws CityGMLReadException {
-        return readAndKeepModel(new Citygml3FeatureMapper(config, file, nested), inputFactory, ois, verbose);
+                                                       boolean verbose, boolean unconnected) throws CityGMLReadException {
+        return readAndKeepModel(new Citygml3FeatureMapper(config, file, unconnected), inputFactory, ois, verbose);
     }
 
     private static CityDoctorModel readAndKeepModel(Citygml3FeatureMapper mapper, CityGMLInputFactory inputFactory,
@@ -445,7 +445,8 @@ public class CityGmlParser {
             List<AbstractCityObject> acos = new ArrayList<>();
             CityModel cgmlModel = null;
             CityDoctorModel cdModel = mapper.getModel();
-            cdModel.getCache().resetCache();
+            AutoCloseable parsingContext = cdModel.getCache().startParsingMode();
+            try (parsingContext){
                 List<AbstractCityObject> queuedObjects = new ArrayList<>();
                 while (reader.hasNext()) {
                     AbstractFeature chunk = reader.next();
@@ -494,8 +495,11 @@ public class CityGmlParser {
                             mapper.getModel().getNumberOfFeatures());
                 }
                 mapper.setCityGMLVersion(version);
-            CacheManager.finishParsing();
                 return mapper.getModel();
+            } catch (Exception e) {
+                throw new CityGMLReadException(e);
+            }
+
         }
     }
 

CityDoctorParent/CityDoctorModel/src/test/java/de/hft/stuttgart/citydoctor2/database/FeatureCacheTest.java

@@ -5,15 +5,16 @@ import org.citygml4j.core.model.building.AbstractBuilding;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.mockito.Mockito.mock;
 
 public class FeatureCacheTest {
 
 
-
     @Test
     public void testPut(){
-        FeatureCache cache = new FeatureCache();
+        FeatureCache cache = CacheManager.getFeatureCache();
+
         Building b = new Building();
         b.setGmlObject(mock(AbstractBuilding.class));
         b.setGmlId(new GmlId("test"));
@@ -21,18 +22,22 @@ public class FeatureCacheTest {
 
         assertEquals(0, cache.getTotalFeatureCount());
         cache.put(b);
-        cache.finishParsing();
+
+
         assertEquals(1, cache.getTotalFeatureCount());
     }
 
     @Test
     public void testGet(){
-        FeatureCache cache = new FeatureCache();
-        EmbeddedDatabaseHandler handler = new EmbeddedDatabaseHandler();
+        FeatureCache cache = CacheManager.getFeatureCache();
+        assertNull(cache.get(new GmlId("building")));
         Building b = new Building();
-        b.setGmlId(new GmlId("building"));
+
+        b.setGmlObject(mock(AbstractBuilding.class));
+        b.setGmlId(new GmlId("test"));
         b.addGeometry(GeometryTestUtils.createDummyGeometry(GeometryType.SOLID, Lod.LOD0));
-        handler.marshallCityObject(b);
+        assertEquals(0, cache.getTotalFeatureCount());
+        cache.put(b);
 
         CityObject b2 = cache.get(new GmlId("building"));
         assertEquals(b.getGmlId(), b2.getGmlId());

CityDoctorParent/CityDoctorValidation/src/main/java/de/hft/stuttgart/citydoctor2/check/Checker.java

@@ -193,13 +193,23 @@ public class Checker {
         if (config == null) {
             config = ValidationConfiguration.loadStandardValidationConfig();
         }
-
+        AutoCloseable checkingContext = model.getCache().startCheckingMode();
+        try(checkingContext){
             Callable<SvrlContentHandler> schematronThread = () -> executeSchematronValidationIfAvailable(config, model.getFile());
             ExecutorService executor = Executors.newSingleThreadExecutor();
             Future<SvrlContentHandler> handlerFuture =  executor.submit(schematronThread);
             executor.shutdown();
 
             checkCityModel(model, l);
+            getSchematronResults(handlerFuture, l);
+
+            model.setValidated(createValidationPlan());
+        } catch (Exception e){
+            logger.error("Validation failed due to encountering error", e);
+        }
+    }
+
+    private void getSchematronResults(Future<SvrlContentHandler> handlerFuture, ProgressListener l) {
         if (!handlerFuture.isDone()) {
             if (logger.isInfoEnabled()) {
                 logger.info(Localization.getText("Checker.schematronStillRunning"));
@@ -219,8 +229,6 @@ public class Checker {
             logger.error(Localization.getText("Checker.schematronInterrupted"), e);
             Thread.currentThread().interrupt();
         }
-        CacheManager.synchronizeCache();
-        model.setValidated(createValidationPlan());
     }
 
     private void handleSchematronResults(SvrlContentHandler handler) {