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

        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.
        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.
        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.
        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" } },
        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
        Hide
        Benoit Chesneau added a comment -

        bump.

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

        same patch, fix a test.

        Show
        Benoit Chesneau added a comment - same patch, fix a test.
        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.
        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.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development