From ee3024c1b76f732f4faa2e946c38593f2ba7ef47 Mon Sep 17 00:00:00 2001 From: dotasek Date: Thu, 22 Aug 2024 13:36:54 -0400 Subject: [PATCH 1/5] WIP cherry-pick fix for NPE in IGLoader --- .../main/java/org/hl7/fhir/validation/IgLoader.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/IgLoader.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/IgLoader.java index e0a1a9edc..9f64cdc62 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/IgLoader.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/IgLoader.java @@ -497,10 +497,12 @@ public class IgLoader implements IValidationEngineLoader { private Map fetchByPackage(String src, boolean loadInContext) throws FHIRException, IOException { NpmPackage pi; - InputStream stream = directProvider.fetchByPackage(src); - if (stream != null) { - pi = NpmPackage.fromPackage(stream); - return loadPackage(pi, loadInContext); + if (directProvider != null) { + InputStream stream = directProvider.fetchByPackage(src); + if (stream != null) { + pi = NpmPackage.fromPackage(stream); + return loadPackage(pi, loadInContext); + } } String id = src; String version = null; From 8094d72314945c6cfa605110de70e0f24c4b443c Mon Sep 17 00:00:00 2001 From: dotasek Date: Thu, 22 Aug 2024 15:42:18 -0400 Subject: [PATCH 2/5] Improved basic SimpleHttpClientTest and add one for redirects --- .../fhir/utilities/http/SimpleHTTPClient.java | 26 +++--- .../fhir/utilities/SimpleHTTPClientTest.java | 91 +++++++++++++++++-- 2 files changed, 100 insertions(+), 17 deletions(-) diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java index ad3f5c7d5..7bb81fceb 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java @@ -90,17 +90,21 @@ public class SimpleHTTPClient { setHeaders(c); c.setInstanceFollowRedirects(false); - switch (c.getResponseCode()) { - case HttpURLConnection.HTTP_MOVED_PERM: - case HttpURLConnection.HTTP_MOVED_TEMP: - String location = c.getHeaderField("Location"); - location = URLDecoder.decode(location, "UTF-8"); - URL base = new URL(url); - URL next = new URL(base, location); // Deal with relative URLs - url = next.toExternalForm(); - continue; - default: - done = true; + int responseCode = c.getResponseCode(); + System.out.println("Processing response code "+ responseCode); + switch (responseCode) { + case HttpURLConnection.HTTP_MOVED_PERM: + case HttpURLConnection.HTTP_MOVED_TEMP: + case 308: // Same as HTTP_MOVED_PERM, but does not allow changing the request method from POST to GET + System.out.println("Processing redirect to "+c.getHeaderField("Location")); + String location = c.getHeaderField("Location"); + location = URLDecoder.decode(location, "UTF-8"); + URL base = new URL(url); + URL next = new URL(base, location); // Deal with relative URLs + url = next.toExternalForm(); + continue; + default: + done = true; } } diff --git a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java index 866894056..fda01dd32 100644 --- a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java +++ b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java @@ -1,24 +1,103 @@ package org.hl7.fhir.utilities; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; +import java.util.stream.Stream; +import okhttp3.HttpUrl; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; import org.hl7.fhir.utilities.http.HTTPResult; import org.hl7.fhir.utilities.http.SimpleHTTPClient; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.MethodSource; public class SimpleHTTPClientTest { + + private MockWebServer server; + + + + @BeforeEach + void setup() { + setupMockServer(); + } + + void setupMockServer() { + server = new MockWebServer(); + } + @Test - public void testSimpleHTTPClient() throws IOException { + public void testGetApplicationJson() throws IOException, InterruptedException { + + HttpUrl serverUrl = server.url("fhir/us/core/package-list.json?nocache=1724353440974"); + + server.enqueue( + new MockResponse() + .setBody("Monkeys").setResponseCode(200) + ); SimpleHTTPClient http = new SimpleHTTPClient(); - String url = "https://hl7.org/fhir/us/core/package-list.json?nocache=" + System.currentTimeMillis(); - HTTPResult res = http.get(url, "application/json"); + HTTPResult res = http.get(serverUrl.url().toString(), "application/json"); + + assertThat(res.getCode()).isEqualTo(200); + + RecordedRequest packageRequest = server.takeRequest(); + + assert packageRequest.getRequestUrl() != null; + assertThat(packageRequest.getRequestUrl().toString()).isEqualTo(serverUrl.url().toString()); + assertThat(packageRequest.getMethod()).isEqualTo("GET"); + assertThat(packageRequest.getHeader("Accept")).isEqualTo("application/json"); + + } + + public static Stream getRedirectArgs() { + return Stream.of( + Arguments.of(301, new String[]{"url1", "url2"}), + Arguments.of(301, new String[]{"url1", "url2", "url3"}), + Arguments.of(301, new String[]{"url1", "url2", "url3", "url4"}), + Arguments.of(302, new String[]{"url1", "url2"}), + Arguments.of(302, new String[]{"url1", "url2", "url3"}), + Arguments.of(302, new String[]{"url1", "url2", "url3", "url4"}), + Arguments.of(308, new String[]{"url1", "url2"}), + Arguments.of(308, new String[]{"url1", "url2", "url3"}), + Arguments.of(308, new String[]{"url1", "url2", "url3", "url4"}) + ); + } + + @ParameterizedTest + @MethodSource("getRedirectArgs") + public void testRedirects(int code, String[] urlArgs) throws IOException, InterruptedException { + + HttpUrl[] url = new HttpUrl[urlArgs.length]; + for (int i = 0; i < urlArgs.length; i++) { + url[i] = server.url(urlArgs[i]); + if (i > 0 && i < urlArgs.length - 1) { + server.enqueue( + new MockResponse() + .setResponseCode(code) + .addHeader("Location", url[i].url().toString())); + } else if (i == urlArgs.length - 1) { + server.enqueue( + new MockResponse() + .setBody("Monkeys").setResponseCode(200) + ); + } + } + + SimpleHTTPClient http = new SimpleHTTPClient(); + + HTTPResult res = http.get(url[0].url().toString(), "application/json"); + + assertThat(res.getCode()).isEqualTo(200); -// System.out.println(res.getCode()); -// System.out.println(new String(res.getContent(), StandardCharsets.UTF_8)); - assertTrue(res.getCode() != 400); } } \ No newline at end of file From 59f5eeb3c4679e4a13c82eb50b2a99d05baf3fa5 Mon Sep 17 00:00:00 2001 From: dotasek Date: Thu, 22 Aug 2024 15:51:31 -0400 Subject: [PATCH 3/5] Clean up --- .../java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java | 5 +---- .../java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java | 2 -- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java index 7bb81fceb..3385682ee 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java @@ -90,13 +90,10 @@ public class SimpleHTTPClient { setHeaders(c); c.setInstanceFollowRedirects(false); - int responseCode = c.getResponseCode(); - System.out.println("Processing response code "+ responseCode); - switch (responseCode) { + switch (c.getResponseCode()) { case HttpURLConnection.HTTP_MOVED_PERM: case HttpURLConnection.HTTP_MOVED_TEMP: case 308: // Same as HTTP_MOVED_PERM, but does not allow changing the request method from POST to GET - System.out.println("Processing redirect to "+c.getHeaderField("Location")); String location = c.getHeaderField("Location"); location = URLDecoder.decode(location, "UTF-8"); URL base = new URL(url); diff --git a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java index fda01dd32..1a2308673 100644 --- a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java +++ b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java @@ -1,7 +1,6 @@ package org.hl7.fhir.utilities; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; import java.util.stream.Stream; @@ -16,7 +15,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.MethodSource; public class SimpleHTTPClientTest { From 62758b57c5ad9acacbeb8aeb2902bd307535a9af Mon Sep 17 00:00:00 2001 From: dotasek Date: Thu, 22 Aug 2024 17:09:03 -0400 Subject: [PATCH 4/5] Smarter test. --- .../fhir/utilities/SimpleHTTPClientTest.java | 21 ++++++++++++------- pom.xml | 2 +- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java index 1a2308673..a5a2549cf 100644 --- a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java +++ b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java @@ -21,8 +21,6 @@ public class SimpleHTTPClientTest { private MockWebServer server; - - @BeforeEach void setup() { setupMockServer(); @@ -78,24 +76,31 @@ public class SimpleHTTPClientTest { HttpUrl[] url = new HttpUrl[urlArgs.length]; for (int i = 0; i < urlArgs.length; i++) { url[i] = server.url(urlArgs[i]); - if (i > 0 && i < urlArgs.length - 1) { + if (i > 0 && i < urlArgs.length) { server.enqueue( new MockResponse() .setResponseCode(code) + .setBody("Pumas") .addHeader("Location", url[i].url().toString())); - } else if (i == urlArgs.length - 1) { - server.enqueue( - new MockResponse() - .setBody("Monkeys").setResponseCode(200) - ); } } + server.enqueue( + new MockResponse() + .setBody("Monkeys").setResponseCode(200) + ); SimpleHTTPClient http = new SimpleHTTPClient(); HTTPResult res = http.get(url[0].url().toString(), "application/json"); assertThat(res.getCode()).isEqualTo(200); + assertThat(res.getContentAsString()).isEqualTo("Monkeys"); + assertThat(server.getRequestCount()).isEqualTo(urlArgs.length); + for (int i = 0; i < urlArgs.length; i++) { + RecordedRequest packageRequest = server.takeRequest(); + assertThat(packageRequest.getMethod()).isEqualTo("GET"); + assertThat(packageRequest.getHeader("Accept")).isEqualTo("application/json"); + } } } \ No newline at end of file diff --git a/pom.xml b/pom.xml index c5f5508ca..f10d71bc8 100644 --- a/pom.xml +++ b/pom.xml @@ -21,7 +21,7 @@ 1.26.0 32.0.1-jre 6.4.1 - 1.5.19 + 1.5.20-SNAPSHOT 2.17.0 5.9.2 1.8.2 From cf1a58e76c4e57fb38cd6d6f7f4cfd5632d26835 Mon Sep 17 00:00:00 2001 From: dotasek Date: Thu, 22 Aug 2024 17:24:17 -0400 Subject: [PATCH 5/5] Add 307 as well --- .../hl7/fhir/utilities/http/SimpleHTTPClient.java | 1 + .../hl7/fhir/utilities/SimpleHTTPClientTest.java | 15 ++++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java index 3385682ee..52ce02bd6 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/http/SimpleHTTPClient.java @@ -93,6 +93,7 @@ public class SimpleHTTPClient { switch (c.getResponseCode()) { case HttpURLConnection.HTTP_MOVED_PERM: case HttpURLConnection.HTTP_MOVED_TEMP: + case 307: case 308: // Same as HTTP_MOVED_PERM, but does not allow changing the request method from POST to GET String location = c.getHeaderField("Location"); location = URLDecoder.decode(location, "UTF-8"); diff --git a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java index a5a2549cf..3973cdd94 100644 --- a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java +++ b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/SimpleHTTPClientTest.java @@ -63,6 +63,9 @@ public class SimpleHTTPClientTest { Arguments.of(302, new String[]{"url1", "url2"}), Arguments.of(302, new String[]{"url1", "url2", "url3"}), Arguments.of(302, new String[]{"url1", "url2", "url3", "url4"}), + Arguments.of(307, new String[]{"url1", "url2"}), + Arguments.of(307, new String[]{"url1", "url2", "url3"}), + Arguments.of(307, new String[]{"url1", "url2", "url3", "url4"}), Arguments.of(308, new String[]{"url1", "url2"}), Arguments.of(308, new String[]{"url1", "url2", "url3"}), Arguments.of(308, new String[]{"url1", "url2", "url3", "url4"}) @@ -71,23 +74,24 @@ public class SimpleHTTPClientTest { @ParameterizedTest @MethodSource("getRedirectArgs") - public void testRedirects(int code, String[] urlArgs) throws IOException, InterruptedException { + public void testRedirectsGet(int code, String[] urlArgs) throws IOException, InterruptedException { - HttpUrl[] url = new HttpUrl[urlArgs.length]; + HttpUrl[] urls = new HttpUrl[urlArgs.length]; for (int i = 0; i < urlArgs.length; i++) { - url[i] = server.url(urlArgs[i]); - if (i > 0 && i < urlArgs.length) { + urls[i] = server.url(urlArgs[i]); + if (i > 0) { server.enqueue( new MockResponse() .setResponseCode(code) .setBody("Pumas") - .addHeader("Location", url[i].url().toString())); + .addHeader("Location", urls[i].url().toString())); } } server.enqueue( new MockResponse() .setBody("Monkeys").setResponseCode(200) ); + HttpUrl[] url = urls; SimpleHTTPClient http = new SimpleHTTPClient(); @@ -103,4 +107,5 @@ public class SimpleHTTPClientTest { assertThat(packageRequest.getHeader("Accept")).isEqualTo("application/json"); } } + } \ No newline at end of file