Skip to content

Commit

Permalink
Cleanup random findings
Browse files Browse the repository at this point in the history
- Add error handling.
- Prevent millions of nonsense string comparions where a simple int
  compare is enough.
- Shorten useless complicated messages. Cache result of toString().
- Replace ugly imports.

References (www):
- https://m.youtube.com/watch?v=x2EOOJg8FkA
- https://m.youtube.com/watch?v=EpYr3T5VP6w
- https://medium.com/swlh/yagni-and-dry-the-kiss-of-death-for-your-software-project-cfd44b0654b6#fc82

References (I'm not allowed to make those publicly available):
- https://wikit.post.ch/x/_Bv6Rw
- https://wikit.post.ch/x/iRepPQ
- https://jira.post.ch/browse/SDCISA-10871

(wip @ 284d263)
  • Loading branch information
hiddenalpha committed Jun 28, 2023
1 parent 6a30fd1 commit 67f673e
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,24 @@
import io.vertx.core.MultiMap;
import io.vertx.core.Vertx;
import io.vertx.core.buffer.Buffer;
import io.vertx.core.http.*;
import io.vertx.core.http.HttpClient;
import io.vertx.core.http.HttpClientRequest;
import io.vertx.core.http.HttpClientResponse;
import io.vertx.core.http.HttpMethod;
import io.vertx.core.http.HttpServerRequest;
import io.vertx.core.json.JsonArray;
import io.vertx.core.json.JsonObject;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.swisspush.gateleen.core.http.RequestLoggerFactory;
import org.swisspush.gateleen.core.storage.ResourceStorage;
import org.swisspush.gateleen.core.util.*;
import org.swisspush.gateleen.core.util.ExpansionDeltaUtil;
import org.swisspush.gateleen.core.util.ExpansionDeltaUtil.CollectionResourceContainer;
import org.swisspush.gateleen.core.util.ExpansionDeltaUtil.SlashHandling;
import org.swisspush.gateleen.core.util.HttpServerRequestUtil;
import org.swisspush.gateleen.core.util.ResourceCollectionException;
import org.swisspush.gateleen.core.util.ResponseStatusCodeLogUtil;
import org.swisspush.gateleen.core.util.StatusCode;
import org.swisspush.gateleen.routing.Rule;
import org.swisspush.gateleen.routing.RuleFeaturesProvider;
import org.swisspush.gateleen.routing.RuleProvider;
Expand All @@ -25,6 +33,7 @@
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;

import static org.swisspush.gateleen.core.util.StatusCode.INTERNAL_SERVER_ERROR;
import static org.swisspush.gateleen.routing.RuleFeatures.Feature.EXPAND_ON_BACKEND;
import static org.swisspush.gateleen.routing.RuleFeatures.Feature.STORAGE_EXPAND;
import static org.swisspush.gateleen.routing.RuleProvider.RuleChangesObserver;
Expand Down Expand Up @@ -491,7 +500,9 @@ private void removeZipParameter(final HttpServerRequest request) {

private void makeStorageExpandRequest(final String targetUri, final List subResourceNames, final HttpServerRequest req, final DeltaHandler<ResourceNode> handler) {
Logger log = RequestLoggerFactory.getLogger(ExpansionHandler.class, req);
httpClient.request(HttpMethod.POST, targetUri + "?storageExpand=true").onComplete(asyncResult -> {
HttpMethod reqMethod = HttpMethod.POST;
String reqUri = targetUri + "?storageExpand=true";
httpClient.request(reqMethod, reqUri, asyncResult -> {

Check warning on line 505 in gateleen-expansion/src/main/java/org/swisspush/gateleen/expansion/ExpansionHandler.java

View check run for this annotation

Codecov / codecov/patch

gateleen-expansion/src/main/java/org/swisspush/gateleen/expansion/ExpansionHandler.java#L503-L505

Added lines #L503 - L505 were not covered by tests
if (asyncResult.failed()) {
log.warn("Failed request to {}: {}", targetUri + "?storageExpand=true", asyncResult.cause());
return;
Expand All @@ -512,16 +523,23 @@ private void makeStorageExpandRequest(final String targetUri, final List subReso
cReq.write(payload);

cReq.send(event -> {
if (event.failed()) {
Throwable ex = event.cause();
log.debug("{} {}", reqMethod, reqUri, ex);
var exWrappr = new ResourceCollectionException(ex.getMessage(), INTERNAL_SERVER_ERROR);
handler.handle(new ResourceNode(SERIOUS_EXCEPTION, exWrappr));

Check warning on line 530 in gateleen-expansion/src/main/java/org/swisspush/gateleen/expansion/ExpansionHandler.java

View check run for this annotation

Codecov / codecov/patch

gateleen-expansion/src/main/java/org/swisspush/gateleen/expansion/ExpansionHandler.java#L527-L530

Added lines #L527 - L530 were not covered by tests
}
HttpClientResponse cRes = event.result();
cRes.bodyHandler(data -> {
if (StatusCode.NOT_FOUND.getStatusCode() == cRes.statusCode()) {
log.debug("requested resource could not be found: {}", targetUri);
log.debug("NotFound: {}", targetUri);

Check warning on line 535 in gateleen-expansion/src/main/java/org/swisspush/gateleen/expansion/ExpansionHandler.java

View check run for this annotation

Codecov / codecov/patch

gateleen-expansion/src/main/java/org/swisspush/gateleen/expansion/ExpansionHandler.java#L535

Added line #L535 was not covered by tests
handler.handle(new ResourceNode(SERIOUS_EXCEPTION, new ResourceCollectionException(cRes.statusMessage(), StatusCode.NOT_FOUND)));
} else if (StatusCode.INTERNAL_SERVER_ERROR.getStatusCode() == cRes.statusCode()) {
log.error("error in request resource : {} message : {}", targetUri, data.toString());
handler.handle(new ResourceNode(SERIOUS_EXCEPTION, new ResourceCollectionException(data.toString(), StatusCode.INTERNAL_SERVER_ERROR)));
String fullResponseBody = data.toString();
log.error("{}: {}: {}", INTERNAL_SERVER_ERROR, targetUri, fullResponseBody);
handler.handle(new ResourceNode(SERIOUS_EXCEPTION, new ResourceCollectionException(fullResponseBody, StatusCode.INTERNAL_SERVER_ERROR)));

Check warning on line 540 in gateleen-expansion/src/main/java/org/swisspush/gateleen/expansion/ExpansionHandler.java

View check run for this annotation

Codecov / codecov/patch

gateleen-expansion/src/main/java/org/swisspush/gateleen/expansion/ExpansionHandler.java#L538-L540

Added lines #L538 - L540 were not covered by tests
} else if (StatusCode.METHOD_NOT_ALLOWED.getStatusCode() == cRes.statusCode()) {
log.error("POST requests (storageExpand) not allowed for uri: {}", targetUri);
log.error("storageExpand not allowed for: {}", targetUri);

Check warning on line 542 in gateleen-expansion/src/main/java/org/swisspush/gateleen/expansion/ExpansionHandler.java

View check run for this annotation

Codecov / codecov/patch

gateleen-expansion/src/main/java/org/swisspush/gateleen/expansion/ExpansionHandler.java#L542

Added line #L542 was not covered by tests
handler.handle(new ResourceNode(SERIOUS_EXCEPTION, new ResourceCollectionException(cRes.statusMessage(), StatusCode.METHOD_NOT_ALLOWED)));
} else {
String eTag = geteTag(cRes.headers());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import static io.vertx.core.http.HttpMethod.DELETE;
import static io.vertx.core.http.HttpMethod.PUT;
import static org.swisspush.gateleen.core.util.HttpRequestHeader.CONTENT_LENGTH;

/**
Expand Down Expand Up @@ -502,24 +504,25 @@ public boolean handle(final RoutingContext ctx) {
/*
* 1) Un- / Register Listener / Routes
*/
if (isHookListenerRegistration(request)) {
handleListenerRegistration(request);
return true;
}

if (isHookListenerUnregistration(request)) {
handleListenerUnregistration(request);
return true;
}

if (isHookRouteRegistration(request)) {
handleRouteRegistration(request);
return true;
if (request.method() == PUT) {
if (request.uri().contains(HOOKS_LISTENERS_URI_PART)) {
handleListenerRegistration(request);
return true;
}
if (request.uri().contains(HOOKS_ROUTE_URI_PART)) {
handleRouteRegistration(request);
return true;
}
}

if (isHookRouteUnregistration(request)) {
handleRouteUnregistration(request);
return true;
if (request.method() == DELETE) {
if (request.uri().contains(HOOKS_LISTENERS_URI_PART)) {
handleListenerUnregistration(request);
return true;

Check warning on line 520 in gateleen-hook/src/main/java/org/swisspush/gateleen/hook/HookHandler.java

View check run for this annotation

Codecov / codecov/patch

gateleen-hook/src/main/java/org/swisspush/gateleen/hook/HookHandler.java#L519-L520

Added lines #L519 - L520 were not covered by tests
}
if (request.uri().contains(HOOKS_ROUTE_URI_PART)) {
handleRouteUnregistration(request);
return true;

Check warning on line 524 in gateleen-hook/src/main/java/org/swisspush/gateleen/hook/HookHandler.java

View check run for this annotation

Codecov / codecov/patch

gateleen-hook/src/main/java/org/swisspush/gateleen/hook/HookHandler.java#L523-L524

Added lines #L523 - L524 were not covered by tests
}
}

/*
Expand Down Expand Up @@ -1654,46 +1657,6 @@ private String getListenerUrlSegment(String requestUrl) {
return requestUrl.substring(pos + HOOKS_LISTENERS_URI_PART.length());
}

/**
* Checks if the given request is a listener unregistration instruction.
*
* @param request request
* @return boolean
*/
private boolean isHookListenerUnregistration(HttpServerRequest request) {
return request.uri().contains(HOOKS_LISTENERS_URI_PART) && HttpMethod.DELETE == request.method();
}

/**
* Checks if the given request is a listener registration instruction.
*
* @param request request
* @return boolean
*/
private boolean isHookListenerRegistration(HttpServerRequest request) {
return request.uri().contains(HOOKS_LISTENERS_URI_PART) && HttpMethod.PUT == request.method();
}

/**
* Checks if the given request is a route registration instruction.
*
* @param request request
* @return boolean
*/
private boolean isHookRouteRegistration(HttpServerRequest request) {
return request.uri().contains(HOOKS_ROUTE_URI_PART) && HttpMethod.PUT == request.method();
}

/**
* Checks if the given request is a route registration instruction.
*
* @param request request
* @return boolean
*/
private boolean isHookRouteUnregistration(HttpServerRequest request) {
return request.uri().contains(HOOKS_ROUTE_URI_PART) && HttpMethod.DELETE == request.method();
}

/**
* @param request Request to extract the value from. This instance gets manipulated
* internally during call.
Expand Down

0 comments on commit 67f673e

Please sign in to comment.