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

Generated PropertyAccessor fails lookup of setter accepting a primitive type [DATACMNS-916] #1371

Closed
spring-projects-issues opened this issue Sep 22, 2016 · 10 comments
Assignees
Labels
in: core type: bug

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Sep 22, 2016

Mark Paluch opened DATACMNS-916 and commented

Class:
https://github.com/spring-projects/spring-integration/blob/master/spring-integration-mongodb/src/main/java/org/springframework/integration/mongodb/store/MongoDbMessageStore.java#L841

	private static final class MessageWrapper {

		@SuppressWarnings("unused")
		private int sequence;

		public void setSequence(int sequence) {
			this.sequence = sequence;
		}
	}
java.lang.ExceptionInInitializerError: java.lang.ExceptionInInitializerError
java.lang.ExceptionInInitializerError
	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
	at org.springframework.data.mapping.model.ClassGeneratingPropertyAccessorFactory.getPropertyAccessor(ClassGeneratingPropertyAccessorFactory.java:84)
	at org.springframework.data.mapping.model.BasicPersistentEntity.getPropertyAccessor(BasicPersistentEntity.java:417)
	at org.springframework.data.mongodb.core.MongoTemplate.assertUpdateableIdIfNotSet(MongoTemplate.java:1294)
	at org.springframework.data.mongodb.core.MongoTemplate.doInsert(MongoTemplate.java:843)
	at org.springframework.data.mongodb.core.MongoTemplate.insert(MongoTemplate.java:791)
	at org.springframework.integration.mongodb.store.MongoDbMessageStore.addMessageDocument(MongoDbMessageStore.java:216)
	at org.springframework.integration.mongodb.store.MongoDbMessageStore.addMessagesToGroup(MongoDbMessageStore.java:291)
	at org.springframework.integration.store.AbstractMessageGroupStore.addMessageToGroup(AbstractMessageGroupStore.java:209)
	at org.springframework.integration.handler.DelayHandler.releaseMessageAfterDelay(DelayHandler.java:315)
	at org.springframework.integration.handler.DelayHandler.handleRequestMessage(DelayHandler.java:243)
	at org.springframework.integration.handler.AbstractReplyProducingMessageHandler.handleMessageInternal(AbstractReplyProducingMessageHandler.java:109)
	at org.springframework.integration.handler.AbstractMessageHandler.handleMessage(AbstractMessageHandler.java:127)
	at org.springframework.integration.dispatcher.AbstractDispatcher.tryOptimizedDispatch(AbstractDispatcher.java:116)
	at org.springframework.integration.dispatcher.UnicastingDispatcher.doDispatch(UnicastingDispatcher.java:148)
	at org.springframework.integration.dispatcher.UnicastingDispatcher.dispatch(UnicastingDispatcher.java:121)
	at org.springframework.integration.channel.AbstractSubscribableChannel.doSend(AbstractSubscribableChannel.java:77)
	at org.springframework.integration.channel.AbstractMessageChannel.send(AbstractMessageChannel.java:423)
	at org.springframework.integration.channel.AbstractMessageChannel.send(AbstractMessageChannel.java:373)
	at org.springframework.integration.mongodb.store.DelayerHandlerRescheduleIntegrationTests.testDelayerHandlerRescheduleWithMongoDbMessageStore(DelayerHandlerRescheduleIntegrationTests.java:82)
	at org.springframework.integration.mongodb.store.DelayerHandlerRescheduleIntegrationTests.testWithMongoDbMessageStore(DelayerHandlerRescheduleIntegrationTests.java:62)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.springframework.integration.mongodb.rules.MongoDbAvailableRule$1.evaluate(MongoDbAvailableRule.java:59)
	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.runTestClass(JUnitTestClassExecuter.java:114)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.execute(JUnitTestClassExecuter.java:57)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassProcessor.processTestClass(JUnitTestClassProcessor.java:66)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:51)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:32)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:93)
	at com.sun.proxy.$Proxy2.processTestClass(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.processTestClass(TestWorker.java:109)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:377)
	at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:54)
	at org.gradle.internal.concurrent.StoppableExecutorImpl$1.run(StoppableExecutorImpl.java:40)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NoSuchMethodException: org.springframework.integration.mongodb.store.MongoDbMessageStore$MessageWrapper.setSequence(java.lang.Integer)
	at java.lang.Class.getDeclaredMethod(Class.java:2130)
	at org.springframework.integration.mongodb.store.MongoDbMessageStore$MessageWrapper_Accessor_lxoqe3.<clinit>(Unknown Source)
	... 70 more

Update: Generated property accessors initialize field and property accessors as far as possible upon class initialization. Although setter initialization is not required for this case, it revealed a bug in setter lookup. The call to getDeclaredMethod used java.lang.Integer as argument type instead of java.lang.Integer#TYPE. The root cause is that a setter lookup uses wrapped types instead of the primitive type for getDeclaredMethod lookup


Affects: 1.13 M1 (Ingalls)

Reference URL: https://build.spring.io/browse/PLATFORM-COM-JOB1-232/test/case/202163589

Issue Links:

  • INT-4119 Mark MessageWrapper and MessageDocument with @AccessType(PROPERTY) for better performance

Referenced from: pull request #178

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 22, 2016

Oliver Drotbohm commented

Although the class' structure is rather unconventional, I wonder why we're actually trying to look up the setter in the first place as we should use field access by default, shouldn't we?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 22, 2016

Artem Bilan commented

Guys, don't mix please.

The code for IO Brussels is here: https://github.com/spring-projects/spring-integration/blob/4.3.x/spring-integration-mongodb/src/main/java/org/springframework/integration/mongodb/store/MongoDbMessageStore.java#L851.

We are talking about Spring Integration 4.3.x and Spring Data Ingalls support.
In SI 5.0 (current master) it has been fixed the way as:

public void set_Sequence(int sequence) 

Since that is a private class I won't mind to fix it any proper way which you will point.
That won't be breaking change in our side and we will have compatibility with both Hopper and Ingalls.

When I remove private modifier from that class (and many other test entities) it starts work, too.

Another thought, looking at the StackTrace one more time:

java.lang.NoSuchMethodException: org.springframework.integration.mongodb.store.MongoDbMessageStore$MessageWrapper.setSequence(java.lang.Integer)

Really, there is no such a method because it is like:

public void setSequence(int sequence) {
	this.sequence = sequence;
}

If I change it to accept Integer instead of primitive it starts to work again.

WDYT?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 22, 2016

Mark Paluch commented

I will investigate the issue to find the cause. Thanks for clarifying the, I was on the wrong branch

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 23, 2016

Oliver Drotbohm commented

Artem Bilan — Can you elaborate on the weird property names and especially the non-JavaBeans convention following sequence VS. set_Sequence?

Mark Paluch — Two things: shouldn't we ignore private static final types completely and fall back to the reflection based approach? Looks like we're still trying to generate the accessor class here

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 23, 2016

Mark Paluch commented

Non-accessible types are accessed using MethodHandles (the initial approach) but in this case, the Spring Integration code revealed a bug in setter lookup.

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 23, 2016

Oliver Drotbohm commented

That's merged into master. Ingalls snapshots should be available in a few seconds

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 23, 2016

Artem Bilan commented

Thank you, guys, for such a quick turnaround!

Re. non-JavaBeans convention.
Well that is enough old class and I'm not sure that there will be some hooks to understand why.
Although that feels like just arbitrary choice for the inner class.

Let me know if switching it to the JavaBeans convention makes the stuff faster and I'll fix that issue!

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 23, 2016

Oliver Drotbohm commented

Unless you explicitly activate property access, we now use the fields in the first place (just like the reflection based bean wrapper we have used in Hopper did)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 23, 2016

Artem Bilan commented

I guess you mean @AccessType(PROPERTY) on the class level.

Well, since we have a couple domain entities which are actively used in the Spring Integration, I think that will be good idea to optimize there as well.

Thank you for the help!

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 23, 2016

Oliver Drotbohm commented

Well, we default to field access as we actually think property access is too invasive regarding class design. With it active, you need accessor methods for everything that's supposed to be persisted. That however might conflict with the API you want to expose. Generally speaking, the state of an object consists of its fields. Hence using those directly for persistence seems to be the better option.

With the new code in place (i.e. the stuff that Mark just fixed) we're now using method handles instead of reflection to access the field's values which gives us some performance gains in that area

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

No branches or pull requests

2 participants