RFR: 7455: Add support for jolokia JMX service connection [v2]

[Alex Macdonald]

On Thu, 25 Jan 2024 17:42:51 GMT, Martin Skarsaune <duke at openjdk.org> wrote:

>> Replaces parts of https://github.com/openjdk/jmc/pull/332
>
> Martin Skarsaune has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit:
> 
>   JMC-7455: Add plugin with support for Jolokia connections

Gave this a quick read through, the builds aren't happy because a couple of the imports need to be updated in `ServerConnectionDescriptor` and `JolokiaTest`; both need `IConnectionDescriptor` and `IServerDescriptor` to be updated to come from `rjmx.common`. After these changes it looks like everything builds and tests pass.

I've commented on a handful more nits I found, but haven't actually tested the functionality yet so I can't comment on anything else.

Another PR where the check license script isn't happy though, I'll have to a take a look at why it's trying to check all these different files that weren't touched by your commit.

application/org.openjdk.jmc.feature.core/feature.xml line 234:

> 232: 
> 233:     <plugin
> 234:             id="org.openjdk.jmc.jolokia"

Indented too far?

application/org.openjdk.jmc.jolokia/build.properties line 2:

> 1: #
> 2: #  Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.

2021 -> 2024, there's a handful more of these so I'll only write it once here

application/org.openjdk.jmc.jolokia/src/main/java/org/openjdk/jmc/jolokia/ServerConnectionDescriptor.java line 38:

> 36: import javax.management.remote.JMXServiceURL;
> 37: 
> 38: import org.openjdk.jmc.rjmx.IConnectionDescriptor;

These two imports are now in rjmx.common

application/tests/org.openjdk.jmc.jolokia.test/META-INF/MANIFEST.MF line 3:

> 1: Manifest-Version: 1.0
> 2: Bundle-ManifestVersion: 2
> 3: Bundle-Name: RJMX Test

Update the name to Jolokia Test or something similar

application/tests/org.openjdk.jmc.jolokia.test/META-INF/MANIFEST.MF line 7:

> 5: Bundle-Version: 9.0.0.qualifier
> 6: Bundle-Vendor: Oracle Corporation
> 7: Bundle-RequiredExecutionEnvironment: JavaSE-1.8

Should be JavaSE-17?

application/tests/org.openjdk.jmc.jolokia.test/pom.xml line 67:

> 65: 				<configuration>
> 66: 					<includes>${test.includes}</includes>
> 67: 					<!-- Start jolokia on a random free port to avoid requiring

Comment could be cleaned up to not span 3 lines

application/tests/org.openjdk.jmc.jolokia.test/src/test/java/org/openjdk/jmc/jolokia/JolokiaTest.java line 68:

> 66: import org.openjdk.jmc.common.IDescribable;
> 67: import org.openjdk.jmc.jolokia.preferences.PreferenceConstants;
> 68: import org.openjdk.jmc.rjmx.IConnectionDescriptor;

IConnectionDescriptor and IServerDescriptor are now in rjmx.common now

application/tests/org.openjdk.jmc.jolokia.test/src/test/java/org/openjdk/jmc/jolokia/JolokiaTest.java line 73:

> 71: 
> 72: /**
> 73:  * I test that JMX connections done with JmcJolokiaJmxConnectionProvider are functional

Could probably substitute the "I test that" with "Tests that" or something similar

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

PR Review: https://git.openjdk.org/jmc/pull/548#pullrequestreview-1844462989
PR Review Comment: https://git.openjdk.org/jmc/pull/548#discussion_r1466818710
PR Review Comment: https://git.openjdk.org/jmc/pull/548#discussion_r1466819270
PR Review Comment: https://git.openjdk.org/jmc/pull/548#discussion_r1466820210
PR Review Comment: https://git.openjdk.org/jmc/pull/548#discussion_r1466821456
PR Review Comment: https://git.openjdk.org/jmc/pull/548#discussion_r1466821563
PR Review Comment: https://git.openjdk.org/jmc/pull/548#discussion_r1466823347
PR Review Comment: https://git.openjdk.org/jmc/pull/548#discussion_r1466823932
PR Review Comment: https://git.openjdk.org/jmc/pull/548#discussion_r1466822668