Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue311 improve hook searching #613

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions gateleen-hook/README_hook.md
Original file line number Diff line number Diff line change
Expand Up @@ -240,10 +240,43 @@ hookHandler.enableResourceLogging(true);
```


## Query-Based Listener and Route Search

Gateleen allows searching for listeners and routes using the query parameter `q`. This simplifies filtering the registered hooks based on query parameters.

### Listener Search with `q`
Search for listeners based on a query parameter like this:

```
GET http://myserver:7012/gateleen/server/listenertest/_hooks/listeners/listener/1?q=testQuery
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not how listener hooks are searched. The URL does not contain the listener-ID.

```

The response will contain the matching listeners. If no match is found, an empty list is returned:

**Example response with matches:**
```json
{
"listeners": [
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content of searching listeners and routes is not an array of objects (with destination, methods properties) but an array containing a list of names (strings). How did you even get this response?

"destination": "/path/to/destination",
"methods": ["GET", "POST"]
}
]
}
```

**Example response with no matches:**
```json
{
"listeners": []
}
```

### Route Search with `q`
Similarly, you can search for routes using a query parameter:

```
GET http://myserver:7012/gateleen/server/listenertest/_hooks/routes?q=testRoute
```

The response contains the matching routes, or an empty list if no match is found.
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -123,6 +124,13 @@ public class HookHandler implements LoggableResource {
public static final String LISTABLE = "listable";
public static final String COLLECTION = "collection";

private static final String CONTENT_TYPE_JSON = "application/json";
private static final String LISTENERS_KEY = "listeners";
private static final String ROUTES_KEY = "routes";
private static final String DESTINATION_KEY = "destination";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your newly created DESTINATION_KEY static variable is not used

private static final String CONTENT_TYPE_HEADER = "content-type";


private final Comparator<String> collectionContentComparator;
private static final Logger log = LoggerFactory.getLogger(HookHandler.class);

Expand Down Expand Up @@ -569,6 +577,22 @@ public boolean handle(final RoutingContext ctx) {
}
}

// 1. Check if the request method is GET
if (request.method() == HttpMethod.GET) {
String uri = request.uri();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract var requestUri = request.uri(); only once in the handle method. It is already done for PUTand DELETE requests. We should optimize this.

String queryParam = request.getParam("q");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using any other query params than q in GET requests should result in a 400 Bad Request and not just return all listeners/hooks. Also create tests to verify this.

// 2. Check if the URI is for listeners or routes and has a query parameter
if (queryParam != null && !queryParam.isEmpty()) {
if (uri.contains(LISTENERS_KEY)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't look for listeners only. This will also match other requests not related to hooks. E.g.

/nemo/some/url/listeners/foobar

The same applies for routes. You always have a configurable prefix like playground/server/hooks/v1/registrations which should be used for this.

handleListenerSearch(queryParam, request.response());
return true;
} else if (uri.contains(ROUTES_KEY)) {
handleRouteSearch(queryParam, request.response());
return true;
}
}
}

/*
* 2) Check if we have to queue a request for listeners
*/
Expand All @@ -592,6 +616,65 @@ public boolean handle(final RoutingContext ctx) {
}
}

private void handleListenerSearch(String queryParam, HttpServerResponse response) {
handleSearch(
listenerRepository.getListeners().stream().collect(Collectors.toMap(Listener::getListenerId, listener -> listener)),
listener -> listener.getHook().getDestination(),
queryParam,
LISTENERS_KEY,
response
);
}

private void handleRouteSearch(String queryParam, HttpServerResponse response) {
handleSearch(
routeRepository.getRoutes(),
route -> route.getHook().getDestination(),
queryParam,
ROUTES_KEY,
response
);
}

/**
* Search the repository for items matching the query parameter.
* Returns a JSON response with the matched results.
* If any essential parameter (repository, response, getDestination) is null,
* a 400 Bad Request is returned.
*
* @param repository The items to search.
* @param getDestination Function to extract destinations.
* @param queryParam The query string to match.
* @param resultKey The key for the result in the response.
* @param response The HTTP response to return the results. Must not be null.
*/
private <T> void handleSearch(Map<String, T> repository, Function<T, String> getDestination, String queryParam, String resultKey, HttpServerResponse response) {
if (repository == null || getDestination == null) {
response.setStatusCode(400).end(); // Bad request for missing parameters
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use StatusCode.BAD_REQUEST.getStatusCode() instead of 400

return;
}

JsonArray matchingResults = new JsonArray();

repository.forEach((key, value) -> {
String destination = getDestination.apply(value);
if (destination != null && destination.contains(queryParam != null ? queryParam : "")) {
matchingResults.add(key);
}
});

JsonObject result = new JsonObject();
result.put(resultKey, matchingResults);

String encodedResult = result.encode();

response.putHeader(CONTENT_TYPE_HEADER, CONTENT_TYPE_JSON);
response.putHeader("Content-Length", String.valueOf(encodedResult.length()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two lines are not needed:

response.putHeader("Content-Length", String.valueOf(encodedResult.length()));
response.write(encodedResult);

use response.end(encodedResult) directly.

response.write(encodedResult);
response.end();
}


/**
* Create a listing of routes in the given parent. This happens
* only if we have a GET request, the routes are listable and
Expand Down
Loading
Loading