Skip to content

Add generatedNameOf accepting package name and class name #63

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.

Sign up for GitHub

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

Open
wendigo wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add generatedNameOf accepting package name and class name #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions src/main/java/org/weakref/jmx/ObjectNameGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public interface ObjectNameGenerator
{
static ObjectNameGenerator defaultObjectNameGenerator()
{
return (type, properties) -> new ObjectNameBuilder(type.getPackage().getName())
return (domain, properties) -> new ObjectNameBuilder(domain)
.withProperties(properties)
.build();
}
Expand All @@ -26,5 +26,30 @@ default String generatedNameOf(Class<?> type, String name)
.build());
}

String generatedNameOf(Class<?> type, Map<String, String> properties);
default String generatedNameOf(String packageName, String className)
{
return generatedNameOf(packageName, ImmutableMap.of("name", className));
}

default String generatedNameOf(String packageName, String className, String name)
{
return generatedNameOf(packageName, ImmutableMap.of("name", className, "type", name));
}

default String generatedNameOf(Package pkg, String className, String name)
{
return generatedNameOf(pkg.getName(), ImmutableMap.of("name", className, "type", name));
}

default String generatedNameOf(Package pkg, String className)
{
return generatedNameOf(pkg.getName(), ImmutableMap.of("name", className));
}

default String generatedNameOf(Class<?> type, Map<String, String> properties)
{
return generatedNameOf(type.getPackage().getName(), properties);
}

String generatedNameOf(String domain, Map<String, String> properties);
Comment on lines +29 to +54
Copy link
Owner

@martint martint Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no callers for these methods. How do you intend for these to be used?

Copy link
Contributor Author

@wendigo wendigo Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To generate a name for the class that was renamed and we want to keep backward compatibility providing previous name by hand

Copy link
Owner

@martint martint Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This interface is meant to be used by the mbean exporter, not for users to call directly.

Copy link
Contributor Author

@wendigo wendigo Dec 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come?

newExporter(binder).export(DispatchManager.class).as(generator -> generator.generatedNameOf(QueryManager.class));

generator is of type ObjectNameGenerator

Copy link
Contributor Author

@wendigo wendigo Dec 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to be able to do:

newExporter(binder).export(DispatchManager.class).as(generator -> generator.generatedNameOf("io.trino.execution", "QueryManager"));

to avoid a need to keep a class around just to get a package name and simple name from it

Copy link
Contributor Author

@wendigo wendigo Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a test

Copy link
Contributor Author

@wendigo wendigo Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martint PTAL

Copy link
Owner

@martint martint Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right. However, that's a pattern of usage I've been wanting to change. There's really no need to pass an object around when the caller can generate the names directly. Let me take a closer look at this PR to see if there's a better way to make it fit.

Copy link
Contributor Author

@wendigo wendigo Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let me know. Right now in Trino we are renaming beans to ensure backward compatibility of metrics

}
44 changes: 43 additions & 1 deletion src/test/java/org/weakref/jmx/guice/TestMBeanModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,48 @@ public void testCustomSet()
binder -> binder.bind(ObjectNameGenerator.class).to(TestObjectNameGenerator.class));
}

@Test
public void testCustomNaming()
throws Exception
{
Injector injector = Guice.createInjector(PRODUCTION, new MBeanModule(), new AbstractModule()
{
@Override
protected void configure()
{
binder().requireExplicitBindings();
binder().disableCircularProxies();

bind(SimpleObject.class).annotatedWith(named("1")).toInstance(new SimpleObject());
bind(SimpleObject.class).annotatedWith(named("2")).toInstance(new SimpleObject());
bind(SimpleObject.class).annotatedWith(named("3")).toInstance(new SimpleObject());

bind(MBeanServer.class).toInstance(ManagementFactory.getPlatformMBeanServer());
ExportBinder.newExporter(binder()).export(Key.get(SimpleObject.class, named("1")))
.as(generator -> generator.generatedNameOf("org.example", "LegacyObject"));

ExportBinder.newExporter(binder()).export(Key.get(SimpleObject.class, named("2")))
.as(generator -> generator.generatedNameOf(SimpleObject.class));

ExportBinder.newExporter(binder()).export(Key.get(SimpleObject.class, named("3")))
.as(generator -> generator.generatedNameOf(TestMBeanModule.class.getPackage(), "AnotherObject"));
}
});

ObjectName name1 = new ObjectName("org.example", "name", "LegacyObject");
ObjectName name2 = new ObjectName(generatedNameOf(SimpleObject.class));
ObjectName name3 = new ObjectName("org.weakref.jmx.guice", "name", "AnotherObject");

MBeanServer server = injector.getInstance(MBeanServer.class);
Assert.assertNotNull(server.getMBeanInfo(name1));
Assert.assertNotNull(server.getMBeanInfo(name2));
Assert.assertNotNull(server.getMBeanInfo(name3));

server.unregisterMBean(name1);
server.unregisterMBean(name2);
server.unregisterMBean(name3);
}

private static void assertSet(ObjectName name1, ObjectName name2, Module additionalBindings)
throws InstanceNotFoundException, IntrospectionException, ReflectionException, MBeanRegistrationException
{
Expand Down Expand Up @@ -330,7 +372,7 @@ public static final class TestObjectNameGenerator
implements ObjectNameGenerator
{
@Override
public String generatedNameOf(Class<?> type, Map<String, String> properties)
public String generatedNameOf(String packageName, Map<String, String> properties)
{
return new ObjectNameBuilder("test")
.withProperties(properties)
Expand Down