Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup ADL14Converter code #648

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Cleanup ADL14Converter code #648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d600f02
Remove unused setExistingRepository method
VeraPrinsen Dec 10, 2024
3513d35
Add documentation to DescriptionConverter
VeraPrinsen Dec 10, 2024
3776862
remove commented code and add to documentation of DefaultMultipliciti…
VeraPrinsen Dec 11, 2024
628c078
Cleanup ConversionResult
VeraPrinsen Dec 11, 2024
78b22c5
cleanup DefaultRmStructureRemover
VeraPrinsen Dec 11, 2024
c83a563
Rename ArchetypeParsePostProcessor and formatting
VeraPrinsen Dec 11, 2024
8571a29
Cleanup ADL14NodeIDConverter
VeraPrinsen Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 6 additions & 17 deletions aom/src/main/java/com/nedap/archie/adl14/ADL14DefaultMultiplicitiesSetter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
import com.nedap.archie.rminfo.MetaModels;

/**
* Sets the default occurrences with ADL 1.4 rules, if not explicitly set in a given Archetype.
* Useful for conversion to ADL 2, where the default values are different, and it is good to start
* Sets the default occurrences with ADL 1.4 rules ({1..1}), if not explicitly set in a given Archetype.
* Useful for conversion to ADL 2, where the default values are different ({0..*}), and it is good to start
Copy link
Collaborator

@pieterbos pieterbos Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they are not {0..*} default - they are whatever is in the reference model for that property. Which can be one of at least 0..1, 1..1, 0..* or 1..*.

* with the correct values already present.
*
* <p>
* Cardinality and existence are also specified to have a default value. However, this is not used
* in practice (source, several openEHR community members). Adding that to the conversion would
* lead to problems. So these are left out.
Expand All @@ -31,20 +31,9 @@ public void setDefaults(Archetype archetype) {
}

private void correctItemsMultiplicities(CObject cObject) {
for(CAttribute attribute:cObject.getAttributes()) {
// according to the specification, the following lines must be added.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this been removed from the specification? Otherwise it is a valid comment.

// however, in practice this is not followed, and adding it would
// lead to more problems
/*
if(attribute.getCardinality() == null) {
attribute.setCardinality(new Cardinality(0, 1));
}
if(attribute.getExistence() == null) {
attribute.setExistence(new MultiplicityInterval(1, 1));
}*/
for(CObject child:attribute.getChildren()) {
if(child.getOccurrences() == null &&
metaModels.isMultiple(cObject.getRmTypeName(), attribute.getRmAttributeName())) {
for (CAttribute attribute : cObject.getAttributes()) {
for (CObject child : attribute.getChildren()) {
if (child.getOccurrences() == null && metaModels.isMultiple(cObject.getRmTypeName(), attribute.getRmAttributeName())) {
child.setOccurrences(new MultiplicityInterval(1, 1));
}
correctItemsMultiplicities(child);
Expand Down
7 changes: 5 additions & 2 deletions aom/src/main/java/com/nedap/archie/adl14/ADL14DescriptionConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
import java.util.LinkedHashMap;
import java.util.Map;

/**
* Converts the Description section from ADL1.4 to ADL2
* <a href="https://specifications.openehr.org/releases/AM/latest/ADL1.4.html#_description_section">Specifications ADL1.4</a>
* <a href="https://specifications.openehr.org/releases/AM/Release-2.3.0/ADL2.html#_description_section">Specifications ADL2</a>
*/
public class ADL14DescriptionConverter {

public void convert(Archetype archetype) {
Expand Down Expand Up @@ -44,12 +49,10 @@ public void convert(Archetype archetype) {
}
description.setIpAcknowledgements(acknowledgements);
}

String revision = description.getOtherDetails().remove("revision");
if(revision != null) {
archetype.getArchetypeId().setReleaseVersion(revision);
}

archetype.setGenerated(true);
}
}
271 changes: 140 additions & 131 deletions aom/src/main/java/com/nedap/archie/adl14/ADL14NodeIDConverter.java
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions aom/src/main/java/com/nedap/archie/adl14/ADL14Parser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import com.nedap.archie.antlr.errors.ANTLRParserErrors;
import com.nedap.archie.antlr.errors.ArchieErrorListener;
import com.nedap.archie.aom.Archetype;
import com.nedap.archie.aom.utils.ArchetypeParsePostProcesser;
import com.nedap.archie.aom.utils.ArchetypeParsePostProcessor;
import com.nedap.archie.rminfo.MetaModels;
import org.antlr.v4.runtime.CharStream;
import org.antlr.v4.runtime.CharStreams;
Expand Down Expand Up @@ -74,7 +74,7 @@ public Archetype parse(CharStream stream, ADL14ConversionConfiguration conversio
walker = new ParseTreeWalker();
walker.walk(listener, tree);
result = listener.getArchetype();
ArchetypeParsePostProcesser.fixArchetype(result);
ArchetypeParsePostProcessor.fixArchetype(result);
if (metaModels != null) {
metaModels.selectModel(result);
if (metaModels.getSelectedBmmModel() != null) {
Expand Down
39 changes: 10 additions & 29 deletions aom/src/main/java/com/nedap/archie/adl14/ADL2ConversionResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

/**
* ADL 2 conversion result. Always has the archetypeId field set.
* Either has archetype and conversionLog non-null in case of a succesful conversion, or
* Either has archetype and conversionLog non-null in case of a successful conversion, or
* exception non-null in case of an unexpected Exception
*/
public class ADL2ConversionResult {
Expand All @@ -18,24 +18,13 @@ public class ADL2ConversionResult {
private MessageLogger log;
private Exception exception;

/**
* empty constructor for JSON parsing. Do not use
*/
public ADL2ConversionResult() {

/* Empty construction for Jackson parsing */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have moved javadoc comments, which are actually converted to documentation on the build, to an internal comment. That does not seem like a good idea, as the javadoc will no longer list that this constructor should not be used.

}

public ADL2ConversionResult(Archetype archetype) {
this.archetypeId = archetype.getArchetypeId().getFullId();
this(archetype.getArchetypeId().getFullId(), null);
this.archetype = archetype;
log = new MessageLogger();
}

public ADL2ConversionResult(Archetype archetype, ADL2ConversionLog conversionLog) {
this.archetypeId = archetype.getArchetypeId().getFullId();
this.archetype = archetype;
this.conversionLog = conversionLog;
log = new MessageLogger();
}

public ADL2ConversionResult(String archetypeId, Exception exception) {
Expand All @@ -44,14 +33,11 @@ public ADL2ConversionResult(String archetypeId, Exception exception) {
log = new MessageLogger();
}

/** GETTERS **/
public String getArchetypeId() {
return archetypeId;
}

public void setArchetypeId(String archetypeId) {
this.archetypeId = archetypeId;
}

public Archetype getArchetype() {
return archetype;
}
Expand All @@ -60,14 +46,6 @@ public ADL2ConversionLog getConversionLog() {
return conversionLog;
}

public void setConversionLog(ADL2ConversionLog conversionLog) {
this.conversionLog = conversionLog;
}

public void setArchetype(Archetype archetype) {
this.archetype = archetype;
}

public MessageLogger getLog() {
return log;
}
Expand All @@ -76,9 +54,12 @@ public Exception getException() {
return exception;
}

public void setException(Exception exception) {
this.exception = exception;
/** SETTERS **/
public void setArchetype(Archetype archetype) {
this.archetype = archetype;
}


public void setConversionLog(ADL2ConversionLog conversionLog) {
this.conversionLog = conversionLog;
}
}
4 changes: 2 additions & 2 deletions aom/src/main/java/com/nedap/archie/adlparser/ADLParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import com.nedap.archie.antlr.errors.ArchieErrorListener;
import com.nedap.archie.antlr.errors.ANTLRParserErrors;
import com.nedap.archie.aom.Archetype;
import com.nedap.archie.aom.utils.ArchetypeParsePostProcesser;
import com.nedap.archie.aom.utils.ArchetypeParsePostProcessor;
import com.nedap.archie.rminfo.MetaModels;
import org.antlr.v4.runtime.CharStream;
import org.antlr.v4.runtime.*;
Expand Down Expand Up @@ -97,7 +97,7 @@ public Archetype parse(CharStream stream) throws ADLParseException {
walker.walk(listener, tree);
result = listener.getArchetype();
//set some values that are not directly in ODIN or ADL
ArchetypeParsePostProcesser.fixArchetype(result);
ArchetypeParsePostProcessor.fixArchetype(result);

if (modelConstraintImposer != null && result.getDefinition() != null) {
modelConstraintImposer.imposeConstraints(result.getDefinition());
Expand Down
4 changes: 2 additions & 2 deletions aom/src/main/java/com/nedap/archie/aom/Archetype.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import com.nedap.archie.aom.terminology.ArchetypeTerminology;
import com.nedap.archie.aom.terminology.ValueSet;
import com.nedap.archie.aom.utils.AOMUtils;
import com.nedap.archie.aom.utils.ArchetypeParsePostProcesser;
import com.nedap.archie.aom.utils.ArchetypeParsePostProcessor;
import com.nedap.archie.definitions.AdlCodeDefinitions;
import com.nedap.archie.query.AOMPathQuery;
import com.nedap.archie.rminfo.RMProperty;
Expand Down Expand Up @@ -236,7 +236,7 @@ public Archetype clone() {

Archetype result = (Archetype) super.clone();
//fix some things that are not handled automatically
ArchetypeParsePostProcesser.fixArchetype(result);
ArchetypeParsePostProcessor.fixArchetype(result);
return result;

}
Expand Down
30 changes: 15 additions & 15 deletions ...om/utils/ArchetypeParsePostProcesser.java → ...om/utils/ArchetypeParsePostProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
/**
* Sets some values that are not directly in ADL or ODIN, such as original language in terminology, etc.
*/
public class ArchetypeParsePostProcesser {
public class ArchetypeParsePostProcessor {

Check warning on line 19 in aom/src/main/java/com/nedap/archie/aom/utils/ArchetypeParsePostProcessor.java

View check run for this annotation

Codecov / codecov/patch

aom/src/main/java/com/nedap/archie/aom/utils/ArchetypeParsePostProcessor.java#L19 

Added line #L19 was not covered by tests

public static void fixArchetype(Archetype archetype) {
if(archetype.getTerminology() != null) {
if (archetype.getTerminology() != null) {
ArchetypeTerminology terminology = archetype.getTerminology();
//codes are in model, but do not appear in odin. Set them here
fillArchetypeTermCodes(terminology.getTermDefinitions());
Expand All @@ -37,33 +37,33 @@
private static void setParents(Archetype archetype) {
Stack<CObject> workList = new Stack<>();
workList.add(archetype.getDefinition());
while(!workList.empty()) {
while (!workList.empty()) {
CObject cObject = workList.pop();
if(cObject instanceof CPrimitiveObject) {
if (cObject instanceof CPrimitiveObject) {
cObject.setNodeId("id9999");//also in the implementation, but check to be sure
}
for(CAttribute attribute:cObject.getAttributes()) {
for (CAttribute attribute : cObject.getAttributes()) {

attribute.setParent(cObject);
for(CObject child:attribute.getChildren()) {
for (CObject child : attribute.getChildren()) {
child.setParent(attribute);
}
workList.addAll(attribute.getChildren());
}
if(cObject instanceof CComplexObject) {
if (cObject instanceof CComplexObject) {
//TODO: fix tuple so it has proper multiple references to the separate structures, instead of a complete duplication of data in json
CComplexObject cComplexObject = (CComplexObject) cObject;
for(CAttributeTuple tuple: cComplexObject.getAttributeTuples()) {
for(CAttribute attribute:tuple.getMembers()) {
for (CAttributeTuple tuple : cComplexObject.getAttributeTuples()) {
for (CAttribute attribute : tuple.getMembers()) {
attribute.setSocParent(tuple);
attribute.setParent(cObject);
workList.addAll(attribute.getChildren());
cComplexObject.replaceAttribute(attribute);
}
for(CPrimitiveTuple primitiveTuple:tuple.getTuples()) {
for (CPrimitiveTuple primitiveTuple : tuple.getTuples()) {
int index = 0;
for(CPrimitiveObject<?, ?> object:primitiveTuple.getMembers()) {
if(index < tuple.getMembers().size()) {
for (CPrimitiveObject<?, ?> object : primitiveTuple.getMembers()) {
if (index < tuple.getMembers().size()) {
CAttribute attribute = tuple.getMember(index);
object.setSocParent(primitiveTuple);
object.setParent(attribute);
Expand All @@ -79,9 +79,9 @@
}

private static void fillArchetypeTermCodes(Map<String, Map<String, ArchetypeTerm>> termSet) {
if(termSet != null) {
for(Map<String, ArchetypeTerm> language:termSet.values()) {
for(String term:language.keySet()) {
if (termSet != null) {
for (Map<String, ArchetypeTerm> language : termSet.values()) {
for (String term : language.keySet()) {
language.get(term).setCode(term);
}
}
Expand Down
Loading