openEHR / archie

OpenEHR library implementing ADL 2, AOM 2, BMM, RM 1.0.4 and many tools
Apache License 2.0
53 stars 25 forks source link

NPE in Validation of DvDuration #379

Closed stefanspiska closed 2 years ago

stefanspiska commented 2 years ago

Try to run the following test

@Test
  public void validateDvDuration(){

    DvDuration dvDuration = new DvDuration(Period.of(10,5,5));
    dvDuration.setNormalStatus(new CodePhrase(new TerminologyId("openehr_normal_statuses"),"N"));

    dvDuration.setNormalRange(new DvInterval<>());
    dvDuration.getNormalRange().setLower(new DvDuration(Period.of(10,5,5)));
    dvDuration.getNormalRange().setUpper(new DvDuration(Period.of(10,6,5)));

    RMObjectValidator rmObjectValidator =
            new RMObjectValidator(ArchieRMInfoLookup.getInstance(), s -> null);

    assertThat(rmObjectValidator.validate(dvDuration)).isEmpty();
  }

This fails with

Exception NullPointerException invoking invariant Normal_range_and_status_consistency on DV_DURATION: Cannot invoke "java.lang.Comparable.compareTo(Object)" because the return value of "com.nedap.archie.rm.datavalues.quantity.DvQuantified.getMagnitude()" is null

I assume this is do to DvDuration not correctly Implementing getMagnitude.

pieterbos commented 2 years ago

Good find!

The cause is the following, from DvDuration.java:

    @XmlTransient
    @Override
    @JsonIgnore
    public Long getMagnitude() {
        return null; //no magnitude defined in spec
    }

The problem is, how to calculate the magnitude is not defined in the specification. The javalibs implementation calculated a double value of the number of seconds:

https://github.com/openEHR/java-libs/blob/master/openehr-rm-core/src/main/java/org/openehr/rm/datatypes/quantity/datetime/DvDuration.java#L248-L252

Then of course converting years or months to seconds will need a specification, as the number of seconds in a month or year is not the same for every month or year.

I guess I'll raise a question about how this should be in the specifcation.

pieterbos commented 2 years ago

See https://github.com/openEHR/archie/pull/383 for the preliminary fix. the specification of converting years and months is defined in https://specifications.openehr.org/releases/BASE/latest/foundation_types.html#_iso8601_duration_class

this leaves whether magnitude of DV_DURATION should be seconds or something else. Once we have that answer, the pull requests can be fixed and merged.

pieterbos commented 2 years ago

Ah, it was defined after all. Ok, the pull request should be correct as is.