Skip to content

FindByXXX fails to work with field marked as Id #1321

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

Closed
shaniiwild opened this issue Feb 3, 2022 · 10 comments · Fixed by #1338
Closed

FindByXXX fails to work with field marked as Id #1321

shaniiwild opened this issue Feb 3, 2022 · 10 comments · Fixed by #1338
Labels
type: bug A general bug

Comments

@shaniiwild
Copy link

shaniiwild commented Feb 3, 2022

Hi!
Before the upgrade to Spring Data 4.3 we had the following derived query working fine:
image

The find by was accessing this field with the name 'key' which is marked as Id by annotation and results were retrieves as expected.
image
(Please note: It worked without n1ql implementation)

Following the upgrade, derived methods which are approaching field marked with @id failed to retrieve results (Also with combination of another fields, For example: findByKeyAndStatus()).
We solved it using CrudRepository inherited method of findById()
But We would still like to understand what has changed that it's now fails to work? is it a bug?
Thanks.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 3, 2022
@mikereiche
Copy link
Collaborator

Thanks. Roi told me of the issue and I was not able to reproduce it. Please provide everything needed to reproduce it - the implementation of FileHeader and the repository and the code that calls findByKey.

btw - I believe you should be using the findById method which uses the kv API. It will be about 10x faster than the query API that findByKey uses.

@mikereiche
Copy link
Collaborator

spring-data-couchbase does not store the @id property in the document. It stores it only as the document id and in a query can be referenced by meta(bucket).id Derived queries should have references to "key" replaced with meta(bucket).id

@mikereiche
Copy link
Collaborator

Please also provide the document that was to be retrieved.

@mikereiche mikereiche added the status: waiting-for-feedback We need additional information before we can continue label Feb 3, 2022
@mikereiche
Copy link
Collaborator

If there is a constructor that has a 'key' argument, and that argument is ignored in the constructor (i.e. because the id is @generated), then the 'key' field in the class will not be populated from meta().id on find operations, thus a subsequent findByKey(found.getKey()) will not return anything because getKey() returned null because it was not set in the constructor.
So there are all kinds of things that could occur, which is why I need to be able to run a reproducer in the debugger to see what is going on. But given the description here and assuming that nothing else is out of the ordinary - I'm not able to reproduce the issue. Is FileHeader an abstract class or interface, perhaps? That would prevent any non-@query methods from returning results - see #1315.

@shaniiwild
Copy link
Author

shaniiwild commented Feb 8, 2022

Hi @mikereiche
Sorry for the late response.
I understand from you comments that it's better performance to use findById.
It's fine, However we have cases where we search for multiple fields (for example findByKeyAndStatus) and in order to overcome this issue we had to implement the query in n1ql.

I understand from your comment there could be some explanations for this issue. Also wanted to clarify again that it did work before we switched to new spring data and defined collections. So here are the details, Please let us know if we need to implement some adjustment for it to work again.
File header:
image
Class inherits from abstract Couchbase entity class:
image
The derived method in repository:
image

Thanks alot!

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 8, 2022
@mikereiche
Copy link
Collaborator

mikereiche commented Feb 8, 2022

There was a bug introduced in 4.3.0 when the entity is an abstract class.
It's possible that this is the same issue, but I still need to reproduce it to be sure.
(that issue will be fixed in the 4.3.2 release later this week. #1315)

@shaniiwild
Copy link
Author

Okay thanks @mikereiche

@mikereiche
Copy link
Collaborator

mikereiche commented Feb 10, 2022

I reconstructed the classes as best I could by re-typing from the screenshots. It would have been convenient if the actual files were posted as text.

@mikereiche
Copy link
Collaborator

mikereiche commented Feb 10, 2022

Found the issue. It manifests when a derived query is on a collection and the 'id' field (or version or expiry) is in a predicate. Instead of being replaced by META(collection).id, it is replaced by META(bucket).id - and therefore does not match.

Here is my CouchbaseEntity, FileHeader and FileHeader repository classes and my test case (below under the Code section). [The bug I mentioned above was that _class is not projected, but it does not affect this case as the repository is defined using the concrete class FileHeader and not the abstract class CouchbaseEntity ]

SELECT META(`my_collection`).id AS __id, META(`my_collection`).cas AS __cas, `uploadStartDate`,
 `uploadEndDate`, `status`, `userName`, `fileName`, `mappingDefinitionKey`, `source`, `uploadTypeKey`,
 `fileType`, `fileHeaderAnalysis`, `entityAudit` FROM `my_collection`
 WHERE `t` = "org.springframework.data.couchbase.domain.FileHeader" AND META(`my_bucket`).`id` = $1
found: FileHeader(uploadStartDate=2022-02-10, uploadEndDate=2022-02-10, status=UPLOAD_STATUS, 
userName=12345, fileName=file.txt, mappingDefinitionKey=mappingDefinitionKey, source=source, 
uploadTypeKey=uploadKeyType, fileType=FILE_TYPE, fileHeaderAnalysis=FILE_HEADER_ANALYSIS)
Code ``` package org.springframework.data.couchbase.domain;

import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.NoArgsConstructor;
import lombok.experimental.SuperBuilder;
import org.jetbrains.annotations.NotNull;
import org.springframework.data.annotation.Id;
import org.springframework.data.couchbase.core.mapping.Field;
import org.springframework.data.couchbase.core.mapping.id.GeneratedValue;
import org.springframework.data.couchbase.core.mapping.id.GenerationStrategy;

@DaTa
@SuperBuilder( toBuilder = true)
@NoArgsConstructor
@AllArgsConstructor
public abstract class CouchbaseEntity {
@id @GeneratedValue(strategy = GenerationStrategy.UNIQUE)
@NotNull
private String key;
@field
private String entityAudit;
}


package org.springframework.data.couchbase.domain;

import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.EqualsAndHashCode;
import lombok.NoArgsConstructor;
import org.springframework.data.couchbase.core.mapping.Document;
import org.springframework.data.couchbase.core.mapping.Field;

import javax.validation.constraints.NotNull;
import java.time.LocalDate;

@document
@DaTa
@EqualsAndHashCode
@NoArgsConstructor
@AllArgsConstructor
public class FileHeader extends CouchbaseEntity {
@field
private LocalDate uploadStartDate = LocalDate.now();
@field
private LocalDate uploadEndDate = LocalDate.now();
@field
private UploadStatus status = UploadStatus.UPLOAD_STATUS;
@field
private Long userName = new Long("12345");
@field
private String fileName = "fileName";
@field
@NotNull
private String mappingDefinitionKey;
@field
private String source;
@field
private String uploadTypeKey;
@field
private FileType fileType = FileType.FILE_TYPE;
@field
private FileHeaderAnalysis fileHeaderAnalysis = FileHeaderAnalysis.FILE_HEADER_ANALYSIS;

public enum UploadStatus {
UPLOAD_STATUS
}

public enum FileType {
FILE_TYPE
}

public enum FileHeaderAnalysis {
FILE_HEADER_ANALYSIS
}

}


package org.springframework.data.couchbase.domain;

import com.couchbase.client.java.query.QueryScanConsistency;
import org.springframework.data.couchbase.repository.ScanConsistency;
import org.springframework.data.repository.PagingAndSortingRepository;
import org.springframework.stereotype.Repository;

@repository
@scope("my_scope")
@Collection("my_collection")
public interface FileHeaderRepository extends PagingAndSortingRepository<FileHeader, String> {
@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
FileHeader findByKey(String key);
}


@test
void findByKey() {
FileHeader fileHeader = new FileHeader(LocalDate.now(),
LocalDate.now(),
FileHeader.UploadStatus.UPLOAD_STATUS.UPLOAD_STATUS,
new Long(12345),
"file.txt",
"mappingDefinitionKey",
"source",
"uploadKeyType",
FileHeader.FileType.FILE_TYPE,
FileHeader.FileHeaderAnalysis.FILE_HEADER_ANALYSIS);
fileHeaderRepository.save(fileHeader);
FileHeader found = fileHeaderRepository.findByKey(fileHeader.getKey());
System.err.println("found: " + found);
assertEquals(fileHeader,
found);
fileHeaderRepository.delete(fileHeader);
}

</details>

@mikereiche mikereiche added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Feb 10, 2022
@mikereiche mikereiche added this to the 4.3.2 (2021.1.2) milestone Feb 10, 2022
@shaniiwild
Copy link
Author

@mikereiche Thanks a lot for all the effort.

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

Successfully merging a pull request may close this issue.

3 participants