From b05735d742fefee15ae179796f136b21373d6db0 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 22 Aug 2024 12:05:04 +0200 Subject: [PATCH] MongoDB Panache - Consistently encode list with [] 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 #27326 --- docs/src/main/asciidoc/mongodb-panache.adoc | 4 +++- .../common/binder/CommonQueryBinder.java | 2 +- .../common/binder/MongoParserVisitor.java | 4 ++-- .../common/runtime/MongoOperationsTest.java | 21 ++++++++++++------- .../panache/book/BookEntityResource.java | 7 +++++++ .../panache/book/BookRepositoryResource.java | 7 +++++++ .../panache/MongodbPanacheResourceTest.java | 15 +++++++++++++ 7 files changed, 48 insertions(+), 12 deletions(-) diff --git a/docs/src/main/asciidoc/mongodb-panache.adoc b/docs/src/main/asciidoc/mongodb-panache.adoc index 60ab35867ab0e..bef7cf9c63556 100644 --- a/docs/src/main/asciidoc/mongodb-panache.adoc +++ b/docs/src/main/asciidoc/mongodb-panache.adoc @@ -572,12 +572,14 @@ Here are some query examples: - `amount > ?1 and firstname != ?2` will be mapped to `{'amount': {'$gt': ?1}, 'firstname': {'$ne': ?2}}` - `lastname like ?1` will be mapped to `{'lastname': {'$regex': ?1}}`. Be careful that this will be link:https://docs.mongodb.com/manual/reference/operator/query/regex/#op._S_regex[MongoDB regex] support and not SQL like pattern. - `lastname is not null` will be mapped to `{'lastname':{'$exists': true}}` -- `status in ?1` will be mapped to `{'status':{$in: [?1]}}` +- `status in ?1` will be mapped to `{'status':{$in: ?1}}` WARNING: MongoDB queries must be valid JSON documents, using the same field multiple times in a query is not allowed using PanacheQL as it would generate an invalid JSON (see link:https://github.com/quarkusio/quarkus/issues/12086[this issue on GitHub]). +WARNING: Prior to Quarkus 3.16, when using `$in` with a list, you had to write your query with `{'status':{$in: [?1]}}`. Starting with Quarkus 3.16, make sure you use `{'status':{$in: ?1}}` instead. The list will be properly expanded with surrounding square brackets. + We also handle some basic date type transformations: all fields of type `Date`, `LocalDate`, `LocalDateTime` or `Instant` will be mapped to the link:https://docs.mongodb.com/manual/reference/bson-types/#date[BSON Date] using the `ISODate` type (UTC datetime). The MongoDB POJO codec doesn't support `ZonedDateTime` and `OffsetDateTime` so you should convert them prior usage. diff --git a/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/binder/CommonQueryBinder.java b/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/binder/CommonQueryBinder.java index 1900974bcd166..2824a3ef2d9f5 100644 --- a/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/binder/CommonQueryBinder.java +++ b/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/binder/CommonQueryBinder.java @@ -32,7 +32,7 @@ static String escape(Object value) { return "null"; } if (value.getClass().isArray() || Collection.class.isAssignableFrom(value.getClass())) { - return arrayAsString(value); + return "[" + arrayAsString(value) + "]"; } if (Number.class.isAssignableFrom(value.getClass()) || value instanceof Boolean) { return value.toString(); diff --git a/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/binder/MongoParserVisitor.java b/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/binder/MongoParserVisitor.java index fc3294487c00b..7ecae81441b63 100644 --- a/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/binder/MongoParserVisitor.java +++ b/extensions/panache/mongodb-panache-common/runtime/src/main/java/io/quarkus/mongodb/panache/common/binder/MongoParserVisitor.java @@ -152,9 +152,9 @@ private String unquote(String text) { @Override public String visitInPredicate(HqlParser.InPredicateContext ctx) { StringBuilder sb = new StringBuilder(ctx.expression().accept(this)) - .append(":{'$in':[") + .append(":{'$in':") .append(ctx.inList().accept(this)) - .append("]}"); + .append("}"); return sb.toString(); } diff --git a/extensions/panache/mongodb-panache-common/runtime/src/test/java/io/quarkus/mongodb/panache/common/runtime/MongoOperationsTest.java b/extensions/panache/mongodb-panache-common/runtime/src/test/java/io/quarkus/mongodb/panache/common/runtime/MongoOperationsTest.java index 14089ccafae32..8a1cb1f0081ef 100644 --- a/extensions/panache/mongodb-panache-common/runtime/src/test/java/io/quarkus/mongodb/panache/common/runtime/MongoOperationsTest.java +++ b/extensions/panache/mongodb-panache-common/runtime/src/test/java/io/quarkus/mongodb/panache/common/runtime/MongoOperationsTest.java @@ -51,6 +51,7 @@ protected Object createQuery(MongoCollection collection, ClientSession session, private static class DemoObj { public String field; + public List listField; public boolean isOk; @BsonProperty("value") public String property; @@ -146,19 +147,19 @@ public void testBindNativeFilterByIndex() { //queries related to '$in' operator List list = Arrays.asList("f1", "f2"); - query = operations.bindFilter(DemoObj.class, "{ field: { '$in': [?1] } }", new Object[] { list }); + query = operations.bindFilter(DemoObj.class, "{ field: { '$in': ?1 } }", new Object[] { list }); assertEquals("{ field: { '$in': ['f1', 'f2'] } }", query); - query = operations.bindFilter(DemoObj.class, "{ field: { '$in': [?1] }, isOk: ?2 }", new Object[] { list, true }); + query = operations.bindFilter(DemoObj.class, "{ field: { '$in': ?1 }, isOk: ?2 }", new Object[] { list, true }); assertEquals("{ field: { '$in': ['f1', 'f2'] }, isOk: true }", query); query = operations.bindFilter(DemoObj.class, - "{ field: { '$in': [?1] }, $or: [ {'property': ?2}, {'property': ?3} ] }", + "{ field: { '$in': ?1 }, $or: [ {'property': ?2}, {'property': ?3} ] }", new Object[] { list, "jpg", "gif" }); assertEquals("{ field: { '$in': ['f1', 'f2'] }, $or: [ {'property': 'jpg'}, {'property': 'gif'} ] }", query); query = operations.bindFilter(DemoObj.class, - "{ field: { '$in': [?1] }, isOk: ?2, $or: [ {'property': ?3}, {'property': ?4} ] }", + "{ field: { '$in': ?1 }, isOk: ?2, $or: [ {'property': ?3}, {'property': ?4} ] }", new Object[] { list, true, "jpg", "gif" }); assertEquals("{ field: { '$in': ['f1', 'f2'] }, isOk: true, $or: [ {'property': 'jpg'}, {'property': 'gif'} ] }", query); @@ -205,21 +206,21 @@ public void testBindNativeFilterByName() { //queries related to '$in' operator List ids = Arrays.asList("f1", "f2"); - query = operations.bindFilter(DemoObj.class, "{ field: { '$in': [:fields] } }", + query = operations.bindFilter(DemoObj.class, "{ field: { '$in': :fields } }", Parameters.with("fields", ids).map()); assertEquals("{ field: { '$in': ['f1', 'f2'] } }", query); - query = operations.bindFilter(DemoObj.class, "{ field: { '$in': [:fields] }, isOk: :isOk }", + query = operations.bindFilter(DemoObj.class, "{ field: { '$in': :fields }, isOk: :isOk }", Parameters.with("fields", ids).and("isOk", true).map()); assertEquals("{ field: { '$in': ['f1', 'f2'] }, isOk: true }", query); query = operations.bindFilter(DemoObj.class, - "{ field: { '$in': [:fields] }, $or: [ {'property': :p1}, {'property': :p2} ] }", + "{ field: { '$in': :fields }, $or: [ {'property': :p1}, {'property': :p2} ] }", Parameters.with("fields", ids).and("p1", "jpg").and("p2", "gif").map()); assertEquals("{ field: { '$in': ['f1', 'f2'] }, $or: [ {'property': 'jpg'}, {'property': 'gif'} ] }", query); query = operations.bindFilter(DemoObj.class, - "{ field: { '$in': [:fields] }, isOk: :isOk, $or: [ {'property': :p1}, {'property': :p2} ] }", + "{ field: { '$in': :fields }, isOk: :isOk, $or: [ {'property': :p1}, {'property': :p2} ] }", Parameters.with("fields", ids) .and("isOk", true) .and("p1", "jpg") @@ -388,6 +389,10 @@ public void testBindUpdate() { String update = operations.bindUpdate(DemoObj.class, "{'field': ?1}", new Object[] { "a value" }); assertEquals("{'$set':{'field': 'a value'}}", update); + // native update by index without $set + update = operations.bindUpdate(DemoObj.class, "{'listField': ?1}", new Object[] { List.of("value1", "value2") }); + assertEquals("{'$set':{'listField': ['value1', 'value2']}}", update); + // native update by name without $set update = operations.bindUpdate(Object.class, "{'field': :field}", Parameters.with("field", "a value").map()); diff --git a/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/book/BookEntityResource.java b/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/book/BookEntityResource.java index 5622ed83d2f05..2c360e39eb2fc 100644 --- a/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/book/BookEntityResource.java +++ b/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/book/BookEntityResource.java @@ -115,6 +115,13 @@ public BookEntity search2(@QueryParam("author") String author, @QueryParam("titl Parameters.with("dateFrom", LocalDate.parse(dateFrom)).and("dateTo", LocalDate.parse(dateTo))).firstResult(); } + @PUT + @Path("/update-categories/{id}") + public Response updateCategories(@PathParam("id") String id) { + BookEntity.update("categories = ?1", List.of("novel", "fiction")).where("_id", new ObjectId(id)); + return Response.accepted().build(); + } + @DELETE public void deleteAll() { BookEntity.deleteAll(); diff --git a/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/book/BookRepositoryResource.java b/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/book/BookRepositoryResource.java index 16705ac803f0e..d341827a8d9d1 100644 --- a/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/book/BookRepositoryResource.java +++ b/integration-tests/mongodb-panache/src/main/java/io/quarkus/it/mongodb/panache/book/BookRepositoryResource.java @@ -110,6 +110,13 @@ public Book search2(@QueryParam("author") String author, @QueryParam("title") St Parameters.with("dateFrom", LocalDate.parse(dateFrom)).and("dateTo", LocalDate.parse(dateTo))).firstResult(); } + @PUT + @Path("/update-categories/{id}") + public Response updateCategories(@PathParam("id") String id) { + bookRepository.update("categories = ?1", List.of("novel", "fiction")).where("_id", new ObjectId(id)); + return Response.accepted().build(); + } + @DELETE public void deleteAll() { bookRepository.deleteAll(); diff --git a/integration-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java b/integration-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java index 14c2f29761c21..d07a9733a4744 100644 --- a/integration-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java +++ b/integration-tests/mongodb-panache/src/test/java/io/quarkus/it/mongodb/panache/MongodbPanacheResourceTest.java @@ -183,6 +183,21 @@ private void callBookEndpoint(String endpoint) { book = get(endpoint + "/optional/" + book.getId().toString()).as(BookDTO.class); Assertions.assertNotNull(book); + // update categories list using HQL + response = RestAssured + .given() + .header("Content-Type", "application/json") + .put(endpoint + "/update-categories/" + book.getId()) + .andReturn(); + Assertions.assertEquals(202, response.statusCode()); + + //check that the title and categories have been updated and the transient description ignored + book = get(endpoint + "/" + book.getId().toString()).as(BookDTO.class); + Assertions.assertNotNull(book); + Assertions.assertEquals("Notre-Dame de Paris 2", book.getTitle()); + Assertions.assertNull(book.getTransientDescription()); + Assertions.assertEquals(List.of("novel", "fiction"), book.getCategories()); + //delete a book response = RestAssured .given()