From 791e0162b074569ddbbc42b89e1e37ea4f36690d Mon Sep 17 00:00:00 2001 From: runner Date: Fri, 19 Apr 2024 09:27:06 +0000 Subject: [PATCH 1/8] updating poms for 3.1.4-SNAPSHOT development --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f862d79..2eb2443 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 org.swisspush rest-storage - 3.1.3-SNAPSHOT + 3.1.4-SNAPSHOT rest-storage Persistence for REST resources in the filesystem or a redis database From 3e906c2e5d6b6e9fc24fb2b0dc8f83bda3c11889 Mon Sep 17 00:00:00 2001 From: runner Date: Fri, 19 Apr 2024 09:27:07 +0000 Subject: [PATCH 2/8] updating poms for branch'release-3.1.3' with non-snapshot versions --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index f862d79..865ffa1 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 org.swisspush rest-storage - 3.1.3-SNAPSHOT + 3.1.3 rest-storage Persistence for REST resources in the filesystem or a redis database From 95af67865b91ca1e08e1310760400e3b650213bc Mon Sep 17 00:00:00 2001 From: runner Date: Fri, 19 Apr 2024 09:28:34 +0000 Subject: [PATCH 3/8] updating develop poms to master versions to avoid merge conflicts --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 2eb2443..865ffa1 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 org.swisspush rest-storage - 3.1.4-SNAPSHOT + 3.1.3 rest-storage Persistence for REST resources in the filesystem or a redis database From 35b482eab02f1f3b6a0bf4d51f5ca22e8c271107 Mon Sep 17 00:00:00 2001 From: runner Date: Fri, 19 Apr 2024 09:28:34 +0000 Subject: [PATCH 4/8] Updating develop poms back to pre merge state --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 865ffa1..2eb2443 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 org.swisspush rest-storage - 3.1.3 + 3.1.4-SNAPSHOT rest-storage Persistence for REST resources in the filesystem or a redis database From 330952f3a2614972486a8939c42e07e29067fb79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Weber?= Date: Wed, 1 May 2024 11:10:09 +0200 Subject: [PATCH 5/8] #173 Add limit for StorageExpand feature --- README.md | 8 ++- .../reststorage/RestStorageHandler.java | 38 ++++++++--- .../reststorage/util/HttpRequestHeader.java | 1 + .../reststorage/util/ModuleConfiguration.java | 10 +++ .../reststorage/util/StatusCode.java | 1 + .../StorageExpandIntegrationTest.java | 68 +++++++++++++++++++ .../RedisStorageIntegrationTestCase.java | 1 + .../util/ModuleConfigurationTest.java | 9 +++ 8 files changed, 126 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 9db9eaa..d860fff 100644 --- a/README.md +++ b/README.md @@ -127,12 +127,17 @@ would lead to this result To use the StorageExpand feature you have to make a POST request to the desired collection to expand having the url parameter **storageExpand=true**. Also, you wil have to send the names of the sub resources in the body of the request. Using the example above, the request would look like this: -**POST /yourStorageURL/collection** with the body: +**POST /yourStorageURL/collection?storageExpand=true** with the body: ```json { "subResources" : ["resource1", "resource2", "resource3"] } ``` +The amount of sub resources that can be provided is defined in the configuration by the property _maxStorageExpandSubresources_. + +To override this for a single request, add the following request header with an appropriate value: +> x-max-expand-resources: 1500 + ### Reject PUT requests on low memory (redis only) The redis storage provides a feature to reject PUT requests when the memory gets low. The information about the used memory is provided by the @@ -201,6 +206,7 @@ The following configuration values are available: | storageAddress | common | resource-storage | The eventbus address the mod listens to. | | editorConfig | common | | Additional configuration values for the editor | | confirmCollectionDelete | common | false | When set to _true_, an additional _recursive=true_ url parameter has to be set to delete collections | +| maxStorageExpandSubresources | common | 1000 | The amount of sub resources to expand. When limit exceeded, _413 Payload Too Large_ is returned | | redisHost | redis | localhost | The host where redis is running on | | redisPort | redis | 6379 | The port where redis is running on | | redisReconnectAttempts | redis | 0 | The amount of reconnect attempts when connection to redis is lost. Use _-1_ for continuous reconnects or _0_ for no reconnects at all | diff --git a/src/main/java/org/swisspush/reststorage/RestStorageHandler.java b/src/main/java/org/swisspush/reststorage/RestStorageHandler.java index a91ec1d..c271438 100644 --- a/src/main/java/org/swisspush/reststorage/RestStorageHandler.java +++ b/src/main/java/org/swisspush/reststorage/RestStorageHandler.java @@ -42,6 +42,7 @@ public class RestStorageHandler implements Handler { private final boolean rejectStorageWriteOnLowMemory; private final boolean return200onDeleteNonExisting; private final DecimalFormat decimalFormat; + private final Integer maxStorageExpandSubresources; public RestStorageHandler(Vertx vertx, final Logger log, final Storage storage, final ModuleConfiguration config) { this.router = Router.router(vertx); @@ -51,6 +52,7 @@ public RestStorageHandler(Vertx vertx, final Logger log, final Storage storage, this.confirmCollectionDelete = config.isConfirmCollectionDelete(); this.rejectStorageWriteOnLowMemory = config.isRejectStorageWriteOnLowMemory(); this.return200onDeleteNonExisting = config.isReturn200onDeleteNonExisting(); + this.maxStorageExpandSubresources = config.getMaxStorageExpandSubresources(); this.decimalFormat = new DecimalFormat(); this.decimalFormat.setMaximumFractionDigits(1); @@ -586,17 +588,35 @@ private void deleteResource(RoutingContext ctx) { }); } + private boolean checkMaxSubResourcesCount(HttpServerRequest request, int subResourcesArraySize) { + MultiMap headers = request.headers(); + + // get max expand value from request header of use the configuration value as fallback + Integer maxStorageExpand = getInteger(headers, MAX_EXPAND_RESOURCES_HEADER, maxStorageExpandSubresources); + if (maxStorageExpand < subResourcesArraySize) { + respondWith(request.response(), StatusCode.PAYLOAD_TOO_LARGE, + "Resources provided: "+subResourcesArraySize+". Allowed are: " + maxStorageExpand); + return true; + } + return false; + } + private void storageExpand(RoutingContext ctx) { - if (!containsParam(ctx.request().params(), STORAGE_EXPAND_PARAMETER)) { - respondWithNotAllowed(ctx.request()); + HttpServerRequest request = ctx.request(); + if (!containsParam(request.params(), STORAGE_EXPAND_PARAMETER)) { + respondWithNotAllowed(request); } else { - ctx.request().bodyHandler(bodyBuf -> { + request.bodyHandler(bodyBuf -> { List subResourceNames = new ArrayList<>(); try { JsonObject body = new JsonObject(bodyBuf); JsonArray subResourcesArray = body.getJsonArray("subResources"); if (subResourcesArray == null) { - respondWithBadRequest(ctx.request(), "Bad Request: Expected array field 'subResources' with names of resources"); + respondWithBadRequest(request, "Bad Request: Expected array field 'subResources' with names of resources"); + return; + } + + if (checkMaxSubResourcesCount(request, subResourcesArray.size())) { return; } @@ -606,12 +626,12 @@ private void storageExpand(RoutingContext ctx) { ResourceNameUtil.replaceColonsAndSemiColonsInList(subResourceNames); } catch (RuntimeException ex) { log.warn("KISS handler is not interested in error details. I'll report them here then.", ex); - respondWithBadRequest(ctx.request(), "Bad Request: Unable to parse body of storageExpand POST request"); + respondWithBadRequest(request, "Bad Request: Unable to parse body of storageExpand POST request"); return; } - final String path = cleanPath(ctx.request().path().substring(prefixFixed.length())); - final String etag = ctx.request().headers().get(IF_NONE_MATCH_HEADER.getName()); + final String path = cleanPath(request.path().substring(prefixFixed.length())); + final String etag = request.headers().get(IF_NONE_MATCH_HEADER.getName()); storage.storageExpand(path, etag, subResourceNames, resource -> { var rsp = ctx.response(); @@ -649,7 +669,7 @@ private void storageExpand(RoutingContext ctx) { if (resource.exists) { if (log.isTraceEnabled()) { - log.trace("RestStorageHandler resource is a DocumentResource: {}", ctx.request().uri()); + log.trace("RestStorageHandler resource is a DocumentResource: {}", request.uri()); } String mimeType = mimeTypeResolver.resolveMimeType(path); @@ -669,7 +689,7 @@ private void storageExpand(RoutingContext ctx) { } else { if (log.isTraceEnabled()) { - log.trace("RestStorageHandler Could not find resource: {}", ctx.request().uri()); + log.trace("RestStorageHandler Could not find resource: {}", request.uri()); } rsp.setStatusCode(StatusCode.NOT_FOUND.getStatusCode()); rsp.setStatusMessage(StatusCode.NOT_FOUND.getStatusMessage()); diff --git a/src/main/java/org/swisspush/reststorage/util/HttpRequestHeader.java b/src/main/java/org/swisspush/reststorage/util/HttpRequestHeader.java index 60db461..b34287e 100644 --- a/src/main/java/org/swisspush/reststorage/util/HttpRequestHeader.java +++ b/src/main/java/org/swisspush/reststorage/util/HttpRequestHeader.java @@ -16,6 +16,7 @@ public enum HttpRequestHeader { LOCK_EXPIRE_AFTER_HEADER("x-lock-expire-after"), EXPIRE_AFTER_HEADER("x-expire-after"), IMPORTANCE_LEVEL_HEADER("x-importance-level"), + MAX_EXPAND_RESOURCES_HEADER("x-max-expand-resources"), COMPRESS_HEADER("x-stored-compressed"), CONTENT_TYPE("Content-Type"), CONTENT_LENGTH("Content-Length"); diff --git a/src/main/java/org/swisspush/reststorage/util/ModuleConfiguration.java b/src/main/java/org/swisspush/reststorage/util/ModuleConfiguration.java index efaebd8..5e5604f 100644 --- a/src/main/java/org/swisspush/reststorage/util/ModuleConfiguration.java +++ b/src/main/java/org/swisspush/reststorage/util/ModuleConfiguration.java @@ -67,6 +67,7 @@ public enum StorageType { private int maxRedisConnectionPoolSize = 24; private int maxQueueWaiting = 24; private int maxRedisWaitingHandlers = 2048; + private int maxStorageExpandSubresources = 1000; public ModuleConfiguration root(String root) { @@ -280,6 +281,11 @@ public ModuleConfiguration return200onDeleteNonExisting(boolean deleteNonExistin return this; } + public ModuleConfiguration maxStorageExpandSubresources(int maxStorageExpandSubresources) { + this.maxStorageExpandSubresources = maxStorageExpandSubresources; + return this; + } + public String getRoot() { return root; } @@ -445,6 +451,10 @@ public int getMaxRedisWaitingHandlers() { return maxRedisWaitingHandlers; } + public int getMaxStorageExpandSubresources() { + return maxStorageExpandSubresources; + } + public JsonObject asJsonObject() { return JsonObject.mapFrom(this); } diff --git a/src/main/java/org/swisspush/reststorage/util/StatusCode.java b/src/main/java/org/swisspush/reststorage/util/StatusCode.java index 3d5bc5c..74aaced 100644 --- a/src/main/java/org/swisspush/reststorage/util/StatusCode.java +++ b/src/main/java/org/swisspush/reststorage/util/StatusCode.java @@ -13,6 +13,7 @@ public enum StatusCode { UNAUTHORIZED(401, "Unauthorized"), NOT_FOUND(404, "Not Found"), METHOD_NOT_ALLOWED(405, "Method Not Allowed"), + PAYLOAD_TOO_LARGE(413, "Payload Too Large"), INTERNAL_SERVER_ERROR(500, "Internal Server Error"), INSUFFICIENT_STORAGE(507, "Insufficient Storage"), CONFLICT(409, "Conflict"); diff --git a/src/test/java/org/swisspush/reststorage/StorageExpandIntegrationTest.java b/src/test/java/org/swisspush/reststorage/StorageExpandIntegrationTest.java index 6c117ff..8c67927 100644 --- a/src/test/java/org/swisspush/reststorage/StorageExpandIntegrationTest.java +++ b/src/test/java/org/swisspush/reststorage/StorageExpandIntegrationTest.java @@ -18,6 +18,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.swisspush.reststorage.redis.RedisStorageIntegrationTestCase; +import org.swisspush.reststorage.util.HttpRequestHeader; import static io.restassured.RestAssured.*; import static org.hamcrest.Matchers.*; @@ -250,6 +251,73 @@ public void testSimpleWithEmptyResult(TestContext context) { async.complete(); } + @Test + public void testWithTooManySubResources(TestContext context) { + Async async = context.async(); + delete("/server/resources"); + + // by configuration, only 5 sub resources are allowed + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\"] }") + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(413).body(containsString("Resources provided: 6. Allowed are: 5")); + + // request header overwrites configuration + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\"] }") + .header(HttpRequestHeader.MAX_EXPAND_RESOURCES_HEADER.getName(), 2) + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(413).body(containsString("Resources provided: 6. Allowed are: 2")); + + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\", \"res7\", \"res8\", \"res9\"] }") + .header(HttpRequestHeader.MAX_EXPAND_RESOURCES_HEADER.getName(), 8) + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(413).body(containsString("Resources provided: 9. Allowed are: 8")); + + // invalid header value + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\", \"res7\", \"res8\", \"res9\"] }") + .header(HttpRequestHeader.MAX_EXPAND_RESOURCES_HEADER.getName(), "Not a number") + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(413).body(containsString("Resources provided: 9. Allowed are: 5")); + + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\", \"res7\", \"res8\", \"res9\"] }") + .header(HttpRequestHeader.MAX_EXPAND_RESOURCES_HEADER.getName(), 999999999999999999L) + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(413).body(containsString("Resources provided: 9. Allowed are: 5")); + + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\", \"res7\", \"res8\", \"res9\"] }") + .header(HttpRequestHeader.MAX_EXPAND_RESOURCES_HEADER.getName(), 25.5) + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(413).body(containsString("Resources provided: 9. Allowed are: 5")); + + // no 413 Payload Too Large response when limit not exceeded + given() + .body("{ \"subResources\": [\"res1\", \"res2\", \"res3\", \"res4\", \"res5\", \"res6\"] }") + .header(HttpRequestHeader.MAX_EXPAND_RESOURCES_HEADER.getName(), 10) + .when() + .post(POST_STORAGE_EXP) + .then() + .assertThat().statusCode(404); + + async.complete(); + } + @Test public void testSimpleWithResourcesAndCollections(TestContext context) { Async async = context.async(); diff --git a/src/test/java/org/swisspush/reststorage/redis/RedisStorageIntegrationTestCase.java b/src/test/java/org/swisspush/reststorage/redis/RedisStorageIntegrationTestCase.java index e9831e1..138bd6c 100644 --- a/src/test/java/org/swisspush/reststorage/redis/RedisStorageIntegrationTestCase.java +++ b/src/test/java/org/swisspush/reststorage/redis/RedisStorageIntegrationTestCase.java @@ -34,6 +34,7 @@ public void setUp(TestContext context) { ModuleConfiguration modConfig = new ModuleConfiguration() .storageType(ModuleConfiguration.StorageType.redis) .confirmCollectionDelete(true) + .maxStorageExpandSubresources(5) .storageAddress("rest-storage"); updateModuleConfiguration(modConfig); diff --git a/src/test/java/org/swisspush/reststorage/util/ModuleConfigurationTest.java b/src/test/java/org/swisspush/reststorage/util/ModuleConfigurationTest.java index b1be26f..125647d 100644 --- a/src/test/java/org/swisspush/reststorage/util/ModuleConfigurationTest.java +++ b/src/test/java/org/swisspush/reststorage/util/ModuleConfigurationTest.java @@ -60,6 +60,7 @@ public void testDefaultConfiguration(TestContext testContext) { testContext.assertEquals(config.getFreeMemoryCheckIntervalMs(), 60000L); testContext.assertFalse(config.isReturn200onDeleteNonExisting()); testContext.assertEquals(config.getMaxRedisWaitingHandlers(), 2048); + testContext.assertEquals(config.getMaxStorageExpandSubresources(), 1000); } @Test @@ -85,6 +86,7 @@ public void testOverrideConfiguration(TestContext testContext) { .redisPublishMetrcisAddress("metrics-eb-address") .redisPublishMetrcisPrefix("my-storage") .redisPublishMetrcisRefreshPeriodSec(20) + .maxStorageExpandSubresources(500) .return200onDeleteNonExisting(true); // go through JSON encode/decode @@ -127,6 +129,7 @@ public void testOverrideConfiguration(TestContext testContext) { testContext.assertEquals(config.getRedisPublishMetrcisAddress(), "metrics-eb-address"); testContext.assertEquals(config.getRedisPublishMetrcisPrefix(), "my-storage"); testContext.assertEquals(config.getRedisPublishMetrcisRefreshPeriodSec(), 20); + testContext.assertEquals(config.getMaxStorageExpandSubresources(), 500); } @Test @@ -167,6 +170,7 @@ public void testGetDefaultAsJsonObject(TestContext testContext){ testContext.assertFalse(json.getBoolean("confirmCollectionDelete")); testContext.assertFalse(json.getBoolean("rejectStorageWriteOnLowMemory")); testContext.assertEquals(json.getLong("freeMemoryCheckIntervalMs"), 60000L); + testContext.assertEquals(json.getInteger("maxStorageExpandSubresources"), 1000); } @Test @@ -193,6 +197,7 @@ public void testGetOverriddenAsJsonObject(TestContext testContext){ .confirmCollectionDelete(true) .rejectStorageWriteOnLowMemory(true) .resourceCleanupIntervalSec(15) + .maxStorageExpandSubresources(500) .freeMemoryCheckIntervalMs(5000); JsonObject json = config.asJsonObject(); @@ -225,6 +230,7 @@ public void testGetOverriddenAsJsonObject(TestContext testContext){ testContext.assertEquals(json.getLong("freeMemoryCheckIntervalMs"), 5000L); testContext.assertEquals(json.getInteger("maxRedisWaitingHandlers"), 4096); testContext.assertEquals(json.getInteger("resourceCleanupIntervalSec"), 15); + testContext.assertEquals(json.getInteger("maxStorageExpandSubresources"), 500); testContext.assertNotNull(json.getJsonObject("editorConfig")); testContext.assertTrue(json.getJsonObject("editorConfig").containsKey("myKey")); @@ -275,6 +281,7 @@ public void testGetDefaultFromJsonObject(TestContext testContext){ testContext.assertNull(config.getRedisPublishMetrcisAddress(), "metrics-eb-address"); testContext.assertEquals(config.getRedisPublishMetrcisPrefix(), "storage"); testContext.assertEquals(config.getRedisPublishMetrcisRefreshPeriodSec(), 10); + testContext.assertEquals(config.getMaxStorageExpandSubresources(), 1000); } @Test @@ -312,6 +319,7 @@ public void testGetOverriddenFromJsonObject(TestContext testContext){ json.put("redisPublishMetrcisAddress", "metrics-eb-address"); json.put("redisPublishMetrcisPrefix", "my-storage"); json.put("redisPublishMetrcisRefreshPeriodSec", 30); + json.put("maxStorageExpandSubresources", 250); ModuleConfiguration config = fromJsonObject(json); testContext.assertEquals(config.getRoot(), "newroot"); @@ -346,6 +354,7 @@ public void testGetOverriddenFromJsonObject(TestContext testContext){ testContext.assertTrue(config.isConfirmCollectionDelete()); testContext.assertTrue(config.isRejectStorageWriteOnLowMemory()); testContext.assertEquals(config.getFreeMemoryCheckIntervalMs(), 30000L); + testContext.assertEquals(config.getMaxStorageExpandSubresources(), 250); testContext.assertEquals(config.getRedisPublishMetrcisAddress(), "metrics-eb-address"); testContext.assertEquals(config.getRedisPublishMetrcisPrefix(), "my-storage"); From ff5b5114155e1fea4257ab8963298445a1ef22bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Weber?= Date: Wed, 1 May 2024 11:35:55 +0200 Subject: [PATCH 6/8] Some code cleanup --- .../FileSystemRestStorageRunner.java | 1 - .../reststorage/MimeTypeResolver.java | 3 +- .../ModuleConfigurationAuthentication.java | 1 - .../reststorage/RedisRestStorageRunner.java | 1 - .../reststorage/RestStorageHandler.java | 13 ++---- .../reststorage/redis/RedisStorage.java | 46 ++----------------- .../reststorage/util/ResourcesUtils.java | 2 +- src/main/resources/mime-types.properties | 2 +- .../reststorage/EtagIntegrationTest.java | 2 +- .../reststorage/FilesystemStorageTest.java | 2 +- .../reststorage/RestStorageHandlerTest.java | 2 +- .../lua/RedisCleanupLuaScriptTests.java | 2 +- .../lua/RedisDelLuaScriptTests.java | 3 +- .../lua/RedisGetLuaScriptTests.java | 2 +- .../lua/RedisPutLuaScriptTests.java | 2 +- .../lua/RedisStorageExpandLuaScriptTests.java | 2 +- .../mocks/FailFastVertxWebRoutingContext.java | 1 - 17 files changed, 19 insertions(+), 68 deletions(-) diff --git a/src/main/java/org/swisspush/reststorage/FileSystemRestStorageRunner.java b/src/main/java/org/swisspush/reststorage/FileSystemRestStorageRunner.java index 7c0cb31..94e9ef6 100644 --- a/src/main/java/org/swisspush/reststorage/FileSystemRestStorageRunner.java +++ b/src/main/java/org/swisspush/reststorage/FileSystemRestStorageRunner.java @@ -3,7 +3,6 @@ import io.vertx.core.AsyncResult; import io.vertx.core.Vertx; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import static org.slf4j.LoggerFactory.getLogger; diff --git a/src/main/java/org/swisspush/reststorage/MimeTypeResolver.java b/src/main/java/org/swisspush/reststorage/MimeTypeResolver.java index a87ea8d..928d721 100644 --- a/src/main/java/org/swisspush/reststorage/MimeTypeResolver.java +++ b/src/main/java/org/swisspush/reststorage/MimeTypeResolver.java @@ -1,7 +1,6 @@ package org.swisspush.reststorage; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.InputStream; @@ -14,7 +13,7 @@ public class MimeTypeResolver { private static final Logger log = getLogger(MimeTypeResolver.class); - private Map mimeTypes = new HashMap<>(); + private final Map mimeTypes = new HashMap<>(); private final String defaultMimeType; diff --git a/src/main/java/org/swisspush/reststorage/ModuleConfigurationAuthentication.java b/src/main/java/org/swisspush/reststorage/ModuleConfigurationAuthentication.java index 167ca7c..f3cceb2 100644 --- a/src/main/java/org/swisspush/reststorage/ModuleConfigurationAuthentication.java +++ b/src/main/java/org/swisspush/reststorage/ModuleConfigurationAuthentication.java @@ -9,7 +9,6 @@ import io.vertx.ext.auth.authentication.Credentials; import io.vertx.ext.auth.authentication.UsernamePasswordCredentials; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.swisspush.reststorage.util.ModuleConfiguration; import java.util.Objects; diff --git a/src/main/java/org/swisspush/reststorage/RedisRestStorageRunner.java b/src/main/java/org/swisspush/reststorage/RedisRestStorageRunner.java index 8a95ed4..2a91874 100644 --- a/src/main/java/org/swisspush/reststorage/RedisRestStorageRunner.java +++ b/src/main/java/org/swisspush/reststorage/RedisRestStorageRunner.java @@ -4,7 +4,6 @@ import io.vertx.core.DeploymentOptions; import io.vertx.core.Vertx; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.swisspush.reststorage.util.ModuleConfiguration; import static org.slf4j.LoggerFactory.getLogger; diff --git a/src/main/java/org/swisspush/reststorage/RestStorageHandler.java b/src/main/java/org/swisspush/reststorage/RestStorageHandler.java index a91ec1d..209f0e7 100644 --- a/src/main/java/org/swisspush/reststorage/RestStorageHandler.java +++ b/src/main/java/org/swisspush/reststorage/RestStorageHandler.java @@ -16,7 +16,6 @@ import org.slf4j.Logger; import org.swisspush.reststorage.util.*; -import java.nio.charset.StandardCharsets; import java.text.DecimalFormat; import java.util.*; @@ -192,7 +191,7 @@ public void handle(Resource resource) { StringBuilder body = new StringBuilder(1024); String editor = null; - if (editors.size() > 0) { + if (!editors.isEmpty()) { editor = editors.values().iterator().next(); } body.append("\n"); @@ -277,12 +276,8 @@ public void handle(Resource resource) { documentResource.closeHandler.handle(null); rsp.end(); }); - documentResource.addErrorHandler(ex -> { - log.error("TODO error handling", new Exception(ex)); - }); - documentResource.readStream.exceptionHandler((Handler) ex -> { - log.error("TODO error handling", new Exception(ex)); - }); + documentResource.addErrorHandler(ex -> log.error("TODO error handling", new Exception(ex))); + documentResource.readStream.exceptionHandler(ex -> log.error("TODO error handling", new Exception(ex))); pump.start(); } } @@ -737,7 +732,7 @@ private void respondWith(HttpServerResponse response, StatusCode statusCode, Str } private String collectionName(String path) { - if (path.equals("/") || path.equals("")) { + if (path.equals("/") || path.isEmpty()) { return "root"; } else { return path.substring(path.lastIndexOf("/") + 1); diff --git a/src/main/java/org/swisspush/reststorage/redis/RedisStorage.java b/src/main/java/org/swisspush/reststorage/redis/RedisStorage.java index 94e67ed..e623fc4 100644 --- a/src/main/java/org/swisspush/reststorage/redis/RedisStorage.java +++ b/src/main/java/org/swisspush/reststorage/redis/RedisStorage.java @@ -51,6 +51,8 @@ import java.util.UUID; import java.util.stream.Stream; +import static org.swisspush.reststorage.redis.RedisUtils.toPayload; + public class RedisStorage implements Storage { private final Logger log = LoggerFactory.getLogger(RedisStorage.class); @@ -815,7 +817,7 @@ private void handleJsonArrayValues(Response values, Handler handler, b } } } - if (allowEmptyReturn && items.size() == 0) { + if (allowEmptyReturn && items.isEmpty()) { notFound(handler); } else { r.items = new ArrayList<>(items); @@ -1293,46 +1295,4 @@ public void cleanup(Handler handler, String cleanupResourcesAm private boolean isEmpty(CharSequence cs) { return cs == null || cs.length() == 0; } - - /** - * from https://github.com/vert-x3/vertx-redis-client/blob/3.9/src/main/java/io/vertx/redis/impl/RedisClientImpl.java#L94 - * - * @param parameters - * @return - */ - private static List toPayload(Object... parameters) { - List result = new ArrayList<>(parameters.length); - - for (Object param : parameters) { - // unwrap - if (param instanceof JsonArray) { - param = ((JsonArray) param).getList(); - } - // unwrap - if (param instanceof JsonObject) { - param = ((JsonObject) param).getMap(); - } - - if (param instanceof Collection) { - ((Collection) param).stream().filter(Objects::nonNull).forEach(o -> result.add(o.toString())); - } else if (param instanceof Map) { - for (Map.Entry pair : ((Map) param).entrySet()) { - result.add(pair.getKey().toString()); - result.add(pair.getValue().toString()); - } - } else if (param instanceof Stream) { - ((Stream) param).forEach(e -> { - if (e instanceof Object[]) { - Collections.addAll(result, (String[]) e); - } else { - result.add(e.toString()); - } - }); - } else if (param != null) { - result.add(param.toString()); - } - } - return result; - } - } diff --git a/src/main/java/org/swisspush/reststorage/util/ResourcesUtils.java b/src/main/java/org/swisspush/reststorage/util/ResourcesUtils.java index 853e02b..7727773 100644 --- a/src/main/java/org/swisspush/reststorage/util/ResourcesUtils.java +++ b/src/main/java/org/swisspush/reststorage/util/ResourcesUtils.java @@ -16,7 +16,7 @@ */ public class ResourcesUtils { - private static Logger log = LoggerFactory.getLogger(ResourcesUtils.class); + private static final Logger log = LoggerFactory.getLogger(ResourcesUtils.class); private ResourcesUtils() { // prevent instantiation diff --git a/src/main/resources/mime-types.properties b/src/main/resources/mime-types.properties index bafe6c9..71b4ac0 100644 --- a/src/main/resources/mime-types.properties +++ b/src/main/resources/mime-types.properties @@ -28,7 +28,7 @@ aso=application/vnd.accpac.simply.aso atc=application/vnd.acucorp atomcat=application/atomcat+xml atomsvc=application/atomsvc+xml -atom, xml=application/atom+xml +atomxml=application/atom+xml atx=application/vnd.antix.game-component au=audio/basic avi=video/x-msvideo diff --git a/src/test/java/org/swisspush/reststorage/EtagIntegrationTest.java b/src/test/java/org/swisspush/reststorage/EtagIntegrationTest.java index a98c3cc..182aa6b 100644 --- a/src/test/java/org/swisspush/reststorage/EtagIntegrationTest.java +++ b/src/test/java/org/swisspush/reststorage/EtagIntegrationTest.java @@ -26,7 +26,7 @@ public void testEtag(TestContext context) { String etag = get("resources/res1").getHeader(ETAG_HEADER); context.assertNotNull(etag, "Etag header should be available in response headers"); - context.assertTrue(etag.length() > 0, "Etag header should not be empty"); + context.assertFalse(etag.isEmpty(), "Etag header should not be empty"); // get requests with no if-none-match header should result in statuscode 200 when().get("resources/res1").then().assertThat() diff --git a/src/test/java/org/swisspush/reststorage/FilesystemStorageTest.java b/src/test/java/org/swisspush/reststorage/FilesystemStorageTest.java index 4f18fb9..491b01f 100644 --- a/src/test/java/org/swisspush/reststorage/FilesystemStorageTest.java +++ b/src/test/java/org/swisspush/reststorage/FilesystemStorageTest.java @@ -346,6 +346,6 @@ private void doAsserts() { * check if such a directory exists or someone has access to it. */ private static String createPseudoFileStorageRoot() { - return new File( "target/fileStorage-"+ UUID.randomUUID().toString() ).getAbsolutePath().replaceAll("\\\\","/"); // <-- Fix windows + return new File( "target/fileStorage-"+ UUID.randomUUID()).getAbsolutePath().replaceAll("\\\\","/"); // <-- Fix windows } } diff --git a/src/test/java/org/swisspush/reststorage/RestStorageHandlerTest.java b/src/test/java/org/swisspush/reststorage/RestStorageHandlerTest.java index 42d56fe..e45c1c4 100644 --- a/src/test/java/org/swisspush/reststorage/RestStorageHandlerTest.java +++ b/src/test/java/org/swisspush/reststorage/RestStorageHandlerTest.java @@ -312,7 +312,7 @@ public void notifiesResourceAboutExceptionsOnRequest(TestContext testContext) th @Override public void put(String path, String etag, boolean merge, long expire, String lockOwner, LockMode lockMode, long lockExpire, boolean storeCompressed, Handler handler) { final DocumentResource resource = new DocumentResource(); - resource.writeStream = new FailFastVertxWriteStream() { + resource.writeStream = new FailFastVertxWriteStream<>() { @Override public Future write(Buffer t) { log.debug("Somewhat irrelevant got written to the resource."); diff --git a/src/test/java/org/swisspush/reststorage/lua/RedisCleanupLuaScriptTests.java b/src/test/java/org/swisspush/reststorage/lua/RedisCleanupLuaScriptTests.java index cb9d1bf..14d9b5e 100644 --- a/src/test/java/org/swisspush/reststorage/lua/RedisCleanupLuaScriptTests.java +++ b/src/test/java/org/swisspush/reststorage/lua/RedisCleanupLuaScriptTests.java @@ -11,7 +11,7 @@ import java.util.UUID; import static org.hamcrest.CoreMatchers.equalTo; -import static org.junit.Assert.assertThat; +import static org.hamcrest.MatcherAssert.assertThat; public class RedisCleanupLuaScriptTests extends AbstractLuaScriptTest { diff --git a/src/test/java/org/swisspush/reststorage/lua/RedisDelLuaScriptTests.java b/src/test/java/org/swisspush/reststorage/lua/RedisDelLuaScriptTests.java index 5fae971..2bde3d4 100644 --- a/src/test/java/org/swisspush/reststorage/lua/RedisDelLuaScriptTests.java +++ b/src/test/java/org/swisspush/reststorage/lua/RedisDelLuaScriptTests.java @@ -9,7 +9,8 @@ import static org.awaitility.Awaitility.await; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; +import static org.hamcrest.MatcherAssert.assertThat; + public class RedisDelLuaScriptTests extends AbstractLuaScriptTest { diff --git a/src/test/java/org/swisspush/reststorage/lua/RedisGetLuaScriptTests.java b/src/test/java/org/swisspush/reststorage/lua/RedisGetLuaScriptTests.java index 51310a0..5c53629 100644 --- a/src/test/java/org/swisspush/reststorage/lua/RedisGetLuaScriptTests.java +++ b/src/test/java/org/swisspush/reststorage/lua/RedisGetLuaScriptTests.java @@ -5,7 +5,7 @@ import java.util.List; import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.assertThat; +import static org.hamcrest.MatcherAssert.assertThat; public class RedisGetLuaScriptTests extends AbstractLuaScriptTest { diff --git a/src/test/java/org/swisspush/reststorage/lua/RedisPutLuaScriptTests.java b/src/test/java/org/swisspush/reststorage/lua/RedisPutLuaScriptTests.java index 625492a..b0e4bed 100644 --- a/src/test/java/org/swisspush/reststorage/lua/RedisPutLuaScriptTests.java +++ b/src/test/java/org/swisspush/reststorage/lua/RedisPutLuaScriptTests.java @@ -11,7 +11,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; import static org.awaitility.Awaitility.await; import static org.hamcrest.CoreMatchers.*; -import static org.junit.Assert.assertThat; +import static org.hamcrest.MatcherAssert.assertThat; public class RedisPutLuaScriptTests extends AbstractLuaScriptTest { diff --git a/src/test/java/org/swisspush/reststorage/lua/RedisStorageExpandLuaScriptTests.java b/src/test/java/org/swisspush/reststorage/lua/RedisStorageExpandLuaScriptTests.java index 2cc4096..c57bb5c 100644 --- a/src/test/java/org/swisspush/reststorage/lua/RedisStorageExpandLuaScriptTests.java +++ b/src/test/java/org/swisspush/reststorage/lua/RedisStorageExpandLuaScriptTests.java @@ -19,7 +19,7 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; +import static org.hamcrest.MatcherAssert.assertThat; public class RedisStorageExpandLuaScriptTests extends AbstractLuaScriptTest { diff --git a/src/test/java/org/swisspush/reststorage/mocks/FailFastVertxWebRoutingContext.java b/src/test/java/org/swisspush/reststorage/mocks/FailFastVertxWebRoutingContext.java index d454f4e..bd6f630 100644 --- a/src/test/java/org/swisspush/reststorage/mocks/FailFastVertxWebRoutingContext.java +++ b/src/test/java/org/swisspush/reststorage/mocks/FailFastVertxWebRoutingContext.java @@ -17,7 +17,6 @@ import java.nio.charset.Charset; import java.util.List; import java.util.Map; -import java.util.Set; public class FailFastVertxWebRoutingContext implements RoutingContext { From 8ba246d647910782b36c94e149e842ec6cf8680b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Weber?= Date: Thu, 2 May 2024 08:20:50 +0200 Subject: [PATCH 7/8] #173 Set Content-Type for response payload containing text only --- src/main/java/org/swisspush/reststorage/RestStorageHandler.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/swisspush/reststorage/RestStorageHandler.java b/src/main/java/org/swisspush/reststorage/RestStorageHandler.java index c271438..d55fdc1 100644 --- a/src/main/java/org/swisspush/reststorage/RestStorageHandler.java +++ b/src/main/java/org/swisspush/reststorage/RestStorageHandler.java @@ -750,6 +750,7 @@ private void respondWith(HttpServerResponse response, StatusCode statusCode, Str response.setStatusCode(statusCode.getStatusCode()); response.setStatusMessage(statusCode.getStatusMessage()); if (responseBody != null) { + response.putHeader(CONTENT_TYPE.getName(), "text/plain"); response.end(responseBody); } else { response.end(); From 604fa52079f583c112f1c576f3334001c3e8c813 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Weber?= Date: Thu, 2 May 2024 11:05:33 +0200 Subject: [PATCH 8/8] typo --- src/main/java/org/swisspush/reststorage/RestStorageHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/swisspush/reststorage/RestStorageHandler.java b/src/main/java/org/swisspush/reststorage/RestStorageHandler.java index d55fdc1..cd1f05b 100644 --- a/src/main/java/org/swisspush/reststorage/RestStorageHandler.java +++ b/src/main/java/org/swisspush/reststorage/RestStorageHandler.java @@ -591,7 +591,7 @@ private void deleteResource(RoutingContext ctx) { private boolean checkMaxSubResourcesCount(HttpServerRequest request, int subResourcesArraySize) { MultiMap headers = request.headers(); - // get max expand value from request header of use the configuration value as fallback + // get max expand value from request header or use the configuration value as fallback Integer maxStorageExpand = getInteger(headers, MAX_EXPAND_RESOURCES_HEADER, maxStorageExpandSubresources); if (maxStorageExpand < subResourcesArraySize) { respondWith(request.response(), StatusCode.PAYLOAD_TOO_LARGE,