Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support annotations on private and package local methods #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

johngmyers
Copy link
Contributor

Depends on pull #15

A jmxutils annotation on a method explicitly indicates the class's intent to have that method managed by jmxutils. It shouldn't also be necessary for the class to make that method callable by anybody else.

@martint
Copy link
Owner

martint commented Jun 25, 2013

I'm not sure I agree with this change. I generally dislike framework code that can magically "see" into private methods/state. There are some philosophical and some practical issues with allowing and encouraging this form.

The intent of jmxutils is to make exposing objects via jmx easy, but not to be the canonical way to access or manage them. Today, a class that is annotated with @Managed is fully usable, even if jmxutils is not in the classpath (annotations are optional dependencies in java). This would not be possible for private methods marked with @Managed. Moreover, all @Managed methods should really be part of the public interface of an object, they should be testable, etc.

From an implementation point of view, the call to setAccessible is problematic since it will not work when running within a SecurityManager.

@johngmyers
Copy link
Contributor Author

The placing of @Managed on a method is an explicit declaration by the author of that object that access to that method through JMX is part of the contract. An object should not be required to also expose that method to other packages in order to get that JMX management (or whatever semantics the framework provides).

Testing of such methods should be through the framework, not directly through the method. Tests of the latter are incomplete, as they do not catch errors in annotation or naming. Furthermore, @Nested and @Flatten hide implementation details--tests directly to the methods are testing the implementation, not the (JMX-exposed) API.

I would be willing to write code to handle running within a SecurityManager.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants