Tapestry
  1. Tapestry
  2. TAPESTRY-234

contrib:Table can't render column blocks when column name has period in it

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0.1
    • Fix Version/s: 4.0.1, 4.1.5
    • Component/s: Contrib
    • Labels:
      None

      Description

      I'm using the following column attribute in my contrib:Table:

      columns="user.username:username, user.firstName:firstName, user.lastName:lastName, user.email:email"

      If I try to override one of the column with an @Block, an error gets thrown. It'd be nice if periods were allowed, possibly converted to underscores or something for the @Block element. I'll attach a patch to SimpleTableColumn that makes this work.

      1. AbstractTableColumn.patch
        2 kB
        Matt Raible
      2. contribTable.patch
        2 kB
        Matt Raible
      3. contribTable-20070829.patch
        1.0 kB
        Matt Raible
      4. SimpleTableColumn.patch
        0.7 kB
        Matt Raible

        Issue Links

          Activity

          Hide
          Matt Raible added a comment -

          The first attachment is invalid and doesn't work. This attached patch is much better and works as desired. This patch solves the following issue.

          If I use "user.username:username" as a column, I'm unable to override its columnValueBlock. If I use:

          <tr jwcid="user.usernameColumnValue@Block">
          <a jwcid="@DirectLink" listener="ognl:listeners.edit" parameters="ognl:row.username">
          <span jwcid="@Insert" value="ognl:row.username"/>
          </a>
          </tr>

          I get the following error:

          Tag <tr> on line 20 contains an invalid jwcid 'user.usernameColumnValue@Block'.

          This patch allows to replaces . with _ for the column name and allows you to override the column value with:

          <tr jwcid="user_usernameColumnValue@Block">
          <a jwcid="@DirectLink" listener="ognl:listeners.edit" parameters="ognl:row.username">
          <span jwcid="@Insert" value="ognl:row.username"/>
          </a>
          </tr>

          Show
          Matt Raible added a comment - The first attachment is invalid and doesn't work. This attached patch is much better and works as desired. This patch solves the following issue. If I use "user.username:username" as a column, I'm unable to override its columnValueBlock. If I use: <tr jwcid="user.usernameColumnValue@Block"> <a jwcid="@DirectLink" listener="ognl:listeners.edit" parameters="ognl:row.username"> <span jwcid="@Insert" value="ognl:row.username"/> </a> </tr> I get the following error: Tag <tr> on line 20 contains an invalid jwcid 'user.usernameColumnValue@Block'. This patch allows to replaces . with _ for the column name and allows you to override the column value with: <tr jwcid="user_usernameColumnValue@Block"> <a jwcid="@DirectLink" listener="ognl:listeners.edit" parameters="ognl:row.username"> <span jwcid="@Insert" value="ognl:row.username"/> </a> </tr>
          Hide
          Matt Raible added a comment -

          Any chance of getting this patch committed?

          Show
          Matt Raible added a comment - Any chance of getting this patch committed?
          Hide
          Jesse Kuhnert added a comment -

          I'll do it tomorrow on my "faster" computer. No idea if a 3.x release is intended to go out anytime soon though...I guess if this is the only bug we care about there's no reason why not? heh

          Show
          Jesse Kuhnert added a comment - I'll do it tomorrow on my "faster" computer. No idea if a 3.x release is intended to go out anytime soon though...I guess if this is the only bug we care about there's no reason why not? heh
          Hide
          Matt Raible added a comment -

          Actually, I'm not so concerned about 3.0 since I already patched that JAR. This bug still happens in 4.0 and it'd be great to see this fixed in the next 4.x release. Thanks!

          Show
          Matt Raible added a comment - Actually, I'm not so concerned about 3.0 since I already patched that JAR. This bug still happens in 4.0 and it'd be great to see this fixed in the next 4.x release. Thanks!
          Hide
          Jesse Kuhnert added a comment -

          Fixed in subversion 4.0 branch

          Show
          Jesse Kuhnert added a comment - Fixed in subversion 4.0 branch
          Hide
          Matt Raible added a comment -

          I updated branches/4.0 and compiled. However, my column block doesn't override anything. The following worked with my 3.0.3 patch.

          <table jwcid="table@contrib:Table" class="list userList" id="userList"
          rowsClass="ognl:beans.evenOdd.next" row="ognl:row"
          columns="user.username:username, user.firstName:firstName, user.lastName:lastName, user.email:email, user.enabled:enabled"
          source="ognl:users" initialSortColumn="username"
          arrowUpAsset="ognl:assets.upArrow" arrowDownAsset="ognl:assets.downArrow">
          <tr jwcid="user_usernameColumnValue@Block">
          <a jwcid="@DirectLink" listener="ognl:listeners.edit" parameters="ognl:row.username">
          <span jwcid="@Insert" value="ognl:row.username"/>
          </a>
          </tr>
          <tr jwcid="user_emailColumnValue@Block">
          <a jwcid="@Any" href="ognl:+'mailto:'+row.email">
          <span jwcid="@Insert" value="ognl:row.email"/>
          </a>
          </tr>
          </table>

          Show
          Matt Raible added a comment - I updated branches/4.0 and compiled. However, my column block doesn't override anything. The following worked with my 3.0.3 patch. <table jwcid="table@contrib:Table" class="list userList" id="userList" rowsClass="ognl:beans.evenOdd.next" row="ognl:row" columns="user.username:username, user.firstName:firstName, user.lastName:lastName, user.email:email, user.enabled:enabled" source="ognl:users" initialSortColumn="username" arrowUpAsset="ognl:assets.upArrow" arrowDownAsset="ognl:assets.downArrow"> <tr jwcid="user_usernameColumnValue@Block"> <a jwcid="@DirectLink" listener="ognl:listeners.edit" parameters="ognl:row.username"> <span jwcid="@Insert" value="ognl:row.username"/> </a> </tr> <tr jwcid="user_emailColumnValue@Block"> <a jwcid="@Any" href="ognl:+'mailto:'+row.email"> <span jwcid="@Insert" value="ognl:row.email"/> </a> </tr> </table>
          Hide
          Matt Raible added a comment -

          Jesse,

          It looks like the wrong patch got committed. Here's a new one (contribTable.patch) that works and fixes the problem.

          Show
          Matt Raible added a comment - Jesse, It looks like the wrong patch got committed. Here's a new one (contribTable.patch) that works and fixes the problem.
          Hide
          Matt Raible added a comment -

          After upgrading from 4.0.2 to 4.1.1, this doesn't seem to work again. The first part of the patch (contribTable.patch) seems to be there (for SimpleTableColumn.java), but not the 2nd part (for AbstractTableColumn.java).

          Index: src/java/org/apache/tapestry/contrib/table/model/common/AbstractTableColumn.java
          ===================================================================
          — src/java/org/apache/tapestry/contrib/table/model/common/AbstractTableColumn.java (revision 389802)
          +++ src/java/org/apache/tapestry/contrib/table/model/common/AbstractTableColumn.java (working copy)
          @@ -222,13 +222,16 @@
          */
          public void loadSettings(IComponent objSettingsContainer)
          {

          • IComponent objColumnRendererSource = (IComponent) objSettingsContainer.getComponents().get(getColumnName() + COLUMN_RENDERER_BLOCK_SUFFIX);
            + // Replace any periods in the column name with underscores so columns can be referenced in a @Block
            + String columnName = getColumnName().replace('.', '_');
            +
            + IComponent objColumnRendererSource = (IComponent) objSettingsContainer.getComponents().get(columnName + COLUMN_RENDERER_BLOCK_SUFFIX);
            if (objColumnRendererSource == null)
            objColumnRendererSource = (IComponent) objSettingsContainer.getComponents().get(COLUMN_RENDERER_BLOCK_SUFFIX);
            if (objColumnRendererSource != null && objColumnRendererSource instanceof Block)
            setColumnRendererSource(new BlockTableRendererSource((Block) objColumnRendererSource));
          • IComponent objValueRendererSource = (IComponent) objSettingsContainer.getComponents().get(getColumnName() + VALUE_RENDERER_BLOCK_SUFFIX);
            + IComponent objValueRendererSource = (IComponent) objSettingsContainer.getComponents().get(columnName + VALUE_RENDERER_BLOCK_SUFFIX);
            if (objValueRendererSource == null)
            objValueRendererSource = (IComponent) objSettingsContainer.getComponents().get(VALUE_RENDERER_BLOCK_SUFFIX);
            if (objValueRendererSource != null && objValueRendererSource instanceof Block)
          Show
          Matt Raible added a comment - After upgrading from 4.0.2 to 4.1.1, this doesn't seem to work again. The first part of the patch (contribTable.patch) seems to be there (for SimpleTableColumn.java), but not the 2nd part (for AbstractTableColumn.java). Index: src/java/org/apache/tapestry/contrib/table/model/common/AbstractTableColumn.java =================================================================== — src/java/org/apache/tapestry/contrib/table/model/common/AbstractTableColumn.java (revision 389802) +++ src/java/org/apache/tapestry/contrib/table/model/common/AbstractTableColumn.java (working copy) @@ -222,13 +222,16 @@ */ public void loadSettings(IComponent objSettingsContainer) { IComponent objColumnRendererSource = (IComponent) objSettingsContainer.getComponents().get(getColumnName() + COLUMN_RENDERER_BLOCK_SUFFIX); + // Replace any periods in the column name with underscores so columns can be referenced in a @Block + String columnName = getColumnName().replace('.', '_'); + + IComponent objColumnRendererSource = (IComponent) objSettingsContainer.getComponents().get(columnName + COLUMN_RENDERER_BLOCK_SUFFIX); if (objColumnRendererSource == null) objColumnRendererSource = (IComponent) objSettingsContainer.getComponents().get(COLUMN_RENDERER_BLOCK_SUFFIX); if (objColumnRendererSource != null && objColumnRendererSource instanceof Block) setColumnRendererSource(new BlockTableRendererSource((Block) objColumnRendererSource)); IComponent objValueRendererSource = (IComponent) objSettingsContainer.getComponents().get(getColumnName() + VALUE_RENDERER_BLOCK_SUFFIX); + IComponent objValueRendererSource = (IComponent) objSettingsContainer.getComponents().get(columnName + VALUE_RENDERER_BLOCK_SUFFIX); if (objValueRendererSource == null) objValueRendererSource = (IComponent) objSettingsContainer.getComponents().get(VALUE_RENDERER_BLOCK_SUFFIX); if (objValueRendererSource != null && objValueRendererSource instanceof Block)
          Hide
          Matt Raible added a comment -

          Has my comment from Dec-28-2006 been fixed in 4.1.2?

          Show
          Matt Raible added a comment - Has my comment from Dec-28-2006 been fixed in 4.1.2?
          Hide
          Jesse Kuhnert added a comment -

          It's possible I didn't see that last comment. The svn logs for the ticket are for much earlier in the year. I'll take a look tonight.

          Show
          Jesse Kuhnert added a comment - It's possible I didn't see that last comment. The svn logs for the ticket are for much earlier in the year. I'll take a look tonight.
          Hide
          Jesse Kuhnert added a comment -

          All fixed up now, slightly different from the patch but with the same result. (just where the string replace happens is all..)

          Show
          Jesse Kuhnert added a comment - All fixed up now, slightly different from the patch but with the same result. (just where the string replace happens is all..)
          Hide
          Matt Raible added a comment -

          Are snapshot builds posted somewhere? I was hoping to upgraded from 4.0.2 to 4.1.2 tonight, but I need this patch to do the upgrade.

          Thanks,

          Matt

          Show
          Matt Raible added a comment - Are snapshot builds posted somewhere? I was hoping to upgraded from 4.0.2 to 4.1.2 tonight, but I need this patch to do the upgrade. Thanks, Matt
          Hide
          Jesse Kuhnert added a comment -

          Yeah, they always go to http://people.apache.org/repo/m2-snapshot-repository/ .

          If I resolve a ticket it's usually safe to expect that a new build has gone out within an hour or two or at least in the same day. In this case it's already out so you should be good to go on 4.1.3-SNAPSHOT.

          4.1.3 should be getting released as soon as I get organized enough to release it.

          Show
          Jesse Kuhnert added a comment - Yeah, they always go to http://people.apache.org/repo/m2-snapshot-repository/ . If I resolve a ticket it's usually safe to expect that a new build has gone out within an hour or two or at least in the same day. In this case it's already out so you should be good to go on 4.1.3-SNAPSHOT. 4.1.3 should be getting released as soon as I get organized enough to release it.
          Hide
          Matt Raible added a comment -

          Is a SNAPSHOT of OGNL available?

          [INFO] ------------------------------------------------------------------------
          [ERROR] BUILD ERROR
          [INFO] ------------------------------------------------------------------------
          [INFO] Failed to resolve artifact.

          Missing:
          ----------
          1) ognl:ognl:jar:2.7.1-SNAPSHOT

          Show
          Matt Raible added a comment - Is a SNAPSHOT of OGNL available? [INFO] ------------------------------------------------------------------------ [ERROR] BUILD ERROR [INFO] ------------------------------------------------------------------------ [INFO] Failed to resolve artifact. Missing: ---------- 1) ognl:ognl:jar:2.7.1-SNAPSHOT
          Show
          Jesse Kuhnert added a comment - Oops...yeah: http://opencomponentry.com/repository/m2-snapshot-repo/
          Hide
          Matt Raible added a comment - - edited

          This doesn't quite work as expected - maybe it has something to do with TAPESTRY-881. My column headers are rendered as user_username, user_email, user_enabled, etc. when in 4.0.x, they got rendered with i18n keys.

          Here's my table:

          <table jwcid="table@contrib:Table" class="table contribTable" id="userList"
          rowsClass="ognl:beans.rowsClass.next" row="ognl:row"
          columns="user.username:username, activeUsers.fullName:fullName, user.email:email, user.enabled:enabled"
          source="ognl:users" initialSortColumn="username"
          arrowUpAsset="asset:upArrow" arrowDownAsset="asset:downArrow">
          <tr jwcid="user_usernameColumnValue@Block">
          <a jwcid="@DirectLink" listener="listener:edit" parameters="ognl:row.id">
          <span jwcid="@Insert" value="ognl:row.username"/>
          </a>
          </tr>
          <tr jwcid="user_emailColumnValue@Block">
          <a jwcid="@Any" href="ognl:+'mailto:'+row.email">
          <span jwcid="@Insert" value="ognl:row.email"/>
          </a>
          </tr>
          <tr jwcid="user_enabledColumnValue@Block">
          <span jwcid="@If" condition="ognl:row.enabled == true"><input type="checkbox" disabled="disabled" checked="checked" style="margin-left: 15px"/></span>
          <span jwcid="@Else"><input type="checkbox" disabled="disabled" style="margin-left: 15px"/></span>
          </tr>
          </table>

          Show
          Matt Raible added a comment - - edited This doesn't quite work as expected - maybe it has something to do with TAPESTRY-881 . My column headers are rendered as user_username, user_email, user_enabled, etc. when in 4.0.x, they got rendered with i18n keys. Here's my table: <table jwcid="table@contrib:Table" class="table contribTable" id="userList" rowsClass="ognl:beans.rowsClass.next" row="ognl:row" columns="user.username:username, activeUsers.fullName:fullName, user.email:email, user.enabled:enabled" source="ognl:users" initialSortColumn="username" arrowUpAsset="asset:upArrow" arrowDownAsset="asset:downArrow"> <tr jwcid="user_usernameColumnValue@Block"> <a jwcid="@DirectLink" listener="listener:edit" parameters="ognl:row.id"> <span jwcid="@Insert" value="ognl:row.username"/> </a> </tr> <tr jwcid="user_emailColumnValue@Block"> <a jwcid="@Any" href="ognl:+'mailto:'+row.email"> <span jwcid="@Insert" value="ognl:row.email"/> </a> </tr> <tr jwcid="user_enabledColumnValue@Block"> <span jwcid="@If" condition="ognl:row.enabled == true"><input type="checkbox" disabled="disabled" checked="checked" style="margin-left: 15px"/></span> <span jwcid="@Else"><input type="checkbox" disabled="disabled" style="margin-left: 15px"/></span> </tr> </table>
          Hide
          Jesse Kuhnert added a comment -

          Hmmm ok. I guess I'll have to actually try it out to see for myself what is happening. I'll get back to you through jira. (obviously)

          Show
          Jesse Kuhnert added a comment - Hmmm ok. I guess I'll have to actually try it out to see for myself what is happening. I'll get back to you through jira. (obviously)
          Hide
          Matt Raible added a comment -

          I'd like to try and solve this myself - what class should I look at?

          Show
          Matt Raible added a comment - I'd like to try and solve this myself - what class should I look at?
          Hide
          Matt Raible added a comment -

          The attached patch (contribTable-20070829.patch) fixes the latest issue.

          Show
          Matt Raible added a comment - The attached patch (contribTable-20070829.patch) fixes the latest issue.
          Hide
          Jesse Kuhnert added a comment -

          Hopefully the apache ibiblio rsync scripts haven't run yet.

          Show
          Jesse Kuhnert added a comment - Hopefully the apache ibiblio rsync scripts haven't run yet.
          Hide
          Patrick Klein added a comment -

          The way to fix this bug seems to have side effects. In our application we use e.g. FIRST_NAME as column name which is rendered localized. After this fix, it renders now to FIRST.NAME which is not as it's supposed to be!

          Show
          Patrick Klein added a comment - The way to fix this bug seems to have side effects. In our application we use e.g. FIRST_NAME as column name which is rendered localized. After this fix, it renders now to FIRST.NAME which is not as it's supposed to be!
          Hide
          Jesse Kuhnert added a comment -

          Yeah my bad on this. I hope to address it in the next couple of days once I've got a firm understanding of all things involved.

          Show
          Jesse Kuhnert added a comment - Yeah my bad on this. I hope to address it in the next couple of days once I've got a firm understanding of all things involved.
          Hide
          Claudio Mantovani added a comment -

          This behaviour has pretty bad side effects.
          First of all, as Patrick Klein reported, we cannot use column names with underscores, since they get "changed" in dots when displayed.
          Second, we can't use sorting on columns whose value contains dots, since they get converted to underscores when passed to the bound table model.
          It would better to come up with a different solution, that does not break usage of dots and underscores in column paths/names.

          In any case, column display code should be fixed to preserve original format.
          Some ideas to fix the second problem:

          • use a more elaborate pattern, less likely to cause conflicts (e.g.: "___", three underscores) (simple, easy to adapt for users but hacky, although less that the current broken solution)
          • connect the "overriding" blocks to the tablevalues with a different method, that does not involve a particular component name (would also be more robust, but harder and requires more refactoring by users)

          A "compatibility" mode could be used to allow old code to work as before, specified by a parameter on the tablevalues component.

          This has caused us multiple problems in multiple situations, and should REALLY be fixed ASAP.
          Thanks

          Show
          Claudio Mantovani added a comment - This behaviour has pretty bad side effects. First of all, as Patrick Klein reported, we cannot use column names with underscores, since they get "changed" in dots when displayed. Second, we can't use sorting on columns whose value contains dots, since they get converted to underscores when passed to the bound table model. It would better to come up with a different solution, that does not break usage of dots and underscores in column paths/names. In any case, column display code should be fixed to preserve original format. Some ideas to fix the second problem: use a more elaborate pattern, less likely to cause conflicts (e.g.: "___", three underscores) (simple, easy to adapt for users but hacky, although less that the current broken solution) connect the "overriding" blocks to the tablevalues with a different method, that does not involve a particular component name (would also be more robust, but harder and requires more refactoring by users) A "compatibility" mode could be used to allow old code to work as before, specified by a parameter on the tablevalues component. This has caused us multiple problems in multiple situations, and should REALLY be fixed ASAP. Thanks
          Hide
          Jesse Kuhnert added a comment -

          Ok, re-did the logic to work for everyone concerned. (I hope)

          Show
          Jesse Kuhnert added a comment - Ok, re-did the logic to work for everyone concerned. (I hope)
          Hide
          Claudio Mantovani added a comment -

          Thank you!
          You were very fast!! We will try as soon as possible the changes.

          Show
          Claudio Mantovani added a comment - Thank you! You were very fast!! We will try as soon as possible the changes.

            People

            • Assignee:
              Jesse Kuhnert
              Reporter:
              Matt Raible
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development