markt-asf commented on code in PR #1079:
URL: https://github.com/apache/tomcat/pull/1079#discussion_r4294502354
##########
java/org/apache/tomcat/util/modeler/BaseAttributeFilter.java:
##########
@@ -60,11 +62,15 @@ public BaseAttributeFilter(String name) {
/**
- * The set of attribute names that are accepted by this filter. If this
list is empty, all attribute names are
- * accepted.
+ * The set of attribute names that are accepted or excluded by this
filter, depending on {@link #acceptAll}.
Review Comment:
The comment needs to explicitly state how the value of acceptAll affects
include/exclude
##########
java/org/apache/tomcat/util/modeler/BaseAttributeFilter.java:
##########
@@ -77,7 +83,11 @@ public BaseAttributeFilter(String name) {
public void addAttribute(String name) {
synchronized (names) {
- names.add(name);
+ if (acceptAll) {
+ names.remove(name);
+ } else {
+ names.add(name);
+ }
Review Comment:
I think a few short comments are required here to explain what is going on.
##########
java/org/apache/tomcat/util/modeler/BaseAttributeFilter.java:
##########
@@ -89,21 +99,25 @@ public void addAttribute(String name) {
public void clear() {
synchronized (names) {
+ acceptAll = true;
names.clear();
}
}
/**
- * Return the set of names that are accepted by this filter. If this
filter accepts all attribute names, a zero
- * length array will be returned.
+ * Return the set of names that are explicitly accepted by this filter. If
this filter is in accept-all mode,
+ * including when individual names have been excluded, a zero length array
will be returned.
Review Comment:
That is ... unexpected. Is that really the expected behavioour?
##########
java/org/apache/tomcat/util/modeler/BaseAttributeFilter.java:
##########
@@ -151,9 +161,27 @@ public boolean isNotificationEnabled(Notification
notification) {
public void removeAttribute(String name) {
synchronized (names) {
- names.remove(name);
+ if (acceptAll) {
+ names.add(name);
+ } else {
+ names.remove(name);
+ }
Review Comment:
More comments here too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
