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 37 commits into
base: develop
Choose a base branch
from

Conversation

steniobhz
Copy link
Collaborator

@steniobhz steniobhz commented Oct 4, 2024

Documentation for the Hook Search API (Listeners and Routes)

Available Endpoints

  1. Search Listeners
  • Endpoint: /registrations/listeners
  • HTTP Method: GET
  • Description: Searches for all Listeners whose destination field contains the provided query string in the q parameter.
  1. Search Routes
  • Endpoint: /registrations/routes
  • HTTP Method: GET
  • Description: Searches for all Routes whose destination field contains the provided query string in the q parameter.

Query Parameter

Both endpoints support the q query parameter, which is used to filter the search results.

  • q: A string to search for within the destination field of Listeners or Routes.

Example Request:
GET /registrations/listeners?q=myDestination
GET /registrations/routes?q=myDestination

Response Structure
Example Response - Searching for Listeners:

{
    "listeners": [
        "listener1",
        "listener2",
        "listener3"
    ]
}

Example Response - Searching for Routes:

{
    "routes": [
        "route1",
        "route2",
        "route3"
    ]
}

Key Features

  1. Separate Endpoints for Listeners and Routes: The API provides distinct endpoints for Listeners and Routes searches, ensuring optimized search results based on specific hook types.

  2. String-Based Search: The API looks for matches in the destination field of both Listeners and Routes. It searches for any substring match rather than an exact match, making the search more flexible.

  3. Optimized Search Logic: The API uses efficient search mechanisms internally to quickly find results across registered listeners or routes, reducing response time even with large datasets.

Example Workflow

  1. A client wants to find all Listeners with a destination that contains the string "myDestination". They make the following request:
    GET /registrations/listeners?q=myDestination

  2. The API returns a JSON object containing the identifiers of matching Listeners:

{
    "listeners": [
        "listener1",
        "listener2"
    ]
}

Copy link
Collaborator

@mcweba mcweba left a comment

Choose a reason for hiding this comment

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

As discussed on the phone, you should also add some integration tests in the gateleen-test module. Where is the documentation of this new API feature?

Add new tests for HookHandler
Create testes with storage
- Verifies successful listener search when multiple listeners are present in storage.
- Ensures correct retrieval of a single listener from storage.
- Tests failure case when searching for a non-existent listener among multiple listeners.
- Ensures proper handling of search when no listeners are registered in storage.
…istener is found for a given query parameter.
…eners registered.

- Test for handling searches with no matching listeners, returning an empty list.
- Test for handling searches when no listeners are registered, ensuring an empty list is returned.
- Fix hookHandler Search
…tory. It checks if the destination of each item contains the query string (queryParam), adding matching items to the result.
- improve listeners tests
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 90.47619% with 4 lines in your changes missing coverage. Please review.

Project coverage is 49.08%. Comparing base (45f39c1) to head (9956eba).
Report is 23 commits behind head on develop.

Files with missing lines Patch % Lines
.../java/org/swisspush/gateleen/hook/HookHandler.java 89.74% 1 Missing and 3 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop     #613      +/-   ##
=============================================
+ Coverage      48.44%   49.08%   +0.63%     
- Complexity      1862     1907      +45     
=============================================
  Files            238      238              
  Lines          11953    12049      +96     
  Branches        1261     1276      +15     
=============================================
+ Hits            5791     5914     +123     
+ Misses          5659     5626      -33     
- Partials         503      509       +6     
Flag Coverage Δ
?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@steniobhz steniobhz force-pushed the Issue311_Improve-hook-searching branch from 4fdab6c to 4004646 Compare October 17, 2024 12:49
@steniobhz steniobhz requested a review from mcweba October 17, 2024 12:54
@steniobhz steniobhz requested a review from mcweba October 21, 2024 18:33
@steniobhz steniobhz requested a review from mcweba October 22, 2024 14:20
@steniobhz steniobhz marked this pull request as draft October 24, 2024 13:10
@steniobhz steniobhz self-assigned this Oct 24, 2024
@steniobhz steniobhz marked this pull request as ready for review October 30, 2024 17:32
@steniobhz steniobhz requested a review from mcweba October 30, 2024 17:33
Copy link
Collaborator

@mcweba mcweba left a comment

Choose a reason for hiding this comment

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

There seems to be an issue in the implementation of route searching. When listing the routes with


GET /playground/server/hooks/v1/registrations/routes

you get the following response

{
    "routes": [
        "+playground+server+destination1+_hooks+routes+http+myhook54"
    ]
}

However when you add the search query parameter like this

GET /playground/server/hooks/v1/registrations/routes?q=r1

you get the following response

{
    "routes": [
        "/playground/server/destination1"
    ]
}

The response should still be the same. This is the case for listeners hooks but not for route hooks.

…h and new implementation to search with parameters

Add more unit tests
Fix route search with parameters
Update readme with more information about search with query parameters
create a hookIdentify to get a requestUrl when search using parameters
Add more unit and integrate testes
improve code implementations
improve testes
organize code
remove unused variables
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants