Skip to content

Commit

Permalink
#1706 fix review findings:
Browse files Browse the repository at this point in the history 
* simplify form parameter extraction by using Akka HTTP's `formFieldMultiMap(Route)`
* thereby also fix NPEs when a form parameter's value was empty
* thereby also don't require URL encoding of form params

Signed-off-by: Thomas Jäckle <thomas.jaeckle@beyonnex.io>
  • Loading branch information
@thjaeckle
thjaeckle committed Sep 13, 2023
1 parent 10e84dc commit 95589f8
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 97 deletions.
18 changes: 9 additions & 9 deletions documentation/src/main/resources/pages/ditto/httpapi-search.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Optionally a `namespaces` parameter can be added to search only in the given nam
## GET
### Query parameters

In order to define for which `Things` to search, the `filter` query parameter has to be added.<br/>
In order to define for which `Things` to search, the `filter` query parameter has to be added.
In order to change the sorting and limit the result (also to do paging), the `option` parameter has to be added.
Default values of each option is documented [here](basic-search.html#sorting-and-paging-options).

Expand All @@ -44,7 +44,7 @@ GET .../search/things?filter=eq(attributes/location,"living-room")&namespaces=or
```

The HTTP search API can also profit from the [partial request](httpapi-concepts.html#partial-requests) concept
of the API:<br/>
of the API:
Additionally to a `filter` and `options`, a `fields` parameter may be specified in order to select which data
of the result set to retrieve.

Expand Down Expand Up @@ -75,39 +75,39 @@ GET .../search/things/count?filter=eq(attributes/location,"living-room")
## POST
### x-www-form-urlencoded

In order to define for which `Things` to search, the key `filter` has to be used.<br/>
In order to define for which `Things` to search, the key `filter` has to be used.
In order to change the sorting and limit the result (also to do paging), the key `option` has to be used.
Default values of each option is documented [here](basic-search.html#sorting-and-paging-options).

Complex example:
```
POST .../search/things
body: filter=eq%28attributes%2Flocation%2C%22living-room%22%29&namespaces=org.eclipse.ditto%2Cfoo.bar&option=sort%28%2BthingId%29%2Climit%280%2C5%29
body: filter=eq(attributes/location,"living-room")&namespaces=org.eclipse.ditto,foo.bar&option=sort(+thingId),limit(0,5)
```

Another Complex example with the `namespaces` parameter:
```
POST .../search/things
body: filter=filter=eq%28attributes%2Flocation%2C%22living-room%22%29&namespaces=org.eclipse.ditto%2Cfoo.bar
body: filter=eq(attributes/location,"living-room")&namespaces=org.eclipse.ditto,foo.bar
```

The HTTP search API can also profit from the [partial request](httpapi-concepts.html#partial-requests) concept
of the API:<br/>
of the API:
Additionally to a `filter` and `options`, the key `fields` may be specified in order to select which data
of the result set to retrieve.

Example which only returns `thingId` and the `manufacturer` attribute of the found Things:
```
POST .../search/things
body: filter=eq%28attributes%2Flocation%2C%22living-room%22%29&fields=thingId%2Cattributes%2Fmanufacturer
body: filter=eq(attributes/location,"living-room")&fields=thingId,attributes/manufacturer
```

With the `namespaces` parameter, the result can be limited to the given namespaces.

Example which only returns Things with the given namespaces prefix:
```
POST .../search/things
body: namespaces=org.eclipse.ditto%2Cfoo.bar
body: namespaces=org.eclipse.ditto,foo.bar
```

### Search count
Expand All @@ -120,5 +120,5 @@ http://localhost:8080/api/2/search/things/count
Complex example:
```
POST .../search/things/count
body: filter=eq%28attributes%2Flocation%2C%22living-room%22%29
body: filter=eq(attributes/location,"living-room")
```
12 changes: 6 additions & 6 deletions ...rvice/src/main/java/org/eclipse/ditto/gateway/service/endpoints/routes/AbstractRoute.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.io.InputStreamReader;
import java.time.Duration;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
Expand Down Expand Up @@ -303,13 +304,12 @@ protected ActorRef createHttpPerRequestActor(final RequestContext ctx,
protected Route ensureMediaTypeFormUrlEncodedThenExtractData(
final RequestContext ctx,
final DittoHeaders dittoHeaders,
final java.util.function.Function<Source<ByteString, Object>, Route> inner
final java.util.function.Function<Map<String, List<String>>, Route> inner
) {
return ContentTypeValidationDirective.ensureValidContentType(Set.of(MediaTypes.APPLICATION_X_WWW_FORM_URLENCODED.toString()), ctx, dittoHeaders,
() -> {
final var res = extractDataBytes(inner);
return res;
});
return ContentTypeValidationDirective.ensureValidContentType(
Set.of(MediaTypes.APPLICATION_X_WWW_FORM_URLENCODED.toString()), ctx, dittoHeaders,
() -> formFieldMultiMap(inner)
);
}

/**
Expand Down
128 changes: 55 additions & 73 deletions ...java/org/eclipse/ditto/gateway/service/endpoints/routes/thingsearch/ThingSearchRoute.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,26 @@
*/
package org.eclipse.ditto.gateway.service.endpoints.routes.thingsearch;

import akka.http.javadsl.server.Directives;
import akka.http.javadsl.server.PathMatchers;
import akka.http.javadsl.server.RequestContext;
import akka.http.javadsl.server.Route;
import java.util.Arrays;
import java.util.EnumMap;
import java.util.List;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.annotation.Nullable;

import org.eclipse.ditto.base.model.headers.DittoHeaders;
import org.eclipse.ditto.base.model.signals.commands.Command;
import org.eclipse.ditto.gateway.service.endpoints.routes.AbstractRoute;
import org.eclipse.ditto.gateway.service.endpoints.routes.RouteBaseProperties;
import org.eclipse.ditto.thingsearch.model.signals.commands.query.CountThings;
import org.eclipse.ditto.thingsearch.model.signals.commands.query.QueryThings;

import javax.annotation.Nullable;
import java.util.*;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import akka.http.javadsl.server.Directives;
import akka.http.javadsl.server.PathMatchers;
import akka.http.javadsl.server.RequestContext;
import akka.http.javadsl.server.Route;

/**
* Builder for creating Akka HTTP routes for {@code /search/things}.
Expand Down Expand Up @@ -78,25 +82,25 @@ private Route countThings(final RequestContext ctx, final DittoHeaders dittoHead
// GET things/count?filter=<filterString>&namespaces=<namespacesString>
get(() -> thingSearchParameterOptional(
params -> handlePerRequest(ctx,
CountThings.of(calculateFilter(params.get(ThingSearchParameter.FILTER)),
calculateNamespaces(params.get(ThingSearchParameter.NAMESPACES)),
dittoHeaders)))),
CountThings.of(calculateFilter(params.get(ThingSearchParameter.FILTER)),
calculateNamespaces(params.get(ThingSearchParameter.NAMESPACES)),
dittoHeaders)))),
// POST things/count
post(() -> ensureMediaTypeFormUrlEncodedThenExtractData(
ctx,
dittoHeaders,
payloadSource -> handlePerRequest(
formFields -> handlePerRequest(
ctx,
dittoHeaders,
payloadSource,
requestString -> getFormParameters(
requestString,
params -> CountThings.of(
calculateFilter(params.get(ThingSearchParameter.FILTER)),
calculateNamespaces(params.get(ThingSearchParameter.NAMESPACES)),
dittoHeaders)
)
)))
CountThings.of(
calculateFilter(
formFields.getOrDefault(ThingSearchParameter.FILTER.toString(),
List.of())),
calculateNamespaces(
formFields.getOrDefault(ThingSearchParameter.NAMESPACES.toString(),
List.of())),
dittoHeaders)
)
))
);
}

Expand All @@ -111,65 +115,43 @@ private Route searchThings(final RequestContext ctx, final DittoHeaders dittoHea
// &options=<optionsString>
// &fields=<fieldsString>
// &namespaces=<namespacesString>
// &nextPageKey=<nextPageKey>
get(() -> thingSearchParameterOptional(
params -> handlePerRequest(ctx,
QueryThings.of(
calculateFilter(params.get(ThingSearchParameter.FILTER)),
calculateOptions(params.get(ThingSearchParameter.OPTION)),
AbstractRoute.calculateSelectedFields(params.get(ThingSearchParameter.FIELDS))
.orElse(null),
calculateNamespaces(params.get(ThingSearchParameter.NAMESPACES)),
dittoHeaders
)
)
)
),
// POST /search/things
post(() -> ensureMediaTypeFormUrlEncodedThenExtractData(
ctx,
dittoHeaders,
payloadSource -> handlePerRequest(
ctx,
dittoHeaders,
payloadSource,
requestString -> getFormParameters(
requestString,
params -> QueryThings.of(
params -> handlePerRequest(ctx,
QueryThings.of(
calculateFilter(params.get(ThingSearchParameter.FILTER)),
calculateOptions(params.get(ThingSearchParameter.OPTION)),
AbstractRoute.calculateSelectedFields(params.get(ThingSearchParameter.FIELDS))
.orElse(null),
calculateNamespaces(params.get(ThingSearchParameter.NAMESPACES)),
dittoHeaders
)
)
)))
);

}

private Command<?> getFormParameters(final String requestString, final Function<EnumMap<ThingSearchParameter, List<String>>, Command<?>> inner) {
return inner.apply(parseFormParameters(requestString));
}

private EnumMap<ThingSearchParameter, List<String>> parseFormParameters(final String requestString){
final var thingSearchParameterMap = new EnumMap<ThingSearchParameter, List<String>>(ThingSearchParameter.class);
final var keyValues = Arrays.stream(requestString.split("&")).map(item -> item.split("=")).toList();

Arrays.stream(ThingSearchParameter.values()).forEach(
thingSearchParameter -> {
final var valueList = new ArrayList<String>();
keyValues.forEach( keyValue -> {
if (Objects.equals(keyValue[0], thingSearchParameter.toString())) {
valueList.add(keyValue[1]);
}
});
thingSearchParameterMap.put(thingSearchParameter, valueList);
}
)
)
),
// POST /search/things
post(() -> ensureMediaTypeFormUrlEncodedThenExtractData(
ctx,
dittoHeaders,
formFields -> handlePerRequest(ctx,
QueryThings.of(
calculateFilter(
formFields.getOrDefault(ThingSearchParameter.FILTER.toString(),
List.of())),
calculateOptions(
formFields.getOrDefault(ThingSearchParameter.OPTION.toString(),
List.of())),
AbstractRoute.calculateSelectedFields(
formFields.getOrDefault(ThingSearchParameter.FIELDS.toString(),
List.of())).orElse(null),
calculateNamespaces(
formFields.getOrDefault(ThingSearchParameter.NAMESPACES.toString(),
List.of())),
dittoHeaders
)
)
))
);

return thingSearchParameterMap;
}

private Route thingSearchParameterOptional(
Expand Down
21 changes: 12 additions & 9 deletions .../org/eclipse/ditto/gateway/service/endpoints/routes/thingsearch/ThingSearchRouteTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,22 @@
*/
package org.eclipse.ditto.gateway.service.endpoints.routes.thingsearch;

import akka.http.javadsl.model.*;
import akka.http.javadsl.server.Route;
import akka.http.javadsl.testkit.TestRoute;
import akka.japi.Pair;
import static org.eclipse.ditto.json.assertions.DittoJsonAssertions.assertThat;

import java.util.List;

import org.eclipse.ditto.gateway.service.endpoints.EndpointTestBase;
import org.eclipse.ditto.json.JsonKey;
import org.eclipse.ditto.json.JsonObject;
import org.junit.Before;
import org.junit.Test;

import java.util.List;

import static org.eclipse.ditto.json.assertions.DittoJsonAssertions.assertThat;
import akka.http.javadsl.model.FormData;
import akka.http.javadsl.model.HttpRequest;
import akka.http.javadsl.model.StatusCodes;
import akka.http.javadsl.server.Route;
import akka.http.javadsl.testkit.TestRoute;
import akka.japi.Pair;

/**
* Builder for creating Akka HTTP routes for {@code /search/things}.
Expand Down Expand Up @@ -55,7 +58,7 @@ public void postSearchThingsShouldGetParametersFromBody() {
.withEntity(formData.toEntity()));

result.assertStatusCode(StatusCodes.OK);
result.assertEntity("{\"type\":\"thing-search.commands:queryThings\",\"filter\":\"and(and%28like%28definition%2C%22%2Atest%2A%22%29%29)\",\"options\":[\"sort%28%2BthingId%29\",\"limit%280%2C5%29\"],\"namespaces\":[\"org.eclipse.ditto%2Cfoo.bar\"]}");
result.assertEntity("{\"type\":\"thing-search.commands:queryThings\",\"filter\":\"and(and(like(definition,\\\"*test*\\\")))\",\"options\":[\"limit(0\",\"5)\",\"sort(+thingId)\"],\"namespaces\":[\"foo.bar\",\"org.eclipse.ditto\"]}");
}

@Test
Expand Down Expand Up @@ -90,7 +93,7 @@ public void countThingsShouldGetFilterFromBody() {
assertThat(JsonObject.of(result.entityString()))
.contains(
JsonKey.of("filter"),
"and(and%28like%28definition%2C%22%2Atest%2A%22%29%29)"
"and(and(like(definition,\"*test*\")))"
);
}

Expand Down

0 comments on commit 95589f8

Please sign in to comment.