diff --git a/base/src/main/java/com/nedap/archie/base/Interval.java b/base/src/main/java/com/nedap/archie/base/Interval.java index 7222aa291..50be8f8f1 100644 --- a/base/src/main/java/com/nedap/archie/base/Interval.java +++ b/base/src/main/java/com/nedap/archie/base/Interval.java @@ -1,11 +1,14 @@ package com.nedap.archie.base; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.nedap.archie.rminfo.RMPropertyIgnore; + import javax.annotation.Nullable; import javax.xml.bind.annotation.XmlAccessType; import javax.xml.bind.annotation.XmlAccessorType; import javax.xml.bind.annotation.XmlAttribute; +import javax.xml.bind.annotation.XmlTransient; import javax.xml.bind.annotation.XmlType; -import java.time.Duration; import java.time.temporal.TemporalAmount; import java.util.Objects; @@ -146,11 +149,11 @@ public boolean has(T value) { Comparable comparableValue; Comparable comparableLower; Comparable comparableUpper; - if (value instanceof TemporalAmount && !(value instanceof Comparable) && isNonComparableTemporalAmount(lower) && isNonComparableTemporalAmount(upper)) { + if (value instanceof TemporalAmount && lower instanceof TemporalAmount && upper instanceof TemporalAmount) { //TemporalAmount is not comparable, but can always be converted to a duration that is comparable. - comparableValue = value == null ? null : Duration.from((TemporalAmount) value); - comparableLower = lower == null ? null : Duration.from((TemporalAmount) lower); - comparableUpper = upper == null ? null : Duration.from((TemporalAmount) upper); + comparableValue = toComparable(value); + comparableLower = toComparable(lower); + comparableUpper = toComparable(upper); } else if (!(isComparable(lower) && isComparable(upper) && isComparable(value))) { throw new UnsupportedOperationException("subclasses of interval not implementing comparable should implement their own has method"); } else { @@ -180,13 +183,50 @@ public boolean has(T value) { return true; } + /** + * Get the lower value as a comparable amount. Required because some temporal amounts are not always directly comparable + * @return + */ + @JsonIgnore + @XmlTransient + @RMPropertyIgnore + public Comparable getComparableLower() { + return toComparable(lower); + } + + /** + * Get the lower value as a comparable amount. Required because some temporal amounts are not always directly comparable + * @return + */ + @JsonIgnore + @XmlTransient + @RMPropertyIgnore + public Comparable getComparableUpper() { + return toComparable(upper); + } + + private Comparable toComparable(T value) { + if(value == null) { + return null; + } + if (value instanceof TemporalAmount && !(value instanceof Comparable) && isNonComparableTemporalAmount(value)) { + //TemporalAmount is not comparable, but can always be converted to a duration that is comparable. + return IntervalDurationConverter.from((TemporalAmount) value); + + } else if (!isComparable(value)) { + throw new UnsupportedOperationException("subclasses of interval not implementing comparable should implement their own has method"); + } else { + return (Comparable) value; + } + } + private int compareTo(T intervalValue, T value) { Comparable comparableIntervalValue; Comparable comparableValue; if (value instanceof TemporalAmount && !(value instanceof Comparable) && isNonComparableTemporalAmount(intervalValue)) { //TemporalAmount is not comparable, but can always be converted to a duration that is comparable. - comparableValue = value == null ? null : Duration.from((TemporalAmount) value); - comparableIntervalValue = intervalValue == null ? null : Duration.from((TemporalAmount) intervalValue); + comparableValue = value == null ? null : IntervalDurationConverter.from((TemporalAmount) value); + comparableIntervalValue = intervalValue == null ? null : IntervalDurationConverter.from((TemporalAmount) intervalValue); } else if (!(isComparable(intervalValue) && isComparable(value))) { throw new UnsupportedOperationException("subclasses of interval not implementing comparable should implement their own has method"); } else { @@ -200,8 +240,8 @@ private boolean isNonComparableTemporalAmount(T value) { return value == null || (!(value instanceof Comparable) && value instanceof TemporalAmount); } - private boolean isComparable(T upper) { - return upper == null || upper instanceof Comparable; + private boolean isComparable(T value) { + return value == null || value instanceof Comparable; } diff --git a/base/src/main/java/com/nedap/archie/base/IntervalDurationConverter.java b/base/src/main/java/com/nedap/archie/base/IntervalDurationConverter.java new file mode 100644 index 000000000..a20d87e84 --- /dev/null +++ b/base/src/main/java/com/nedap/archie/base/IntervalDurationConverter.java @@ -0,0 +1,65 @@ +package com.nedap.archie.base; + + +import java.time.Duration; +import java.time.temporal.ChronoUnit; +import java.time.temporal.TemporalAmount; +import java.time.temporal.TemporalUnit; +import java.time.temporal.UnsupportedTemporalTypeException; +import java.util.Objects; + +import static java.time.temporal.ChronoUnit.DAYS; + +/** + * A tool to convert Durations from TemporalAmounts in the specific use case of Intervals + * It handles the unit Month as an exact number of seconds to do a conversion. This is a bit tricky, since + * a number of months has a lower number of seconds and an upper number of seconds. but we need some way of doing this... + * + * This may need a separate method for lower and upper, which returns the lower and upper bounds of the seconds in a month? + */ +class IntervalDurationConverter { + +// private final static long MINIMUM_SECONDS_IN_DAY = 60*60*23; +// private final static long MINIMUM_SECONDS_IN_MONTH = 28*MINIMUM_SECONDS_IN_DAY; +// private final static long MINIMUM_SECONDS_IN_WEEK = 7*MINIMUM_SECONDS_IN_DAY; +// +// private final static long MAXIMUM_SECONDS_IN_DAY = 60*60*25; +// private final static long MAXIMUM_SECONDS_IN_WEEK = 7*MAXIMUM_SECONDS_IN_DAY; +// private final static long MAXIMUM_SECONDS_IN_MONTH = 28*MAXIMUM_SECONDS_IN_WEEK; + + /** + * This is a literal copy of Duration.from, with the only change being that this supports adding + * estimate TemporalAmounts + * @param amount + * @return + */ + public static Duration from(TemporalAmount amount) { + Objects.requireNonNull(amount, "amount"); + Duration duration = Duration.ofSeconds(0); + for (TemporalUnit unit : amount.getUnits()) { + duration = plus(duration, amount.get(unit), unit); + } + return duration; + } + + + /** + * Returns a copy of this duration with the specified duration added. Works the same as {@link Duration#plus(Duration)}, + * but does work for estimate units such as weeks, months and years + + * @param amountToAdd the amount to add, measured in terms of the unit, positive or negative + * @param unit the unit that the amount is measured in, must have an exact duration, not null + * @return a {@code Duration} based on this duration with the specified duration added, not null + * @throws UnsupportedTemporalTypeException if the unit is not supported + * @throws ArithmeticException if numeric overflow occurs + */ + private static Duration plus(Duration value, long amountToAdd, TemporalUnit unit) { + Objects.requireNonNull(unit, "unit"); + if (unit != DAYS && unit.isDurationEstimated()) { + Duration duration = unit.getDuration().multipliedBy(amountToAdd); + return value.plusSeconds(duration.getSeconds()).plusNanos(duration.getNano()); + } else { + return value.plus(amountToAdd, unit); + } + } +} diff --git a/aom/src/main/java/com/nedap/archie/rminfo/RMAttributeInfo.java b/base/src/main/java/com/nedap/archie/rminfo/RMAttributeInfo.java similarity index 100% rename from aom/src/main/java/com/nedap/archie/rminfo/RMAttributeInfo.java rename to base/src/main/java/com/nedap/archie/rminfo/RMAttributeInfo.java diff --git a/aom/src/main/java/com/nedap/archie/rminfo/RMPackageId.java b/base/src/main/java/com/nedap/archie/rminfo/RMPackageId.java similarity index 100% rename from aom/src/main/java/com/nedap/archie/rminfo/RMPackageId.java rename to base/src/main/java/com/nedap/archie/rminfo/RMPackageId.java diff --git a/aom/src/main/java/com/nedap/archie/rminfo/RMProperty.java b/base/src/main/java/com/nedap/archie/rminfo/RMProperty.java similarity index 100% rename from aom/src/main/java/com/nedap/archie/rminfo/RMProperty.java rename to base/src/main/java/com/nedap/archie/rminfo/RMProperty.java diff --git a/aom/src/main/java/com/nedap/archie/rminfo/RMPropertyIgnore.java b/base/src/main/java/com/nedap/archie/rminfo/RMPropertyIgnore.java similarity index 100% rename from aom/src/main/java/com/nedap/archie/rminfo/RMPropertyIgnore.java rename to base/src/main/java/com/nedap/archie/rminfo/RMPropertyIgnore.java diff --git a/aom/src/main/java/com/nedap/archie/rminfo/RMTypeInfo.java b/base/src/main/java/com/nedap/archie/rminfo/RMTypeInfo.java similarity index 100% rename from aom/src/main/java/com/nedap/archie/rminfo/RMTypeInfo.java rename to base/src/main/java/com/nedap/archie/rminfo/RMTypeInfo.java diff --git a/base/src/test/java/com/nedap/archie/base/IntervalOfPeriodTest.java b/base/src/test/java/com/nedap/archie/base/IntervalOfPeriodTest.java new file mode 100644 index 000000000..3c0d94d29 --- /dev/null +++ b/base/src/test/java/com/nedap/archie/base/IntervalOfPeriodTest.java @@ -0,0 +1,51 @@ +package com.nedap.archie.base; + +import org.junit.Test; + +import java.time.Duration; +import java.time.Period; +import java.time.temporal.ChronoUnit; +import java.time.temporal.TemporalAmount; +import java.time.temporal.TemporalUnit; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + + +public class IntervalOfPeriodTest { + + @Test + public void testMonths() { + Period oneMonth = Period.of(0,1, 0); + Period twoMonths = Period.of(0,2, 0); + Interval interval = new Interval(oneMonth, twoMonths); + Duration oneAndAHalfMonth = Duration.of(45, ChronoUnit.DAYS); + assertTrue(interval.has(oneAndAHalfMonth)); + Duration threeMonths = Duration.of(31*3, ChronoUnit.DAYS); + assertFalse(interval.has(threeMonths)); + + + assertTrue(interval.has(Period.of(0, 1, 0))); + assertTrue(interval.has(Period.of(0, 2, 0))); + + //testing edge cases of Duration in hours would be even better, but once makes sense once we have proper + //converter support, with difference between converting lower and upper values + + } + + @Test + public void testDurations() { + Duration oneHour = Duration.of(1, ChronoUnit.HOURS); + Duration twoHour = Duration.of(2, ChronoUnit.HOURS); + Interval interval = new Interval(oneHour, twoHour); + assertTrue(interval.has(Duration.of(60, ChronoUnit.MINUTES))); + assertTrue(interval.has(Duration.of(120, ChronoUnit.MINUTES))); + + assertFalse(interval.has(Duration.of(59, ChronoUnit.MINUTES))); + assertFalse(interval.has(Duration.of(121, ChronoUnit.MINUTES))); + + Duration oneAndAHalfMonth = Duration.of(45, ChronoUnit.DAYS); + assertFalse(interval.has(oneAndAHalfMonth)); + + } +} diff --git a/tools/src/main/java/com/nedap/archie/archetypevalidator/ArchetypeValidator.java b/tools/src/main/java/com/nedap/archie/archetypevalidator/ArchetypeValidator.java index e480361e9..e86cea5d2 100644 --- a/tools/src/main/java/com/nedap/archie/archetypevalidator/ArchetypeValidator.java +++ b/tools/src/main/java/com/nedap/archie/archetypevalidator/ArchetypeValidator.java @@ -52,6 +52,7 @@ public ArchetypeValidator(MetaModels models) { //but there's no reason this cannot be parsed, so check them here validationsPhase0.add(new AttributeUniquenessValidation()); validationsPhase0.add(new NodeIdValidation()); + validationsPhase0.add(new BasicDefinitionObjectValidation()); validationsPhase0.add(new AttributeTupleValidation()); diff --git a/tools/src/main/java/com/nedap/archie/archetypevalidator/validations/BasicDefinitionObjectValidation.java b/tools/src/main/java/com/nedap/archie/archetypevalidator/validations/BasicDefinitionObjectValidation.java new file mode 100644 index 000000000..9d41260c4 --- /dev/null +++ b/tools/src/main/java/com/nedap/archie/archetypevalidator/validations/BasicDefinitionObjectValidation.java @@ -0,0 +1,64 @@ +package com.nedap.archie.archetypevalidator.validations; + +import com.nedap.archie.aom.CAttribute; +import com.nedap.archie.aom.CObject; +import com.nedap.archie.aom.primitives.COrdered; +import com.nedap.archie.archetypevalidator.ErrorType; +import com.nedap.archie.archetypevalidator.ValidatingVisitor; +import com.nedap.archie.base.Interval; +import org.openehr.utils.message.I18n; + +/** + * Contains validation for basic object validity. Many of these checks are also included in the ADL grammar, but\ + * of course not every AOM object comes from ADL :) + */ +public class BasicDefinitionObjectValidation extends ValidatingVisitor { + + protected void validate(CObject cObject) { + if(cObject.getOccurrences() != null) { + validateOccurrences(cObject); + } + if(cObject instanceof COrdered) { + validateCOrdered((COrdered) cObject); + } + } + + private void validateCOrdered(COrdered cOrdered) { + for(Interval interval:cOrdered.getConstraint()) { + if(!isValidInterval(interval)) { + this.addMessageWithPath(ErrorType.OTHER, cOrdered.path(), I18n.t("The constraint interval for this {0} has lower > upper, this is not allowed", cOrdered.getClass().getSimpleName())); + } + } + } + + protected void validate(CAttribute cAttribute) { + + if(cAttribute.getCardinality() != null) { + if (cAttribute.getCardinality().getInterval() == null) { + this.addMessageWithPath(ErrorType.OTHER, cAttribute.path(), I18n.t("Cardinality must have an interval present, but it was null")); + } else if(!isValidInterval(cAttribute.getCardinality().getInterval())) { + this.addMessageWithPath(ErrorType.OTHER, cAttribute.path(), I18n.t("The attribute cardinality interval has lower > upper, this is not allowed")); + } + } + + } + + + + private void validateOccurrences(CObject cObject) { + if(!isValidInterval(cObject.getOccurrences())) { + this.addMessageWithPath(ErrorType.OTHER, cObject.path(), I18n.t("The occurrences interval has lower > upper, this is not allowed")); + } + } + + private boolean isValidInterval(Interval interval) { + if(interval.getUpper() != null && interval.getLower() != null && + !interval.isLowerUnbounded() && + !interval.isUpperUnbounded()){ + if(interval.getComparableUpper().compareTo(interval.getComparableLower()) < 0){ + return false; + } + } + return true; + } +} diff --git a/tools/src/test/java/com/nedap/archie/archetypevalidator/BigArchetypeValidatorTest.java b/tools/src/test/java/com/nedap/archie/archetypevalidator/BigArchetypeValidatorTest.java index 78bfd2c30..5ef8f9cfa 100644 --- a/tools/src/test/java/com/nedap/archie/archetypevalidator/BigArchetypeValidatorTest.java +++ b/tools/src/test/java/com/nedap/archie/archetypevalidator/BigArchetypeValidatorTest.java @@ -103,7 +103,7 @@ private FullArchetypeRepository parseAll() { Reflections reflections = new Reflections("adl2-tests", new ResourcesScanner()); List adlFiles = new ArrayList(reflections.getResources(Pattern.compile(".*\\.adls"))); for(String file:adlFiles) { - if (file.contains("legacy_adl_1.4")) { + if (file.contains("legacy_adl1.4")) { continue; } Archetype archetype = null; diff --git a/tools/src/test/resources/adl2-tests/validity/basics/openEHR-TEST_PKG-WHOLE.OTHER_bad_cardinality.v1.0.0.adls b/tools/src/test/resources/adl2-tests/validity/basics/openEHR-TEST_PKG-WHOLE.OTHER_bad_cardinality.v1.0.0.adls new file mode 100644 index 000000000..542606aa7 --- /dev/null +++ b/tools/src/test/resources/adl2-tests/validity/basics/openEHR-TEST_PKG-WHOLE.OTHER_bad_cardinality.v1.0.0.adls @@ -0,0 +1,36 @@ +archetype (adl_version=2.0.5; rm_release=1.0.2) + openehr-TEST_PKG-WHOLE.bad_cardinality.v1.0.0 + +language + original_language = <[ISO_639-1::en]> + +description + original_author = < + ["name"] = <"Pieter Bos"> + > + details = < + ["en"] = < + language = <[ISO_639-1::en]> + purpose = <"test archetype with bad existence interval (upper > 1)"> + keywords = <"ADL", "test"> + > + > + lifecycle_state = <"published"> + other_details = < + ["regression"] = <"OTHER"> + > + +definition + WHOLE[id1] matches { -- test entry + integer_attr3 cardinality matches {5..3} + } + +terminology + term_definitions = < + ["en"] = < + ["id1"] = < + text = <"test entry"> + description = <"test entry"> + > + > + > diff --git a/tools/src/test/resources/adl2-tests/validity/basics/openEHR-TEST_PKG-WHOLE.OTHER_bad_existence.v1.0.0.adls b/tools/src/test/resources/adl2-tests/validity/basics/openEHR-TEST_PKG-WHOLE.OTHER_bad_existence.v1.0.0.adls new file mode 100644 index 000000000..18192b2a9 --- /dev/null +++ b/tools/src/test/resources/adl2-tests/validity/basics/openEHR-TEST_PKG-WHOLE.OTHER_bad_existence.v1.0.0.adls @@ -0,0 +1,36 @@ +archetype (adl_version=2.0.5; rm_release=1.0.2) + openehr-TEST_PKG-WHOLE.bad_existence.v1.0.0 + +language + original_language = <[ISO_639-1::en]> + +description + original_author = < + ["name"] = <"Pieter Bos"> + > + details = < + ["en"] = < + language = <[ISO_639-1::en]> + purpose = <"test archetype with bad existence interval (upper > 1)"> + keywords = <"ADL", "test"> + > + > + lifecycle_state = <"published"> + other_details = < + ["regression"] = <"SEXLU"> + > + +definition + WHOLE[id1] matches { -- test entry + integer_attr3 existence matches {0..5} + } + +terminology + term_definitions = < + ["en"] = < + ["id1"] = < + text = <"test entry"> + description = <"test entry"> + > + > + > diff --git a/tools/src/test/resources/adl2-tests/validity/basics/openEHR-TEST_PKG-WHOLE.OTHER_bad_interval.v1.0.0.adls b/tools/src/test/resources/adl2-tests/validity/basics/openEHR-TEST_PKG-WHOLE.OTHER_bad_interval.v1.0.0.adls new file mode 100644 index 000000000..52698f739 --- /dev/null +++ b/tools/src/test/resources/adl2-tests/validity/basics/openEHR-TEST_PKG-WHOLE.OTHER_bad_interval.v1.0.0.adls @@ -0,0 +1,36 @@ +archetype (adl_version=2.0.5; rm_release=1.0.2) + openehr-TEST_PKG-WHOLE.bad_interval.v1.0.0 + +language + original_language = <[ISO_639-1::en]> + +description + original_author = < + ["name"] = <"Pieter Bos"> + > + details = < + ["en"] = < + language = <[ISO_639-1::en]> + purpose = <"test archetype with bad existence interval (upper > 1)"> + keywords = <"ADL", "test"> + > + > + lifecycle_state = <"published"> + other_details = < + ["regression"] = <"OTHER"> + > + +definition + WHOLE[id1] matches { -- test entry + integer_attr3 matches {|101..100|} + } + +terminology + term_definitions = < + ["en"] = < + ["id1"] = < + text = <"test entry"> + description = <"test entry"> + > + > + >