vertx-web icon indicating copy to clipboard operation
vertx-web copied to clipboard

Adding the initial prototype backported from yoke

Open pmlopes opened this issue 2 years ago • 6 comments

Signed-off-by: Paulo Lopes [email protected]

Motivation:

Addressing the issue #2087

pmlopes avatar Nov 16 '21 12:11 pmlopes

this should be merged when we consider doing 4.3

vietj avatar Nov 17 '21 09:11 vietj

Looks like a useful addition

jponge avatar Nov 25 '21 13:11 jponge

@jponge I think ATM we have a concise and simple way to handle REST API implementations. I've one question for you.

The current draft assumes that users will always work with JsonObject as type, would it make sense to allow POJOs too?

This means that users need to have jackson-databind in the class-path so all the decoding and encoding happens behind the scenes?

pmlopes avatar Nov 26 '21 10:11 pmlopes

@jponge I think ATM we have a concise and simple way to handle REST API implementations. I've one question for you.

The current draft assumes that users will always work with JsonObject as type, would it make sense to allow POJOs too?

This means that users need to have jackson-databind in the class-path so all the decoding and encoding happens behind the scenes?

Yes that'd make lots of sense to be able to map POJOs

jponge avatar Nov 27 '21 09:11 jponge

I took a look at the code and have a few points I want to mention:

ETags

As far as I can tell does the crud handler implementation not take care of ETag handling. Please keep in mind that the ETag may not just be inferred from the entity id. The response information defines the etag value. A user response may for example also contain references to other resources. Deletion or addition of the referenced resource may also affect the user etag. Personally I think this is a very complicated but required task since more and more REST APIs are directly used by browsers.

Handler parameters

The handler implementation does not provide the routing context. How can a implementation access the user to check if the required permissions are granted to access the resource?

Paging

Currently the start/end paging parameter pattern must be used. Another very common pattern is page / page size. These parameter can directly be used to infer the start, end values. (pagepageSize=start, pagepageSize+pageSize=end, 0 checks not included). It would be nice to have both options. Imho the page, pageSize variant is more common.

Architecture

For Mesh I always wished to be able to make the CrudHandlers 100% HTTP agnostic. This way I could better test them (no need to mock RoutingContext). The current approach seems to fulfill this. If you change parts I would try to keep it this way.

In Mesh there were only a few areas where this was not easy to do. A crud handler which returned files with nio needed to access the RoutingContext to use sendFile. ETag handlers needed to read and modify headers.

Query Parameters

I did not find the place where the CrudQuery was populated. How can custom query parameters be added to this query? A client may request a shallow variant of a resource or a CRUD request for image handling may contain resize parameters.

Jotschi avatar Nov 28 '21 19:11 Jotschi

@Jotschi good points!

ETags

Regarding etags I think it is a good addition. We can capture the response for the verbs that return data, GET in this case, and compute the etag, then other verbs like PUT/PATCH can compute the etag from the local storage and perform cache checks.

Not yet sure how to handle DELETE and POST though.

Parameters

My initial idea was using composition, I was thinking of:

router.route().handler(Oauth2Handler.create(...))
router.route("/persons").handler(CrudHandler.create(...))

Yet, you have a valid point, Oauth2 in this case is too coarse and the next handler CrudHandler may require specific rules per verb. In yoke, I had a pre[Create|Read|Update|Delete]Handler() and post[...]Handler() I was thinking we could live without them, but it seems there was a point after all. :)

Paging

Paging was using the Range header to avoid using the query string, because of the following question.

Query

The query is a JSON representation of the query string, for example:

HTTP GET /persons?name=P&active=true&sort(+foo,-bar)

Will create an object:

crudQuery = {
  query: {
    name: "P*",
    active: "true"
  },
  sort: {
    foo: 1
    bar: -1
  }
}

You may ask why this format? The reason was that yoke was heavily used on projects with https://dojotoolkit.org/reference-guide/1.10/dojo/store/JsonRest.html

The same way, the sort parameter is handled differently, we can also handle pagination properties. The query object is just a simplified representation of the request, so users can extract fields to build mongodb, couchdb or even SQL queries. There was intentionally no specification on what what the query means, or operators for this reason.

pmlopes avatar Nov 30 '21 10:11 pmlopes