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

Fail to set content on nested content properties when using JPA. #1480

Closed
vierbergenlars opened this issue Jun 22, 2023 · 2 comments · Fixed by #1510
Closed

Fail to set content on nested content properties when using JPA. #1480

vierbergenlars opened this issue Jun 22, 2023 · 2 comments · Fixed by #1510

Comments

@vierbergenlars
Copy link
Contributor

Describe the bug

We're using nested content properties with JPA.

When trying to set the content via the REST API, an exception occurs because the field that contains the embedded object is null.

Example setup
@Entity
@NoArgsConstructor
@Getter
@Setter
public class Person {
	@Id
	@GeneratedValue(strategy = GenerationType.AUTO)
	private UUID id;

	private String name;

	@Embedded
	private Content picture;
}

@Embeddable
@NoArgsConstructor
@Getter
@Setter
public class Content {
	@ContentId
	private String id;

	@ContentLength
	private Long length;

	@MimeType
	private String mimetype;

	@OriginalFileName
	private String filename;
}
Exception
2023-06-22 09:20:55.587 ERROR 2974793 --- [nio-8889-exec-5] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is org.springframework.beans.NullValueInNestedPathException: Invalid property 'picture' of bean class [com.contentgrid.userapps.holmes.dcm.model.Person]: Value of nested property 'picture' is null] with root cause

org.springframework.beans.NullValueInNestedPathException: Invalid property 'picture' of bean class [com.contentgrid.userapps.holmes.dcm.model.Person]: Value of nested property 'picture' is null
	at org.springframework.beans.AbstractNestablePropertyAccessor.getNestedPropertyAccessor(AbstractNestablePropertyAccessor.java:849) ~[spring-beans-5.3.26.jar:5.3.26]
	at org.springframework.beans.AbstractNestablePropertyAccessor.getPropertyAccessorForPropertyPath(AbstractNestablePropertyAccessor.java:820) ~[spring-beans-5.3.26.jar:5.3.26]
	at org.springframework.beans.AbstractNestablePropertyAccessor.getPropertyValue(AbstractNestablePropertyAccessor.java:615) ~[spring-beans-5.3.26.jar:5.3.26]
	at org.springframework.content.commons.mappingcontext.ContentProperty.getContentId(ContentProperty.java:43) ~[spring-content-commons-2.9.0.jar:na]
	at internal.org.springframework.content.fs.repository.DefaultFilesystemStoreImpl.getResource(DefaultFilesystemStoreImpl.java:97) ~[spring-content-fs-2.9.0.jar:na]
	at internal.org.springframework.content.commons.repository.factory.StoreImpl.getResource(StoreImpl.java:346) ~[spring-content-commons-2.9.0.jar:na]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) ~[na:na]
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
	at java.base/java.lang.reflect.Method.invoke(Method.java:568) ~[na:na]
	at internal.org.springframework.content.commons.repository.factory.StoreMethodInterceptor.invoke(StoreMethodInterceptor.java:68) ~[spring-content-commons-2.9.0.jar:na]
	at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186) ~[spring-aop-5.3.26.jar:5.3.26]
	at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:215) ~[spring-aop-5.3.26.jar:5.3.26]
	at com.contentgrid.userapps.holmes.dcm.store.$Proxy162.getResource(Unknown Source) ~[na:na]
	at internal.org.springframework.content.rest.controllers.resolvers.AssociativeStoreResourceResolver.resolve(AssociativeStoreResourceResolver.java:32) ~[spring-content-rest-2.9.0.jar:na]
	at internal.org.springframework.content.rest.controllers.ResourceHandlerMethodArgumentResolver.resolveArgument(ResourceHandlerMethodArgumentResolver.java:140) ~[spring-content-rest-2.9.0.jar:na]
	at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:122) ~[spring-web-5.3.26.jar:5.3.26]
	at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:179) ~[spring-web-5.3.26.jar:5.3.26]
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:146) ~[spring-web-5.3.26.jar:5.3.26]
	at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:117) ~[spring-webmvc-5.3.26.jar:5.3.26]
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:895) ~[spring-webmvc-5.3.26.jar:5.3.26]
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:808) ~[spring-webmvc-5.3.26.jar:5.3.26]
	at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87) ~[spring-webmvc-5.3.26.jar:5.3.26]
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1072) ~[spring-webmvc-5.3.26.jar:5.3.26]
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:965) ~[spring-webmvc-5.3.26.jar:5.3.26]
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006) ~[spring-webmvc-5.3.26.jar:5.3.26]
	at org.springframework.web.servlet.FrameworkServlet.doPut(FrameworkServlet.java:920) ~[spring-webmvc-5.3.26.jar:5.3.26]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:520) ~[jakarta.servlet-api-4.0.4.jar:4.0.4]
	at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883) ~[spring-webmvc-5.3.26.jar:5.3.26]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:584) ~[jakarta.servlet-api-4.0.4.jar:4.0.4]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:209) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53) ~[tomcat-embed-websocket-9.0.73.jar:9.0.73]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at com.contentgrid.thunx.spring.data.rest.AbacRequestFilter.doFilter(AbacRequestFilter.java:43) ~[thunx-spring-0.6.0.jar:na]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100) ~[spring-web-5.3.26.jar:5.3.26]
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117) ~[spring-web-5.3.26.jar:5.3.26]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93) ~[spring-web-5.3.26.jar:5.3.26]
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117) ~[spring-web-5.3.26.jar:5.3.26]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.springframework.boot.actuate.metrics.web.servlet.WebMvcMetricsFilter.doFilterInternal(WebMvcMetricsFilter.java:96) ~[spring-boot-actuator-2.7.10.jar:2.7.10]
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117) ~[spring-web-5.3.26.jar:5.3.26]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201) ~[spring-web-5.3.26.jar:5.3.26]
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117) ~[spring-web-5.3.26.jar:5.3.26]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:178) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:153) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:167) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:90) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:492) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:130) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:93) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:389) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:63) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:926) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1791) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) ~[tomcat-embed-core-9.0.73.jar:9.0.73]
	at java.base/java.lang.Thread.run(Thread.java:833) ~[na:na]

To Reproduce
Steps to reproduce the behavior:

  1. Run attached zip with ./gradlew bootRun demo.zip
  2. Go to http://localhost:8080/, create a person object (provide at least a name)
  3. Take the url to the pic_t content object and try to upload a file to it (e.g. curl --upload-file gradle.properties -H "Content-Type: text/plain" http://localhost:8080/persons/830ccc3d-5131-4285-b462-2f0995e5f492/pic_t ; note that you need to change the ID)
  4. Receive an internal server error back, the exception is in the application logs.

Expected behavior

I expect the content to be actually set on the object, even though the embedded object is not created yet.

Additional context

All columns defined by the @Embeddable Content object are nullable in the database. If all fields are null, Hibernate does not create a Content object at all for the @Embedded Content picture field. Instead, it sets it to null. (https://en.wikibooks.org/wiki/Java_Persistence/Embeddables#Nulls)

The result is that, when loaded the entity looks like this

{
  "id": "830ccc3d-5131-4285-b462-2f0995e5f492",
  "name": "xyz",
  "picture": null
}

Traversing null with a spring BeanWrapper throws an exception by default, unless ConfigurablePropertyAccessor#setAutoGrowNestedPaths(true) is called before accessing properties. That will automatically instanciate the object if it's there was are nulls in the path.

It looks like this issue would be resolved by using wrapper.setAutoGrowNestedPaths(true) everywhere in ContentProperty where a BeanWrapper is created for an entity.

A side-effect of this is that trying to read the contentId for a nested content property will also instanciate the embedded object. For JPA itself, that is not really an issue since embedded objects with all-null fields are the same as not having the embedded object at all.

However, for user code it may be an unexpected side-effect. If you want to cover this side-effect, you could catch the org.springframework.beans.NullValueInNestedPathException exception in the property read paths and return null in that case.

@paulcwarren
Copy link
Owner

paulcwarren commented Jul 11, 2023

Thanks @vierbergenlars . I see what you are asking for. I think I would have to make it configurable so I need to look into how best to do that (given its depth in the architecture).

Just curious on your opinion in general. I think this can be easily solved in the application later with an @PostLoad annotation on Person; i.e:

	@PostLoad
	private void initData() {
		if(picT == null) {
			picT = new Content();
		}
	}

Would that work for you all? Or are there reasons/justifications why that wouldn't work (well) beyond keeping the app model simple?

@vierbergenlars
Copy link
Contributor Author

vierbergenlars commented Jul 11, 2023

Putting some code in @PostLoad would work, but I consider that a workaround for this bug.

Although it's perhaps a small detail, the response provided by spring-data-rest would also be impacted, instead of looking like I would expect, we will have an object with all null fields when there is no content.

ExpectedActual
{
  "id": "830ccc3d-5131-4285-b462-2f0995e5f492",
  "name": "xyz",
  "picture": null
}
{
  "id": "830ccc3d-5131-4285-b462-2f0995e5f492",
  "name": "xyz",
  "picture": {
    "length": null,
    "mimetype": null,
    "filename": null
  }
}

You might consider it a detail, but this makes it more difficult for API users to check if there is content.

Adding some code to every entity that uses content is certainly possible, but to me it feels like additional effort that should not be needed to be able to use spring-content.

I think I would have to make it configurable

If taking the second approach (setAutoGrowNestedPaths in write paths; catching NullValueInNestedPathException and treating it as null in read paths) does not need configuration.

I can't immediately think of a situation where you would want an exception while trying to read/write content. That effectively makes two paths that need to be handled for the "no content present" case, one with the read returning null, and one with it throwing an (undeclared/undocumented) exception which is an internal implementation detail.

paulcwarren added a commit that referenced this issue Jul 13, 2023
#1507)

* feat: content property now uses setAutoGrowNestedPaths when setting content to provide better support for null embedded content objects

- filesystem storage only

* test: add @Embedded content tests for azure storage

* test: add @Embedded content tests for gcp storage

* test: add @Embedded content tests for s3 storage

Fixes #1480
paulcwarren added a commit that referenced this issue Jul 14, 2023
#1507)

* feat: content property now uses setAutoGrowNestedPaths when setting content to provide better support for null embedded content objects

- filesystem storage only

* test: add @Embedded content tests for azure storage

* test: add @Embedded content tests for gcp storage

* test: add @Embedded content tests for s3 storage

Fixes #1480
paulcwarren added a commit that referenced this issue Jul 14, 2023
…content (#1510)

* feat: content property now uses setAutoGrowNestedPaths when setting content to provide better support for null embedded content objects

- filesystem storage only

* test: add @Embedded content support/tests for azure storage

* test: add @Embedded content support/tests for gcp storage

* test: add @Embedded content support/tests for s3 storage

* test: add @Embedded content support/tests for jpa storage

Fixes #1480
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants