From 99e35c9f30c436602051e44d4c9fbc6ca4afe202 Mon Sep 17 00:00:00 2001 From: Paul Bui-Quang Date: Thu, 19 Dec 2019 16:13:47 +0100 Subject: [PATCH 1/3] Add a circular dependency check on some objects deps Signed-off-by: Paul Bui-Quang --- .../afs/AfsCircularDependencyException.java | 22 +++++++++++++++++++ .../java/com/powsybl/afs/ProjectFile.java | 11 ++++++++++ .../java/com/powsybl/afs/AfsBaseTest.java | 15 +++++++++++++ afs-ext-base/pom.xml | 4 ++++ .../powsybl/afs/ext/base/AbstractScript.java | 7 ++++++ .../com/powsybl/afs/ext/base/VirtualCase.java | 8 +++---- .../afs/ext/base/ModificationScriptTest.java | 3 +++ .../powsybl/afs/ext/base/VirtualCaseTest.java | 3 +++ 8 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 afs-core/src/main/java/com/powsybl/afs/AfsCircularDependencyException.java diff --git a/afs-core/src/main/java/com/powsybl/afs/AfsCircularDependencyException.java b/afs-core/src/main/java/com/powsybl/afs/AfsCircularDependencyException.java new file mode 100644 index 00000000..f09e21f6 --- /dev/null +++ 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 + */ +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 index 3b7a75a8..327d874e 100644 --- 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 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 index abc94426..188ab5b4 100644 --- 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 index e703b4a6..97411380 100644 --- a/afs-ext-base/pom.xml +++ b/afs-ext-base/pom.xml @@ -77,6 +77,10 @@ junit test + + org.assertj + assertj-core + org.mockito mockito-all 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 index df67bd28..f0c089fb 100644 --- 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 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 index 02558e1a..3f9739ff 100644 --- 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 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 index f56035e4..a2147ff1 100644 --- 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 @@ -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 index 9683373b..8ff0b787 100644 --- 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); } } From 28934b2f4763413f855f141fa34effec08ee3830 Mon Sep 17 00:00:00 2001 From: Paul Bui-Quang Date: Tue, 7 Jan 2020 14:25:03 +0100 Subject: [PATCH 2/3] Sonar Signed-off-by: Paul Bui-Quang --- .../powsybl/afs/AfsCircularDependencyException.java | 7 +++---- .../powsybl/afs/ext/base/ModificationScriptTest.java | 12 ++++++++++++ 2 files changed, 15 insertions(+), 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 index f09e21f6..029fe63f 100644 --- a/afs-core/src/main/java/com/powsybl/afs/AfsCircularDependencyException.java +++ b/afs-core/src/main/java/com/powsybl/afs/AfsCircularDependencyException.java @@ -8,13 +8,12 @@ package com.powsybl.afs; +import com.powsybl.commons.PowsyblException; + /** * @author Paul Bui-Quang */ -public class AfsCircularDependencyException extends AfsException { - public AfsCircularDependencyException(String message) { - super(message); - } +public class AfsCircularDependencyException extends PowsyblException { public AfsCircularDependencyException() { super("Circular dependency detected"); 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 index a2147ff1..47bb597b 100644 --- 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 @@ -11,6 +11,8 @@ import com.powsybl.afs.mapdb.storage.MapDbAppStorage; import com.powsybl.afs.storage.AppStorage; import com.powsybl.afs.storage.InMemoryEventsBus; +import com.powsybl.afs.storage.NodeGenericMetadata; +import com.powsybl.afs.storage.NodeInfo; import org.junit.Test; import java.util.List; @@ -138,5 +140,15 @@ public void test() { assertEquals(includes.get(1).getId(), include3.getId()); assertThatCode(() -> script.addScript(script)).isInstanceOf(AfsCircularDependencyException.class); + + NodeInfo genScript1NodeInfo = storage.createNode(rootFolder.getId(), "genScript1", GenericScript.PSEUDO_CLASS, "", GenericScript.VERSION, new NodeGenericMetadata()); + ProjectFileCreationContext projectFileCreationContext = new ProjectFileCreationContext(genScript1NodeInfo, storage, project); + GenericScript genericScript1 = new GenericScript(projectFileCreationContext); + storage.setConsistent(genScript1NodeInfo.getId()); + genericScript1.writeScript("list of things"); + storage.flush(); + script.addGenericScript(genericScript1); + + assertThatCode(() -> genericScript1.addGenericScript(genericScript1)).isInstanceOf(AfsCircularDependencyException.class); } } From 9d803f86e06081494a310f1f0c4bddfa8236ec34 Mon Sep 17 00:00:00 2001 From: Paul Bui-Quang Date: Tue, 7 Jan 2020 16:01:17 +0100 Subject: [PATCH 3/3] Change depdency check condition order Signed-off-by: Paul Bui-Quang --- .../main/java/com/powsybl/afs/ext/base/AbstractScript.java | 4 ++-- .../src/main/java/com/powsybl/afs/ext/base/VirtualCase.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) 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 index f0c089fb..54fc0c8a 100644 --- 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 @@ -60,14 +60,14 @@ public List getIncludedScripts() { } public void addGenericScript(GenericScript genericScript) { - if (genericScript.hasDeepDependency(this) || getId().equals(genericScript.getId())) { + if (getId().equals(genericScript.getId()) || genericScript.hasDeepDependency(this)) { 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())) { + if (getId().equals(includeScript.getId()) || includeScript.hasDeepDependency(this)) { 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 index 3f9739ff..4606ac17 100644 --- 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 @@ -38,7 +38,7 @@ public Optional getCase() { public void setCase(ProjectFile aCase) { Objects.requireNonNull(aCase); - if (aCase.hasDeepDependency(this, CASE_DEPENDENCY_NAME) || getId().equals(aCase.getId())) { + if (getId().equals(aCase.getId()) || aCase.hasDeepDependency(this, CASE_DEPENDENCY_NAME)) { throw new AfsCircularDependencyException(); } setDependencies(CASE_DEPENDENCY_NAME, Collections.singletonList(aCase));