useid / handlersjs

MIT License
3 stars 3 forks source link

`HttpHandlerOperation` does not accept more than one HTTP method #200

Closed angelaraya closed 1 year ago

angelaraya commented 2 years ago

There are cases where a handler can be used irrespective of the HTTP method of the request, or it could handle a subset of multiple methods. A common example would be using the same handler for HEAD and GET.

┆Issue is synchronized with this Wrike task by Unito

elf-pavlik commented 1 year ago

We need it for https://github.com/janeirodigital/sai-impl-service/issues/39

What would be the easiest config to use the same handler for both GETand HEAD?

I also see #134 without much activity

AbelVandenBriel commented 1 year ago

@elf-pavlik apologies, this issue went off our radar!

Instead of having a HttpHandlerOperation take multiple methods, we define our routes with multiple operations.

An example in componentsjs:

{
      "@id": "urn:some-id",
      "@type": "HttpHandlerRoute",
      "operations": [
        {
          "@type": "HttpHandlerOperation",
          "method": "GET",
          "publish": false
        },
        {
           "@type": "HttpHandlerOperation",
           "method": "POST",
           "publish": false
        },
      ],
      "handler": {
        "@id": "urn:some-handler"
      },
      "path": "/path"
    },

Semantically a HttpHandlerOperation should be just that, a definition of a single operation, but we support the use of multiple operations. Let me know if there are any more questions!

woutermont commented 1 year ago

@elf-pavlik @angelaraya

As @AbelVandenBriel wrote, the HttpHandlerRoute's operations field is a a array of HttpHandlerOperation, each representing a possible method. The reason we do this so verbose, is that we aim to easily export the routes in OpenAPI format.

About HEAD requests: I'd foreseen that for ease of use, these can by default be handled in the RoutedHttpRequestHandler, where they are passed to the applicable HttpHandler, of which the response's body is then removed before serving. This is the issue #134, which shouldn't take long for us to implement.

woutermont commented 1 year ago

Actually, this should all work out of the box (see my comment on #134).

If you feel like this answers your questions, you can close the issue.

elf-pavlik commented 1 year ago

Thank you @AbelVandenBriel and @woutermont! I'm going to test it later today and close this issue ✅

angelaraya commented 1 year ago

Thanks for the update all!