Skip to content

Commit

Permalink
refactor(#2092): Use HttpStatus in stead of plain strings (#2099)
Browse files Browse the repository at this point in the history
* remove occurrences of hard-coded HTTP results and replace them with the corresponding values of code.HttpStatus

* resolve checkstyle violation

* resolve checkstyle violation2

* After testing,resolve checkstyle violation

* modify

* modify
  • Loading branch information
bagechengzi authored Oct 30, 2023
1 parent a34a4dd commit 0497444
Show file tree
Hide file tree
Showing 21 changed files with 77 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package org.apache.streampipes.client.api;


import org.apache.streampipes.client.http.DeleteRequest;
import org.apache.streampipes.client.http.GetRequest;
import org.apache.streampipes.client.http.PostRequestWithPayloadResponse;
Expand All @@ -30,6 +31,8 @@
import org.apache.streampipes.commons.exceptions.SpHttpErrorStatusCode;
import org.apache.streampipes.commons.exceptions.SpRuntimeException;

import org.apache.http.HttpStatus;

import java.util.Optional;

public class AbstractClientApi {
Expand Down Expand Up @@ -89,7 +92,7 @@ protected <T> Optional<T> getSingleOpt(StreamPipesApiPath apiPath,
ObjectSerializer<Void, T> serializer = new ObjectSerializer<>();
return Optional.of(new GetRequest<>(clientConfig, apiPath, targetClass, serializer).executeRequest());
} catch (SpHttpErrorStatusCode e) {
if (e.getHttpStatusCode() == 404) {
if (e.getHttpStatusCode() == HttpStatus.SC_NOT_FOUND) {
return Optional.empty();
} else {
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import org.apache.streampipes.commons.exceptions.SpHttpErrorStatusCode;
import org.apache.streampipes.commons.exceptions.SpRuntimeException;

import org.apache.http.HttpStatus;

import java.util.List;
import java.util.Optional;

Expand All @@ -47,7 +49,7 @@ protected Optional<T> getSingle(StreamPipesApiPath apiPath) throws SpRuntimeExce
try {
return Optional.of(new GetRequest<>(clientConfig, apiPath, targetClass, serializer).executeRequest());
} catch (SpHttpErrorStatusCode e) {
if (e.getHttpStatusCode() == 404) {
if (e.getHttpStatusCode() == HttpStatus.SC_NOT_FOUND) {
return Optional.empty();
} else {
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,10 @@ public T executeRequest() {
switch (status.getStatusCode()) {
case HttpStatus.SC_UNAUTHORIZED -> throw new SpHttpErrorStatusCode(
" 401 - Access to this resource is forbidden - did you provide a poper API key or client secret?",
401);
HttpStatus.SC_UNAUTHORIZED);
case HttpStatus.SC_NOT_FOUND ->
throw new SpHttpErrorStatusCode(" 404 - The requested resource could not be found.", 404);
throw new SpHttpErrorStatusCode(" 404 - The requested resource could not be found.",
HttpStatus.SC_NOT_FOUND);
default -> throw new SpHttpErrorStatusCode(status.getStatusCode() + " - " + status.getReasonPhrase(),
status.getStatusCode());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

package org.apache.streampipes.connect.management.management;


import org.apache.streampipes.commons.exceptions.SpConfigurationException;
import org.apache.streampipes.commons.exceptions.connect.AdapterException;
import org.apache.streampipes.connect.management.util.WorkerPaths;
Expand All @@ -35,6 +36,7 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.commons.io.IOUtils;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.client.fluent.Request;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -106,7 +108,7 @@ private static void triggerAdapterStateChange(AdapterDescription ad,
var response = triggerPost(url, ad.getCorrespondingDataStreamElementId(), adapterDescription);
var responseString = getResponseBody(response);

if (response.getStatusLine().getStatusCode() != 200) {
if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
var exception = getSerializer().readValue(responseString, AdapterException.class);
throw new AdapterException(exception.getMessage(), exception.getCause());
}
Expand Down Expand Up @@ -144,7 +146,7 @@ public static RuntimeOptionsResponse getConfiguration(String workerEndpoint,

String responseString = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8);

if (response.getStatusLine().getStatusCode() == 200) {
if (response.getStatusLine().getStatusCode() == HttpStatus.SC_OK) {
return getSerializer().readValue(responseString, RuntimeOptionsResponse.class);
} else {
var exception = getSerializer().readValue(responseString, SpConfigurationException.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

package org.apache.streampipes.connect.adapters.netio;


import org.apache.streampipes.commons.exceptions.connect.AdapterException;
import org.apache.streampipes.connect.adapters.netio.model.NetioAllPowerOutputs;
import org.apache.streampipes.connect.adapters.netio.model.NetioPowerOutput;
Expand All @@ -41,6 +42,7 @@

import com.google.gson.Gson;
import org.apache.http.HttpHost;
import org.apache.http.HttpStatus;
import org.apache.http.client.HttpResponseException;
import org.apache.http.client.fluent.Executor;
import org.apache.http.client.fluent.Request;
Expand Down Expand Up @@ -159,7 +161,8 @@ public GuessSchema onSchemaRequested(IAdapterParameterExtractor extractor,
requestData();
return NetioUtils.getNetioSchema();
} catch (IOException e) {
if (e instanceof HttpResponseException && ((HttpResponseException) e).getStatusCode() == 401) {
if (e instanceof HttpResponseException && ((HttpResponseException) e).getStatusCode()
== HttpStatus.SC_UNAUTHORIZED) {
throw new AdapterException(
"Unauthorized! Could not connect to NETIO sensor: " + this.ip + " with username " + this.username);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
//
// stubFor(get(urlEqualTo("/"))
// .willReturn(aResponse()
// .withStatus(200)
// .withStatus(HttpStatus.SC_OK)
// .withBody(expected)));
//
// String result = PullRestAdapter.getDataFromEndpointString("http://localhost:" + port + "/");
Expand All @@ -44,7 +44,7 @@
// public void getDataFromEndpointStringFail() throws AdapterException {
// stubFor(get(urlEqualTo("/"))
// .willReturn(aResponse()
// .withStatus(404)
// .withStatus(HttpStatus.SC_NOT_FOUND)
// .withBody("")));
//
// PullRestAdapter.getDataFromEndpointString("http://localhost:" + port + "/");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
//
// stubFor(get(urlEqualTo("/"))
// .willReturn(aResponse()
// .withStatus(200)
// .withStatus(HttpStatus.SC_OK)
// .withBody(expected)));
//
//
Expand Down Expand Up @@ -77,7 +77,7 @@
//
// stubFor(get(urlEqualTo("/"))
// .willReturn(aResponse()
// .withStatus(200)
// .withStatus(HttpStatus.SC_OK)
// .withBody("Example response")));
//
//
Expand Down
4 changes: 4 additions & 0 deletions streampipes-integration-tests/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpcore</artifactId>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package org.apache.streampipes.integration.containers;

import org.apache.http.HttpStatus;
import org.testcontainers.containers.GenericContainer;
import org.testcontainers.containers.wait.strategy.HttpWaitStrategy;

Expand All @@ -35,7 +36,7 @@ public PulsarContainer() {
public void start() {
this.waitStrategy = new HttpWaitStrategy()
.forPort(BROKER_HTTP_PORT)
.forStatusCode(200)
.forStatusCode(HttpStatus.SC_OK)
.forPath("/admin/v2/namespaces/public/default")
.withStartupTimeout(Duration.of(300, SECONDS));
this.withExposedPorts(BROKER_SERVICE_PORT, BROKER_HTTP_PORT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@

package org.apache.streampipes.manager.health;


import org.apache.streampipes.manager.execution.ExtensionServiceExecutions;
import org.apache.streampipes.model.extensions.svcdiscovery.SpServiceRegistration;
import org.apache.streampipes.storage.api.CRUDStorage;
import org.apache.streampipes.storage.management.StorageDispatcher;

import org.apache.http.HttpStatus;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -53,7 +55,7 @@ private void checkServiceHealth(SpServiceRegistration service) {
try {
var request = ExtensionServiceExecutions.extServiceGetRequest(healthCheckUrl);
var response = request.execute();
if (response.returnResponse().getStatusLine().getStatusCode() != 200) {
if (response.returnResponse().getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
processUnhealthyService(service);
} else {
if (!service.isHealthy()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

package org.apache.streampipes.rest.extensions;

import org.apache.http.HttpStatus;

import jakarta.ws.rs.core.Response;

public class AbstractExtensionsResource {
Expand All @@ -31,13 +33,13 @@ protected <T> Response ok(T entity) {

protected Response clientError() {
return Response
.status(400)
.status(HttpStatus.SC_BAD_REQUEST)
.build();
}

protected Response serverError() {
return Response
.status(500)
.status(HttpStatus.SC_INTERNAL_SERVER_ERROR)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import com.google.common.base.Charsets;
import com.google.common.io.Resources;
import org.apache.http.HttpStatus;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -88,7 +89,7 @@ public Response getIconAsset(@PathParam("appId") String appId) throws IOExceptio
return ok(Resources.toByteArray(iconUrl));
} catch (IllegalArgumentException e) {
LOG.warn("No icon resource found for pipeline element {}", appId);
return Response.status(400).build();
return Response.status(HttpStatus.SC_BAD_REQUEST).build();
}
}

Expand All @@ -101,7 +102,7 @@ public Response getDocumentationAsset(@PathParam("id") String elementId) throws
return ok(Resources.toString(documentationUrl, Charsets.UTF_8));
} catch (IllegalArgumentException e) {
LOG.warn("No documentation resource found for pipeline element {}", elementId);
return Response.status(400).build();
return Response.status(HttpStatus.SC_BAD_REQUEST).build();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import org.apache.streampipes.extensions.management.connect.HttpServerAdapterManagement;

import org.apache.http.HttpStatus;

import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
Expand All @@ -36,7 +38,7 @@ public Response receiveEvent(@PathParam("endpointId") String endpointId,
HttpServerAdapterManagement.INSTANCE.notify(endpointId, body);
return Response.ok().build();
} catch (Exception e) {
return Response.status(400).entity(e.getMessage()).build();
return Response.status(HttpStatus.SC_BAD_REQUEST).entity(e.getMessage()).build();
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import org.apache.streampipes.rest.shared.annotation.JacksonSerialized;
import org.apache.streampipes.rest.shared.impl.AbstractSharedRestInterface;

import org.apache.http.HttpStatus;

import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
Expand Down Expand Up @@ -65,7 +67,7 @@ public Response fetchConfigurations(@PathParam("id") String elementId,
}
} catch (SpConfigurationException e) {
return jakarta.ws.rs.core.Response
.status(400)
.status(HttpStatus.SC_BAD_REQUEST)
.entity(e)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import org.apache.streampipes.rest.extensions.AbstractPipelineElementResource;
import org.apache.streampipes.svcdiscovery.api.model.SpServicePathPrefix;

import org.apache.http.HttpStatus;

import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
Expand All @@ -50,7 +52,7 @@ public jakarta.ws.rs.core.Response getAssets(@PathParam("streamId") String strea
.getIncludedAssets()).makeZip());
} catch (IOException e) {
e.printStackTrace();
return jakarta.ws.rs.core.Response.status(500).build();
return jakarta.ws.rs.core.Response.status(HttpStatus.SC_INTERNAL_SERVER_ERROR).build();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.apache.streampipes.rest.shared.annotation.JacksonSerialized;
import org.apache.streampipes.sdk.extractor.AbstractParameterExtractor;

import org.apache.http.HttpStatus;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -130,11 +131,11 @@ public jakarta.ws.rs.core.Response fetchConfigurations(@PathParam("elementId") S
new RuntimeResolvableRequestHandler().handleRuntimeResponse((SupportsRuntimeConfig) declarer, req);
return ok(responseOptions);
} else {
return jakarta.ws.rs.core.Response.status(500).build();
return jakarta.ws.rs.core.Response.status(HttpStatus.SC_INTERNAL_SERVER_ERROR).build();
}
} catch (SpConfigurationException e) {
return jakarta.ws.rs.core.Response
.status(400)
.status(HttpStatus.SC_BAD_REQUEST)
.entity(e)
.build();
}
Expand Down
9 changes: 9 additions & 0 deletions streampipes-rest-shared/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@
<groupId>org.glassfish.jersey.media</groupId>
<artifactId>jersey-media-multipart</artifactId>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpcore</artifactId>
</dependency>

<!-- Test dependencies -->
<dependency>
Expand Down Expand Up @@ -95,6 +99,11 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<configuration>
<propertyExpansion>
checkstyle.config.base.path=${project.parent.basedir}/tools/maven
</propertyExpansion>
</configuration>
</plugin>
</plugins>
</build>
Expand Down
Loading

0 comments on commit 0497444

Please sign in to comment.