Sct initial push

[syllamoh]

RSR-31 / Create a new SCD file with Header
EPIC RSR-51 / RSR-6 : S79 : Add DataTypeTemplates from ICD to current SCD
EPIC RSR-51 / RSR-60 : Add a new IED from ICD to current SCD
EPIC RSR-51 / RSR-64 : Associate a list of Connected Access Point (ConnectedAP) to a SubNetwork
RSR-86 COMPAS List for a given signal ExtRef, all possible IED, LDevice, LN and DO that can be defined as source
RSR-81 COMPAS List all External References signals ExtRef belonging to an IED and LDevice.inst of an SCD file
EPIC RSR-51 / RSR-62 : create a new SubNetwork to current SCD

[Flurb]

How do I correctly review this?
It's a pretty big PR!

[syllamoh]

Hi @dlabordus @Flurb
Who is merging commit on branch under pull request ?
Why you're doing this? Seriously!

[syllamoh]

@dlabordus @Flurb
Plus when doing stuff like this (which is totally not recommended), make sure you do test.

[Flurb]

Hi @syllamoh we saw that the SonarCloud action wasn't added yet. So we added it :)
Otherwise we are merging code into the develop branch that hasn't been scanned. Not something we want!

[Flurb]

I'm also going to add a Build Github Action (like the one in compas-cim-mapping repository https://github.com/com-pas/compas-cim-mapping/actions/workflows/compas-ci.yml) to check if the projects build correctly.
Right now, it doesn't. We had to check it manually, this is not something we want.

[syllamoh]

Hi @Flurb @Sander3003
That's not how collaborative development works.
First, if you want to add code or document on existing branch, you create issue and do pull request.
Second, you don't add change to branch under pull request whom you're not the requester (you create issue and do pull request).
Third the least you can do in general, is to test your change before merging. If you did this you'd have noticed that your script is buggy.

Worst this pull request target is the branch develop, and you merge a code from develop to this branch! I'm sorry, but this is not OK at all.

It's better you remove what you added, and follow the process correctly.

[Sander3003]

@syllamoh I think we need to discuss how we can collaborate more closely. It would be nice if we can do (technical) refinements together to prevent any surprises in pull-requests and have the discussions up-front. Can you make it today during the community call to discuss this?

[syllamoh]

@Sander3003
No problem for me.
It was just very surprising the way things were done.

[dlabordus]

• sct-data: Some of the DTO Classes in this project aren't used, only by test-classes.
• sct-data: H2XMLTypeMapper is only used in the test part, why not put it in "src/test/java"
• sct-data: There are Test XML Files in "src/test/resource", but I don't think there are used in this project.
• sct-data: I think the idea is that ScdJpaRepository also implements IScdCrudRepository, and that this interface is used in de Service Layer. Now ScdJpaRepository is used in the Service Layer.
• sct-service: SimpleSqlScdService is working with the class SimpleSCD, but I see SimpleSCD as a data object how stuff is stored. Should we build a Service that will be handling the SCL and how this is stored is up to a Storage Service. In that Storage Service class we can use SimpleSCD.
• sct-service: Also, SimpleSqlScdService has a lot of extra methods add*, extract*. These are only working on the SCL class, so by passing the SimpleSCD a lot of extra Unmarshalling and Marshalling is done. It looks like these methods are used by some GUI Interface, but that's unclear to me. And it's now becoming quite a big service with a lot of responsibilities. We prefer smaller services.
• sct-service: In the directory "src/main/resources" there are xml files, that probably should be there. I think there are by mistake copied there because they are also in "src/test/resources".
• sct-service: currently this project only contains a Singletone Spring Bean, and it can't be access as a microservice through a REST Interface.

[syllamoh]

@dlabordus

• sct-data: Some of the DTO Classes in this project aren't used, only by test-classes.

This not the final version for compas-sct. Developement is ongoing. This pull request only addressed issue mentioned before. nothing else. DTO classes as you see are model(sct-data), will be used in sct-data and in the future sct-app (coming soon).

• sct-data: H2XMLTypeMapper is only used in the test part, why not put it in "src/test/java"

Indeed it could be there as today that's the only use.

• sct-data: I think the idea is that ScdJpaRepository also implements IScdCrudRepository, and that this interface is used in de Service Layer. Now ScdJpaRepository is used in the Service Layer.

Almost, but not really. I'm writing the conceptual architecture. ScdJpaRepository is a spring JPA interface. There ScdRepository is class that implements IScdCrudRepository, that class delegate DAO operation to spring. Any correct IDE can help you navigate.

• sct-service: SimpleSqlScdService is working with the class SimpleSCD, but I see SimpleSCD as a data object how stuff is stored. Should we build a Service that will be handling the SCL and how this is stored is up to a Storage Service. In that Storage Service class we can use SimpleSCD.

SimpleSCD has no storage capability, juste simple POJO (the SCL and meta data)
SimpleSqlScdService implements IScdService. It contains the DAO class that deal with storage. I suppose you're familiar with generic programing.

• sct-service: Also, SimpleSqlScdService has a lot of extra methods add*, extract*. These are only working on the SCL class, so by passing the SimpleSCD a lot of extra Unmarshalling and Marshalling is done. It looks like these methods are used by some GUI Interface, but that's unclear to me. And it's now becoming quite a big service with a lot of responsibilities. We prefer smaller services.

SimpleSqlScdService has oe single mission : manipulate SCD file. It has no direct interaction with any GUI.
As the XML manipulation is managed by JAXB facilities, one should indeed marshal raw xml data fromDB, process it, the unmarshal to put it back to DB. They are done once par service call.

• sct-service: In the directory "src/main/resources" there are xml files, that probably should be there. I think there are by mistake copied there because they are also in "src/test/resources".

Indeed, those xml file are useless there

• sct-service: currently this project only contains a Singletone Spring Bean, and it can't be access as a microservice through a REST Interface.

Again this not the final version for compas-sct. Developement is ongoing. I mentioned in the begin the issues that were developed in this pull. Any applicative part that deal with controller (caller to service) are defined in issue not addressed by this pull request
compas-sct deal with only one table (in fact only one DB), only one service.

brief, I suppose you're familiar with n-tier architecture. This is simple n-tiers architecture with single responsibility service (with lot of delegation), and non share DB pattern.

Again the development is incremental issue by issue. You'll not have the full and final compas-sct for months to come. Please read the issue that are embedded in the pull request

[dlabordus]

@dlabordus

• sct-data: Some of the DTO Classes in this project aren't used, only by test-classes.

This not the final version for compas-sct. Developement is ongoing. This pull request only addressed issue mentioned before. nothing else. DTO classes as you see are model(sct-data), will be used in sct-data and in the future sct-app (coming soon).

I would prefer to first have the basic in place and then add these classes when needed, then it's also clear how these should look.

• sct-data: I think the idea is that ScdJpaRepository also implements IScdCrudRepository, and that this interface is used in de Service Layer. Now ScdJpaRepository is used in the Service Layer.

Almost, but not really. I'm writing the conceptual architecture. ScdJpaRepository is a spring JPA interface. There ScdRepository is class that implements IScdCrudRepository, that class delegate DAO operation to spring. Any correct IDE can help you navigate.

Oke, I will wait for that and see how we can integrate the BaseX version, because that's not JPA based.

• sct-service: SimpleSqlScdService is working with the class SimpleSCD, but I see SimpleSCD as a data object how stuff is stored. Should we build a Service that will be handling the SCL and how this is stored is up to a Storage Service. In that Storage Service class we can use SimpleSCD.

SimpleSCD has no storage capability, juste simple POJO (the SCL and meta data)
SimpleSqlScdService implements IScdService. It contains the DAO class that deal with storage. I suppose you're familiar with generic programing.

SimpleSCD is a Pojo, but contains all kind of JPA Annotations, but in BaseX these aren't usefull.

• sct-service: Also, SimpleSqlScdService has a lot of extra methods add*, extract*. These are only working on the SCL class, so by passing the SimpleSCD a lot of extra Unmarshalling and Marshalling is done. It looks like these methods are used by some GUI Interface, but that's unclear to me. And it's now becoming quite a big service with a lot of responsibilities. We prefer smaller services.

SimpleSqlScdService has oe single mission : manipulate SCD file. It has no direct interaction with any GUI.
As the XML manipulation is managed by JAXB facilities, one should indeed marshal raw xml data fromDB, process it, the unmarshal to put it back to DB. They are done once par service call.

Maybe it's nice to have a separate Service to manipulate the SCL directly. And only marchalling is then needed when saving to a Database.

[Flurb]

core-commons dependency needs to be added! :)

[Flurb]

It's still not building correctly, SonarCloud can't run

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

100.0% Coverage
0.0% Duplication

[Flurb]

Should be 2021

[Flurb]

Dependency not being used anymore?

[Flurb]

Also, the files in sct-commons/src/test/resources/ are all empty, why are they added?

Rob already reviewed

[Flurb]

You can rebase/merge it @syllamoh