spring-projects / spring-projects/spring-data-rest

Exception with lazy loading DBRef on property access or embedded excerpt [DATAREST-480]

Open
#859 2 comments 0 reactions 1 assignee View on GitHub

@odrotbohm is already working on this.

Since Dec 31, 2020.

type: bug
Dominant language
Java
Stars
958
Forks
568
PR merge metrics
No merged PRs in 30d

Description

Gary Frankel opened DATAREST-480 and commented

Ran into because I'm working on a different database that also has lazy loading and wanted to see what the Mongo implementation does.

public class Person {
       @Id private String id;
       public String getId() {
             return id;
        }
	public void setId(String id) {
		this.id = id;
	}
	public Address getTheAddress() {
		return theAddress;
	}
	public void setTheAddress(Address theAddress) {
		this.theAddress = theAddress;
	}
	@DBRef (lazy=true)
	private Address theAddress;
}
public class Address {
	@Id private String id;
	public String getId() {
		return id;
	}
	public void setId(String id) {
		this.id = id;
	}
	public String getCity() {
		return city;
	}
	public void setCity(String city) {
		this.city = city;
	}
	String street;
}

When getting /persons/xxx/theAddress, I get the exception shown at the bottom below.. Basically, it is failing to do the lazy load and eventually having problems dealing with the generated proxy type. It tries to serialize the proxy and that causes a failure when trying to serialize the callback. But the core is that it is trying after doing the "findOne" to deal with the proxy type - that causes other issues with not finding the right projection, etc...
The same kind of problem occurs when using an excerpt projection on Address that causes the lazy loaded reference to be expanded in the /persons feed.

Of Course, I'll be glad to learn if there is an existing solution and I just didn't find the right usage of annotations or interfaces, but I couldnt find anything that answered this particular problem.

I tried a couple of approaches to fixing it - one where I extended PersistentProperty to have a "getTarget" method and used that from various places (since MongoPersistentProperty extends it). BUT that was pretty messy and involved changes in core Spring Data classes and interfaces which just felt wrong.

So, I tried an alternative where I only made changes in RepositoryEntityController.java and RepositoryPropertyReferenceController.java. This approach was at least localized and the proper reference type was discovered early and the rest of the processing was normal. (code below)... This was based on checking association properties for the existence of "getTarget" on the type. And then invoking that method to ensure that the proper object was obtained at the beginning. This call is only made on assocations/references - and shouldnt happen unless these are "lazy loaded".

As noted in the comments, this is a bit clunky since it depends on that particular method - a better answer would be for lazy loading proxies (of any database) to support a particular LazyLoadingProxy interface with the getTarget method - and then test for that interface rather than just the existence of getTarget. I didnt go that far in what I did yet.

Now, I'm posting all this because (1) not sure how it will be for me to actually do a pull request given EMC's messy open source policies - and (2) not enough of a Spring Data expert to know if this is really the right answer. It does work (and it also works for the database that we are building that does lazy loading), but there might well be a "better way". (and the relevant getTarget method in the controllers should be shared between the controllers rather than repeated of course).

I'll be glad to finish this and clean it up and send attach the whole classes ("pull" sort of) IF Oliver/others believe that this is the right direction / approach.

 //at the end of doWithReferencedProperty()
                // may need to "load" the actual object (for lazy load situations)
		// all proxies that support this must support a getTarget call
		if (prop.isAssociation()) {
			Class<?> propValType = propVal.getClass(); 
			propVal = getTarget(propVal, propValType);
		}

		return handler.apply(new ReferencedProperty(prop, propVal, wrapper));

// and added the getTarget method (same code in both Controllers)
//TODO copied from RepositoryEntityController
		private Object getTarget(Object value, Class<?> targetType) {
			// first test if this is a Proxy
			// best way would be for the Proxy to support an interface that defines getTarget
			// 	 and then test for that interface (and require all Lazy Loaders to implement it
			Object result = null;
			try {
				Method getTargetMethod = targetType.getMethod("getTarget");
				result = getTargetMethod.invoke(value);
			} catch (NoSuchMethodException exc) {
				// OK - not a proxy
			} catch (InvocationTargetException exc ) {
				// ??
			} catch (IllegalAccessException exc) {
				// ??
			}
			return result;
		}

getCollectionResource() //just after doing the findAll

                PersistentEntity<?,?> entity = resourceInformation.getPersistentEntity();
		
		for (Object foundItem : results) {
			AssociationHandler itemHandler = new AssociationHandler(foundItem);
			entity.doWithAssociations(itemHandler);	
		}

getItemResource()  //just after doing findOne
                AssociationHandler itemHandler = new AssociationHandler(foundObject);
		entity.doWithAssociations(itemHandler);	

//calling ->
      private class AssociationHandler implements SimpleAssociationHandler {
		Object foundItem = null;
		public AssociationHandler (Object item) {
			foundItem = item;
		}
		public void doWithAssociation(Association<? extends PersistentProperty<?>> association) {
			PersistentProperty<?> prop = association.getInverse();
			Class<?> propType = prop.getActualType();
			ResourceMetadata metadata = mappings.getMappingFor(propType);
			if (metadata.getExcerptProjection() != null) {
				// really should only do this if there is an excerptProjection for now since that is the trigger
			       // for embedding
				try {
					Method propGetter = prop.getGetter();
					Object assocValue = propGetter.invoke(foundItem);
					if (assocValue != null) {
						Class<?> assocValueType = assocValue.getClass();
						assocValue = getTarget(assocValue, assocValueType);
						try {
							Method propSetter = prop.getSetter();
							if (propSetter != null) {
								propSetter.invoke(foundItem, assocValue);
							}
						} catch (InvocationTargetException exc ) {
							// ??
						}
					}
				} catch (IllegalAccessException exc)  {
					// can this happen??
				} catch (InvocationTargetException exc) {
					// ??
				}
			}
		}
		
	}

Affects: 2.2.1 (Evans SR1)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.