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

String to ObjectId mapping error when using query method #4490

Closed
grwang91 opened this issue Aug 31, 2023 · 12 comments
Closed

String to ObjectId mapping error when using query method #4490

grwang91 opened this issue Aug 31, 2023 · 12 comments
Labels
type: bug A general bug

Comments

@grwang91
Copy link
Contributor

grwang91 commented Aug 31, 2023

Having problem using query method, as query generated by spring-data-mongodb is consist of string type _id, not ObjectId.

@Document("test_entity")
public class TestEntity {
	@MongoId(targetType = FieldType.OBJECT_ID)
	private String id;

	private String testField;
}
public interface TestRepository extends MongoRepository<TestEntity, String> {
	List<TestEntity> findAllByIdGreaterThan(String id);
}

Entity and Repository are declared like above code.

And I tested by below simple test code.

@SpringBootTest
class TestRepositoryTest {

	@Autowired
	private TestRepository testRepository;

	@Test
	void test() {
		TestEntity entity = new TestEntity();
		testRepository.save(entity);

		List<TestEntity> result = testRepository.findAllByIdGreaterThan("000000000000000000000000");
	}
}

It is supposed to return all documents in DB, as "000000000000000000000000" is minimum value of ObjectId. But nothing returned.
Also, It seems Mongotemplate is querying by string type _id.

2023-08-31T22:20:55.784+09:00 DEBUG 72457 --- [    Test worker] o.s.d.m.r.query.MongoQueryCreator        : Created query Query: { "id" : { "$gt" : "000000000000000000000000"}}, Fields: {}, Sort: {}
2023-08-31T22:20:55.789+09:00 DEBUG 72457 --- [    Test worker] o.s.data.mongodb.core.MongoTemplate      : find using query: { "_id" : { "$gt" : "000000000000000000000000"}} fields: Document{{}} for class: class com.example.mongodb_test.entity.TestEntity in collection: test_entity

I could solve this problem with a temporary solution, by using custom @query like below, with appropriate debug log using $oid.

@Query("{'_id': {'$gte': {'$oid': ?0}}}")
	List<TestEntity> findAllByIdGreaterThan(String id);
2023-08-31T22:24:31.959+09:00 DEBUG 72886 --- [    Test worker] o.s.d.m.r.query.StringBasedMongoQuery    : Created query Document{{_id=Document{{$gte=000000000000000000000000}}}} for Document{{}} fields.
2023-08-31T22:24:31.965+09:00 DEBUG 72886 --- [    Test worker] o.s.data.mongodb.core.MongoTemplate      : find using query: { "_id" : { "$gte" : { "$oid" : "000000000000000000000000"}}} fields: Document{{}} for class: class com.example.mongodb_test.entity.TestEntity in collection: test_entity

But still I can't use query method with _id.

Am I using query method in wrong way? or It's id type bug?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 31, 2023
@Lucasark
Copy link

I dont know if you have any particular reason to use "findAll", but if you call "findByIdGreaterThan" (return Optional) it will do what you describe. Btw, ID's should be unique, so always it will be found 1 or 0, and not a List.

PS.: On your custom query you use GTE and your jpa is on GT.

@grwang91
Copy link
Contributor Author

grwang91 commented Sep 1, 2023

I dont know if you have any particular reason to use "findAll", but if you call "findByIdGreaterThan" (return Optional) it will do what you describe. Btw, ID's should be unique, so always it will be found 1 or 0, and not a List.

PS.: On your custom query you use GTE and your jpa is on GT.

I need to use "findAll" by my business logic requirement, so "findByIdGreaterThan" is not appropriate choice. Also, "findByIdGreaterThan" still queries by string type _id.

@Lucasark
Copy link

Lucasark commented Sep 1, 2023

I dont know if you have any particular reason to use "findAll", but if you call "findByIdGreaterThan" (return Optional) it will do what you describe. Btw, ID's should be unique, so always it will be found 1 or 0, and not a List.
PS.: On your custom query you use GTE and your jpa is on GT.

I need to use "findAll" by my business logic requirement, so "findByIdGreaterThan" is not appropriate choice. Also, "findByIdGreaterThan" still queries by string type _id.

Can you describe better your case? Because I can't see where it could be appropriate. I can imagine only one scenario when you have compound id, like:

@Document("test_entity")
public class TestEntity {

	@Id
	private TestId id;

	private String testField;
}



public class TestId{

	private String id1;

	private String id2;
}

Your document will be:

{
_id:{
  id1: "val",
  id2: "val
},
testField: "val"
}

About "findByIdGreaterThan", you should use ObjectId in your method on interface, like
:

Optional<TestEntity> findByIdGreaterThan(ObjectId id);

It will produce same @query that you describe on your custom

@grwang91
Copy link
Contributor Author

grwang91 commented Sep 1, 2023

I need to get list of documents greater than certain _id, for pagination. _id is used as cursor, and I need range query.

@Lucasark
Copy link

Lucasark commented Sep 1, 2023

I need to get list of documents greater than certain _id, for pagination. _id is used as cursor, and I need range query.

Now I got it! And I think we had miss conception of "ALL", if you have a look on "findAllById" from JPA it is Iterable, like:

findAllById(List.of(id1, id2, id3..., idN))

So, it will need construct $in, like: { "_id" : { "$in" : [{ "$oid" : "64f17a0ef6bfc9364e25cef7"}]}} , and it will return a List from this id`s

Ok, now move on for what you need, that is a List<> by 1 id (I think is it), for my example, I build two method on interface:

public interface TestRepository extends MongoRepository<TestEntity, String> {
    List<TestEntity> findByIdGreaterThan(String id);
    List<TestEntity> findByIdGreaterThan(ObjectId id);
}

GreatThan does not know your "comparable" otherwise you explicit. So, take my example:

image

Using both method in order, it will produce from JPA:

image

findByIdGreaterThan(String id); will return none, like you already said, because in mongodb this is not a String value.
findByIdGreaterThan(ObjectId id) will return one (because I used GT), and now it is explicit that is ObjectId.

Same scenario can be reproduce with other types like: Date, Integer, Geo...

PS.: I can use findAllBy"Something", but it will be same findBy

PS2.: This where you can find when Criteria construct query

org.springframework.data.mongodb.core.query

    public Criteria gt(Object value) {
        this.criteria.put("$gt", value);
        return this;
    }

@grwang91
Copy link
Contributor Author

grwang91 commented Sep 1, 2023

It works well if I declare id with ObjectId.
However, I think string type id should work well, as it seems spring data mongodb supports String to ObjectId conversion.

If I use String type id and call testRepository.findById, id field is automatically converted to ObjectId. Also, I can identify query log using $oid, unlike query method.

So I think there's hole regarding id field conversion in query method.

@christophstrobl
Copy link
Member

The generated query should take the targetType of @MongoId into account. @grwang91, which version of data-mongodb are you using? Any customisations to the mapping? Ideally please take the time to provide a complete minimal sample (something that we can unzip or git clone, build, and deploy) that reproduces the problem.

@christophstrobl christophstrobl added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 1, 2023
@grwang91
Copy link
Contributor Author

grwang91 commented Sep 1, 2023

@christophstrobl
I'm using spring-boot-starter-data-mongodb:3.1.3, no customize at mapping.

Here's sample git repository.
https://github.com/grwang91/mongodb_test

@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 Sep 1, 2023
@grwang91
Copy link
Contributor Author

@christophstrobl
If it is a real bug that has to be fixed, can I take a look?

@christophstrobl
Copy link
Member

@grwang91 it's on the list of issues to tackle. If you've time to investigate further and propose a solution we'd be happy to review it.

@grwang91
Copy link
Contributor Author

@christophstrobl
I found the reason and there are two options to solve.

First option is modifying at MongoQueryCreator level. (Submitted PR)
Second option is modifying at QueryMapper level that modifies 'else' branch on below code. (line 496)
image

As a maintainer, can you advise which option is better?

@christophstrobl
Copy link
Member

Thank you for digging into it. The QueryMapper is the place where the conversion should happen.

@mp911de mp911de added type: bug A general bug and removed status: feedback-provided Feedback has been provided labels Feb 12, 2024
@mp911de mp911de added this to the 4.3 M1 (2024.0.0) milestone Feb 12, 2024
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
5 participants