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.

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

Jump to bottom

Cannot use name of field converted with a CompositeAttributeConverter in converted map #932

Closed
oxisto opened this issue Jan 25, 2023 · 2 comments · Fixed by #933
Closed

Cannot use name of field converted with a CompositeAttributeConverter in converted map #932

oxisto opened this issue Jan 25, 2023 · 2 comments · Fixed by #933

Comments

@oxisto
Copy link
Contributor

oxisto commented Jan 25, 2023 •
edited
Loading

In our project, we are trying to analyse source code in a graph structure. Each source code element is represented as a Node. Each Node has a name attribute, which is not a string, but a special Name class. This is primarily for reasons such as parsing of fully qualified names (e.g. MyClass::test). To serialise this special class, we have written a CompositeAttributeConverter that serialises the individual elements into a map of strings (fullName, delimiter, localName). However, we want also want to serialise a string representation into a property called name (the same as the original field name). While this works perfectly when serialising into the graph; it does not when we try to load a node again.

Expected Behavior

I would expect the OGM to recognise the field name as part of the converted set of attributes and that the toEntityAttribute is called.

Current Behavior

Currently, OGM recognises name as a field property and tries to directly set the string value to the Java object, without consulting the composite attribute converter. If I remove the name key from the map, everything works.

Error mapping GraphModel
org.neo4j.ogm.exception.core.MappingException: Error mapping GraphModel
	at app//org.neo4j.ogm.context.GraphEntityMapper.mapContentOf(GraphEntityMapper.java:160)
	at app//org.neo4j.ogm.context.GraphEntityMapper.lambda$map$1(GraphEntityMapper.java:106)
	at app//org.neo4j.ogm.context.GraphEntityMapper.map(GraphEntityMapper.java:111)
	at app//org.neo4j.ogm.context.GraphRowModelMapper.map(GraphRowModelMapper.java:55)
	at app//org.neo4j.ogm.session.delegates.LoadByTypeDelegate.lambda$loadAll$0(LoadByTypeDelegate.java:108)
	at app//org.neo4j.ogm.session.Neo4jSession.doInTransaction(Neo4jSession.java:589)
	at app//org.neo4j.ogm.session.Neo4jSession.doInTransaction(Neo4jSession.java:563)
	at app//org.neo4j.ogm.session.delegates.LoadByTypeDelegate.loadAll(LoadByTypeDelegate.java:95)
	at app//org.neo4j.ogm.session.delegates.LoadByTypeDelegate.loadAll(LoadByTypeDelegate.java:115)
	at app//org.neo4j.ogm.session.Neo4jSession.loadAll(Neo4jSession.java:193)
	at app//de.fraunhofer.aisec.cpg_vis_neo4j.ApplicationTest.testPush(ApplicationTest.kt:71)
	at java.base@17.0.5/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base@17.0.5/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base@17.0.5/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base@17.0.5/java.lang.reflect.Method.invoke(Method.java:568)
	at app//org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:686)
	at app//org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at app//org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at app//org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
	at app//org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
	at app//org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
	at app//org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
	at app//org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
	at app//org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at app//org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at app//org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at app//org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at app//org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
	at app//org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
	at app//org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:212)
	at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at app//org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:208)
	at app//org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:137)
	at app//org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:71)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135)
	at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at app//org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at java.base@17.0.5/java.util.ArrayList.forEach(ArrayList.java:1511)
	at app//org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at app//org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at java.base@17.0.5/java.util.ArrayList.forEach(ArrayList.java:1511)
	at app//org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:38)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
	at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
	at app//org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
	at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
	at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
	at app//org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:32)
	at app//org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at app//org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:51)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:107)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
	at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
	at org.junit.platform.launcher.core.SessionPerRequestLauncher.execute(SessionPerRequestLauncher.java:53)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:99)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:79)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:75)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:62)
	at java.base@17.0.5/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base@17.0.5/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base@17.0.5/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base@17.0.5/java.lang.reflect.Method.invoke(Method.java:568)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at jdk.proxy2/jdk.proxy2.$Proxy5.stop(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:193)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:113)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:65)
	at app//worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at app//worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
Caused by: java.lang.IllegalArgumentException: Can not set de.fraunhofer.aisec.cpg.graph.Name field de.fraunhofer.aisec.cpg.graph.Node.name to java.lang.String
	at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:167)
	at java.base/jdk.internal.reflect.UnsafeFieldAccessorImpl.throwSetIllegalArgumentException(UnsafeFieldAccessorImpl.java:171)
	at java.base/jdk.internal.reflect.UnsafeObjectFieldAccessorImpl.set(UnsafeObjectFieldAccessorImpl.java:81)
	at java.base/java.lang.reflect.Field.set(Field.java:799)
	at org.neo4j.ogm.metadata.FieldInfo.write(FieldInfo.java:421)
	at org.neo4j.ogm.metadata.FieldInfo.write(FieldInfo.java:446)
	at org.neo4j.ogm.context.GraphEntityMapper.writeProperty(GraphEntityMapper.java:293)
	at org.neo4j.ogm.context.GraphEntityMapper.setProperties(GraphEntityMapper.java:264)
	at org.neo4j.ogm.context.GraphEntityMapper.mapNodes(GraphEntityMapper.java:224)
	at org.neo4j.ogm.context.GraphEntityMapper.mapContentOf(GraphEntityMapper.java:150)
	... 95 more

The writeProperty function would even know that a composite converter is in place, however that information is not used (either in the function directly or above).

Screenshot 2023-01-25 at 14 12 10

Your Environment
@oxisto oxisto mentioned this issue Jan 25, 2023
Merged
oxisto added a commit to oxisto/neo4j-ogm that referenced this issue Jan 25, 2023
@oxisto
Checking for possible composite converters in `GraphEntityMapper.writ…
c5184aa 
…eProperty`

This allows to resolve conflicts when a property is using a composite converter.

Fixes neo4j#932
@oxisto oxisto mentioned this issue Jan 25, 2023
Merged
oxisto added a commit to Fraunhofer-AISEC/cpg that referenced this issue Jan 25, 2023
@oxisto
Also serializing the full name of a Node as name in Neo4J
6d3159d 
Currently blocked by neo4j/neo4j-ogm#932
@meistermeier
Copy link
Collaborator

Thanks for reporting this and I am happy to review your PR.
Looking into the mapNodes method that calls setProperties, I can see that it does two things:

  1. check for composite properties
  2. call writeProperty for all properties that come back for this node

The first operation will succeed, I am pretty sure for this. But the second does not because it pulls the name from the database node and wants to put it on your Java Node.
Maybe you wanted to describe exactly this but the "field property" is not the root of the problem, the graph property is.

Renaming the Java Node's name property will already to the job as a workaround. But refactoring this throughout the code base is not ideal, I would say.

Just wanted to leave this here as a general summary of what the root problem is.

@oxisto
Copy link
Contributor Author

oxisto commented Jan 25, 2023

Thanks for reporting this and I am happy to review your PR. Looking into the mapNodes method that calls setProperties, I can see that it does two things:

Thanks for the quick feedback

  1. check for composite properties
  2. call writeProperty for all properties that come back for this node

The first operation will succeed, I am pretty sure for this. But the second does not because it pulls the name from the database node and wants to put it on your Java Node. Maybe you wanted to describe exactly this but the "field property" is not the root of the problem, the graph property is.

Yes, you are correct :) It seems that for all other graph properties that are put in by the composite converter, the writer within the writeProperty is null. But for the one that has the same name as the class field, it is non-null. That's why I added a quick check for isComposite. Not quite sure if this is really the best way to do it though. We can continue this on the PR itself.

Renaming the Java Node's name property will already to the job as a workaround. But refactoring this throughout the code base is not ideal, I would say.

Hehe, yes exactly :)

Just wanted to leave this here as a general summary of what the root problem is.

oxisto added a commit to oxisto/neo4j-ogm that referenced this issue Jan 25, 2023
@oxisto
Checking for possible composite converters in `GraphEntityMapper.writ…
85a1ed1 
…eProperty`

This allows to resolve conflicts when a property is using a composite converter.

Fixes neo4j#932
oxisto added a commit to oxisto/neo4j-ogm that referenced this issue Jan 25, 2023
@oxisto
Checking for possible composite converters in `GraphEntityMapper.writ…
9d59ed2 
…eProperty`

This allows to resolve conflicts when a property is using a composite converter.

Fixes neo4j#932
oxisto added a commit to Fraunhofer-AISEC/cpg that referenced this issue Jan 26, 2023
@oxisto
Also serializing the full name of a Node as name in Neo4J
e6d4eb6 
Currently blocked by neo4j/neo4j-ogm#932
meistermeier pushed a commit that referenced this issue Jan 27, 2023
@oxisto @meistermeier
Checking for possible composite converters in `GraphEntityMapper.writ…
260c50f 
…eProperty`

This allows to resolve conflicts when a property is using a composite converter.

Fixes #932
meistermeier pushed a commit to oxisto/neo4j-ogm that referenced this issue Jan 27, 2023
@oxisto @meistermeier
Checking for possible composite converters in `GraphEntityMapper.writ…
5166b5c 
…eProperty`

This allows to resolve conflicts when a property is using a composite converter.

Fixes neo4j#932
meistermeier pushed a commit that referenced this issue Jan 27, 2023
@oxisto @meistermeier
Checking for possible composite converters in `GraphEntityMapper.writ…
364c9a0 
…eProperty`

This allows to resolve conflicts when a property is using a composite converter.

Fixes #932
oxisto added a commit to Fraunhofer-AISEC/cpg that referenced this issue Feb 1, 2023
@oxisto
Also serializing the full name of a Node as name in Neo4J
10d5ee9 
Currently blocked by neo4j/neo4j-ogm#932
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Checking for possible composite converters in GraphEntityMapper.writeProperty oxisto/neo4j-ogm
2 participants
@meistermeier @oxisto