Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 1.2
    • Fix Version/s: 1.2
    • Component/s: Database Core
    • Labels:
    • Environment:

      linux

    • Skill Level:
      New Contributors Level (Easy)

      Description

      When PUTting a document you have to specify the current revision as a _rev key in the JSON; whereas many related requests (like DELETE, COPY and PUT of an attachment) specify the revision in a "?rev=" URL query parameter instead. The first example, will result into a document conflict because rev is not specified in the JSON and the rev from the url gets ignored. All other examples work. I strongly believe JSON bodies like this should never contain _id and _rev and will improve in much better client code.

      curl -X PUT http://localhost:5984/_users/user%3Agert?rev=5xxx -H
      'Content-Type: application/json' -d

      { "_id" : "user:gert", "type" : "user", "name" : "gert", "roles" : [], "password_sha" : "", "salt" : "" }

      '

      curl -X PUT http://localhost:5984/_users/user%3Agert -H 'Content-Type:
      application/json' -d

      { "_id" : "user:gert", "_rev":"5xxx", "type" : "user", "name" : "gert", "roles" : [], "password_sha" : "", "salt" : "" }

      '

      curl -X PUT http://localhost:5984/_users/user%3Agert/picture?rev=5xxx
      -H 'Content-Type: image/png' -d @picture.png

        Activity

        gert cuykens created issue -
        Hide
        Adam Kocoloski added a comment -

        I'll see your query string and raise you a request header – set the value of the If-Match header to the document revision and you can omit both the qs parameter and the _rev in the JSON. I definitely agree that the underscore-prefixed special document properties are a wart and one that we should have tried harder to avoid.

        Regardless of the If-Match option we should be consistent in our API. If the "rev" query-string parameter works everywhere else I see no reason why it shouldn't work for regular document PUTs as well.

        Show
        Adam Kocoloski added a comment - I'll see your query string and raise you a request header – set the value of the If-Match header to the document revision and you can omit both the qs parameter and the _rev in the JSON. I definitely agree that the underscore-prefixed special document properties are a wart and one that we should have tried harder to avoid. Regardless of the If-Match option we should be consistent in our API. If the "rev" query-string parameter works everywhere else I see no reason why it shouldn't work for regular document PUTs as well.
        Hide
        gert cuykens added a comment - - edited

        Thanks, if-match works. This is by far the most logic way for me, no _id and no _rev. Pleas consider development in this direction that _id and _rev should always be header material. In futon both should also not be represented in the fields but in the bottom bar menu between previous an next version for example. Id is already represented in the top menu.

        curl -X PUT http://localhost:5984/_users/user%3Agert -H 'If-Match: 5xxx' -H 'Content-Type: application/json' -d '

        { "type" : "user", "name" : "gert", "roles" : [], "password_sha" : "...", "salt" : "..." }

        '

        Show
        gert cuykens added a comment - - edited Thanks, if-match works. This is by far the most logic way for me, no _id and no _rev. Pleas consider development in this direction that _id and _rev should always be header material. In futon both should also not be represented in the fields but in the bottom bar menu between previous an next version for example. Id is already represented in the top menu. curl -X PUT http://localhost:5984/_users/user%3Agert -H 'If-Match: 5xxx' -H 'Content-Type: application/json' -d ' { "type" : "user", "name" : "gert", "roles" : [], "password_sha" : "...", "salt" : "..." } '
        Hide
        Jan Lehnardt added a comment -

        I agree that ?rev should be honoured, but I disagree that _id and _rev should not be part of the document itself. Other than theoretical purity I haven't seen any strong arguments against this and I've only seen good things coming out of it and I conclude that it is a very nice practical compromise. But we've had that discussion numerous times.

        Show
        Jan Lehnardt added a comment - I agree that ?rev should be honoured, but I disagree that _id and _rev should not be part of the document itself. Other than theoretical purity I haven't seen any strong arguments against this and I've only seen good things coming out of it and I conclude that it is a very nice practical compromise. But we've had that discussion numerous times.
        Hide
        gert cuykens added a comment -

        I am not experienced enough to come up with better arguments, except for compatibility between other document based databases maybe. I trust you will have better judgment on this.

        Show
        gert cuykens added a comment - I am not experienced enough to come up with better arguments, except for compatibility between other document based databases maybe. I trust you will have better judgment on this.
        Randall Leeds made changes -
        Field Original Value New Value
        Assignee Randall Leeds [ tilgovi ]
        Randall Leeds made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Randall Leeds made changes -
        Status In Progress [ 3 ] Open [ 1 ]
        Hide
        Randall Leeds added a comment -

        Patch is finished and as a side affect I think I'm enabling new_edits=false and batch=true for things like COPY and POST. Speak up if you have a reason for that not to be the case, but what I've done cleans up couch_httpd_db.erl quite a bit. Just need to separate the patches into a couple logical chunks and write a test.

        Show
        Randall Leeds added a comment - Patch is finished and as a side affect I think I'm enabling new_edits=false and batch=true for things like COPY and POST. Speak up if you have a reason for that not to be the case, but what I've done cleans up couch_httpd_db.erl quite a bit. Just need to separate the patches into a couple logical chunks and write a test.
        Randall Leeds made changes -
        Status Open [ 1 ] Closed [ 6 ]
        Fix Version/s 1.2 [ 12315198 ]
        Resolution Fixed [ 1 ]

          People

          • Assignee:
            Randall Leeds
            Reporter:
            gert cuykens
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 24h
              24h
              Remaining:
              Remaining Estimate - 24h
              24h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development