RFR: 8344976: Remove the jmx.invoke.getters compatibility property [v2]

[Kevin Walls]

On Fri, 24 Jan 2025 20:31:01 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Kevin Walls has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   (C)
>
> src/java.management/share/classes/com/sun/jmx/mbeanserver/PerInterface.java line 149:
> 
>> 147:             throws MBeanException, ReflectionException {
>> 148: 
>> 149:         // Construct the exception that we will throw
> 
> Although your changes look fine, the existing code for constructing this exception is odd in that it artificially introduces a `cause` exception.  It seems to mostly want to capture additional msg information, but most of it seems to be duplicated in the msg passed in, and all this could instead just  be handled with a single msg constructed at the call site (and the ReflectionException could be allocated at the call site). Also, what it the @SuppressWarnings("removal") for?

Thanks -
  
Ah, "removal" was added  for SM removal, that can surely go now.  Actually I don't see anything in what we're removing which required it, seems to have been unnecessary.

The method is almost trivial now, and only used in two places.  Yes it is a bit odd to manufacture a new cause, rather than throw something immediately.

Elsewhere in this file, e.g. the getAttribute method does just throw new AttributeNotFoundException(msg),
but here in invoke() we use "return noSuchMethod(...)" which effectively does:

  throw new ReflectionException(new NoSuchMethodException(operation + sigString(signature)), msg);

(Hah, yes unlike a new Exception(String message, Throwable cause), ReflectionException uses new ReflectionException(Exception e, String message).)

Two lines like that mean that the noSuchMethod method can be removed.  I'm trying that out...

But changing the behaviour (throwing a different Exception) might come with some compatibilty risk.  I think it's good not to change the behaviour in this property removal change.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23132#discussion_r1929381074