Add a circular dependency check on some objects deps

Signed-off-by: Paul Bui-Quang <[email protected]>
powsybl · Jan 6, 2020 · 99e35c9 · 99e35c9
1 parent 12c386f
commit 99e35c9
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 4 deletions.
diff --git a/afs-core/src/main/java/com/powsybl/afs/AfsCircularDependencyException.java b/afs-core/src/main/java/com/powsybl/afs/AfsCircularDependencyException.java
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2019, RTE (http://www.rte-france.com)
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ */
+
+package com.powsybl.afs;
+
+/**
+ * @author Paul Bui-Quang <paul.buiquang at rte-france.com>
+ */
+public class AfsCircularDependencyException extends AfsException {
+    public AfsCircularDependencyException(String message) {
+        super(message);
+    }
+
+    public AfsCircularDependencyException() {
+        super("Circular dependency detected");
+    }
+}
diff --git a/afs-core/src/main/java/com/powsybl/afs/ProjectFile.java b/afs-core/src/main/java/com/powsybl/afs/ProjectFile.java
@@ -104,6 +104,17 @@ public void removeDependencies(String name) {
         storage.flush();
     }
 
+    public boolean hasDeepDependency(ProjectFile candidateDependency) {
+        return hasDeepDependency(candidateDependency, null);
+    }
+
+    public boolean hasDeepDependency(ProjectFile candidateDependency, String dependencyName) {
+        List<ProjectFile> dependencies = (dependencyName != null) ?
+                getDependencies(dependencyName, ProjectFile.class) :
+                getDependencies().stream().map(ProjectDependency::getProjectNode).filter(dep -> ProjectFile.class.isAssignableFrom(dep.getClass())).map(ProjectFile.class::cast).collect(Collectors.toList());
+        return dependencies.stream().anyMatch(dep -> dep.getId().equals(candidateDependency.getId()) || dep.hasDeepDependency(candidateDependency, dependencyName));
+    }
+
     public boolean mandatoryDependenciesAreMissing() {
         return false;
     }

diff --git a/afs-core/src/test/java/com/powsybl/afs/AfsBaseTest.java b/afs-core/src/test/java/com/powsybl/afs/AfsBaseTest.java
@@ -335,4 +335,19 @@ public void fetchNodeTest() {
         checkResult.accept(projectFolder, afs.fetchNode(projectFolder.getId()));
     }
 
+    @Test
+    public void hasDeepDependencyTest() {
+        Project project = afs.getRootFolder().createProject("test");
+        FooFile createdFile = project.getRootFolder().fileBuilder(FooFileBuilder.class)
+                .withName("foo")
+                .build();
+        FooFile otherFile = project.getRootFolder().fileBuilder(FooFileBuilder.class)
+                .withName("bar")
+                .build();
+        createdFile.setDependencies("dep", Collections.singletonList(otherFile));
+        assertTrue(createdFile.hasDeepDependency(otherFile));
+        assertFalse(createdFile.hasDeepDependency(createdFile));
+        otherFile.setDependencies("dep", Collections.singletonList(createdFile));
+        assertTrue(createdFile.hasDeepDependency(createdFile, "dep"));
+    }
 }
diff --git a/afs-ext-base/pom.xml b/afs-ext-base/pom.xml
@@ -77,6 +77,10 @@
             <artifactId>junit</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.mockito</groupId>
             <artifactId>mockito-all</artifactId>

diff --git a/afs-ext-base/src/main/java/com/powsybl/afs/ext/base/AbstractScript.java b/afs-ext-base/src/main/java/com/powsybl/afs/ext/base/AbstractScript.java
@@ -7,6 +7,7 @@
 package com.powsybl.afs.ext.base;
 
 import com.google.common.io.CharStreams;
+import com.powsybl.afs.AfsCircularDependencyException;
 import com.powsybl.afs.OrderedDependencyManager;
 import com.powsybl.afs.ProjectFile;
 import com.powsybl.afs.ProjectFileCreationContext;
@@ -59,10 +60,16 @@ public List<AbstractScript> getIncludedScripts() {
     }
 
     public void addGenericScript(GenericScript genericScript) {
+        if (genericScript.hasDeepDependency(this) || getId().equals(genericScript.getId())) {
+            throw new AfsCircularDependencyException();
+        }
         orderedDependencyManager.appendDependencies(INCLUDED_SCRIPTS_DEPENDENCY_NAME, Collections.singletonList(genericScript));
     }
 
     public void addScript(T includeScript) {
+        if (includeScript.hasDeepDependency(this) || getId().equals(includeScript.getId())) {
+            throw new AfsCircularDependencyException();
+        }
         orderedDependencyManager.appendDependencies(INCLUDED_SCRIPTS_DEPENDENCY_NAME, Collections.singletonList(includeScript));
     }
 

diff --git a/afs-ext-base/src/main/java/com/powsybl/afs/ext/base/VirtualCase.java b/afs-ext-base/src/main/java/com/powsybl/afs/ext/base/VirtualCase.java
@@ -6,10 +6,7 @@
  */
 package com.powsybl.afs.ext.base;
 
-import com.powsybl.afs.AfsException;
-import com.powsybl.afs.DependencyCache;
-import com.powsybl.afs.ProjectFile;
-import com.powsybl.afs.ProjectFileCreationContext;
+import com.powsybl.afs.*;
 import com.powsybl.iidm.network.Network;
 
 import java.util.Collections;
@@ -41,6 +38,9 @@ public Optional<ProjectFile> getCase() {
 
     public void setCase(ProjectFile aCase) {
         Objects.requireNonNull(aCase);
+        if (aCase.hasDeepDependency(this, CASE_DEPENDENCY_NAME) || getId().equals(aCase.getId())) {
+            throw new AfsCircularDependencyException();
+        }
         setDependencies(CASE_DEPENDENCY_NAME, Collections.singletonList(aCase));
         projectCaseDependency.invalidate();
     }

diff --git a/afs-ext-base/src/test/java/com/powsybl/afs/ext/base/ModificationScriptTest.java b/afs-ext-base/src/test/java/com/powsybl/afs/ext/base/ModificationScriptTest.java
@@ -17,6 +17,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import static org.junit.Assert.*;
+import static org.assertj.core.api.Assertions.assertThatCode;
 
 /**
  * @author Geoffroy Jamgotchian <geoffroy.jamgotchian at rte-france.com>
@@ -135,5 +136,7 @@ public void test() {
         assertEquals(includes.size(), 2);
         assertEquals(includes.get(0).getId(), include1.getId());
         assertEquals(includes.get(1).getId(), include3.getId());
+
+        assertThatCode(() -> script.addScript(script)).isInstanceOf(AfsCircularDependencyException.class);
     }
 }
diff --git a/afs-ext-base/src/test/java/com/powsybl/afs/ext/base/VirtualCaseTest.java b/afs-ext-base/src/test/java/com/powsybl/afs/ext/base/VirtualCaseTest.java
@@ -22,6 +22,7 @@
 import java.io.IOException;
 import java.util.List;
 
+import static org.assertj.core.api.Assertions.assertThatCode;
 import static org.junit.Assert.*;
 
 /**
@@ -190,5 +191,7 @@ public void test() {
 
         assertNotEquals(importedCase2.getName(), virtualCase3.getCase().map(ProjectFile::getName).orElse(null));
         assertEquals(importedCase3.getName(), virtualCase3.getCase().map(ProjectFile::getName).orElse(null));
+
+        assertThatCode(() -> virtualCase3.setCase(virtualCase3)).isInstanceOf(AfsCircularDependencyException.class);
     }
 }