Refactor: Change Checker workflow

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

@@ -31,6 +31,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
@@ -41,9 +42,15 @@ import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 import javax.annotation.Nullable;
+import javax.swing.text.html.Option;
 
+import de.hft.stuttgart.citydoctor2.utils.Pair;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -90,8 +97,61 @@ public class Checker {
 
     private static final Logger logger = LogManager.getLogger(Checker.class);
 
+    private static class CheckExecutionLayers {
+
+        private final List<List<Check>> fullLayersList;
+        private boolean hasTopoLayers;
+
+        CheckExecutionLayers(List<List<Check>> layerList){
+            this.fullLayersList = layerList;
+            hasTopoLayers = layerList.stream()
+                    .anyMatch(layer -> layer.stream()
+                            .anyMatch(c -> c.getType().equals(RequirementType.TOPOLOGY)));
+        }
+
+        public List<List<Check>> getFullLayersList(){
+            return this.fullLayersList;
+        }
+
+        public List<List<Check>> getGeometricLayersList(){
+            return filterLayers(check ->
+                    check.getType().equals(RequirementType.GEOMETRY) ||
+                            check.getType().equals(RequirementType.SEMANTIC));
+        }
+
+        public List<List<Check>> getTopoLayersList(){
+            return filterLayers(check ->
+                    check.getType().equals(RequirementType.TOPOLOGY));
+        }
+
+        public boolean isEmpty(){
+            return this.fullLayersList.isEmpty();
+        }
+
+        public boolean hasTopoLayers(){
+            return this.hasTopoLayers;
+        }
+
+        private List<List<Check>> filterLayers(Predicate<Check> filter) {
+            List<List<Check>> result = new ArrayList<>();
+
+            for (List<Check> layer : fullLayersList) {
+                List<Check> filtered = layer.stream()
+                        .filter(filter)
+                        .collect(Collectors.toList());
+
+                if (!filtered.isEmpty()) {
+                    result.add(filtered);
+                }
+            }
+
+            return result;
+        }
+    }
+
+
     private ValidationConfiguration config;
-    private List<List<Check>> execLayers;
+    private CheckExecutionLayers execLayers;
 
     private List<Filter> includeFilters;
     private List<Filter> excludeFilters;
@@ -351,6 +411,24 @@ public class Checker {
         Checking waterChecking = new Checking();
         waterChecking.setFeatureType(TopLevelFeatureType.WATER);
         filter.getChecking().add(new CheckingProperty(waterChecking));
+
+        //TODO: Implement filters for additional types
+//
+//        Checking tunnelChecking = new Checking();
+//        tunnelChecking.setFeatureType(TopLevelFeatureType.TUNNEL);
+//        filter.getChecking().add(new CheckingProperty(tunnelChecking));
+//
+//        Checking genericObjectChecking = new Checking();
+//        genericObjectChecking.setFeatureType(TopLevelFeatureType.GENERIC_CITY_OBJECT);
+//        filter.getChecking().add(new CheckingProperty(genericObjectChecking));
+//
+//        Checking cityFurnitureChecking = new Checking();
+//        cityFurnitureChecking.setFeatureType(TopLevelFeatureType.CITY_FURNITURE);
+//        filter.getChecking().add(new CheckingProperty(cityFurnitureChecking));
+//
+//        Checking otherConstructionChecking = new Checking();
+//        otherConstructionChecking.setFeatureType(TopLevelFeatureType.OTHER_CONSTRUCTION_OBJECT);
+//        filter.getChecking().add(new CheckingProperty(otherConstructionChecking));
     }
 
     private void removeFilter(TopLevelFeatureType tlft, de.hft.stuttgart.quality.model.types.Filter filter) {
@@ -418,6 +496,7 @@ public class Checker {
         return checkList;
     }
 
+
     private void insertGlobalParameters(ValidationConfiguration config, Set<CheckId> enabledCheck,
                                         Map<CheckId, Map<String, String>> parameterMap, Entry<String, RequirementConfiguration> e,
                                         de.hft.stuttgart.citydoctor2.check.Requirement req) {
@@ -479,17 +558,34 @@ public class Checker {
                 (threadCount, threadCount, 60L, TimeUnit.SECONDS, new LinkedBlockingQueue<>());
         try{
             long startTime = System.nanoTime();
+            logger.trace("Starting validation");
+            List<GmlId> missedFeatures = new ArrayList<>();
+            List<Callable<Optional<GmlId>>> tasks;
+            List<Future<Optional<GmlId>>> futures;
+            if(execLayers.hasTopoLayers()){
+                // First: geometric validation
+                tasks = getGeometricCheckingTasks(cache, features, l);
+                futures = exec.invokeAll(tasks);
+                missedFeatures = getMissedFeatures(futures);
+                logger.trace("Finished geometric checks, starting topological checks");
+                // Second: topological validation
+                tasks = getTopologicalCheckingTasks(cache, features, l);
+                futures = exec.invokeAll(tasks);
+                missedFeatures.addAll(getMissedFeatures(futures));
 
-            List<Future<GmlId>> futures = runChecksOnFeatures(exec, cache, features, checkedCount, l);
-            List<GmlId> missedFeatures = getMissedFeatures(futures);
-
+            } else {
+                tasks = getFullCheckingTasks(cache, features, l);
+                futures = exec.invokeAll(tasks);
+                missedFeatures = getMissedFeatures(futures);
 
+            }
             if (!missedFeatures.isEmpty()){
                 logger.error(Localization.getText("Checker.dbUnresponsive"));
                 if (logger.isDebugEnabled()){
                     logger.debug(missedFeatures.toString());
                 }
             }
+
             long endTime = System.nanoTime();
             if (logger.isInfoEnabled()){
                 long totalTime = (endTime - startTime) / 1_000_000; // Convert ns to ms
@@ -517,59 +613,78 @@ public class Checker {
         }
     }
 
+
+    private List<Callable<Optional<GmlId>>> getFullCheckingTasks(CityObjectCache cache, List<GmlId> ids,
+                                                       @Nullable ProgressListener l) {
+        float progressTarget = ids.size();
+        Consumer<CityObject> checkFunction = this::executeTopologicalChecksForCityObject;
+        return getCheckWorkingTasks(cache, ids, checkFunction, progressTarget, 0, l);
+    }
+
+    private List<Callable<Optional<GmlId>>> getGeometricCheckingTasks(CityObjectCache cache, List<GmlId> ids,
+                                                            @Nullable ProgressListener l) {
+        float progressTarget = (ids.size() * 2);
+        Consumer<CityObject> checkFunction = this::executeGeometricChecksForCityObject;
+        return getCheckWorkingTasks(cache, ids, checkFunction, progressTarget, 0, l);
+    }
+
+    private List<Callable<Optional<GmlId>>> getTopologicalCheckingTasks(CityObjectCache cache, List<GmlId> ids,
+                                                              ProgressListener l) {
+        float progressTarget = (ids.size() * 2);
+        Consumer<CityObject> checkFunction = this::executeTopologicalChecksForCityObject;
+        return getCheckWorkingTasks(cache, ids, checkFunction, progressTarget, ids.size(), l);
+    }
+
+
     /**
-     * Runs the validation on a list of Features. The checks are run asynchronously and multithreaded. Returns a list
-     * of {@link Future Futures} for evaluation if Features failed to be checked due to exceptions or failing to load from
-     * the cache. If failed, the respective Future will return the affected Feature's GmlID.
-     * <p>Checks finding an error in a Feature, or not having their dependencies met, are not a check-failure in this
-     * context.<p/>
-     * @param exec the threadpool for the tasks
+     * Returns a {@link Callable Callable} tasks list for checking of CityObjects.
      * @param cache the cache of the model
      * @param ids the list of GmlIds to check
-     * @param checkedCount the number of already checked Features for tracking the total progress of validation across retries
+     * @param checkerHook call hook for the checking strategy
+     * @param progressTarget target value for calculating progress percentage
+     * @param counterStartValue start value for the progress counter
      * @param l a listener for the progress of the validation, can be null
-     * @return a list of Futures containing the GmlId of Features that failed to execute all checks, and contain null otherwise
-     * @throws InterruptedException if interrupted while invoking the tasks for checking
+     * @return a list of Callables for an ExecutorService. The returned Optional will be empty if checks were executed
+     * successfully, and contain the gmlID of the feature if checks failed to execute.
+     *
      */
-    private List<Future<GmlId>> runChecksOnFeatures(ExecutorService exec, CityObjectCache cache, List<GmlId> ids, AtomicInteger checkedCount,
-                                           @Nullable ProgressListener l)
-            throws InterruptedException {
-        float featureSum = ids.size() + (float) checkedCount.get();
-
-        logger.trace("Queueing up Checker tasks");
-        List<Callable<GmlId>> tasks = new ArrayList<>();
+    private List<Callable<Optional<GmlId>>> getCheckWorkingTasks(CityObjectCache cache, List<GmlId> ids,
+                                                                 Consumer<CityObject> checkerHook, float progressTarget, int counterStartValue,
+                                                                 @Nullable ProgressListener l) {
+        AtomicInteger counter = new AtomicInteger(counterStartValue);
+        List<Callable<Optional<GmlId>>> tasks = new ArrayList<>();
         for (GmlId id : ids) {
             tasks.add(()->{
+                Optional<GmlId> ret = Optional.of(id);
                 CityObject co = cache.get(id);
                 if (co == null) {
-                    return id;
+                    return ret;
                 }
                 if(Thread.interrupted()){
                     Thread.currentThread().interrupt();
-                    return id;
+                    return ret;
                 }
-                executeChecksForCityObject(co);
+                checkerHook.accept(co);
                 cache.put(co);
-                checkedCount.incrementAndGet();
+                counter.incrementAndGet();
                 if (l!=null){
-                    l.updateProgress((checkedCount.get()) / featureSum);
+                    l.updateProgress((counter.get()) / progressTarget);
                 }
-                return null;
+                return Optional.empty();
             });
         }
-        logger.trace("Queueing up finished, invoking all tasks");
-        return exec.invokeAll(tasks);
+        return tasks;
     }
 
-    private List<GmlId> getMissedFeatures(List<Future<GmlId>> futures) throws InterruptedException{
+    private List<GmlId> getMissedFeatures(List<Future<Optional<GmlId>>> futures) throws InterruptedException{
         List<GmlId> missedList = new ArrayList<>();
         Set<String> errors = new HashSet<>();
-        for (Future<GmlId> future : futures) {
+        for (Future<Optional<GmlId>> future : futures) {
             try{
-                GmlId gmlId = future.get();
-                if (gmlId != null) {
-                    missedList.add(gmlId);
-                }
+
+                Optional<GmlId> gmlId = future.get();
+                gmlId.ifPresent(missedList::add);
+
             } catch (ExecutionException e){
                 logger.debug("A Task failed due to an unexpected exception", e);
                 logger.debug(e.getCause());
@@ -583,11 +698,11 @@ public class Checker {
         return missedList;
     }
 
-    private boolean filterObject(CityObject co) {
-        return isObjectIncluded(co, includeFilters, excludeFilters);
+    private boolean isObjectIncluded(CityObject co) {
+        return applyObjectFilters(co, includeFilters, excludeFilters);
     }
 
-    private boolean isObjectIncluded(CityObject co, List<Filter> includeFilters, List<Filter> excludeFilters) {
+    private boolean applyObjectFilters(CityObject co, List<Filter> includeFilters, List<Filter> excludeFilters) {
         if (!includeFilters.isEmpty()) {
             boolean include = false;
             for (Filter f : includeFilters) {
@@ -618,27 +733,67 @@ public class Checker {
      * @param co the city object that is going to be checked
      */
     private void executeChecksForCityObject(CityObject co) {
-        if (!filterObject(co)) {
+        if (!isObjectIncluded(co)) {
             return;
         }
-        co.prepareForChecking();
-        executeChecksForCheckable(co);
-        co.clearMetaInformation();
+        executeAllChecksForCheckable(co);
+    }
+
+    private void executeGeometricChecksForCityObject(CityObject co) {
+        if (!isObjectIncluded(co)) {
+            return;
+        }
+        executeGeoChecksForCheckable(co);
+    }
+
+    private void executeTopologicalChecksForCityObject(CityObject co) {
+        if (!isObjectIncluded(co)) {
+            return;
+        }
+        executeChecksForCityObject(co);
     }
 
+
     /**
-     * Executes all checks for the checkable. This will bypass the filters. This
-     * will clear the old check results
+     * Executes all checks for the checkable. This method bypasses feature-filters and will clear previous check results.
      *
      * @param co the checkable.
      */
-    public void executeChecksForCheckable(Checkable co) {
+    public void executeAllChecksForCheckable(Checkable co) {
         // throw away old results
         co.clearAllContainedCheckResults();
+        executeChecksForCheckable(co, execLayers.getFullLayersList());
+    }
+
+    /**
+     * Executes all geometric and (geometric-)semantic checks for the checkable. This method bypasses feature-filters and
+     * will clear previous check results.
+     *
+     * @param co the checkable.
+     */
+    private void executeGeoChecksForCheckable(CityObject co) {
+        co.clearAllContainedCheckResults();
+        executeChecksForCheckable(co, execLayers.getGeometricLayersList());
+    }
+
+    /**
+     * Executes all topological checks for the checkable. This method bypasses feature-filters.
+     * <p>
+     * Will <strong>NOT</strong> clear previous check results!
+     * This method is intended to run as part of the workflow for validation of the entire city model
+     * </p>
+     * @param co the checkable.
+     */
+    private void executeTopoChecksForCheckable(Checkable co) {
+        executeChecksForCheckable(co, execLayers.getTopoLayersList());
+    }
+
+    private void executeChecksForCheckable(Checkable co, List<List<Check>> layersList) {
+        co.prepareForChecking();
         if (logger.isDebugEnabled()) {
             logger.debug(Localization.getText("Checker.checkFeature"), co);
         }
-        for (List<Check> execLayer : execLayers) {
+        for (List<Check> execLayer : layersList) {
             for (Check check : execLayer) {
                 if (logger.isTraceEnabled()) {
                     logger.trace(Localization.getText("Checker.executeCheck"), check.getCheckId());
@@ -646,9 +801,15 @@ public class Checker {
                 co.accept(check);
             }
         }
+        co.clearMetaInformation();
     }
 
-    public static List<List<Check>> buildExecutionLayers(List<Check> checks) {
+
+
+
+
+
+    private static CheckExecutionLayers buildExecutionLayers(List<Check> checks) {
         List<List<Check>> result = new ArrayList<>();
 
         Set<Check> availableChecks = new HashSet<>(checks);
@@ -674,7 +835,8 @@ public class Checker {
                 usedChecks.add(c.getCheckId());
             }
         }
-        return result;
+
+        return new CheckExecutionLayers(result);
     }
 
     private static boolean searchForUnusedDependency(Set<CheckId> usedChecks, Check c) {
@@ -786,4 +948,6 @@ public class Checker {
             pdfReporter.report(co);
         }
     }
+
+
 }

CityDoctorParent/Extensions/CityDoctorHealer/src/main/java/de/hft/stuttgart/citydoctor2/healer/Healer.java

@@ -17,7 +17,6 @@ import de.hft.stuttgart.citydoctor2.database.UnconnectedCache;
 import de.hft.stuttgart.citydoctor2.exceptions.CityObjectCacheException;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.citygml4j.core.ade.ADEException;
 import org.citygml4j.core.model.CityGMLVersion;
 import org.citygml4j.core.model.core.AbstractCityObject;
 import org.citygml4j.core.model.core.AbstractCityObjectProperty;
@@ -263,7 +262,7 @@ public class Healer {
 		// recheck for geometry errors as they were removed when executing the
 		// schematron stuff
 		co.prepareForChecking();
-		checker.executeChecksForCheckable(co);
+		checker.executeAllChecksForCheckable(co);
 		if (checker.getConfig().getParserConfiguration().useLowMemoryConsumption()) {
 			co.clearMetaInformation();
 		}
@@ -327,7 +326,7 @@ public class Healer {
 		}
 		if (!co.isValidated()) {
 			// check it if it has not been checked yet
-			checker.executeChecksForCheckable(co);
+			checker.executeAllChecksForCheckable(co);
 		}
 
 		try {
@@ -352,7 +351,7 @@ public class Healer {
 				co.prepareForChecking();
 				filterOutDuplicateVertices(co);
 				// recheck for errors
-				checker.executeChecksForCheckable(co);
+				checker.executeAllChecksForCheckable(co);
 			}
 		} catch (Exception e) {
 			logger.debug("Failed to heal geometry", e);

CityDoctorParent/Extensions/CityDoctorHealerGUI/src/main/java/de/hft/stuttgart/citydoctor2/healer/gui/HealerController.java

@@ -316,7 +316,7 @@ public class HealerController {
 		Healer healer = new Healer(checker);
 		healer.heal(nextGeometry, nextFeature, 1);
 		nextFeature.prepareForChecking();
-		checker.executeChecksForCheckable(nextFeature);
+		checker.executeAllChecksForCheckable(nextFeature);
 		updateNextErrors();
 		updateNextGeometryView();
 	}
@@ -408,7 +408,7 @@ public class HealerController {
 			return;
 		}
 		nextFeature.prepareForChecking();
-		checker.executeChecksForCheckable(nextFeature);
+		checker.executeAllChecksForCheckable(nextFeature);
 		model.replaceFeature(currentFeature, nextFeature);
 		currentFeature = nextFeature;
 		currentGeometry = nextGeometry;