Feat/66 automated std import ied communication #101

AliouDIAITE · 2022-04-28T15:00:54Z

Import multiple STD files (IED, DataTypeTemplates and Communication) into SCD

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

#91) * Update SCL communication import method to allow the import of the attributes Address et PhysConn if present. Signed-off-by: SAINTIER FRANCOIS <francois.saintier@rte-france.com> Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

sct-commons/pom.xml

sct-commons/src/main/java/org/lfenergy/compas/sct/commons/dto/SubNetworkDTO.java

sct-commons/src/main/java/org/lfenergy/compas/sct/commons/scl/SclService.java

SaintierFr · 2022-05-06T08:45:42Z

sct-commons/src/test/resources/scd-ied-dtt-com-import-stds/std_SITESITE1GTW2.xml

+        </History>
+    </Header>
+    <Communication>
+        <SubNetwork type="8-MMS" name="RSPACE_PROCESS_NETWORK">


Like the others, generify file

Suggested change

<SubNetwork type="8-MMS" name="RSPACE_PROCESS_NETWORK">

<SubNetwork type="8-MMS" name="PROCESS_NETWORK">

SaintierFr · 2022-05-06T08:46:02Z

sct-commons/src/test/resources/scd-ied-dtt-com-import-stds/std_SITESITE1GTW2.xml

+                    <Private type="RTE-ProjectStage">Phase 1_Pilotes</Private>
+                    <Private type="RTE-FunctionUUID">9d13ab59-1ec8-4c46-91c8-0326e6a5408e</Private>
+                    <Private type="RTE-LDMaxInst">1</Private>
+                    <Private type="RTE-FunctionIndice">1.0</Private>


Generify file

SaintierFr · 2022-05-06T08:46:25Z

...ommons/src/test/resources/scd-ied-dtt-com-import-stds/std_with_same_ICDSystemVersionUUID.xml

+        </History>
+    </Header>
+    <Communication>
+        <SubNetwork type="8-MMS" name="RSPACE_PROCESS_NETWORK">


Suggested change

<SubNetwork type="8-MMS" name="RSPACE_PROCESS_NETWORK">

<SubNetwork type="8-MMS" name="PROCESS_NETWORK">

SaintierFr · 2022-05-06T08:46:35Z

...ommons/src/test/resources/scd-ied-dtt-com-import-stds/std_with_same_ICDSystemVersionUUID.xml

+                </Address>
+            </ConnectedAP>
+        </SubNetwork>
+        <SubNetwork type="IP" name="RSPACE_ADMIN_NETWORK">


Suggested change

<SubNetwork type="IP" name="RSPACE_ADMIN_NETWORK">

<SubNetwork type="IP" name="ADMIN_NETWORK">

SaintierFr · 2022-05-06T08:46:49Z

...ommons/src/test/resources/scd-ied-dtt-com-import-stds/std_with_same_ICDSystemVersionUUID.xml

+                    <Private type="RTE-ProjectStage">Phase 1_Pilotes</Private>
+                    <Private type="RTE-FunctionUUID">9d13ab59-1ec8-4c46-91c8-0326e6a5408e</Private>
+                    <Private type="RTE-LDMaxInst">1</Private>
+                    <Private type="RTE-FunctionIndice">1.0</Private>


Generify file

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

sct-commons/src/main/java/org/lfenergy/compas/sct/commons/dto/SubNetworkDTO.java

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

sonarqubecloud · 2022-05-10T13:32:23Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

90.8% Coverage
0.0% Duplication

psabatier74 · 2022-05-10T09:23:23Z

sct-app/src/main/java/org/lfenergy/compas/sct/app/SclAutomationService.java


 @Slf4j
 public class SclAutomationService {

+    private static final Map<Pair<String, String>, List<String>> comMap = Map.of(


perhaps a more precise name like communicationMap ?

psabatier74 · 2022-05-10T09:25:31Z

sct-app/src/test/java/org.lfenergy.compas.sct.app/SclAutomationServiceTest.java

@@ -29,11 +31,15 @@ void init(){

    @Test
    void createSCD() throws Exception {
-        SCL ssd = SclTestMarshaller.getSCLFromFile("/scd-substation-import-ssd/ssd.xml");
-        SclRootAdapter expectedSCD = SclAutomationService.createSCD(ssd, headerDTO);
+        SCL ssd = SclTestMarshaller.getSCLFromFile("/scd-ied-dtt-com-import-stds/scd.xml");


I suggest to create blocs \Given \When \Then for better readability in all tests. Here the code is too dense

psabatier74 · 2022-05-10T09:29:21Z

sct-app/src/test/java/org.lfenergy.compas.sct.app/SclAutomationServiceTest.java

+        SCL std1 = SclTestMarshaller.getSCLFromFile("/std_1.xml");
+        SCL std2 = SclTestMarshaller.getSCLFromFile("/std_2.xml");
+        SCL std3 = SclTestMarshaller.getSCLFromFile("/std_3.xml");
+        SclRootAdapter expectedSCD = SclAutomationService.createSCD(ssd, headerDTO, Set.of(std1, std2, std3));
        assertNotNull(expectedSCD.getCurrentElem().getHeader().getId());
        assertEquals(1 ,expectedSCD.getCurrentElem().getHeader().getHistory().getHitem().size());
        assertEquals(1, expectedSCD.getCurrentElem().getSubstation().size());


this assertion is related to HItem creation ?

psabatier74 · 2022-05-10T09:37:58Z

sct-app/src/test/java/org.lfenergy.compas.sct.app/SclAutomationServiceTest.java

        assertNotNull(expectedSCD.getCurrentElem().getHeader().getId());
        assertEquals(1, expectedSCD.getCurrentElem().getHeader().getHistory().getHitem().size());
        assertEquals("what", expectedSCD.getCurrentElem().getHeader().getHistory().getHitem().get(0).getWhat());
    }

-        @Test
+    @Test
    void createSCD_SSD_Without_Substation() throws Exception {


It is important to show the test intention with its name. Here it is not evident to know if we check the creation or an error (except the throws exception). I suggest something like should_not_create_SCD_SSD_without_substation()

sct-commons/src/main/java/org/lfenergy/compas/sct/commons/dto/SubNetworkDTO.java

psabatier74 · 2022-05-10T09:48:07Z

sct-commons/src/main/java/org/lfenergy/compas/sct/commons/scl/SclService.java

+
+    public static SclRootAdapter importSTDElementsInSCD(@NonNull SclRootAdapter scdRootAdapter, Set<SCL> stds,
+                                                        Map<Pair<String, String>, List<String>> comMap) throws ScdException {
+


Big method, could it be devided ?
Instead of comments try to create private methods with good naming.

psabatier74 · 2022-05-10T10:01:18Z

sct-commons/src/main/java/org/lfenergy/compas/sct/commons/scl/SclService.java

+                .collect(Collectors.toMap(tPrivate -> getValueFromPrivate(tPrivate, CommonConstants.IED_NAME), tPrivate -> tPrivate));
+    }
+
+    private static Map<String, Pair<TPrivate, List<SCL>>> createMapICDSystemVersionUuidAndSTDFile(Set<SCL> stds) {


Naming suggestion : createSTDFileByICDSystemVersionUuidMap

psabatier74 · 2022-05-10T10:04:28Z

sct-commons/src/test/java/org/lfenergy/compas/sct/commons/dto/DTO.java

 import org.lfenergy.compas.scl2007b4.model.*;

 import java.time.LocalDateTime;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
 import java.util.UUID;

 public class DTO {


too generic class name. What is the purpose, responsability of this class ?

psabatier74 · 2022-05-10T10:06:59Z

sct-commons/src/main/java/org/lfenergy/compas/sct/commons/scl/SclService.java

+                CommonConstants.HW_REV, CommonConstants.SW_REV, CommonConstants.HEADER_ID, CommonConstants.HEADER_VERSION,
+                CommonConstants.HEADER_REVISION);
+
+        Element iedElement = (Element) iedPrivate.getContent().get(1);


get(1) how do you know it is the second element that you seek ? And there is potentially a risk of NullPointerException

psabatier74 · 2022-05-10T10:09:16Z

sct-commons/src/test/java/org/lfenergy/compas/sct/commons/dto/SubNetworkDTOTest.java

@@ -42,4 +48,27 @@ void testFrom(){

    }

+    @Test
+    void testCreateDefaultSubnetwork() {
+        CommunicationAdapter comAdapter = Mockito.mock(CommunicationAdapter.class);


Reminder for tests devided in \Given \When \Then blocs

AliouDIAITE added this to the 0.0.4 milestone Apr 28, 2022

AliouDIAITE requested review from samirromdhani and SaintierFr April 28, 2022 15:00

AliouDIAITE self-assigned this Apr 28, 2022

AliouDIAITE and others added 8 commits April 28, 2022 17:03

[#66] : WIP

0a9ff1e

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

[#66] : WIP to be refactored

25fdaa6

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

[#65] : correcting history for automation service

962c60f

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

[#65 ==> 95] : correcting history for automation service: add test

55dddfb

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

[#66] : WIP refactoring and test needed

f0f9b87

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

[#66] : WIP add files and tests, refactoring

23dcc7b

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

[#66] : moving code to subnetwork and refactoring

ca1d917

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

AliouDIAITE force-pushed the feat/66-Automated-STD-Import-IED-Communication branch from 62f6eb1 to ca1d917 Compare April 28, 2022 15:03

AliouDIAITE linked an issue Apr 28, 2022 that may be closed by this pull request

RSR-367 : Automated ICD import and then IED renaming and Communication section creation into SCD file - Part 2 of 4 #66

Closed

AliouDIAITE force-pushed the feat/66-Automated-STD-Import-IED-Communication branch from 219ab20 to ca1d917 Compare April 28, 2022 15:20

AliouDIAITE added 4 commits April 28, 2022 17:22

[#66] : delete not yet used code

a4a898a

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

[#66] : WIP

2097116

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

[#66] : WIP to be refactored

a2ae4f7

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

[#66] : rebase branch into develop

d61db13

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

SaintierFr reviewed May 6, 2022

View reviewed changes

[#66] : refactoring after remarks for pull request

be8af28

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

SaintierFr reviewed May 10, 2022

View reviewed changes

sct-commons/src/main/java/org/lfenergy/compas/sct/commons/dto/SubNetworkDTO.java Outdated Show resolved Hide resolved

[#66] : delete commented code and update pom.xml

e710426

Signed-off-by: Aliou DIAITE <aliou.diaite@rte-france.com>

SaintierFr previously approved these changes May 10, 2022

View reviewed changes

AliouDIAITE dismissed SaintierFr’s stale review via e710426 May 10, 2022 13:29

SaintierFr approved these changes May 10, 2022

View reviewed changes

psabatier74 reviewed May 10, 2022

View reviewed changes

AliouDIAITE merged commit 36643fa into develop May 10, 2022

AliouDIAITE deleted the feat/66-Automated-STD-Import-IED-Communication branch May 10, 2022 14:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/66 automated std import ied communication #101

Feat/66 automated std import ied communication #101

AliouDIAITE commented Apr 28, 2022

SaintierFr May 6, 2022

SaintierFr May 6, 2022

SaintierFr May 6, 2022

SaintierFr May 6, 2022

SaintierFr May 6, 2022

sonarqubecloud bot commented May 10, 2022

psabatier74 May 10, 2022

psabatier74 May 10, 2022

psabatier74 May 10, 2022

psabatier74 May 10, 2022

psabatier74 May 10, 2022

psabatier74 May 10, 2022

psabatier74 May 10, 2022

psabatier74 May 10, 2022

psabatier74 May 10, 2022

	<SubNetwork type="8-MMS" name="RSPACE_PROCESS_NETWORK">
	<SubNetwork type="8-MMS" name="PROCESS_NETWORK">

	<SubNetwork type="IP" name="RSPACE_ADMIN_NETWORK">
	<SubNetwork type="IP" name="ADMIN_NETWORK">


		public static SclRootAdapter importSTDElementsInSCD(@NonNull SclRootAdapter scdRootAdapter, Set<SCL> stds,
		Map<Pair<String, String>, List<String>> comMap) throws ScdException {

Feat/66 automated std import ied communication #101

Feat/66 automated std import ied communication #101

Conversation

AliouDIAITE commented Apr 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented May 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment