RFR: 7167: Agent Plugin [v6]

[Brice Dutheil]

On Wed, 28 Apr 2021 17:57:17 GMT, Joshua Matsuoka <jmatsuoka at openjdk.org> wrote:

>> This PR adds the Agent Plugin to JMC. This is a plugin for controlling the jmc agent and provides a way to attach it and add probes to any JVMs in the JVM browser, as well as providing a preset manager for creating/modifying probe presets.
>> 
>> There is still a small amount of cleanup to be done so I'll mark it as a draft for now but it would be great to get a review :)
>
> Joshua Matsuoka has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix rcp feature to use new agent plugin name

This is a big PR !
There's a few nitpick around non localisable strings, that might be worth checking.
I think I'll build JMC to see it in action ;)

application/org.openjdk.jmc.console.agent/src/main/java/org/openjdk/jmc/console/agent/manager/model/PresetRepositoryFactory.java line 85:

> 83: 		for (File file : files) {
> 84: 			if (file.length() == 0) {
> 85: 				// FIXME: delete or just ignore empty files? 

Maybe ignore empty files but report them in a logger ?

application/org.openjdk.jmc.console.agent/src/main/java/org/openjdk/jmc/console/agent/raweditor/internal/XmlDocumentProvider.java line 74:

> 72: 		if (!out.exists()) {
> 73: 			try {
> 74: 				out.createNewFile();

I am not really aware of how `ei.getStorage().getFullPath()`, but could there be non-existant folders as well ?
Additionnally the code here could use `Files.*` methods.

application/org.openjdk.jmc.console.agent/src/main/java/org/openjdk/jmc/console/agent/raweditor/internal/XmlDocumentProvider.java line 81:

> 79: 		}
> 80: 
> 81: 		try (FileOutputStream fos = new FileOutputStream(out)) {

I think the FOS should be decorated by a `BufferedOutputStream`.

application/org.openjdk.jmc.console.agent/src/main/java/org/openjdk/jmc/console/agent/raweditor/internal/XmlDocumentProvider.java line 82:

> 80: 
> 81: 		try (FileOutputStream fos = new FileOutputStream(out)) {
> 82: 			fos.write(document.get().getBytes()); // TODO: encoding?

Yes I think the code should impose UTF-8, this is better for XML.

application/org.openjdk.jmc.console.agent/src/main/java/org/openjdk/jmc/console/agent/raweditor/internal/XmlPartitionScanner.java line 44:

> 42: public class XmlPartitionScanner extends RuleBasedPartitionScanner {
> 43: 	public final static String XML_COMMENT = "__xml_comment";
> 44: 	public final static String XML_TAG = "__xml_tag";

SHouldn't those be commented by `//$NON-NLS-1$`

configuration/spotbugs/spotbugs-exclude.xml line 85:

> 83: 		<Bug pattern="UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR"/>
> 84: 	</Match>
> 85: 	

Tabulation instead of spaces in this file

-------------

Marked as reviewed by bric3 at github.com (no known OpenJDK username).

PR: https://git.openjdk.java.net/jmc/pull/226