With the new ListTag implementation, we evaluate more code in the
afterBody hook than before.
In the old implementation it was not very clear when
setCurrentList(this) was called for a decorator, and as decorators may
appear at different times of the state machine, the code was fragile,
and it created a bug where the entitlements in the system lists are
repeated multiple times, because I was setting the list once and then
again, however the ElaboratorDecorator had the unexpected behavior of
elaborating when setting the list, and then once again even if
setCurrentList was called with the same list.
This patch moves setting the list to one place, when adding the
decorator, this makes the code more robust as it is more predictable
that every decorator will get their list set, once, and in a predictable
place. And fixes the bug mentioned above.
Please don't apply this patch, just review, as it is in the branch with
all the other fixes.
Duncan
commit 95c6f86eef976242094ffe1840d329a1b9ed864e
Author: Duncan Mac-Vicar P <dmacvi...@suse.de>
Date: Tue Nov 26 11:24:25 2013 +0100
Set the current list for decorators as they are added in addDecorator()
Even if counter-intuitive, some decorators like ElaboratorDecorator do
side effects when setCurrentList is called, so we can't call it
"just to be sure".
On the other hand, decorators are added at different points, one
as an attribute to the list tag, others after the columns therefore
there is no clear place where to set the current list, why not do it
when they are about to be added.
diff --git a/java/code/src/com/redhat/rhn/frontend/taglibs/list/ListTag.java b/java/code/src/com/redhat/rhn/frontend/taglibs/list/ListTag.java
index 2f66cb9..ee03aa9 100644
--- a/java/code/src/com/redhat/rhn/frontend/taglibs/list/ListTag.java
+++ b/java/code/src/com/redhat/rhn/frontend/taglibs/list/ListTag.java
@@ -141,6 +141,7 @@ public class ListTag extends BodyTagSupport {
ListDecorator dec = getDecorator(decName);
if (dec != null) {
getDecorators().add(dec);
+ dec.setCurrentList(this);
}
}
@@ -420,7 +421,6 @@ public class ListTag extends BodyTagSupport {
// here decorators should insert other e.g hidden input fields
for (ListDecorator dec : getDecorators()) {
- dec.setCurrentList(this);
dec.afterList();
}
@@ -449,7 +449,6 @@ public class ListTag extends BodyTagSupport {
pageContext.pushBody(topPaginationContent);
if (!isEmpty()) {
for (ListDecorator dec : getDecorators()) {
- dec.setCurrentList(this);
dec.beforeTopPagination();
}
}
@@ -459,7 +458,6 @@ public class ListTag extends BodyTagSupport {
pageContext.pushBody(topAddonsContent);
if (!isEmpty()) {
for (ListDecorator dec : getDecorators()) {
- dec.setCurrentList(this);
dec.onTopExtraAddons();
}
}
@@ -503,7 +501,6 @@ public class ListTag extends BodyTagSupport {
pageContext.pushBody(headAddons);
if (!isEmpty()) {
for (ListDecorator dec : getDecorators()) {
- dec.setCurrentList(this);
dec.onHeadExtraAddons();
}
}
@@ -520,7 +517,6 @@ public class ListTag extends BodyTagSupport {
pageContext.pushBody(headExtraContent);
if (!isEmpty()) {
for (ListDecorator dec : getDecorators()) {
- dec.setCurrentList(this);
dec.onHeadExtraContent();
}
}
@@ -645,9 +641,7 @@ public class ListTag extends BodyTagSupport {
pageContext.pushBody(footAddonsContent);
if (!manip.isListEmpty()) {
for (ListDecorator dec : getDecorators()) {
- dec.setCurrentList(this);
dec.onFooterExtraAddons();
- dec.setCurrentList(null);
}
}
pageContext.popBody();
@@ -655,9 +649,7 @@ public class ListTag extends BodyTagSupport {
pageContext.pushBody(footExtraContent);
if (!manip.isListEmpty()) {
for (ListDecorator dec : getDecorators()) {
- dec.setCurrentList(this);
dec.onFooterExtraContent();
- dec.setCurrentList(null);
}
}
pageContext.popBody();
@@ -722,7 +714,6 @@ public class ListTag extends BodyTagSupport {
"<div class=\"spacewalk-list-bottom-addons-extra\">");
if (!isEmpty()) {
for (ListDecorator dec : getDecorators()) {
- dec.setCurrentList(this);
dec.onBottomExtraAddons();
}
}
@@ -732,7 +723,6 @@ public class ListTag extends BodyTagSupport {
"<div class=\"spacewalk-list-bottom-addons-extra\">");
if (!isEmpty()) {
for (ListDecorator dec : getDecorators()) {
- dec.setCurrentList(this);
dec.onBottomExtraContent();
}
}
@@ -847,7 +837,6 @@ public class ListTag extends BodyTagSupport {
ListCommand.ENUMERATE);
for (ListDecorator dec : getDecorators()) {
- dec.setCurrentList(this);
dec.beforeList();
}
@@ -1040,9 +1029,7 @@ public class ListTag extends BodyTagSupport {
if (!manip.isListEmpty()) {
for (ListDecorator dec : getDecorators()) {
- dec.setCurrentList(this);
dec.afterBottomPagination();
- dec.setCurrentList(null);
}
}
_______________________________________________
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
