Add Streaming Rest Call - returning the response input stream without… by adigfrog · Pull Request #379 · jfrog/artifactory-client-java

adigfrog · 2023-09-11T18:44:13Z

… the wrappers

All tests passed. If this feature is not already covered by the tests, I added new tests.
This pull request is on the dev branch.

github-actions · 2023-09-11T18:44:30Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

alenon

Please consider moving the isSuccessResponse to the common class
And making the common implementation to look like the following
public static boolean isSuccessfulResponseCode(int status) { return 200 <= status && status < 300; }

adigfrog · 2023-09-12T06:51:03Z

Please consider moving the isSuccessResponse to the common class And making the common implementation to look like the following public static boolean isSuccessfulResponseCode(int status) { return 200 <= status && status < 300; }

That was my first change but I saw that in the function getInputStreamWithHeaders:

.../services/src/main/groovy/org/jfrog/artifactory/client/impl)/ArtifactoryImpl.java:

public InputStream getInputStreamWithHeaders(String path, Map<String, String> headers) throws IOException {
        HttpResponse httpResponse = get(path, null, null, headers);
        if (httpResponse.getStatusLine().getStatusCode() == HttpStatus.SC_OK ||
                httpResponse.getStatusLine().getStatusCode() == HttpStatus.SC_PARTIAL_CONTENT) {
            return httpResponse.getEntity().getContent();
        }
        throw newHttpResponseException(httpResponse);
    }

So I understood that for streaming only 200 and 206 are valid

adigfrog · 2023-09-12T06:52:43Z

I have read the CLA Document and I hereby sign the CLA

yahavi · 2023-09-20T06:45:17Z

    private static final ObjectMapper objectMapper = new ObjectMapper();

-    private HttpResponse httpResponse;
+


Redundant new line

Suggested change

yahavi · 2023-09-20T06:45:47Z

-
        return status >= 200 && status < 300;
    }
+


Suggested change

yahavi · 2023-09-20T06:48:57Z

+        ArtifactoryStreamingResponse response = artifactory.streamingRestCall(request);
+        assertFalse(response.isSuccessResponse());
+        assertEquals(response.getStatusLine().getStatusCode(), 404);
+        assertEquals(response.getStatusLine().getReasonPhrase(), "Not Found");


I got an empty reason phrase on my environment:

java.lang.AssertionError: expected [Not Found] but found []

yahavi · 2023-09-20T06:50:57Z

+ * ArtifactoryStreamingResponse object returned from {@link Artifactory#streamingRestCall(ArtifactoryRequest)}.
+ * acts as a wrapper for {@link org.apache.http.HttpResponse}.
+ */
+public interface ArtifactoryStreamingResponse extends BaseArtifactoryResponse {


Suggestion - please consider making ArtifactoryStreamingResponse AutoClosable to allow using easily it with try-with-resources

yahavi · 2023-09-20T06:51:02Z

+public interface ArtifactoryStreamingResponse extends BaseArtifactoryResponse {
+
+    InputStream getInputStream() throws IOException;
+


Suggested change

yahavi · 2023-09-20T06:52:39Z


    InputStream getInputStreamWithHeaders(String path, Map<String, String> headers) throws IOException;

+    HttpResponse execute(HttpUriRequest request) throws IOException;


Is it really needed?

yahavi · 2023-09-20T06:53:25Z


    ArtifactoryResponse restCall(ArtifactoryRequest artifactoryRequest) throws IOException;

+    ArtifactoryStreamingResponse streamingRestCall(ArtifactoryRequest artifactoryRequest) throws IOException;


Let's document it to the README

done and tested

yahavi · 2023-09-20T10:48:17Z

+assert response.isSuccessResponse();
+
+// Get the response raw body using input stream
+String rawBody = IOUtils.toString(response.getInputStream(),StandardCharsets.UTF_8);


Suggested change

String rawBody = IOUtils.toString(response.getInputStream(),StandardCharsets.UTF_8);

String rawBody = IOUtils.toString(response.getInputStream(), StandardCharsets.UTF_8);

… the wrappers

alenon suggested changes Sep 12, 2023

View reviewed changes

adigfrog force-pushed the JR-7345 branch 2 times, most recently from aec4be2 to 618959e Compare September 13, 2023 06:21

yahavi requested changes Sep 20, 2023

View reviewed changes

adigfrog force-pushed the JR-7345 branch from 618959e to c53a38c Compare September 20, 2023 09:49

yahavi reviewed Sep 20, 2023

View reviewed changes

adigfrog force-pushed the JR-7345 branch from c53a38c to f78bf52 Compare September 20, 2023 11:27

Add Streaming Rest Call - returning the response input stream without…

ad4ebdb

… the wrappers

adigfrog force-pushed the JR-7345 branch from f78bf52 to ad4ebdb Compare September 20, 2023 11:32

yahavi approved these changes Sep 20, 2023

View reviewed changes

yahavi added the feature request New feature or request label Sep 20, 2023

yahavi merged commit 102bee9 into jfrog:dev Sep 20, 2023

		private static final ObjectMapper objectMapper = new ObjectMapper();

		private HttpResponse httpResponse;

		public interface ArtifactoryStreamingResponse extends BaseArtifactoryResponse {

		InputStream getInputStream() throws IOException;


		InputStream getInputStreamWithHeaders(String path, Map<String, String> headers) throws IOException;

		HttpResponse execute(HttpUriRequest request) throws IOException;


		ArtifactoryResponse restCall(ArtifactoryRequest artifactoryRequest) throws IOException;

		ArtifactoryStreamingResponse streamingRestCall(ArtifactoryRequest artifactoryRequest) throws IOException;

	String rawBody = IOUtils.toString(response.getInputStream(),StandardCharsets.UTF_8);
	String rawBody = IOUtils.toString(response.getInputStream(), StandardCharsets.UTF_8);

Uh oh!

Conversation

adigfrog commented Sep 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alenon left a comment

Choose a reason for hiding this comment

Uh oh!

adigfrog commented Sep 12, 2023

Uh oh!

adigfrog commented Sep 12, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adigfrog Sep 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adigfrog commented Sep 11, 2023 •

edited

Loading

github-actions Bot commented Sep 11, 2023 •

edited

Loading

adigfrog Sep 20, 2023 •

edited

Loading