Queue browse improvements in webconsole#1937
Conversation
This change makes sure we always use the correct content type for the output based on the configured view and also escape xml content when displaying. Co-authored-by: Jean-Baptiste Onofré <jbonofre@apache.org>
| <bundle dependency="true">mvn:org.apache.geronimo.specs/geronimo-jta_1.1_spec/1.1.1</bundle> | ||
| <bundle>mvn:org.apache.commons/commons-pool2/${commons-pool2-version}</bundle> | ||
| <bundle>mvn:org.apache.commons/commons-lang3/${commons-lang3-version}</bundle> | ||
| <bundle>mvn:org.apache.commons/commons-text/${commons-text-version}</bundle> |
There was a problem hiding this comment.
If we add these dependencies back in, they should be tied to the web console feature, and not the activemq-client
There was a problem hiding this comment.
No, because it goes in the activemq-osgi artifact, so it has to be in this feature.
This feature is core/common to all other features, including broker and webconsole.
So, adding these bundles in the webconsole feature won't work.
There was a problem hiding this comment.
The web console is deployed as an independent feature. Requiring these dependencies for activemq-client only users in Karaf (Camel, etc) is incorrect. Same for users using activemq-broker-noweb feature.
These dependencies should be added to the activemq-web-console feature here:
There was a problem hiding this comment.
You don't get my point: today the activemq-osgi requires commons-text because the web-console import leak in activemq-osgi.
That's not in the webconsole war, it's in the activemq-osgi bundle.
There was a problem hiding this comment.
What is the web leak in activemq-osgi?
There was a problem hiding this comment.
There was a problem hiding this comment.
It's a different issue (unrelated to this PR).
| <commons-collections-version>3.2.2</commons-collections-version> | ||
| <commons-dbcp2-version>2.14.0</commons-dbcp2-version> | ||
| <commons-io-version>2.21.0</commons-io-version> | ||
| <commons-lang3-version>3.20.0</commons-lang3-version> |
There was a problem hiding this comment.
I really struggle with adding these dependencies back in for essentially one method. I think we'd end up being tied to their release cycle for security patches and ActiveMQ get flagged for CVE vulnerability, even if we don't use vulnerable methods.
There was a problem hiding this comment.
Yes, we are back to what we had before 😄 Going in circle 😄
There was a problem hiding this comment.
I don't have any issue with adding dependencies on things like commons libraries and they are quite useful so I disagree with that. Limiting dependencies is good but I'm not going to worry about useful ones like commons-lang3 which I found hard to believe it didn't exist.
I mostly just want to get rid of the Spring dependencies but anything Apache Commons I am fine with.
There was a problem hiding this comment.
Also I will point out if we want to get rid of Spring we actually will want more of these types of libraries. We are currently using Spring utils in a lot of cases for things like HTML validation etc. We can switch to using Apache commons instead
There was a problem hiding this comment.
I believe we'll always have custom buffer, io, and text handling classes due to all the protocol and admin functionality due to the nature of operating a performant broker. Part of the reason to re-home activemq-protobuf is to be able to consolidate those classes UTF8Buffer, ASCIIBuffer, etc.. into a single activemq-io module. We have a couple sets of these classes and custom stream handlers across the project, and the hawt* dependencies. I think once those classes are consolidated to a single module, it'll be more apparent.
I'm not opposed to adding this to the web console, b/c it is expedient and the desire to overhaul/replace this broker.
However, I think adoption of wider use of third-party I/O dependencies warrants a bigger discussion when it comes to client/broker usage.
|
I'm closing this in favor of #1938 |
3 participants
This change makes sure we always use the correct content type for the output based on the configured view and also escape xml content when displaying.