jersey
  1. jersey
  2. JERSEY-2486

Clarify Javadoc of PatternWithGroups.getGroupIndexes()

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.7
    • Fix Version/s: 2.8
    • Component/s: core
    • Labels:
      None

      Description

      The Javadoc for PatternWithGroups.getGroupIndexes() reads Get the group indexes which doesn't actually explain why we need them (i.e. why not just use getGroupCount()?)

      I walked backwards through the code and figured out that indexes come from UriTemplateParser which provides the following (useful) explanation:

          /**
           * Get the group indexes to capturing groups.
           * <p>
           * Any nested capturing groups will be ignored and the
           * the group index will refer to the top-level capturing
           * groups associated with the templates variables.
           *
           * @return the group indexes to capturing groups.
           */
          public final int[] getGroupIndexes() {
      

      Please copy this Javadoc into PatternWithGroups.getGroupIndexes()

        Activity

        Hide
        cowwoc added a comment -

        I just discovered that sometimes (always in my case) this method returns an empty list in spite of the fact that the template has multiple groups. The Javadoc should also clarify when we can depend on this list being populated or not.

        Show
        cowwoc added a comment - I just discovered that sometimes (always in my case) this method returns an empty list in spite of the fact that the template has multiple groups. The Javadoc should also clarify when we can depend on this list being populated or not.
        Hide
        cowwoc added a comment -

        Michael,

        Looking at https://github.com/jersey/jersey/commit/47266536bb588ce54e99875f1bcec63653317dda was the old implementation of getGroupIndexes() broken? Was that why I was receiving an empty array?

        Show
        cowwoc added a comment - Michael, Looking at https://github.com/jersey/jersey/commit/47266536bb588ce54e99875f1bcec63653317dda was the old implementation of getGroupIndexes() broken? Was that why I was receiving an empty array?
        Hide
        Michal Gajdos added a comment -

        Yep. In Jersey < 2.8 you get an empty array in two cases:

        • there are no templates in provided Uri
        • templates don't contain nested groups (you'd get non-empty array just for templates like "/ {a: (abc)+}"). In this case there'd be n groups (where n is number of templates) which you can refer to via 1..n but you need to find out the value of n.

          Now (Jersey > 2.8) you'll get an non-empty array in case there is at least one template in Uri:

          - in case of "/{a}

          /

          {b}" you'll get "identity": a[0] = 1, a[1] = 2, a[2] = 3
          - in case of "/{a: (abc)+}/{b}

          " you'll get: a[0] = 1, a[1] = 3, a[2] = 4

        Show
        Michal Gajdos added a comment - Yep. In Jersey < 2.8 you get an empty array in two cases: there are no templates in provided Uri templates don't contain nested groups (you'd get non-empty array just for templates like "/ {a: (abc)+}"). In this case there'd be n groups (where n is number of templates) which you can refer to via 1..n but you need to find out the value of n. Now (Jersey > 2.8) you'll get an non-empty array in case there is at least one template in Uri: - in case of "/{a} / {b}" you'll get "identity": a [0] = 1, a [1] = 2, a [2] = 3 - in case of "/{a: (abc)+}/{b} " you'll get: a [0] = 1, a [1] = 3, a [2] = 4
        Hide
        cowwoc added a comment -

        Michal,

        In the case of "/

        {a}

        /

        {b}

        " why would I get a[2] = 3? I am only expecting the first two entries (one for "a" and one for "b").

        Show
        cowwoc added a comment - Michal, In the case of "/ {a} / {b} " why would I get a [2] = 3? I am only expecting the first two entries (one for "a" and one for "b").
        Hide
        Michal Gajdos added a comment -

        Hi Gili, good point, you're right. If I am not mistaken the intention (in Jersey 1.x) was that the first member of the array would represent a group for the whole Uri. But it doesn't work now. I'll file a new issue and fix it (unfortunately the fix wouldn't be part of 2.8).

        Show
        Michal Gajdos added a comment - Hi Gili, good point, you're right. If I am not mistaken the intention (in Jersey 1.x) was that the first member of the array would represent a group for the whole Uri. But it doesn't work now. I'll file a new issue and fix it (unfortunately the fix wouldn't be part of 2.8).
        Hide
        cowwoc added a comment -

        Michal,

        Group for the whole URI: Why do we need this? Doesn't group(0) already provide that value?
        New issue: Okay, please post the link here when it's ready.

        Show
        cowwoc added a comment - Michal, Group for the whole URI: Why do we need this? Doesn't group(0) already provide that value? New issue: Okay, please post the link here when it's ready.

          People

          • Assignee:
            Michal Gajdos
            Reporter:
            cowwoc
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: