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

MongoDB panache generates invalid update documents when updating a list or array with HQL #27326

Closed
languitar opened this issue Aug 17, 2022 · 6 comments · Fixed by #42699
Closed
Assignees
Labels
Milestone

Comments

@languitar
Copy link
Contributor

languitar commented Aug 17, 2022

Describe the bug

When using the update method on a MongoDB panache entity with an HQL-like update description and a list or array parameter, the update fails, because an invalid update document is created.

Expected behavior

Arrays or lists of primitives can be updated using HQL or a proper exception is thrown in case this is not supported.

Actual behavior

The update fails with

2022-08-17 09:49:26,619 ERROR [org.jbo.res.rea.com.cor.AbstractResteasyReactiveContext] (executor-thread-0) Request failed: org.bson.json.JsonParseException: JSON reader was expecting ':' but found '}'.
	at org.bson.json.JsonReader.readBsonType(JsonReader.java:151)
	at org.bson.codecs.DocumentCodec.decode(DocumentCodec.java:178)
	at org.bson.codecs.DocumentCodec.decode(DocumentCodec.java:44)
	at org.bson.internal.LazyCodec.decode(LazyCodec.java:48)
	at org.bson.codecs.ContainerCodecHelper.readValue(ContainerCodecHelper.java:61)
	at org.bson.codecs.DocumentCodec.decode(DocumentCodec.java:180)
	at org.bson.codecs.DocumentCodec.decode(DocumentCodec.java:44)
	at org.bson.Document.parse(Document.java:127)
	at org.bson.Document.parse(Document.java:112)
	at io.quarkus.mongodb.panache.common.runtime.MongoOperations.executeUpdate(MongoOperations.java:762)
	at io.quarkus.mongodb.panache.common.runtime.MongoOperations.update(MongoOperations.java:753)
	at org.acme.TestEntity.update(TestEntity.java)
	at org.acme.GreetingResource.hello(GreetingResource.java:24)

How to Reproduce?

Reproduder: https://github.com/languitar/quarkus-mongodb-panache-update-list-reproducer

  1. start the project in dev mode
  2. curl -v http://localhost:8080/hello

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.11.2.Final

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

Currently, if the list with the new values to set in the update operation contains only one element, the data in the database will be switched from an array to string, consequently breaking any further reads using the POJO.

All this happens, because PanacheQlQueryBinder expands a list into multiple comma-separated tokens, omitting the surrounding array square brackets:

PanacheQlQueryBinder.bindQuery(clazz, "list = ?1", new Object[] {Arrays.asList("test", "foo")})

results in:

{'list':'test', 'foo'}
@languitar languitar added the kind/bug Something isn't working label Aug 17, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Aug 17, 2022

/cc @FroMage, @evanchooly, @loicmathieu

@loicmathieu
Copy link
Contributor

@languitar this indeed looks like a bug, I'll have a look.

@loicmathieu loicmathieu self-assigned this Aug 17, 2022
@languitar
Copy link
Contributor Author

Oh, btw, Maps will also not work:
image

@loicmathieu
Copy link
Contributor

I just check the code and I'm not sure it's possible to update it. The code is shared between query document and update document.

For query document, if it's a native query the user must include the $in: [] and for PanacheQL query we add it.
See the following example from our test cases:

        List<Object> ids = Arrays.asList("f1", "f2");
        query = operations.bindFilter(DemoObj.class, "{ field: { '$in': [:fields] } }",
                Parameters.with("fields", ids).map());
        assertEquals("{ field: { '$in': ['f1', 'f2'] } }", query);

        List<Object> list = Arrays.asList("f1", "f2");
        query = operations.bindFilter(DemoObj.class, "field in ?1", new Object[] { list });
        assertEquals("{'field':{'$in':['f1', 'f2']}}", query);

If I change this it may break existing code, at least for the native query, we can deal with the PanacheQL change on our side. I agree that this is unfortunate for update document, maybe different mapping translation rules should be applied.

For your example, a query like this may work: list = [?1] or it may break our query parser.

As you notted, Map is currently not managed by our translation rules.

@loicmathieu
Copy link
Contributor

Just checked and unfortunatly list=[?1] will not work, you need to use a native query like

 Bug27326Entity.update("{$set: {field : [?1]}}", List.of("one", "two", "three")).where("_id = ?1", bug27326Entity.id);

I keep it open for a while, maybe i'll come up with a solution but for now there is no other way than using a native query as to fix this we will need to break existing code that use collection parameters in a native query.

@languitar
Copy link
Contributor Author

I just check the code and I'm not sure it's possible to update it. The code is shared between query document and update document.

Sounds like a different transformation is needed for the update part to get this working. If that is not a viable option, at least a more informative error message would be nice instead of a BSON parsing exception.

gsmet added a commit to gsmet/quarkus that referenced this issue Aug 22, 2024
While this is a breaking change for native, I think we shouldn't have to
add the [] when dealing with lists: that's not something you do in HQL,
it shouldn't be something you have to do in native.

Fixes quarkusio#27326
gsmet added a commit to gsmet/quarkus that referenced this issue Aug 22, 2024
While this is a breaking change for native, I think we shouldn't have to
add the [] when dealing with lists: that's not something you do in HQL,
it shouldn't be something you have to do in native.

Fixes quarkusio#27326
@gsmet gsmet closed this as completed in b05735d Aug 22, 2024
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Aug 22, 2024
danielsoro pushed a commit to danielsoro/quarkus that referenced this issue Sep 20, 2024
While this is a breaking change for native, I think we shouldn't have to
add the [] when dealing with lists: that's not something you do in HQL,
it shouldn't be something you have to do in native.

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

Successfully merging a pull request may close this issue.

2 participants