Tika
  1. Tika
  2. TIKA-891

Use POST in addition to PUT on method calls in tika-server

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Trivial Trivial
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 1.9
    • Component/s: general
    • Labels:

      Description

      Per Jukka's email:

      http://s.apache.org/uR

      It would be a better use of REST/HTTP "verbs" to use POST to put content to a resource where we don't intend to store that content (which is the implication of PUT). Max suggested adding:

      @POST
      

      annotations to the methods we are currently exposing using PUT to take care of this.

        Issue Links

          Activity

          Chris A. Mattmann created issue -
          Hide
          Chris A. Mattmann added a comment -
          • push to 1.3
          Show
          Chris A. Mattmann added a comment - push to 1.3
          Hide
          Chris A. Mattmann added a comment -
          • push to 1.3
          Show
          Chris A. Mattmann added a comment - push to 1.3
          Chris A. Mattmann made changes -
          Field Original Value New Value
          Fix Version/s 1.3 [ 12321647 ]
          Fix Version/s 1.2 [ 12320169 ]
          Raimund Merkert made changes -
          Link This issue is part of TIKA-1047 [ TIKA-1047 ]
          Hide
          Chris A. Mattmann added a comment -
          • push out to 1.4
          Show
          Chris A. Mattmann added a comment - push out to 1.4
          Chris A. Mattmann made changes -
          Fix Version/s 1.4 [ 12323959 ]
          Fix Version/s 1.3 [ 12321647 ]
          Hide
          Chris A. Mattmann added a comment -
          • push out to 1.4
          Show
          Chris A. Mattmann added a comment - push out to 1.4
          Hide
          Chris A. Mattmann added a comment -
          • push to 1.5, get ready for 1.4 RC #1.
          Show
          Chris A. Mattmann added a comment - push to 1.5, get ready for 1.4 RC #1.
          Chris A. Mattmann made changes -
          Fix Version/s 1.5 [ 12324552 ]
          Fix Version/s 1.4 [ 12323959 ]
          Hide
          Dave Meikle added a comment -

          Pushed out to 1.6, preparing for 1.5 RC

          Show
          Dave Meikle added a comment - Pushed out to 1.6, preparing for 1.5 RC
          Dave Meikle made changes -
          Fix Version/s 1.6 [ 12326172 ]
          Fix Version/s 1.5 [ 12324552 ]
          Chris A. Mattmann made changes -
          Fix Version/s 1.7 [ 12327096 ]
          Fix Version/s 1.6 [ 12326172 ]
          Hide
          Chris A. Mattmann added a comment -
          • push to 1.8
          Show
          Chris A. Mattmann added a comment - push to 1.8
          Chris A. Mattmann made changes -
          Fix Version/s 1.8 [ 12328892 ]
          Fix Version/s 1.7 [ 12327096 ]
          Hide
          Tyler Palsulich added a comment -

          I made a couple changes related to this for TIKA-1547 (use POST for forms). Should we still convert the other PUT resources to POST?

          Show
          Tyler Palsulich added a comment - I made a couple changes related to this for TIKA-1547 (use POST for forms). Should we still convert the other PUT resources to POST?
          Tyler Palsulich made changes -
          Labels newbie
          Hide
          Chris A. Mattmann added a comment -

          I think it would be nice to convert the other PUT ones to POST where it makes sense. Do you have a list in mind?

          Show
          Chris A. Mattmann added a comment - I think it would be nice to convert the other PUT ones to POST where it makes sense. Do you have a list in mind?
          Hide
          Tyler Palsulich added a comment -

          There are only 3 – getText, getXML, getHTML. So, easy list.

          Show
          Tyler Palsulich added a comment - There are only 3 – getText, getXML, getHTML. So, easy list.
          Hide
          Chris A. Mattmann added a comment -

          ahh if it's "get" anything I would recommend making them "GET" methods with @GET

          Show
          Chris A. Mattmann added a comment - ahh if it's "get" anything I would recommend making them "GET" methods with @GET
          Hide
          Sergey Beryozkin added a comment -

          IMHO it might make sense to keep PUT as deprecated for the next release so that the existing users can still get it working and provide a documentation suggesting that PUT will be removed in 1.9. Or at the very least offer such a migration guide for 1.8

          As a side note, I don't know PUT was put in the first place, but speaking of the semantics, I'm not 100% sure POST (effectively adding a resource to the collection) is the closest match to what Tika JAX-RS server does, by accepting a resource and echoing its metadata/data back.

          Cheers, Sergey

          Show
          Sergey Beryozkin added a comment - IMHO it might make sense to keep PUT as deprecated for the next release so that the existing users can still get it working and provide a documentation suggesting that PUT will be removed in 1.9. Or at the very least offer such a migration guide for 1.8 As a side note, I don't know PUT was put in the first place, but speaking of the semantics, I'm not 100% sure POST (effectively adding a resource to the collection) is the closest match to what Tika JAX-RS server does, by accepting a resource and echoing its metadata/data back. Cheers, Sergey
          Hide
          Tyler Palsulich added a comment -

          So, if PUT and POST are out, it looks like the winner is GET? We can definitely keep PUT around for another release and mark this issue for 1.9.

          Show
          Tyler Palsulich added a comment - So, if PUT and POST are out, it looks like the winner is GET? We can definitely keep PUT around for another release and mark this issue for 1.9.
          Hide
          Sergey Beryozkin added a comment -

          Why are both PUT and POST out ? GET does not fit, it offers no input content.

          Show
          Sergey Beryozkin added a comment - Why are both PUT and POST out ? GET does not fit, it offers no input content.
          Hide
          Sergey Beryozkin added a comment -

          Just to clarify: I've no objections to migrating to POST from PUT, I guess neither fits perfectly, perhaps POST is marginally 'cleaner', but ultimately is is not that important IMHO .

          The question is how to support the existing users. Keeping PUT for 1.8 only might make sense. I guess it depends on whether 1.8 is considered a major version or not

          Thanks

          Show
          Sergey Beryozkin added a comment - Just to clarify: I've no objections to migrating to POST from PUT, I guess neither fits perfectly, perhaps POST is marginally 'cleaner', but ultimately is is not that important IMHO . The question is how to support the existing users. Keeping PUT for 1.8 only might make sense. I guess it depends on whether 1.8 is considered a major version or not Thanks
          Hide
          Tyler Palsulich added a comment -

          Sergey Berezhnoy, I think I misunderstood your earlier comment regarding POST. I will defer to you on this – if you think PUT is semantically no different than POST, we can leave it be. As Chris said, we should only switch if it makes sense.

          Show
          Tyler Palsulich added a comment - Sergey Berezhnoy , I think I misunderstood your earlier comment regarding POST. I will defer to you on this – if you think PUT is semantically no different than POST, we can leave it be. As Chris said, we should only switch if it makes sense.
          Hide
          Sergey Beryozkin added a comment - - edited

          If I said PUT was the same as POST then I'd probably be seriously criticized at the very least . My point was, that neither verb is probably that semantically close for the type of the action Tika Server supports. Typically I prefer just to get things done though risking sometimes writing a possibly not very pure REST code . However you are right forms do not support PUT hence I guess I'd be better to have POST across the board for the sake of consistency. And if you agree - then keeping PUT for 1.8 as deprecated and removing in 1.9

          Sergey

          Show
          Sergey Beryozkin added a comment - - edited If I said PUT was the same as POST then I'd probably be seriously criticized at the very least . My point was, that neither verb is probably that semantically close for the type of the action Tika Server supports. Typically I prefer just to get things done though risking sometimes writing a possibly not very pure REST code . However you are right forms do not support PUT hence I guess I'd be better to have POST across the board for the sake of consistency. And if you agree - then keeping PUT for 1.8 as deprecated and removing in 1.9 Sergey
          Hide
          Tyler Palsulich added a comment -

          Ah... I meant they were both equally right/wrong in this case. Not all! Let's schedule this for 1.9. I feel like the deprecation note should be more noticeable than in just the JavaDoc, though. So, we should make a note on the website when we release 1.8.

          Show
          Tyler Palsulich added a comment - Ah... I meant they were both equally right/wrong in this case. Not all! Let's schedule this for 1.9. I feel like the deprecation note should be more noticeable than in just the JavaDoc, though. So, we should make a note on the website when we release 1.8.
          Tyler Palsulich made changes -
          Fix Version/s 1.9 [ 12329574 ]
          Fix Version/s 1.8 [ 12328892 ]
          Hide
          Tyler Palsulich added a comment -

          I was mistaken when I said there are only 3 PUT resources in the server:

          ➜  tika-server  grep -RA 4 "@PUT" . | grep public | cut -d'(' -f1
          ./src/main/java/org/apache/tika/server/DetectorResource.java-    public String detect
          ./src/main/java/org/apache/tika/server/MetadataResource.java-    public Response getMetadata
          ./src/main/java/org/apache/tika/server/MetadataResource.java-    public Response getMetadataField
          ./src/main/java/org/apache/tika/server/RecursiveMetadataResource.java-    public Response getMetadata
          ./src/main/java/org/apache/tika/server/TikaResource.java-    public StreamingOutput getText
          ./src/main/java/org/apache/tika/server/TikaResource.java-    public StreamingOutput getHTML
          ./src/main/java/org/apache/tika/server/TikaResource.java-    public StreamingOutput getXML
          ./src/main/java/org/apache/tika/server/UnpackerResource.java-    public Map<String, byte[]> unpack
          ./src/main/java/org/apache/tika/server/UnpackerResource.java-    public Map<String, byte[]> unpackAll
          

          What do you think of the rest of these, Sergey Beryozkin?

          Show
          Tyler Palsulich added a comment - I was mistaken when I said there are only 3 PUT resources in the server: ➜ tika-server grep -RA 4 "@PUT" . | grep public | cut -d'(' -f1 ./src/main/java/org/apache/tika/server/DetectorResource.java- public String detect ./src/main/java/org/apache/tika/server/MetadataResource.java- public Response getMetadata ./src/main/java/org/apache/tika/server/MetadataResource.java- public Response getMetadataField ./src/main/java/org/apache/tika/server/RecursiveMetadataResource.java- public Response getMetadata ./src/main/java/org/apache/tika/server/TikaResource.java- public StreamingOutput getText ./src/main/java/org/apache/tika/server/TikaResource.java- public StreamingOutput getHTML ./src/main/java/org/apache/tika/server/TikaResource.java- public StreamingOutput getXML ./src/main/java/org/apache/tika/server/UnpackerResource.java- public Map< String , byte []> unpack ./src/main/java/org/apache/tika/server/UnpackerResource.java- public Map< String , byte []> unpackAll What do you think of the rest of these, Sergey Beryozkin ?
          Hide
          Sergey Beryozkin added a comment -

          Well, I guess we have to be careful with replacing all PUTs with POSTs. As far as I recall, one of curl commands documented at the Tika JAXRS page uses PUT implicitly. Supporting both methods is a more conservative approach in the short term at least.

          Show
          Sergey Beryozkin added a comment - Well, I guess we have to be careful with replacing all PUTs with POSTs. As far as I recall, one of curl commands documented at the Tika JAXRS page uses PUT implicitly. Supporting both methods is a more conservative approach in the short term at least.
          Hide
          Chris A. Mattmann added a comment -

          yeah I would suggest we support both methods.

          Show
          Chris A. Mattmann added a comment - yeah I would suggest we support both methods.
          Hide
          Tyler Palsulich added a comment -

          I took a stab at adding the @POST annotation to each @PUT resource. But, it didn't work. Turns out, you cannot support two HTTP request methods for the same resource. See this blog for an experiment. I couldn't seem to find anything in the official documentation. But, the blog and my own experience seem like reason enough.

          So, now the real question: Since we can't support @PUT and @POST, do we want to leave everything the way it is, or switch everything to @POST?

          I'm leaning toward leaving the resources the way they are. But, @POST, as discussed above, isn't necessarily wrong...

          Show
          Tyler Palsulich added a comment - I took a stab at adding the @POST annotation to each @PUT resource. But, it didn't work. Turns out, you cannot support two HTTP request methods for the same resource. See this blog for an experiment. I couldn't seem to find anything in the official documentation . But, the blog and my own experience seem like reason enough. So, now the real question: Since we can't support @PUT and @POST , do we want to leave everything the way it is, or switch everything to @POST ? I'm leaning toward leaving the resources the way they are. But, @POST , as discussed above, isn't necessarily wrong...
          Hide
          Sergey Beryozkin added a comment -

          It is not the case, in JAX-RS, if you need to have multiple methods supported on the same path then one just has multiple methods, annotated with PUT, POST, etc, delegating to a common implementation.

          Show
          Sergey Beryozkin added a comment - It is not the case, in JAX-RS, if you need to have multiple methods supported on the same path then one just has multiple methods, annotated with PUT, POST, etc, delegating to a common implementation.
          Hide
          Tyler Palsulich added a comment -

          That's true. But, in my opinion, it's not worth adding two new methods for each existing method (3 total: 1 PUT, 1 POST, 1 implementation) when no one feels strongly about which method we should definitely support.

          Show
          Tyler Palsulich added a comment - That's true. But, in my opinion, it's not worth adding two new methods for each existing method (3 total: 1 PUT, 1 POST, 1 implementation) when no one feels strongly about which method we should definitely support.
          Hide
          Sergey Beryozkin added a comment -

          What I do feel rather strongly about is that the existing users should not start seeing the client code expecting PUT supported breaking.
          The duplication is usually of some concern to developers initially but always proves to be negligible in terms of what actually (how much) is duplicated. Some people can do a ContainerRequestFilter that will adapt PUT requests to POST and save a bit on typing duplicate method definitions though at a minor extra cost of having to maintain yet another provider.

          Show
          Sergey Beryozkin added a comment - What I do feel rather strongly about is that the existing users should not start seeing the client code expecting PUT supported breaking. The duplication is usually of some concern to developers initially but always proves to be negligible in terms of what actually (how much) is duplicated. Some people can do a ContainerRequestFilter that will adapt PUT requests to POST and save a bit on typing duplicate method definitions though at a minor extra cost of having to maintain yet another provider.

            People

            • Assignee:
              Chris A. Mattmann
              Reporter:
              Chris A. Mattmann
            • Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:

                Development