CouchDB
  1. CouchDB
  2. COUCHDB-1074

trouble URL rewriting for key/startkey/endkey qs params

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.1, 1.2
    • Component/s: HTTP Interface
    • Labels:
    • Skill Level:
      Regular Contributors Level (Easy to Medium)

      Description

      the variable substution not to happen, e.g.
      >
      > {
      > "query":

      { "key": "\":arg\"" }

      > }
      >

      relative to this ml thread:

      http://mail-archives.apache.org/mod_mbox/couchdb-dev/201102.mbox/browser

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        10s 1 Benoit Chesneau 23/Feb/11 06:47
        In Progress In Progress Open Open
        42d 2h 11m 1 Benoit Chesneau 06/Apr/11 09:58
        Open Open Resolved Resolved
        48s 1 Benoit Chesneau 06/Apr/11 09:59
        Hide
        Benoit Chesneau added a comment -

        "Rewrites need to be expressible in JSON, and requiring to quote Boolean values breaks that and adds a requirement for additional knowledge ("write JSON docs everywhere but here...") to get rewrites to work in 1.1+.
        " who says ? Why it needs?

        The main problem isn't the format of rewrites. The main problem is to handle miscellaneous query args which are strings. Detecting arrays vs strings, integer vs strings, tuple vs strings, boolean vs strings etc.

        Show
        Benoit Chesneau added a comment - "Rewrites need to be expressible in JSON, and requiring to quote Boolean values breaks that and adds a requirement for additional knowledge ("write JSON docs everywhere but here...") to get rewrites to work in 1.1+. " who says ? Why it needs? The main problem isn't the format of rewrites. The main problem is to handle miscellaneous query args which are strings. Detecting arrays vs strings, integer vs strings, tuple vs strings, boolean vs strings etc.
        Hide
        Benjamin Young added a comment -

        Here's the new ticket for this: COUCHDB-1306

        Show
        Benjamin Young added a comment - Here's the new ticket for this: COUCHDB-1306
        Hide
        Benjamin Young added a comment -

        As mentioned in the comments on COUCHDB-1290 the fix for this issue forces Boolean values to be quoted, and therefore means past rewrite rules will break when someone upgrades to 1.1+

        Rewrites need to be expressible in JSON, and requiring to quote Boolean values breaks that and adds a requirement for additional knowledge ("write JSON docs everywhere but here...") to get rewrites to work in 1.1+.

        Perhaps this is what Jan was referring to in his earlier comments.

        Show
        Benjamin Young added a comment - As mentioned in the comments on COUCHDB-1290 the fix for this issue forces Boolean values to be quoted, and therefore means past rewrite rules will break when someone upgrades to 1.1+ Rewrites need to be expressible in JSON, and requiring to quote Boolean values breaks that and adds a requirement for additional knowledge ("write JSON docs everywhere but here...") to get rewrites to work in 1.1+. Perhaps this is what Jan was referring to in his earlier comments.
        Benoit Chesneau made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Benoit Chesneau added a comment -

        fixed in svn rev 1089361 . Thanks for the review.

        Show
        Benoit Chesneau added a comment - fixed in svn rev 1089361 . Thanks for the review.
        Benoit Chesneau made changes -
        Status In Progress [ 3 ] Open [ 1 ]
        Hide
        Jan Lehnardt added a comment -

        +1 on getting this committed. There are a few trailing white spaces to look out for and I wouldn't mind having tests for the "bool" format, other than that, looks fine to me.

        Show
        Jan Lehnardt added a comment - +1 on getting this committed. There are a few trailing white spaces to look out for and I wouldn't mind having tests for the "bool" format, other than that, looks fine to me.
        Hide
        Jan Lehnardt added a comment -

        Forgive me, I was reading the patches backwards – Looking good then.

        Show
        Jan Lehnardt added a comment - Forgive me, I was reading the patches backwards – Looking good then.
        Hide
        Jan Lehnardt added a comment -

        This looks alright to me, although the 0002 patch removes all tests for this.

        Show
        Jan Lehnardt added a comment - This looks alright to me, although the 0002 patch removes all tests for this.
        Benoit Chesneau made changes -
        Priority Major [ 3 ] Blocker [ 1 ]
        Benoit Chesneau made changes -
        Hide
        Benoit Chesneau added a comment -

        Works with the patch:
        0001-fix-variable-substitutions.-handle-cases.patch

        This second patch fix formatting in variables substitutions by adding the format member to the rewrite rule:

        {
        "from": "simpleForm/basicViewPath/:start/:end",
        "to": "_list/simpleForm/basicView",
        "query":

        { "startkey": ":start", "endkey": ":end" }

        ,
        "formats":

        { "start": "int", "end": "int" }

        },

        Show
        Benoit Chesneau added a comment - Works with the patch: 0001-fix-variable-substitutions.-handle-cases.patch This second patch fix formatting in variables substitutions by adding the format member to the rewrite rule: { "from": "simpleForm/basicViewPath/:start/:end", "to": "_list/simpleForm/basicView", "query": { "startkey": ":start", "endkey": ":end" } , "formats": { "start": "int", "end": "int" } },
        Benoit Chesneau made changes -
        Hide
        Benoit Chesneau added a comment - - edited

        Updated patch. only fix variable substitution:

        • key= ":key", startkey=[":a", ":b"]
        • variable substitution via query arguments
        • variable substituin via reversed path matching variables

        The variable substition is now a lot easier than the old one. Variables are decoded from the query if they are json, and we recode them only at the end.

        Show
        Benoit Chesneau added a comment - - edited Updated patch. only fix variable substitution: key= ":key", startkey= [":a", ":b"] variable substitution via query arguments variable substituin via reversed path matching variables The variable substition is now a lot easier than the old one. Variables are decoded from the query if they are json, and we recode them only at the end.
        Hide
        Benoit Chesneau added a comment - - edited

        currently asking myself if it's worth to split the diff in 2. One fixing var parsing. Second adding format handling. Though two are needed to solve completly the rewriter with this design. any thoughts?

        Edit: finnally splitted in :

        • 0001-fix-variable-substitutions.-handle-cases.patch
        • 0002-add-formating-in-variables-substitution.patch
        Show
        Benoit Chesneau added a comment - - edited currently asking myself if it's worth to split the diff in 2. One fixing var parsing. Second adding format handling. Though two are needed to solve completly the rewriter with this design. any thoughts? Edit: finnally splitted in : 0001-fix-variable-substitutions.-handle-cases.patch 0002-add-formating-in-variables-substitution.patch
        Benoit Chesneau made changes -
        Labels rewriter
        Hide
        Benoit Chesneau added a comment -

        bump.

        Show
        Benoit Chesneau added a comment - bump.
        Benoit Chesneau made changes -
        Hide
        Benoit Chesneau added a comment -

        same patch, fix a test.

        Show
        Benoit Chesneau added a comment - same patch, fix a test.
        Benoit Chesneau made changes -
        Benoit Chesneau made changes -
        Attachment 0001-fix-issue-COUCHDB-1074.-fix-variable-substitution.patch [ 12471699 ]
        Benoit Chesneau made changes -
        Attachment 0001-fix-issue-COUCHDB-1074.-fix-variable-substitution.patch [ 12471699 ]
        Hide
        Benoit Chesneau added a comment - - edited

        same patch, but remove a useless log info.

        Show
        Benoit Chesneau added a comment - - edited same patch, but remove a useless log info.
        Hide
        Benoit Chesneau added a comment -

        last patch is fixing the issue. js tests are OK.

        Show
        Benoit Chesneau added a comment - last patch is fixing the issue. js tests are OK.
        Benoit Chesneau made changes -
        Hide
        Benoit Chesneau added a comment -

        patch fixing the issue.

        handle cases:

        • key= ":key", startkey=[":a", ":b"]
        • formating in variables substitution:

        {
        "from": "simpleForm/basicViewPath/:start/:end",
        "to": "_list/simpleForm/basicView",
        "query":

        { "startkey": ":start", "endkey": ":end" }

        ,
        "formats":

        { "start": "int", "end": "int" }

        },

        • variable substitution via query arguments
        • variable substituin via reversed path matching variables

        The variable substition is now a lot easier than the old one. Variables
        are decoded from the query if they are json, and we recode them only at
        the end.

        Show
        Benoit Chesneau added a comment - patch fixing the issue. handle cases: key= ":key", startkey= [":a", ":b"] formating in variables substitution: { "from": "simpleForm/basicViewPath/:start/:end", "to": "_list/simpleForm/basicView", "query": { "startkey": ":start", "endkey": ":end" } , "formats": { "start": "int", "end": "int" } }, variable substitution via query arguments variable substituin via reversed path matching variables The variable substition is now a lot easier than the old one. Variables are decoded from the query if they are json, and we recode them only at the end.
        Benoit Chesneau made changes -
        Field Original Value New Value
        Status Open [ 1 ] In Progress [ 3 ]
        Benoit Chesneau created issue -

          People

          • Assignee:
            Benoit Chesneau
            Reporter:
            Benoit Chesneau
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development