From a4365f9b83ded4eb8f1a5bf2cbebead2a78d284f Mon Sep 17 00:00:00 2001 From: Michael Lawley Date: Thu, 24 Aug 2017 13:54:59 +1000 Subject: [PATCH] fix bugs with inverted test for If-Modified-Since (impl and tests) --- .../rest/server/method/ReadMethodBinding.java | 6 ++--- .../uhn/fhir/rest/server/ReadDstu2Test.java | 22 +++++++++++++++-- .../uhn/fhir/rest/server/ReadDstu3Test.java | 24 ++++++++++++++++--- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ReadMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ReadMethodBinding.java index 050e2a6d468..710c1b07c9a 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ReadMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ReadMethodBinding.java @@ -161,7 +161,7 @@ public class ReadMethodBinding extends BaseResourceReturningMethodBinding { ifNoneMatch = ParameterUtil.parseETagValue(ifNoneMatch); if (responseResource.getIdElement() != null && responseResource.getIdElement().hasVersionIdPart()) { if (responseResource.getIdElement().getVersionIdPart().equals(ifNoneMatch)) { - ourLog.debug("Returning HTTP 301 because request specified {}={}", Constants.HEADER_IF_NONE_MATCH, ifNoneMatch); + ourLog.debug("Returning HTTP 304 because request specified {}={}", Constants.HEADER_IF_NONE_MATCH, ifNoneMatch); throw new NotModifiedException("Not Modified"); } } @@ -182,8 +182,8 @@ public class ReadMethodBinding extends BaseResourceReturningMethodBinding { lastModified = responseResource.getMeta().getLastUpdated(); } - if (lastModified != null && lastModified.getTime() > ifModifiedSinceDate.getTime()) { - ourLog.debug("Returning HTTP 301 because If-Modified-Since does not match"); + if (lastModified != null && lastModified.getTime() <= ifModifiedSinceDate.getTime()) { + ourLog.debug("Returning HTTP 304 because If-Modified-Since does not match"); throw new NotModifiedException("Not Modified"); } } diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/ReadDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/ReadDstu2Test.java index 58282a11db8..86fd1e9b0a1 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/ReadDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/ReadDstu2Test.java @@ -57,20 +57,38 @@ public class ReadDstu2Test { CloseableHttpResponse status; HttpGet httpGet; + // Fixture was last modified at 2012-01-01T12:12:12Z + // thus it has changed before the later time of 2012-01-01T13:00:00Z + // so we expect a 304 httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/2"); httpGet.addHeader(Constants.HEADER_IF_MODIFIED_SINCE, DateUtils.formatDate(new InstantDt("2012-01-01T13:00:00Z").getValue())); status = ourClient.execute(httpGet); try { - assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals(304, status.getStatusLine().getStatusCode()); } finally { IOUtils.closeQuietly(status); } + // Fixture was last modified at 2012-01-01T12:12:12Z + // thus it has changed at the same time of 2012-01-01T12:12:12Z + // so we expect a 304 + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/2"); + httpGet.addHeader(Constants.HEADER_IF_MODIFIED_SINCE, DateUtils.formatDate(new InstantDt("2012-01-01T12:12:12Z").getValue())); + status = ourClient.execute(httpGet); + try { + assertEquals(304, status.getStatusLine().getStatusCode()); + } finally { + IOUtils.closeQuietly(status); + } + + // Fixture was last modified at 2012-01-01T12:12:12Z + // thus it has changed after the earlier time of 2012-01-01T10:00:00Z + // so we expect a 200 httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/2"); httpGet.addHeader(Constants.HEADER_IF_MODIFIED_SINCE, DateUtils.formatDate(new InstantDt("2012-01-01T10:00:00Z").getValue())); status = ourClient.execute(httpGet); try { - assertEquals(304, status.getStatusLine().getStatusCode()); + assertEquals(200, status.getStatusLine().getStatusCode()); } finally { IOUtils.closeQuietly(status); } diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/ReadDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/ReadDstu3Test.java index f3f05de41bf..ae8065ed5c4 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/ReadDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/ReadDstu3Test.java @@ -69,20 +69,38 @@ public class ReadDstu3Test { CloseableHttpResponse status; HttpGet httpGet; + // Fixture was last modified at 2012-01-01T12:12:12Z + // thus it hasn't changed after the later time of 2012-01-01T13:00:00Z + // so we expect a 304 (Not Modified) httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/2"); httpGet.addHeader(Constants.HEADER_IF_MODIFIED_SINCE, DateUtils.formatDate(new InstantDt("2012-01-01T13:00:00Z").getValue())); status = ourClient.execute(httpGet); try { - assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals(304, status.getStatusLine().getStatusCode()); } finally { IOUtils.closeQuietly(status); } - + + // Fixture was last modified at 2012-01-01T12:12:12Z + // thus it hasn't changed after the same time of 2012-01-01T12:12:12Z + // so we expect a 304 (Not Modified) + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/2"); + httpGet.addHeader(Constants.HEADER_IF_MODIFIED_SINCE, DateUtils.formatDate(new InstantDt("2012-01-01T12:12:12Z").getValue())); + status = ourClient.execute(httpGet); + try { + assertEquals(304, status.getStatusLine().getStatusCode()); + } finally { + IOUtils.closeQuietly(status); + } + + // Fixture was last modified at 2012-01-01T12:12:12Z + // thus it has changed after the earlier time of 2012-01-01T10:00:00Z + // so we expect a 200 httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/2"); httpGet.addHeader(Constants.HEADER_IF_MODIFIED_SINCE, DateUtils.formatDate(new InstantDt("2012-01-01T10:00:00Z").getValue())); status = ourClient.execute(httpGet); try { - assertEquals(304, status.getStatusLine().getStatusCode()); + assertEquals(200, status.getStatusLine().getStatusCode()); } finally { IOUtils.closeQuietly(status); }