From a2ffc6af0538ea3949512f2d64151531044a3632 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 9 Sep 2016 18:18:28 -0400 Subject: [PATCH] Fix #444 - Correct handling of parsing milliseconds in dates before 1970 --- .../fhir/model/primitive/BaseDateTimeDt.java | 7 +- .../primitive/BaseDateTimeDtDstu2Test.java | 48 ++++++-- .../fhir/dstu3/model/BaseDateTimeType.java | 7 +- .../uhn/fhir/parser/XmlParserDstu3Test.java | 112 +++++++++--------- .../uhn/fhir/rest/server/CreateDstu3Test.java | 56 +++++++++ .../model/BaseDateTimeTypeDstu3Test.java | 40 ++++++- src/changes/changes.xml | 4 + 7 files changed, 201 insertions(+), 73 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/primitive/BaseDateTimeDt.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/primitive/BaseDateTimeDt.java index d963701e422..e800baa1668 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/primitive/BaseDateTimeDt.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/primitive/BaseDateTimeDt.java @@ -449,7 +449,12 @@ public abstract class BaseDateTimeDt extends BasePrimitive { myPrecision = thePrecision; myFractionalSeconds = ""; if (theValue != null) { - String fractionalSeconds = Integer.toString((int) (theValue.getTime() % 1000)); + long millis = theValue.getTime() % 1000; + if (millis < 0) { + // This is for times before 1970 (see bug #444) + millis = 1000 + millis; + } + String fractionalSeconds = Integer.toString((int) millis); myFractionalSeconds = StringUtils.leftPad(fractionalSeconds, 3, '0'); } super.setValue(theValue); diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/model/primitive/BaseDateTimeDtDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/model/primitive/BaseDateTimeDtDstu2Test.java index ef851daf366..3a7f51dd375 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/model/primitive/BaseDateTimeDtDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/model/primitive/BaseDateTimeDtDstu2Test.java @@ -3,22 +3,21 @@ package ca.uhn.fhir.model.primitive; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.either; import static org.hamcrest.Matchers.endsWith; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.text.SimpleDateFormat; -import java.util.Calendar; -import java.util.Date; -import java.util.GregorianCalendar; -import java.util.Locale; -import java.util.TimeZone; +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.util.*; import org.apache.commons.lang3.time.FastDateFormat; import org.hamcrest.Matchers; -import org.junit.AfterClass; -import org.junit.Assert; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Test; +import org.junit.*; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.model.api.TemporalPrecisionEnum; @@ -35,6 +34,33 @@ public class BaseDateTimeDtDstu2Test { private SimpleDateFormat myDateInstantParser; private FastDateFormat myDateInstantZoneParser; + /** + * See #444 + */ + @Test + public void testParseAndEncodeDateBefore1970() { + LocalDateTime ldt = LocalDateTime.of(1960, 9, 7, 0, 44, 25, 12387401); + Date from = Date.from(ldt.toInstant(ZoneOffset.UTC)); + InstantDt type = (InstantDt) new InstantDt(from).setTimeZoneZulu(true); + String encoded = type.getValueAsString(); + + ourLog.info("LDT: "+ ldt.toString()); + ourLog.info("Expected: "+"1960-09-07T00:44:25.012"); + ourLog.info("Actual: "+encoded); + + assertEquals("1960-09-07T00:44:25.012Z", encoded); + + type = new InstantDt(encoded); + assertEquals(1960, type.getYear().intValue()); + assertEquals(8, type.getMonth().intValue()); // 0-indexed unlike LocalDateTime.of + assertEquals(7, type.getDay().intValue()); + assertEquals(0, type.getHour().intValue()); + assertEquals(44, type.getMinute().intValue()); + assertEquals(25, type.getSecond().intValue()); + assertEquals(12, type.getMillis().intValue()); + + } + @Test public void testFromTime() { long millis; diff --git a/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/BaseDateTimeType.java b/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/BaseDateTimeType.java index 1f0aee36525..ecc194d87f4 100644 --- a/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/BaseDateTimeType.java +++ b/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/model/BaseDateTimeType.java @@ -429,7 +429,12 @@ public abstract class BaseDateTimeType extends PrimitiveType { myPrecision = thePrecision; myFractionalSeconds = ""; if (theValue != null) { - String fractionalSeconds = Integer.toString((int) (theValue.getTime() % 1000)); + long millis = theValue.getTime() % 1000; + if (millis < 0) { + // This is for times before 1970 (see bug #444) + millis = 1000 + millis; + } + String fractionalSeconds = Integer.toString((int) millis); myFractionalSeconds = StringUtils.leftPad(fractionalSeconds, 3, '0'); } super.setValue(theValue); diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java index 819b3e5bbb3..aa3141db382 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java @@ -23,6 +23,8 @@ import java.io.IOException; import java.io.StringReader; import java.nio.charset.StandardCharsets; import java.text.SimpleDateFormat; +import java.time.LocalDateTime; +import java.time.ZoneOffset; import java.util.*; import org.apache.commons.io.IOUtils; @@ -80,33 +82,33 @@ public class XmlParserDstu3Test { public void testEncodeChainedContainedResourcer() { Organization gp = new Organization(); gp.setName("grandparent"); - + Organization parent = new Organization(); parent.setName("parent"); parent.getPartOf().setResource(gp); - + Organization child = new Organization(); child.setName("child"); child.getPartOf().setResource(parent); - + Patient patient = new Patient(); patient.getManagingOrganization().setResource(child); - + String encoded = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(patient); ourLog.info(encoded); - + patient = ourCtx.newXmlParser().parseResource(Patient.class, encoded); - + child = (Organization) patient.getManagingOrganization().getResource(); assertEquals("child", child.getName()); - + parent = (Organization) child.getPartOf().getResource(); assertEquals("parent", parent.getName()); - - gp = (Organization)parent.getPartOf().getResource(); + + gp = (Organization) parent.getPartOf().getResource(); assertEquals("grandparent", gp.getName()); } - + /** * If a contained resource refers to a contained resource that comes after it, it should still be successfully * woven together. @@ -114,11 +116,11 @@ public class XmlParserDstu3Test { @Test public void testParseWovenContainedResources() throws IOException { String string = IOUtils.toString(getClass().getResourceAsStream("/bundle_with_woven_obs.xml"), StandardCharsets.UTF_8); - + IParser parser = ourCtx.newXmlParser(); parser.setParserErrorHandler(new StrictErrorHandler()); org.hl7.fhir.dstu3.model.Bundle bundle = parser.parseResource(Bundle.class, string); - + DiagnosticReport resource = (DiagnosticReport) bundle.getEntry().get(0).getResource(); Observation obs = (Observation) resource.getResult().get(1).getResource(); assertEquals("#2", obs.getId()); @@ -126,30 +128,29 @@ public class XmlParserDstu3Test { Practitioner performer = (Practitioner) performerFirstRep.getResource(); assertEquals("#3", performer.getId()); } - - @Test(expected=DataFormatException.class) + + @Test(expected = DataFormatException.class) public void testContainedResourceWithNoId() throws IOException { String string = IOUtils.toString(getClass().getResourceAsStream("/bundle_with_contained_with_no_id.xml"), StandardCharsets.UTF_8); - + IParser parser = ourCtx.newXmlParser(); parser.setParserErrorHandler(new StrictErrorHandler()); parser.parseResource(Bundle.class, string); } - @Test() public void testContainedResourceWithNoIdLenient() throws IOException { String string = IOUtils.toString(getClass().getResourceAsStream("/bundle_with_contained_with_no_id.xml"), StandardCharsets.UTF_8); - + IParser parser = ourCtx.newXmlParser(); parser.setParserErrorHandler(new LenientErrorHandler()); parser.parseResource(Bundle.class, string); } - @Test(expected=DataFormatException.class) + @Test(expected = DataFormatException.class) public void testParseWithInvalidLocalRef() throws IOException { String string = IOUtils.toString(getClass().getResourceAsStream("/bundle_with_invalid_contained_ref.xml"), StandardCharsets.UTF_8); - + IParser parser = ourCtx.newXmlParser(); parser.setParserErrorHandler(new StrictErrorHandler()); parser.parseResource(Bundle.class, string); @@ -158,7 +159,7 @@ public class XmlParserDstu3Test { @Test() public void testParseWithInvalidLocalRefLenient() throws IOException { String string = IOUtils.toString(getClass().getResourceAsStream("/bundle_with_invalid_contained_ref.xml"), StandardCharsets.UTF_8); - + IParser parser = ourCtx.newXmlParser(); parser.setParserErrorHandler(new LenientErrorHandler()); parser.parseResource(Bundle.class, string); @@ -194,7 +195,7 @@ public class XmlParserDstu3Test { assertArrayEquals(new byte[] { 1, 2, 3, 4 }, bin.getContent()); } - + @Test public void testContainedResourceInExtensionUndeclared() { Patient p = new Patient(); @@ -230,7 +231,7 @@ public class XmlParserDstu3Test { assertThat(str, not(containsString("meta"))); assertThat(str, containsString("")); } - + @Test public void testEncodeAndParseBundleWithResourceRefs() { @@ -267,7 +268,7 @@ public class XmlParserDstu3Test { assertEquals("Organization/orgid", pt.getManagingOrganization().getReferenceElement().getValue()); assertSame(org, pt.getManagingOrganization().getResource()); } - + @Test public void testEncodeAndParseCompositeExtension() { PatientWithCustomCompositeExtension pat = new PatientWithCustomCompositeExtension(); @@ -275,16 +276,16 @@ public class XmlParserDstu3Test { pat.setFooParentExtension(new FooParentExtension()); pat.getFooParentExtension().setChildA(new StringType("ValueA")); pat.getFooParentExtension().setChildB(new StringType("ValueB")); - + String enc = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(pat); ourLog.info(enc); - + pat = ourCtx.newXmlParser().parseResource(PatientWithCustomCompositeExtension.class, enc); - + assertEquals("ValueA", pat.getFooParentExtension().getChildA().getValue()); assertEquals("ValueB", pat.getFooParentExtension().getChildB().getValue()); } - + @Test public void testEncodeAndParseContained() { IParser xmlParser = ourCtx.newXmlParser().setPrettyPrint(true); @@ -359,20 +360,20 @@ public class XmlParserDstu3Test { ourCtx = FhirContext.forDstu3(); ourCtx.setDefaultTypeForProfile(CustomObservation.PROFILE, CustomObservation.class); ourCtx.setDefaultTypeForProfile(CustomDiagnosticReport.PROFILE, CustomDiagnosticReport.class); - + CustomObservation obs = new CustomObservation(); obs.setStatus(ObservationStatus.FINAL); - + CustomDiagnosticReport dr = new CustomDiagnosticReport(); dr.setStatus(DiagnosticReportStatus.FINAL); dr.addResult().setResource(obs); - + IParser parser = ourCtx.newXmlParser(); parser.setPrettyPrint(true); - + String output = parser.encodeResourceToString(dr); ourLog.info(output); - + //@formatter:off assertThat(output,stringContainsInOrder( "", @@ -394,11 +395,11 @@ public class XmlParserDstu3Test { "", "")); //@formatter:on - + /* * Now PARSE! */ - + dr = (CustomDiagnosticReport) parser.parseResource(output); assertEquals(DiagnosticReportStatus.FINAL, dr.getStatus()); @@ -409,24 +410,23 @@ public class XmlParserDstu3Test { ourCtx = null; } - @Test public void testEncodeAndParseContainedNonCustomTypes() { ourCtx = FhirContext.forDstu3(); - + Observation obs = new Observation(); obs.setStatus(ObservationStatus.FINAL); - + DiagnosticReport dr = new DiagnosticReport(); dr.setStatus(DiagnosticReportStatus.FINAL); dr.addResult().setResource(obs); - + IParser parser = ourCtx.newXmlParser(); parser.setPrettyPrint(true); - + String output = parser.encodeResourceToString(dr); ourLog.info(output); - + //@formatter:off assertThat(output,stringContainsInOrder( "", @@ -442,11 +442,11 @@ public class XmlParserDstu3Test { "", "")); //@formatter:on - + /* * Now PARSE! */ - + dr = (DiagnosticReport) parser.parseResource(output); assertEquals(DiagnosticReportStatus.FINAL, dr.getStatus()); @@ -1417,34 +1417,34 @@ public class XmlParserDstu3Test { @Test public void testEncodeHistoryEncodeVersionsAtPath3() { ourCtx = FhirContext.forDstu3(); - + assertNull(ourCtx.newXmlParser().getStripVersionsFromReferences()); - + AuditEvent auditEvent = new AuditEvent(); auditEvent.addEntity().setReference(new Reference("http://foo.com/Organization/2/_history/1")); - + IParser parser = ourCtx.newXmlParser(); - + parser.setDontStripVersionsFromReferencesAtPaths("AuditEvent.entity.reference"); String enc = parser.setPrettyPrint(true).encodeResourceToString(auditEvent); ourLog.info(enc); assertThat(enc, containsString("")); - + parser.setDontStripVersionsFromReferencesAtPaths(new ArrayList()); enc = parser.setPrettyPrint(true).encodeResourceToString(auditEvent); ourLog.info(enc); assertThat(enc, containsString("")); - parser.setDontStripVersionsFromReferencesAtPaths((String[])null); + parser.setDontStripVersionsFromReferencesAtPaths((String[]) null); enc = parser.setPrettyPrint(true).encodeResourceToString(auditEvent); ourLog.info(enc); assertThat(enc, containsString("")); - parser.setDontStripVersionsFromReferencesAtPaths((List)null); + parser.setDontStripVersionsFromReferencesAtPaths((List) null); enc = parser.setPrettyPrint(true).encodeResourceToString(auditEvent); ourLog.info(enc); assertThat(enc, containsString("")); - + } @Test @@ -1579,17 +1579,17 @@ public class XmlParserDstu3Test { @Test public void testEncodeReferenceWithUuid() { - + Practitioner pract = new Practitioner(); pract.setId(IdType.newRandomUuid()); pract.addName().addFamily("PRACT FAMILY"); - + Patient patient = new Patient(); patient.addGeneralPractitioner().setResource(pract); - + String encoded = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(patient); ourLog.info(encoded); - + assertThat(pract.getId(), startsWith("urn:uuid:")); assertThat(encoded, containsString("")); } @@ -2792,7 +2792,7 @@ public class XmlParserDstu3Test { ""; //@formatter:on Patient pt = ourCtx.newXmlParser().parseResource(Patient.class, input); - + List extList = pt.getExtensionsByUrl("http://aaa.ch/fhir/Patient#mangedcare"); extList = extList.get(0).getExtensionsByUrl("http://aaa.ch/fhir/Patient#mangedcare-aaa-id"); Extension ext = extList.get(0); @@ -2821,7 +2821,7 @@ public class XmlParserDstu3Test { ""; //@formatter:on Patient pt = ourCtx.newXmlParser().parseResource(Patient.class, input); - + List extList = pt.getExtensionsByUrl("http://aaa.ch/fhir/Patient#mangedcare"); extList = extList.get(0).getExtensionsByUrl("http://aaa.ch/fhir/Patient#mangedcare-aaa-id"); Extension ext = extList.get(0); diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/CreateDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/CreateDstu3Test.java index 03e88a3bfbd..914c3461ee4 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/CreateDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/CreateDstu3Test.java @@ -68,6 +68,62 @@ public class CreateDstu3Test { } + @Test + public void testCreateWithIncorrectContent1() throws Exception { + + HttpPost httpPost = new HttpPost("http://localhost:" + ourPort + "/Patient"); + httpPost.setEntity(new StringEntity("{\"foo\":\"bar\"}", ContentType.parse("application/xml+fhir; charset=utf-8"))); + HttpResponse status = ourClient.execute(httpPost); + + String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); + IOUtils.closeQuietly(status.getEntity().getContent()); + + ourLog.info("Response was:\n{}", responseContent); + + assertEquals(400, status.getStatusLine().getStatusCode()); + + assertThat(responseContent, containsString(" + + Times before 1970 with fractional milliseconds were parsed incorrectly. Thanks + to GitHub user @CarthageKing for reporting! +