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

Properties with "@LoadOnly" are unset when calling merge() method #2613

Closed
jonasnyberg opened this issue Oct 16, 2023 · 2 comments
Closed

Properties with "@LoadOnly" are unset when calling merge() method #2613

jonasnyberg opened this issue Oct 16, 2023 · 2 comments
Labels
Milestone

Comments

@jonasnyberg
Copy link

jonasnyberg commented Oct 16, 2023

Describe the bug

Hi!

I'm not completely sure if it's a bug or not. We have an embedded property on some of our models which acts as a cache. It is updated from other parts of the system which is why we have annotated it with @LoadOnly. However, when we perform updates (using the merge() method together with unsetMissing: true) Morphia not only ignores it in the update but also unsets it.

After going through the Morphia source code I found that it first checks if a property should be serialized (included). If not, it calls an unset operation instead as found here: MergingEncoder.java

This also effects properties marked as @LoadOnly as found here: MorphiaPropertySerialization.java

I would have thought that for this annotation it would skip serialization, but not actually convert it into an unset operation?

To Reproduce

Model class:

@Entity(value = "my_entities")
public class MyEntity {
  @Id private ObjectId id;
  
  private String name;
  @LoadOnly private String loadOnlyValue;
  
  public EntityId<CWGenericEntity> getId() {
    return id;
  }
	
  public void setId(EntityId<CWGenericEntity> id) {
    this.id = id;
  }
  
  public String getName() {
    return name;
  }
  
  public void setName() {
    this.name = name;
  }
  
  public String getLoadOnlyValue() {
    return loadOnlyValue;
  }
  
  public void setLoadOnlyValue(String loadOnlyValue) {
    this.loadOnlyValue = loadOnlyValue;
  }
}

Test:

// Saves base entity with a name:
MyEntity entity = new MyEntity();
entity.setName("TestA");
ds.save(entity);

// Updates the "load only" value through an update operation:
ds.find(MyEntity.class).filter(Filters.eq("_id", entity.getId())).update(new UpdateOptions(), UpdateOperators.set("loadOnlyValue", "My Value"));
MyEntity updatedEntity1 = ds.find(MyEntity.class).filter(Filters.eq("_id", entity.getId())).first();
assertNotNull(updatedEntity1.getLoadOnlyValue()); // PASSES

// Updates the name value using the datastore's "merge()" method:
entity.setName("TestB");
ds.merge(entity, new InsertOneOptions().unsetMissing(true));
MyEntity updatedEntity2 = ds.find(MyEntity.class).filter(Filters.eq("_id", entity.getId())).first();
assertNotNull(updatedEntity2.getLoadOnlyValue()); // FAILS

Expected behavior

I expect that properties annotated with @LoadOnly would not be unset when calling datstore.merge() together with unsetMissing: true.

** Please complete the following information: **

  • Server Version: 6.0
  • Driver Version: 4.8.1
  • Morphia Version: 2.3.1
@evanchooly
Copy link
Member

That does sound like a bug. I'm on the road at the moment so it'll be a bit until i can confirm but it looks like you've done most of the leg work already.

@jonasnyberg
Copy link
Author

Thanks for the quick reply Justin!

The fix itself might not be entirely straight forward though. I mean, the code inside shouldSerialize() is correct in itself, but it should probably not add it as an unset operation in some cases.
So for the storeNulls and storeEmpties it should convert to an unset but not for the annotations. But as the shouldSerialize() method returns a boolean there is no way for that code to know if it should add the unset operation or not.

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

No branches or pull requests

2 participants