You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2016/11/16 12:38:14 UTC
[Bug 60381] New: Inconsistent toString() in ValveBase and RealmBase
https://bz.apache.org/bugzilla/show_bug.cgi?id=60381
Bug ID: 60381
Summary: Inconsistent toString() in ValveBase and RealmBase
Product: Tomcat 8
Version: 8.5.x-trunk
Hardware: All
OS: All
Status: NEW
Severity: normal
Priority: P2
Component: Catalina
Assignee: dev@tomcat.apache.org
Reporter: 1983-01-06@gmx.net
Target Milestone: ----
Moving several components from Tomcat 6.0.x to 8.5.x I have noticed that
#getInfo() is gone. RealmBase has an abstact method #getName() which is used in
#toString()(). ValveBase lacks this abstract method. This makes the #toString()
implementations inconsistent.
RealmBase:
> @Override
> public String toString() {
> StringBuilder sb = new StringBuilder("Realm[");
> sb.append(getName());
> sb.append(']');
> return sb.toString();
> }
ValveBase:
> @Override
> public String toString() {
> StringBuilder sb = new StringBuilder(this.getClass().getName());
> sb.append('[');
> if (container == null) {
> sb.append("Container is null");
> } else {
> sb.append(container.getName());
> }
> sb.append(']');
> return sb.toString();
> }
I would rather expect a consistent behavior for both base implementations
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 60381] Inconsistent toString() in ValveBase and RealmBase
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60381
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #6 from Mark Thomas <ma...@apache.org> ---
Fixed in:
- trunk for 9.0.0.M14 onwards
I also applied the clean-up to Realm.getName()
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 60381] Inconsistent toString() in ValveBase and RealmBase
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60381
--- Comment #3 from Michael Osipov <19...@gmx.net> ---
(In reply to Mark Thomas from comment #2)
> The toString() implementations have been pretty much unchanged since the
> lifecycle refactoring in 7.0.x. While users shouldn't not be expecting these
> to be in any particular format, the chances are that there that some users
> are expecting the current format. Therefore, we can look at this for trunk
> but I don't think it should be back-ported.
I do not even expect some specific format, but rather a consistent approach.
Don't you think it is worthwile to port back to 8.5? This is going to be
supported for a long time compared to 6.0/7.0.
> My current thinking for a standardised format is:
> SimpleClassName[container.toString()] / SimpleClassName[Container is null]
Do you think that the simple class name is sufficient? I was used to #getInfo()
previously which has the FQCN, and this method is gone. Consider that people
might copy Tomcat's standard component into their source tree, modify code and
package but leave class name as-is. Still, this is good compromise.
> If we make more use of the Contained interface, it should be possible to do
> this as a single utility method e.g.:
> o.a.c.util.DebugUtil.containedToString(Contained)
>
> Maybe
> o.a.c.util.DebugUtil.containedToString(Object, Container)
> as well for those objects that don't/can't implement Contained.
This make defitively sense!
> Which means we might not need to expand the use of Contained anyway. I need
> to spend some time thinking about how much sense that does or doesn't make.
I have noticed that a lot of components which use Container do not implement
Contained at all. Is there a legacy reason for that? It seems awkward.
It might be worth considering deprecating RealmBase#getName() since only
toString() uses it and it is likely to be superseded.
Moreover, toString() has to be well crafted if it is used in MBeans/JMX or log
statements to clearly identify the component itself and its location in the
server tree.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 60381] Inconsistent toString() in ValveBase and RealmBase
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60381
--- Comment #5 from Michael Osipov <19...@gmx.net> ---
I see that you have picked up my proposal to reuse the Contained interface:
http://www.mail-archive.com/dev@tomcat.apache.org/msg113336.html
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 60381] Inconsistent toString() in ValveBase and RealmBase
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60381
--- Comment #2 from Mark Thomas <ma...@apache.org> ---
The toString() implementations have been pretty much unchanged since the
lifecycle refactoring in 7.0.x. While users shouldn't not be expecting these to
be in any particular format, the chances are that there that some users are
expecting the current format. Therefore, we can look at this for trunk but I
don't think it should be back-ported.
My current thinking for a standardised format is:
SimpleClassName[container.toString()] / SimpleClassName[Container is null]
This could/should apply to any component that is usually associated with a
single container (Valve, Realm, Manager, etc.)
If we make more use of the Contained interface, it should be possible to do
this as a single utility method e.g.:
o.a.c.util.DebugUtil.containedToString(Contained)
Although, that won't work with Manager, Loader etc. that are Context specific.
Maybe
o.a.c.util.DebugUtil.containedToString(Object, Container)
as well for those objects that don't/can't implement Contained.
Which means we might not need to expand the use of Contained anyway. I need to
spend some time thinking about how much sense that does or doesn't make.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 60381] Inconsistent toString() in ValveBase and RealmBase
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60381
--- Comment #4 from Mark Thomas <ma...@apache.org> ---
(In reply to Michael Osipov from comment #3)
> Don't you think it is worthwile to port back to 8.5?
No.
> Do you think that the simple class name is sufficient?
Yes.
> I have noticed that a lot of components which use Container do not implement
> Contained at all. Is there a legacy reason for that? It seems awkward.
It was added to solve a specific problem for Valves. It wasn't added to the
other components because it wasn't required.
> It might be worth considering deprecating RealmBase#getName() since only
> toString() uses it and it is likely to be superseded.
+1.
> Moreover, toString() has to be well crafted if it is used in MBeans/JMX or
> log statements to clearly identify the component itself and its location in
> the server tree.
JMX name generation should be separate from toString().
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[Bug 60381] Inconsistent toString() in ValveBase and RealmBase
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=60381
--- Comment #1 from Michael Osipov <19...@gmx.net> ---
I am willing to provide a patch if we agree on one approach.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org