[infinispan-dev] REST Refactoring - breaking changes

Thu May 25 04:01:19 EDT 2017

On Thu, May 25, 2017 at 7:42 AM Radim Vansa <rvansa at redhat.com> wrote:

> On 05/24/2017 10:44 AM, Sebastian Laskawiec wrote:
> >
> >
> > On Tue, May 23, 2017 at 5:06 PM Radim Vansa <rvansa at redhat.com
> > <mailto:rvansa at redhat.com>> wrote:
> >
> >     On 05/16/2017 11:05 AM, Sebastian Laskawiec wrote:
> >     > Hey guys!
> >     >
> >     > I'm working on REST Server refactoring and I changed some of the
> >     > previous behavior. Having in mind that we are implementing this in
> a
> >     > minor release, I tried to make those changes really cosmetic:
> >     >
> >     >   * RestEASY as well as Servlet API have been removed from
> >     modules and
> >     >     BOM. If your app relied on it, you'll need to specify them
> >     >     separately in your pom.
> >     >   * Previous implementation picked application/text as a default
> >     >     content type. I replaced it with text/plain with charset
> >     which is
> >     >     more precise and seems to be more widely adopted.
> >     >   * Putting an entry without any TTL nor Idle Time made it living
> >     >     forever (which was BTW aligned with the docs). I switched to
> >     >     server configured defaults in this case. If you want to have an
> >     >     entry that lives forever, just specify 0 or -1 there.
> >     >   * Requesting an entry with wrong mime type (imagine it was stored
> >     >     using application/octet-stream and now you're requesting
> >     >     text/plain) cased Bad Request. Now I switched it to Not
> >     Acceptable
> >     >     which was designed specially to cover this type of use case.
> >     >   * In compatibility mode the server often tried to "guess" the
> >     >     mimetype (the decision was often between text/plain and
> >     >     application/octet-stream). I honestly think it was a wrong move
> >     >     and made the server side code very hard to read and predict
> what
> >     >     would be the result. Now the server always returns text/plain
> by
> >     >     default. If you want to get a byte stream back, just add
> >     `Accept:
> >     >     application/octet-stream`.
> >     >   * The server can be started with port 0. This way you are 100%
> >     sure
> >     >     that it will start using a unique port without colliding
> >     with any
> >     >     other service.
> >     >
> >     How can the client now the port number, then? Is the actual port
> >     exposed
> >     through JMX?
> >
> >     >   * The REST server hosts HTML page if queried using GET on default
> >     >     context. I think it was a bug that it didn't work correctly
> >     before.
> >     >
> >     Did it return 404? What's on that page? Do we expose
> >     keys/values/entries
> >     anywhere in the REST endpoint?
> >
> >
> > Exactly. You may try it using our Docker image and invoking something
> > like this: curl -v -u user:changeme http://172.17.0.6:8080/rest
> >
> >
> >     >   * UTF-8 charset is now the default. You may always ask the
> >     server to
> >     >     return different encoding using Accept header. The charset
> >     is not
> >     >     returned with binary mime types.
> >     >   * If a HEAD request results in an error, a message will be
> >     returned
> >     >     to the client. Even though this behavior breaks Commons HTTP
> >     >     Client (HEAD requests are handled slightly differently and
> >     causes
> >     >     the client to hang if a payload is returned), I think it's
> >     >     beneficial to tell the user what went wrong. It's worth to
> >     mention
> >     >     that Jetty/Netty HTTP clients work correctly.
> >     >   * RestServer doesn't implement Lifecycle now. The protocol server
> >     >     doesn't support start() method without any arguments. You
> always
> >     >     need to specify configuration + Embedded Cache Manager.
> >     >
> >     > Even though it's a long list, I think all those changes were
> >     worth it.
> >     > Please let me know if you don't agree.
> >
> >     Couple of other questions:
> >
> >     * do we accept GET with Range header on keys? What about
> >     delta-updating
> >     entries with Content-Range on PUTs?
> >
> >
> > No and AFAK there are no plans to do it (but perhaps Tristan could
> > shed some more light onto this). We could use HTTP PATCH for delta
> > updates...
> >
> >     * For PUTs/POSTs, do we return 200/201/204 according to the spec?
> >     (modified/created/modified)
> >
> >
> > No, I decided to leave it as 200 for compatibility reasons. But I
> > agree, we could change this as well.
>
> Compatibility reasons? You mean compatibility with clients not adhering
> to spec? (clients should accept any 2xy as success by definition).
>

+1, created https://issues.jboss.org/browse/ISPN-7859

> >
> >     * Do we have any way to execute a replace (or the other prev-value
> >     returning ops) through REST using single request? For example let
> >     DELETE
> >     return the prev entity (it should return 200 & entity or 204 and no
> >     response)
> >
> >
> > Yes, PUT replaces previous value [1] if such exists (whereas POST
> > would return a conflict). If for some reason you can not replace
> > current value, you will get a preconditions failed error.
>
> I have misformulated the question. I meant to ask if there is a way to
> return the previous value when you've replaced it?
>

Unfortunately no. And there wasn't in previous REST implementation as far
as I know.

>
>
> >
> > [1]
> >
> https://github.com/infinispan/infinispan/pull/5094/files#diff-58f67698080cc0242320614c921559a8R301
>
> Looking into the code, without considering performance at all, I think
> that you've become too ecstatic about Optionals. These should be used as
> return types for methods, not a) parameters to methods nor b) fields.
> This is a misuse of the API, according to the authors of Optionals in
> JDK. Most of the time, you're not using optionals to have fluent chain
> of method invocations, so -100 to that.
>

Answered in "To Optional or not to Optional thread".

>
> >     * Do we handle OPTIONS in any way?
> >
> >
> > No. Do we need it? I haven't seen any real implementation that uses
> > that for discovering REST operations.
> >
> >
> >     Radim
> >
> >     >
> >     > Thanks,
> >     > Sebastian
> >     >
> >     > --
> >     >
> >     > SEBASTIANŁASKAWIEC
> >     >
> >     > INFINISPAN DEVELOPER
> >     >
> >     > Red HatEMEA <https://www.redhat.com/>
> >     >
> >     > <https://red.ht/sig>
> >     >
> >     >
> >     >
> >     > _______________________________________________
> >     > infinispan-dev mailing list
> >     > infinispan-dev at lists.jboss.org
> >     <mailto:infinispan-dev at lists.jboss.org>
> >     > https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >
> >
> >     --
> >     Radim Vansa <rvansa at redhat.com <mailto:rvansa at redhat.com>>
> >     JBoss Performance Team
> >
> >     _______________________________________________
> >     infinispan-dev mailing list
> >     infinispan-dev at lists.jboss.org <mailto:
> infinispan-dev at lists.jboss.org>
> >     https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >
> > --
> >
> > SEBASTIANŁASKAWIEC
> >
> > INFINISPAN DEVELOPER
> >
> > Red HatEMEA <https://www.redhat.com/>
> >
> > <https://red.ht/sig>
> >
> >
> >
> > _______________________________________________
> > infinispan-dev mailing list
> > infinispan-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Radim Vansa <rvansa at redhat.com>
> JBoss Performance Team
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

-- 

SEBASTIAN ŁASKAWIEC

INFINISPAN DEVELOPER

Red Hat EMEA <https://www.redhat.com/>
<https://red.ht/sig>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/infinispan-dev/attachments/20170525/e24926ea/attachment.html 