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

all-arguments constructor incompatible with '@Version private long version' [DATACOUCH-626] #937

Closed
spring-projects-issues opened this issue Oct 6, 2020 · 4 comments
Assignees

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Oct 6, 2020

jpringle opened DATACOUCH-626 and commented

When attempting to use an all-arguments constructor as the persistence constructor in conjunction with a @Version property, if the type of the version property is a native type then object instantiation will fail.

Queries and key-value gets fail to construct entities due to an attempt to pass "null" as the version parameter in the all-arguments constructor.

 The following simple document class will fail to load with the following exception:

org.springframework.data.mapping.model.MappingInstantiationException: Failed to instantiate SubscriptionToken using constructor public SubscriptionToken(java.lang.String,long,java.lang.String,java.lang.String,java.lang.String,java.lang.String,long) with arguments 19df69a7-db2f-4cc0-94f9-d2a15624b5fc,null,SafetyAll,SafetySuper1,appId unknown,deviceId unknown,1601974697119
@Getter
@ToString
@EqualsAndHashCode
@Document
public class SubscriptionToken {
  private @Id @GeneratedValue(strategy = GenerationStrategy.UNIQUE) String id;
  private @Version long version;
  private @Field String subscriptionType;
  private @Field String userName;
  private @Field String appId;
  private @Field String deviceId;
  private @Field long subscriptionDate;

  public SubscriptionToken(
      String id,
      long version,
      String subscriptionType,
      String userName,
      String appId,
      String deviceId,
      long subscriptionDate) {
    this.id = id;
    this.version = version;
    this.subscriptionType = subscriptionType;
    this.userName = userName;
    this.appId = appId;
    this.deviceId = deviceId;
    this.subscriptionDate = subscriptionDate;
  }

Referenced from: pull request #272

Backported to: 4.0.5 (Neumann SR5)

1 votes, 3 watchers

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 6, 2020

jpringle commented

Presumably this is due to a mismatch in the parameter lists of the constructor (which includes a value for the "version" property) and the ParameterValueProvider of the root entity (which will not have a value for version, as that is not a property in the CouchbaseDocument that was translated from the query/read result).

Perhaps CouchbaseTemplateSupport.decodeEntity() should populate the cas value into CouchbaseDocument's content (if the persistent entity has a version property) so it can then be populated via the normal property population process during entity creation?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 6, 2020

Michael Reiche commented

This is an issue for any document property that is a primitive and not present in a document, where the property is an arg to the constructor and the entity does not have a no-args constructor. 
Adding a no-args constructor will allow the object to be instantiated and populated without using the constructor with the primitive as an arg. 

"Perhaps CouchbaseTemplateSupport.decodeEntity() should populate the cas value into CouchbaseDocument's content (if the persistent entity has a version property)"

That's a possibility

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 7, 2020

jpringle commented

Yes, a no-args constructor could be used.  But then you're incurring the overhead of property-based setters etc - which goes against the explicit advice in the documentation to use immutable classes where possible, and all-args constructors for maximum efficiency.

I can say it does work to use a Long vs a long (I just tried this), but even then it feels unnecessarily inefficient as each object instantiation is forced to construct an object and then set the version property, when the version value is known up front and could be included in the object instantiation to begin with.

I'd be happy to try my hand at a proposed implementation ... what's involved in that?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 7, 2020

Michael Reiche commented

jpringle - it's going to get addressed for cas, but the issue will remain for other fields

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

Successfully merging a pull request may close this issue.

None yet
2 participants