diff --git a/dsl/src/main/kotlin/expr/Expression.kt b/dsl/src/main/kotlin/expr/Expression.kt new file mode 100644 index 00000000..c519fe63 --- /dev/null +++ b/dsl/src/main/kotlin/expr/Expression.kt @@ -0,0 +1,111 @@ +package fr.qsh.ktmongo.dsl.expr + +import fr.qsh.ktmongo.dsl.LowLevelApi +import org.bson.BsonDocument +import org.bson.BsonDocumentWriter +import org.bson.codecs.configuration.CodecRegistry + +/** + * A compound node in the BSON AST. + * + * Compared to regular [ExpressionNode], `Expression` adds [accept], which allows to inject sub-expressions + * into the current expression. + * + * Subclasses of this interface provide DSLs to create BSON expressions. + * + * ### Implementation notes + * + * Much like [ExpressionNode], implementations of this interface **must** implement [toString] to print the generated + * BSON used by the request. We recommend implementing [AbstractExpression], which does this automatically. + */ +@OptIn(LowLevelApi::class) +interface Expression : ExpressionNode { + + /** + * Adds an arbitrary [node] to this expression. + * + * ### Security and correctness + * + * This function makes no verification on the validity of the passed node. + * It is added to this expression as-is. + * + * This function is only publicly available to allow users to add missing operators themselves, by implementing + * [ExpressionNode]. Only implement operators yourself if you are sure of what you are doing! + */ + @LowLevelApi + fun accept(node: ExpressionNode) +} + +/** + * A node in the BSON AST. + * + * Each node knows how to [write] itself into the expression. + * + * ### Security + * + * Implementing this interface allows to inject arbitrary BSON into a request. + * Be very careful not to allow request injections. + * + * ### Implementation notes + * + * To facilitate debugging, **all** implementations of [ExpressionNode] must implement [toString] to print the + * predicted BSON used by the request. + * We recommend implementing [AbstractExpressionNode], which does this automatically. + */ +@LowLevelApi +interface ExpressionNode { + + /** + * Writes the current node into the BSON AST, represented by the [writer]. + */ + @LowLevelApi + fun write(writer: BsonDocumentWriter, codec: CodecRegistry) +} + +/** + * Helper to implement [Expression] that handles [toString] generation and the implementation of [accept]. + */ +@OptIn(LowLevelApi::class) +abstract class AbstractExpression( + codec: CodecRegistry, +) : AbstractExpressionNode(codec), Expression { + + @OptIn(LowLevelApi::class) + private val children = ArrayList() + + @LowLevelApi + override fun accept(node: ExpressionNode) { + children += node + } + + /** + * Writes [children] into the [writer], simplifying the expression in any way as needed. + */ + @LowLevelApi + protected abstract fun write(writer: BsonDocumentWriter, codec: CodecRegistry, children: List) + + @LowLevelApi + override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + write(writer, codec, children) + } +} + +/** + * Helper to implement [ExpressionNode] that handles [toString] generation. + */ +@LowLevelApi +abstract class AbstractExpressionNode( + private val codec: CodecRegistry, +) : ExpressionNode { + + override fun toString(): String { + val document = BsonDocument() + + @OptIn(LowLevelApi::class) + BsonDocumentWriter(document).use { + write(it, codec) + } + + return document.toJson() + } +} diff --git a/dsl/src/main/kotlin/expr/FilterExpression.kt b/dsl/src/main/kotlin/expr/FilterExpression.kt index 2801e3de..e92433a0 100644 --- a/dsl/src/main/kotlin/expr/FilterExpression.kt +++ b/dsl/src/main/kotlin/expr/FilterExpression.kt @@ -130,7 +130,7 @@ class FilterExpression( @KtMongoDsl inline operator fun <@OnlyInputTypes V> KProperty1.invoke(block: PredicateExpression.() -> Unit) { writer.buildDocument(this.path().toString()) { - PredicateExpression(writer, codec).apply(block) + PredicateExpression(codec).apply(block).write(writer, codec) } } @@ -159,7 +159,7 @@ class FilterExpression( * - [Official documentation](https://www.mongodb.com/docs/manual/reference/operator/query/not/) */ @KtMongoDsl - inline infix fun <@OnlyInputTypes V> KProperty1.not(expression: PredicateExpression.() -> Unit) { + infix fun <@OnlyInputTypes V> KProperty1.not(expression: PredicateExpression.() -> Unit) { this { this.not(expression) } } diff --git a/dsl/src/main/kotlin/expr/PredicateExpression.kt b/dsl/src/main/kotlin/expr/PredicateExpression.kt index 0a36952c..b54ab403 100644 --- a/dsl/src/main/kotlin/expr/PredicateExpression.kt +++ b/dsl/src/main/kotlin/expr/PredicateExpression.kt @@ -13,16 +13,25 @@ import org.bson.codecs.configuration.CodecRegistry * DSL for MongoDB operators that are used as predicates in conditions in a context where the targeted field is already * specified. */ -@OptIn(LowLevelApi::class) @KtMongoDsl class PredicateExpression( @property:LowLevelApi - @PublishedApi - internal val writer: BsonDocumentWriter, + val codec: CodecRegistry, +) : AbstractExpression(codec) { - @PublishedApi - internal val codec: CodecRegistry, -) { + // region Low-level operations + + @LowLevelApi + override fun write(writer: BsonDocumentWriter, codec: CodecRegistry, children: List) { + for (child in children) { + child.write(writer, codec) + } + } + + @LowLevelApi + private sealed class PredicateExpressionNode(codec: CodecRegistry) : AbstractExpressionNode(codec) + + // endregion /** * Matches documents where the value of a field equals the [value]. @@ -48,15 +57,26 @@ class PredicateExpression( * * @see FilterExpression.eq Shorthand. */ + @OptIn(LowLevelApi::class) @KtMongoDsl fun eq(value: T) { - writer.buildDocument("\$eq") { - if (value == null) { - writer.writeNull() - } else { - @Suppress("UNNECESSARY_NOT_NULL_ASSERTION", "UNCHECKED_CAST") // Kotlin doesn't smart-cast here, but should, this is safe - (codec.get(value!!::class.java) as Encoder) - .encode(writer, value, EncoderContext.builder().build()) + accept(EqualityExpressionNode(value, codec)) + } + + @LowLevelApi + private class EqualityExpressionNode( + val value: T, + codec: CodecRegistry, + ) : PredicateExpressionNode(codec) { + override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + writer.buildDocument("\$eq") { + if (value == null) { + writer.writeNull() + } else { + @Suppress("UNNECESSARY_NOT_NULL_ASSERTION", "UNCHECKED_CAST") // Kotlin doesn't smart-cast here, but should, this is safe + (codec.get(value!!::class.java) as Encoder) + .encode(writer, value, EncoderContext.builder().build()) + } } } } @@ -126,10 +146,21 @@ class PredicateExpression( * @see doesNotExist Opposite. * @see isNotNull Identical, but does not match elements where the field is `null`. */ + @OptIn(LowLevelApi::class) @KtMongoDsl fun exists() { - writer.buildDocument("\$exists") { - writer.writeBoolean(true) + accept(ExistsPredicateExpressionNode(true, codec)) + } + + @LowLevelApi + private class ExistsPredicateExpressionNode( + val exists: Boolean, + codec: CodecRegistry, + ) : PredicateExpressionNode(codec) { + override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + writer.buildDocument("\$exists") { + writer.writeBoolean(exists) + } } } @@ -160,11 +191,10 @@ class PredicateExpression( * @see exists Opposite. * @see isNull Only matches elements that are specifically `null`. */ + @OptIn(LowLevelApi::class) @KtMongoDsl fun doesNotExist() { - writer.buildDocument("\$exists") { - writer.writeBoolean(false) - } + accept(ExistsPredicateExpressionNode(false, codec)) } /** @@ -196,10 +226,21 @@ class PredicateExpression( * @see isNull Checks if a value has the type [BsonType.NULL]. * @see isUndefined Checks if a value has the type [BsonType.UNDEFINED]. */ + @OptIn(LowLevelApi::class) @KtMongoDsl fun hasType(type: BsonType) { - writer.buildDocument("\$type") { - writer.writeInt32(type.value) + accept(TypePredicateExpressionNode(type, codec)) + } + + @LowLevelApi + private class TypePredicateExpressionNode( + val type: BsonType, + codec: CodecRegistry, + ) : PredicateExpressionNode(codec) { + override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + writer.buildDocument("\$type") { + writer.writeInt32(this.type.value) + } } } @@ -231,10 +272,21 @@ class PredicateExpression( * * @see FilterExpression.not Shorthand. */ + @OptIn(LowLevelApi::class) @KtMongoDsl - inline fun not(expression: PredicateExpression.() -> Unit) { - writer.buildDocument("\$not") { - PredicateExpression(writer, codec).apply(expression) + fun not(expression: PredicateExpression.() -> Unit) { + accept(NotPredicateExpressionNode(PredicateExpression(codec).apply(expression), codec)) + } + + @LowLevelApi + private class NotPredicateExpressionNode( + val expression: PredicateExpression, + codec: CodecRegistry, + ) : PredicateExpressionNode(codec) { + override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + writer.buildDocument("\$not") { + expression.write(writer, codec) + } } } diff --git a/dsl/src/test/kotlin/expr/ExpressionTestUtils.kt b/dsl/src/test/kotlin/expr/ExpressionTestUtils.kt index f47e3da5..08a0f790 100644 --- a/dsl/src/test/kotlin/expr/ExpressionTestUtils.kt +++ b/dsl/src/test/kotlin/expr/ExpressionTestUtils.kt @@ -1,5 +1,6 @@ package fr.qsh.ktmongo.dsl.expr +import fr.qsh.ktmongo.dsl.LowLevelApi import io.kotest.matchers.shouldBe import org.bson.BsonDocument import org.bson.BsonDocumentWriter @@ -11,66 +12,76 @@ import org.bson.codecs.jsr310.LocalDateCodec import org.bson.codecs.jsr310.LocalDateTimeCodec import org.bson.codecs.jsr310.LocalTimeCodec +fun testCodec(): CodecRegistry = CodecRegistries.fromCodecs( + AtomicBooleanCodec(), + AtomicIntegerCodec(), + AtomicLongCodec(), + BigDecimalCodec(), + BinaryCodec(), + BooleanCodec(), + BsonArrayCodec(), + BsonBinaryCodec(), + BsonBooleanCodec(), + BsonDateTimeCodec(), + BsonDBPointerCodec(), + BsonDecimal128Codec(), + BsonDocumentCodec(), + BsonDoubleCodec(), + BsonInt32Codec(), + BsonInt64Codec(), + BsonJavaScriptCodec(), + BsonMaxKeyCodec(), + BsonMinKeyCodec(), + BsonNullCodec(), + BsonObjectIdCodec(), + BsonRegularExpressionCodec(), + BsonStringCodec(), + BsonSymbolCodec(), + BsonTimestampCodec(), + BsonUndefinedCodec(), + BsonValueCodec(), + ByteArrayCodec(), + ByteCodec(), + CharacterCodec(), + CodeCodec(), + DateCodec(), + Decimal128Codec(), + DocumentCodec(), + DoubleCodec(), + FloatCodec(), + InstantCodec(), + IntegerCodec(), + JsonObjectCodec(), + LocalDateCodec(), + LocalDateTimeCodec(), + LocalTimeCodec(), + LongCodec(), + MaxKeyCodec(), + MinKeyCodec(), + ObjectIdCodec(), + OverridableUuidRepresentationUuidCodec(), + PatternCodec(), + RawBsonDocumentCodec(), + ShortCodec(), + StringCodec(), + SymbolCodec(), + UuidCodec(), +) + fun buildExpression(dsl: (BsonDocumentWriter, CodecRegistry) -> T, block: T.() -> Unit): String { val document = BsonDocument() - val registry = CodecRegistries.fromCodecs( - AtomicBooleanCodec(), - AtomicIntegerCodec(), - AtomicLongCodec(), - BigDecimalCodec(), - BinaryCodec(), - BooleanCodec(), - BsonArrayCodec(), - BsonBinaryCodec(), - BsonBooleanCodec(), - BsonDateTimeCodec(), - BsonDBPointerCodec(), - BsonDecimal128Codec(), - BsonDocumentCodec(), - BsonDoubleCodec(), - BsonInt32Codec(), - BsonInt64Codec(), - BsonJavaScriptCodec(), - BsonMaxKeyCodec(), - BsonMinKeyCodec(), - BsonNullCodec(), - BsonObjectIdCodec(), - BsonRegularExpressionCodec(), - BsonStringCodec(), - BsonSymbolCodec(), - BsonTimestampCodec(), - BsonUndefinedCodec(), - BsonValueCodec(), - ByteArrayCodec(), - ByteCodec(), - CharacterCodec(), - CodeCodec(), - DateCodec(), - Decimal128Codec(), - DocumentCodec(), - DoubleCodec(), - FloatCodec(), - InstantCodec(), - IntegerCodec(), - JsonObjectCodec(), - LocalDateCodec(), - LocalDateTimeCodec(), - LocalTimeCodec(), - LongCodec(), - MaxKeyCodec(), - MinKeyCodec(), - ObjectIdCodec(), - OverridableUuidRepresentationUuidCodec(), - PatternCodec(), - RawBsonDocumentCodec(), - ShortCodec(), - StringCodec(), - SymbolCodec(), - UuidCodec(), - ) + dsl(BsonDocumentWriter(document), testCodec()).apply(block) + + return document.toJson() +} + +@OptIn(LowLevelApi::class) +fun buildExpression(dsl: (CodecRegistry) -> E, block: E.() -> Unit): String { + val document = BsonDocument() - dsl(BsonDocumentWriter(document), registry).apply(block) + val codec = testCodec() + dsl(codec).apply(block).write(BsonDocumentWriter(document), codec) return document.toJson() } -- 2.51.2 From 55df651a19ddfd451e17dc153cfcc87abf08fe88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Fri, 10 May 2024 11:25:15 +0200 Subject: [PATCH 02/18] refactor(dsl): Intermediate request representation for FilterExpression --- driver-sync/src/main/kotlin/Count.kt | 6 +- driver-sync/src/main/kotlin/Find.kt | 6 +- dsl/src/main/kotlin/expr/FilterExpression.kt | 75 +++++++++++++++----- 3 files changed, 67 insertions(+), 20 deletions(-) diff --git a/driver-sync/src/main/kotlin/Count.kt b/driver-sync/src/main/kotlin/Count.kt index 1cf4e431..2f13408f 100644 --- a/driver-sync/src/main/kotlin/Count.kt +++ b/driver-sync/src/main/kotlin/Count.kt @@ -30,8 +30,10 @@ fun MongoCollection.countDocuments(): Long { fun MongoCollection.countDocuments(predicate: FilterExpression.() -> Unit): Long { val bson = BsonDocument() - FilterExpression(BsonDocumentWriter(bson), unsafe.codecRegistry) - .and(predicate) // use an 'and' as the default + FilterExpression(unsafe.codecRegistry).apply { + and(predicate) // use an 'and' as the default + write(BsonDocumentWriter(bson), unsafe.codecRegistry) + } return unsafe.countDocuments(filter = bson) } diff --git a/driver-sync/src/main/kotlin/Find.kt b/driver-sync/src/main/kotlin/Find.kt index 7f72c4ea..e3889705 100644 --- a/driver-sync/src/main/kotlin/Find.kt +++ b/driver-sync/src/main/kotlin/Find.kt @@ -45,8 +45,10 @@ fun MongoCollection.find(): FindIterable { fun MongoCollection.find(predicate: FilterExpression.() -> Unit): FindIterable { val bson = BsonDocument() - FilterExpression(BsonDocumentWriter(bson), unsafe.codecRegistry) - .and(predicate) // use an 'and' as the default + FilterExpression(unsafe.codecRegistry).apply { + and(predicate) // use an 'and' as the default + write(BsonDocumentWriter(bson), unsafe.codecRegistry) + } return unsafe.find(bson.asDocument()) } diff --git a/dsl/src/main/kotlin/expr/FilterExpression.kt b/dsl/src/main/kotlin/expr/FilterExpression.kt index e92433a0..c66d5810 100644 --- a/dsl/src/main/kotlin/expr/FilterExpression.kt +++ b/dsl/src/main/kotlin/expr/FilterExpression.kt @@ -19,12 +19,22 @@ import kotlin.reflect.KProperty1 @KtMongoDsl class FilterExpression( @property:LowLevelApi - @PublishedApi - internal val writer: BsonDocumentWriter, + val codec: CodecRegistry, +) : AbstractExpression(codec) { - @PublishedApi - internal val codec: CodecRegistry, -) { + // region Low-level operations + + @LowLevelApi + override fun write(writer: BsonDocumentWriter, codec: CodecRegistry, children: List) { + for (child in children) { + child.write(writer, codec) + } + } + + @LowLevelApi + private sealed class FilterExpressionNode(codec: CodecRegistry) : AbstractExpressionNode(codec) + + // endregion /** * Performs a logical `AND` operation on one or more expressions, @@ -54,10 +64,21 @@ class FilterExpression( */ @OptIn(LowLevelApi::class) @KtMongoDsl - inline fun and(block: FilterExpression.() -> Unit) { - writer.buildDocument("\$and") { - writer.buildArray { - block() + fun and(block: FilterExpression.() -> Unit) { + accept(AndFilterExpressionNode(FilterExpression(codec).apply(block), codec)) + } + + @LowLevelApi + private class AndFilterExpressionNode( + val expression: FilterExpression, + codec: CodecRegistry, + ) : FilterExpressionNode(codec) { + + override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + writer.buildDocument("\$and") { + writer.buildArray { + expression.write(writer, codec) + } } } } @@ -91,10 +112,21 @@ class FilterExpression( */ @OptIn(LowLevelApi::class) @KtMongoDsl - inline fun or(block: FilterExpression.() -> Unit) { - writer.buildDocument("\$or") { - writer.buildArray { - block() + fun or(block: FilterExpression.() -> Unit) { + accept(OrFilterExpressionNode(FilterExpression(codec).apply(block), codec)) + } + + @LowLevelApi + private class OrFilterExpressionNode( + val expression: FilterExpression, + codec: CodecRegistry, + ) : FilterExpressionNode(codec) { + + override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + writer.buildDocument("\$or") { + writer.buildArray { + expression.write(writer, codec) + } } } } @@ -128,9 +160,20 @@ class FilterExpression( */ @OptIn(LowLevelApi::class) @KtMongoDsl - inline operator fun <@OnlyInputTypes V> KProperty1.invoke(block: PredicateExpression.() -> Unit) { - writer.buildDocument(this.path().toString()) { - PredicateExpression(codec).apply(block).write(writer, codec) + operator fun <@OnlyInputTypes V> KProperty1.invoke(block: PredicateExpression.() -> Unit) { + accept(PredicateInFilterExpression(this.path().toString(), PredicateExpression(codec).apply(block), codec)) + } + + @LowLevelApi + private class PredicateInFilterExpression( + val target: String, + val expression: PredicateExpression<*>, + codec: CodecRegistry, + ) : FilterExpressionNode(codec) { + override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + writer.buildDocument(target) { + expression.write(writer, codec) + } } } -- 2.51.2 From 4b0cfa74b7b3d67446f3d50e86c459c256edcbb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Fri, 10 May 2024 11:59:25 +0200 Subject: [PATCH 03/18] feat(dsl): Automatically insert an $and when multiple filters are provided --- driver-sync/src/main/kotlin/Count.kt | 7 ++-- driver-sync/src/main/kotlin/Find.kt | 7 ++-- dsl/src/main/kotlin/expr/Expression.kt | 40 ++++++++++++++++--- dsl/src/main/kotlin/expr/FilterExpression.kt | 14 +++++-- .../main/kotlin/expr/PredicateExpression.kt | 7 ---- .../test/kotlin/expr/ExpressionTestUtils.kt | 2 +- .../test/kotlin/expr/FilterExpressionTest.kt | 22 ++++++++++ 7 files changed, 74 insertions(+), 25 deletions(-) diff --git a/driver-sync/src/main/kotlin/Count.kt b/driver-sync/src/main/kotlin/Count.kt index 2f13408f..6c6603f3 100644 --- a/driver-sync/src/main/kotlin/Count.kt +++ b/driver-sync/src/main/kotlin/Count.kt @@ -30,10 +30,9 @@ fun MongoCollection.countDocuments(): Long { fun MongoCollection.countDocuments(predicate: FilterExpression.() -> Unit): Long { val bson = BsonDocument() - FilterExpression(unsafe.codecRegistry).apply { - and(predicate) // use an 'and' as the default - write(BsonDocumentWriter(bson), unsafe.codecRegistry) - } + FilterExpression(unsafe.codecRegistry) + .apply(predicate) + .simplifyAndWrite(BsonDocumentWriter(bson), unsafe.codecRegistry) return unsafe.countDocuments(filter = bson) } diff --git a/driver-sync/src/main/kotlin/Find.kt b/driver-sync/src/main/kotlin/Find.kt index e3889705..0480da1d 100644 --- a/driver-sync/src/main/kotlin/Find.kt +++ b/driver-sync/src/main/kotlin/Find.kt @@ -45,10 +45,9 @@ fun MongoCollection.find(): FindIterable { fun MongoCollection.find(predicate: FilterExpression.() -> Unit): FindIterable { val bson = BsonDocument() - FilterExpression(unsafe.codecRegistry).apply { - and(predicate) // use an 'and' as the default - write(BsonDocumentWriter(bson), unsafe.codecRegistry) - } + FilterExpression(unsafe.codecRegistry) + .apply(predicate) + .simplifyAndWrite(BsonDocumentWriter(bson), unsafe.codecRegistry) return unsafe.find(bson.asDocument()) } diff --git a/dsl/src/main/kotlin/expr/Expression.kt b/dsl/src/main/kotlin/expr/Expression.kt index c519fe63..3ecba513 100644 --- a/dsl/src/main/kotlin/expr/Expression.kt +++ b/dsl/src/main/kotlin/expr/Expression.kt @@ -1,5 +1,6 @@ package fr.qsh.ktmongo.dsl.expr +import fr.qsh.ktmongo.dsl.KtMongoDsl import fr.qsh.ktmongo.dsl.LowLevelApi import org.bson.BsonDocument import org.bson.BsonDocumentWriter @@ -19,6 +20,7 @@ import org.bson.codecs.configuration.CodecRegistry * BSON used by the request. We recommend implementing [AbstractExpression], which does this automatically. */ @OptIn(LowLevelApi::class) +@KtMongoDsl interface Expression : ExpressionNode { /** @@ -33,9 +35,17 @@ interface Expression : ExpressionNode { * [ExpressionNode]. Only implement operators yourself if you are sure of what you are doing! */ @LowLevelApi + @KtMongoDsl fun accept(node: ExpressionNode) } +@LowLevelApi +@KtMongoDsl +fun Expression.acceptAll(nodes: Iterable) { + for (node in nodes) + accept(node) +} + /** * A node in the BSON AST. * @@ -60,6 +70,22 @@ interface ExpressionNode { */ @LowLevelApi fun write(writer: BsonDocumentWriter, codec: CodecRegistry) + + /** + * Executes simplification against the current node. + * + * By default, no simplifications are executed and this object is returned as-is. + */ + @LowLevelApi + fun simplify(codec: CodecRegistry): ExpressionNode = this + + @LowLevelApi + fun simplifyAndWrite(writer: BsonDocumentWriter, codec: CodecRegistry) = + simplify(codec).write(writer, codec) + + object EmptyExpressionNode : ExpressionNode { + override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) {} + } } /** @@ -78,15 +104,17 @@ abstract class AbstractExpression( children += node } - /** - * Writes [children] into the [writer], simplifying the expression in any way as needed. - */ @LowLevelApi - protected abstract fun write(writer: BsonDocumentWriter, codec: CodecRegistry, children: List) + protected open fun simplify(codec: CodecRegistry, children: List): ExpressionNode = this + + final override fun simplify(codec: CodecRegistry): ExpressionNode = + simplify(codec, children) @LowLevelApi override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { - write(writer, codec, children) + for (child in children) { + child.write(writer, codec) + } } } @@ -103,7 +131,7 @@ abstract class AbstractExpressionNode( @OptIn(LowLevelApi::class) BsonDocumentWriter(document).use { - write(it, codec) + simplifyAndWrite(it, codec) } return document.toJson() diff --git a/dsl/src/main/kotlin/expr/FilterExpression.kt b/dsl/src/main/kotlin/expr/FilterExpression.kt index c66d5810..e7a2985b 100644 --- a/dsl/src/main/kotlin/expr/FilterExpression.kt +++ b/dsl/src/main/kotlin/expr/FilterExpression.kt @@ -4,10 +4,12 @@ import fr.qsh.ktmongo.dsl.KtMongoDsl import fr.qsh.ktmongo.dsl.LowLevelApi import fr.qsh.ktmongo.dsl.buildArray import fr.qsh.ktmongo.dsl.buildDocument +import fr.qsh.ktmongo.dsl.expr.ExpressionNode.EmptyExpressionNode import fr.qsh.ktmongo.dsl.path.path import org.bson.BsonDocumentWriter import org.bson.BsonType import org.bson.codecs.configuration.CodecRegistry +import javax.management.Query.and import kotlin.internal.OnlyInputTypes import kotlin.reflect.KProperty1 @@ -25,9 +27,15 @@ class FilterExpression( // region Low-level operations @LowLevelApi - override fun write(writer: BsonDocumentWriter, codec: CodecRegistry, children: List) { - for (child in children) { - child.write(writer, codec) + override fun simplify(codec: CodecRegistry, children: List): ExpressionNode { + return when (children.size) { + 0 -> EmptyExpressionNode + 1 -> this + else -> FilterExpression(codec).apply { + and { + acceptAll(children) + } + } } } diff --git a/dsl/src/main/kotlin/expr/PredicateExpression.kt b/dsl/src/main/kotlin/expr/PredicateExpression.kt index b54ab403..b4c21683 100644 --- a/dsl/src/main/kotlin/expr/PredicateExpression.kt +++ b/dsl/src/main/kotlin/expr/PredicateExpression.kt @@ -21,13 +21,6 @@ class PredicateExpression( // region Low-level operations - @LowLevelApi - override fun write(writer: BsonDocumentWriter, codec: CodecRegistry, children: List) { - for (child in children) { - child.write(writer, codec) - } - } - @LowLevelApi private sealed class PredicateExpressionNode(codec: CodecRegistry) : AbstractExpressionNode(codec) diff --git a/dsl/src/test/kotlin/expr/ExpressionTestUtils.kt b/dsl/src/test/kotlin/expr/ExpressionTestUtils.kt index 08a0f790..2324e3b8 100644 --- a/dsl/src/test/kotlin/expr/ExpressionTestUtils.kt +++ b/dsl/src/test/kotlin/expr/ExpressionTestUtils.kt @@ -81,7 +81,7 @@ fun buildExpression(dsl: (CodecRegistry) -> E, block: E.() -> U val document = BsonDocument() val codec = testCodec() - dsl(codec).apply(block).write(BsonDocumentWriter(document), codec) + dsl(codec).apply(block).simplifyAndWrite(BsonDocumentWriter(document), codec) return document.toJson() } diff --git a/dsl/src/test/kotlin/expr/FilterExpressionTest.kt b/dsl/src/test/kotlin/expr/FilterExpressionTest.kt index e5a57262..69640022 100644 --- a/dsl/src/test/kotlin/expr/FilterExpressionTest.kt +++ b/dsl/src/test/kotlin/expr/FilterExpressionTest.kt @@ -177,6 +177,28 @@ class FilterExpressionTest : FunSpec({ """.trimIndent() } + test("An automatic $and is generated when multiple filters are given") { + filter { // same example as the previous, but we didn't write the '$and' + User::name eq "foo" + User::age eq null + } shouldBeBson """ + { + "$and": [ + { + "name": { + "$eq": "foo" + } + }, + { + "age": { + "$eq": null + } + } + ] + } + """.trimIndent() + } + test("Or") { filter { or { -- 2.51.2 From 7da0aad3d53c064b07bda1965475170fa078e1ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Fri, 10 May 2024 14:38:14 +0200 Subject: [PATCH 04/18] refactor(dsl): Simplify expressions and codec management --- driver-sync/src/main/kotlin/Count.kt | 8 +- driver-sync/src/main/kotlin/Find.kt | 8 +- dsl/src/main/kotlin/DocumentWriter.kt | 6 +- dsl/src/main/kotlin/expr/Expression.kt | 139 ------------------ dsl/src/main/kotlin/expr/FilterExpression.kt | 44 +++--- .../main/kotlin/expr/PredicateExpression.kt | 27 ++-- .../kotlin/expr/common/CompoundExpression.kt | 105 +++++++++++++ .../kotlin/expr/common/EmptyExpression.kt | 13 ++ dsl/src/main/kotlin/expr/common/Expression.kt | 86 +++++++++++ .../test/kotlin/expr/ExpressionTestUtils.kt | 21 --- .../test/kotlin/expr/FilterExpressionTest.kt | 2 +- .../kotlin/expr/PredicateExpressionTest.kt | 2 +- 12 files changed, 258 insertions(+), 203 deletions(-) delete mode 100644 dsl/src/main/kotlin/expr/Expression.kt create mode 100644 dsl/src/main/kotlin/expr/common/CompoundExpression.kt create mode 100644 dsl/src/main/kotlin/expr/common/EmptyExpression.kt create mode 100644 dsl/src/main/kotlin/expr/common/Expression.kt diff --git a/driver-sync/src/main/kotlin/Count.kt b/driver-sync/src/main/kotlin/Count.kt index 6c6603f3..0e7a8354 100644 --- a/driver-sync/src/main/kotlin/Count.kt +++ b/driver-sync/src/main/kotlin/Count.kt @@ -30,9 +30,11 @@ fun MongoCollection.countDocuments(): Long { fun MongoCollection.countDocuments(predicate: FilterExpression.() -> Unit): Long { val bson = BsonDocument() - FilterExpression(unsafe.codecRegistry) - .apply(predicate) - .simplifyAndWrite(BsonDocumentWriter(bson), unsafe.codecRegistry) + BsonDocumentWriter(bson).use { writer -> + FilterExpression(unsafe.codecRegistry) + .apply(predicate) + .writeTo(writer) + } return unsafe.countDocuments(filter = bson) } diff --git a/driver-sync/src/main/kotlin/Find.kt b/driver-sync/src/main/kotlin/Find.kt index 0480da1d..7ee05c9a 100644 --- a/driver-sync/src/main/kotlin/Find.kt +++ b/driver-sync/src/main/kotlin/Find.kt @@ -45,9 +45,11 @@ fun MongoCollection.find(): FindIterable { fun MongoCollection.find(predicate: FilterExpression.() -> Unit): FindIterable { val bson = BsonDocument() - FilterExpression(unsafe.codecRegistry) - .apply(predicate) - .simplifyAndWrite(BsonDocumentWriter(bson), unsafe.codecRegistry) + BsonDocumentWriter(bson).use { writer -> + FilterExpression(unsafe.codecRegistry) + .apply(predicate) + .writeTo(writer) + } return unsafe.find(bson.asDocument()) } diff --git a/dsl/src/main/kotlin/DocumentWriter.kt b/dsl/src/main/kotlin/DocumentWriter.kt index 38dab93e..aae34a4b 100644 --- a/dsl/src/main/kotlin/DocumentWriter.kt +++ b/dsl/src/main/kotlin/DocumentWriter.kt @@ -1,6 +1,6 @@ package fr.qsh.ktmongo.dsl -import org.bson.BsonDocumentWriter +import org.bson.AbstractBsonWriter /** * Helper to start a document, ensuring it is closed. @@ -9,7 +9,7 @@ import org.bson.BsonDocumentWriter */ @LowLevelApi @PublishedApi -internal inline fun BsonDocumentWriter.buildDocument(name: String? = null, block: () -> Unit) { +internal inline fun AbstractBsonWriter.buildDocument(name: String? = null, block: () -> Unit) { try { writeStartDocument() name?.let(::writeName) @@ -24,7 +24,7 @@ internal inline fun BsonDocumentWriter.buildDocument(name: String? = null, block */ @LowLevelApi @PublishedApi -internal inline fun BsonDocumentWriter.buildArray(block: () -> Unit) { +internal inline fun AbstractBsonWriter.buildArray(block: () -> Unit) { try { writeStartArray() block() diff --git a/dsl/src/main/kotlin/expr/Expression.kt b/dsl/src/main/kotlin/expr/Expression.kt deleted file mode 100644 index 3ecba513..00000000 --- a/dsl/src/main/kotlin/expr/Expression.kt +++ /dev/null @@ -1,139 +0,0 @@ -package fr.qsh.ktmongo.dsl.expr - -import fr.qsh.ktmongo.dsl.KtMongoDsl -import fr.qsh.ktmongo.dsl.LowLevelApi -import org.bson.BsonDocument -import org.bson.BsonDocumentWriter -import org.bson.codecs.configuration.CodecRegistry - -/** - * A compound node in the BSON AST. - * - * Compared to regular [ExpressionNode], `Expression` adds [accept], which allows to inject sub-expressions - * into the current expression. - * - * Subclasses of this interface provide DSLs to create BSON expressions. - * - * ### Implementation notes - * - * Much like [ExpressionNode], implementations of this interface **must** implement [toString] to print the generated - * BSON used by the request. We recommend implementing [AbstractExpression], which does this automatically. - */ -@OptIn(LowLevelApi::class) -@KtMongoDsl -interface Expression : ExpressionNode { - - /** - * Adds an arbitrary [node] to this expression. - * - * ### Security and correctness - * - * This function makes no verification on the validity of the passed node. - * It is added to this expression as-is. - * - * This function is only publicly available to allow users to add missing operators themselves, by implementing - * [ExpressionNode]. Only implement operators yourself if you are sure of what you are doing! - */ - @LowLevelApi - @KtMongoDsl - fun accept(node: ExpressionNode) -} - -@LowLevelApi -@KtMongoDsl -fun Expression.acceptAll(nodes: Iterable) { - for (node in nodes) - accept(node) -} - -/** - * A node in the BSON AST. - * - * Each node knows how to [write] itself into the expression. - * - * ### Security - * - * Implementing this interface allows to inject arbitrary BSON into a request. - * Be very careful not to allow request injections. - * - * ### Implementation notes - * - * To facilitate debugging, **all** implementations of [ExpressionNode] must implement [toString] to print the - * predicted BSON used by the request. - * We recommend implementing [AbstractExpressionNode], which does this automatically. - */ -@LowLevelApi -interface ExpressionNode { - - /** - * Writes the current node into the BSON AST, represented by the [writer]. - */ - @LowLevelApi - fun write(writer: BsonDocumentWriter, codec: CodecRegistry) - - /** - * Executes simplification against the current node. - * - * By default, no simplifications are executed and this object is returned as-is. - */ - @LowLevelApi - fun simplify(codec: CodecRegistry): ExpressionNode = this - - @LowLevelApi - fun simplifyAndWrite(writer: BsonDocumentWriter, codec: CodecRegistry) = - simplify(codec).write(writer, codec) - - object EmptyExpressionNode : ExpressionNode { - override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) {} - } -} - -/** - * Helper to implement [Expression] that handles [toString] generation and the implementation of [accept]. - */ -@OptIn(LowLevelApi::class) -abstract class AbstractExpression( - codec: CodecRegistry, -) : AbstractExpressionNode(codec), Expression { - - @OptIn(LowLevelApi::class) - private val children = ArrayList() - - @LowLevelApi - override fun accept(node: ExpressionNode) { - children += node - } - - @LowLevelApi - protected open fun simplify(codec: CodecRegistry, children: List): ExpressionNode = this - - final override fun simplify(codec: CodecRegistry): ExpressionNode = - simplify(codec, children) - - @LowLevelApi - override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { - for (child in children) { - child.write(writer, codec) - } - } -} - -/** - * Helper to implement [ExpressionNode] that handles [toString] generation. - */ -@LowLevelApi -abstract class AbstractExpressionNode( - private val codec: CodecRegistry, -) : ExpressionNode { - - override fun toString(): String { - val document = BsonDocument() - - @OptIn(LowLevelApi::class) - BsonDocumentWriter(document).use { - simplifyAndWrite(it, codec) - } - - return document.toJson() - } -} diff --git a/dsl/src/main/kotlin/expr/FilterExpression.kt b/dsl/src/main/kotlin/expr/FilterExpression.kt index e7a2985b..6da743f3 100644 --- a/dsl/src/main/kotlin/expr/FilterExpression.kt +++ b/dsl/src/main/kotlin/expr/FilterExpression.kt @@ -4,9 +4,11 @@ import fr.qsh.ktmongo.dsl.KtMongoDsl import fr.qsh.ktmongo.dsl.LowLevelApi import fr.qsh.ktmongo.dsl.buildArray import fr.qsh.ktmongo.dsl.buildDocument -import fr.qsh.ktmongo.dsl.expr.ExpressionNode.EmptyExpressionNode +import fr.qsh.ktmongo.dsl.expr.common.CompoundExpression +import fr.qsh.ktmongo.dsl.expr.common.Expression +import fr.qsh.ktmongo.dsl.expr.common.empty import fr.qsh.ktmongo.dsl.path.path -import org.bson.BsonDocumentWriter +import org.bson.AbstractBsonWriter import org.bson.BsonType import org.bson.codecs.configuration.CodecRegistry import javax.management.Query.and @@ -20,27 +22,26 @@ import kotlin.reflect.KProperty1 */ @KtMongoDsl class FilterExpression( - @property:LowLevelApi - val codec: CodecRegistry, -) : AbstractExpression(codec) { + codec: CodecRegistry, +) : CompoundExpression(codec) { // region Low-level operations @LowLevelApi - override fun simplify(codec: CodecRegistry, children: List): ExpressionNode { - return when (children.size) { - 0 -> EmptyExpressionNode + override fun simplify(children: List): Expression = + when (children.size) { + 0 -> Expression.empty(codec) 1 -> this - else -> FilterExpression(codec).apply { - and { - acceptAll(children) - } - } + // else -> { + // val expression = FilterExpression(codec) + // expression.and { acceptAll(children) } + // expression + // } + else -> this } - } @LowLevelApi - private sealed class FilterExpressionNode(codec: CodecRegistry) : AbstractExpressionNode(codec) + private sealed class FilterExpressionNode(codec: CodecRegistry) : Expression(codec) // endregion @@ -82,10 +83,10 @@ class FilterExpression( codec: CodecRegistry, ) : FilterExpressionNode(codec) { - override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + override fun write(writer: AbstractBsonWriter) { writer.buildDocument("\$and") { writer.buildArray { - expression.write(writer, codec) + expression.writeTo(writer) } } } @@ -130,10 +131,10 @@ class FilterExpression( codec: CodecRegistry, ) : FilterExpressionNode(codec) { - override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + override fun write(writer: AbstractBsonWriter) { writer.buildDocument("\$or") { writer.buildArray { - expression.write(writer, codec) + expression.writeTo(writer) } } } @@ -178,9 +179,10 @@ class FilterExpression( val expression: PredicateExpression<*>, codec: CodecRegistry, ) : FilterExpressionNode(codec) { - override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + + override fun write(writer: AbstractBsonWriter) { writer.buildDocument(target) { - expression.write(writer, codec) + expression.writeTo(writer) } } } diff --git a/dsl/src/main/kotlin/expr/PredicateExpression.kt b/dsl/src/main/kotlin/expr/PredicateExpression.kt index b4c21683..0b2197ab 100644 --- a/dsl/src/main/kotlin/expr/PredicateExpression.kt +++ b/dsl/src/main/kotlin/expr/PredicateExpression.kt @@ -3,7 +3,9 @@ package fr.qsh.ktmongo.dsl.expr import fr.qsh.ktmongo.dsl.KtMongoDsl import fr.qsh.ktmongo.dsl.LowLevelApi import fr.qsh.ktmongo.dsl.buildDocument -import org.bson.BsonDocumentWriter +import fr.qsh.ktmongo.dsl.expr.common.CompoundExpression +import fr.qsh.ktmongo.dsl.expr.common.Expression +import org.bson.AbstractBsonWriter import org.bson.BsonType import org.bson.codecs.Encoder import org.bson.codecs.EncoderContext @@ -15,14 +17,13 @@ import org.bson.codecs.configuration.CodecRegistry */ @KtMongoDsl class PredicateExpression( - @property:LowLevelApi - val codec: CodecRegistry, -) : AbstractExpression(codec) { + codec: CodecRegistry, +) : CompoundExpression(codec) { // region Low-level operations @LowLevelApi - private sealed class PredicateExpressionNode(codec: CodecRegistry) : AbstractExpressionNode(codec) + private sealed class PredicateExpressionNode(codec: CodecRegistry) : Expression(codec) // endregion @@ -61,7 +62,8 @@ class PredicateExpression( val value: T, codec: CodecRegistry, ) : PredicateExpressionNode(codec) { - override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + + override fun write(writer: AbstractBsonWriter) { writer.buildDocument("\$eq") { if (value == null) { writer.writeNull() @@ -150,7 +152,8 @@ class PredicateExpression( val exists: Boolean, codec: CodecRegistry, ) : PredicateExpressionNode(codec) { - override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + + override fun write(writer: AbstractBsonWriter) { writer.buildDocument("\$exists") { writer.writeBoolean(exists) } @@ -230,9 +233,10 @@ class PredicateExpression( val type: BsonType, codec: CodecRegistry, ) : PredicateExpressionNode(codec) { - override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + + override fun write(writer: AbstractBsonWriter) { writer.buildDocument("\$type") { - writer.writeInt32(this.type.value) + writer.writeInt32(type.value) } } } @@ -276,9 +280,10 @@ class PredicateExpression( val expression: PredicateExpression, codec: CodecRegistry, ) : PredicateExpressionNode(codec) { - override fun write(writer: BsonDocumentWriter, codec: CodecRegistry) { + + override fun write(writer: AbstractBsonWriter) { writer.buildDocument("\$not") { - expression.write(writer, codec) + expression.writeTo(writer) } } } diff --git a/dsl/src/main/kotlin/expr/common/CompoundExpression.kt b/dsl/src/main/kotlin/expr/common/CompoundExpression.kt new file mode 100644 index 00000000..8240cff7 --- /dev/null +++ b/dsl/src/main/kotlin/expr/common/CompoundExpression.kt @@ -0,0 +1,105 @@ +package fr.qsh.ktmongo.dsl.expr.common + +import fr.qsh.ktmongo.dsl.KtMongoDsl +import fr.qsh.ktmongo.dsl.LowLevelApi +import org.bson.AbstractBsonWriter +import org.bson.codecs.configuration.CodecRegistry + +/** + * A compound node in the BSON AST. + * This class is an implementation detail of all operator DSLs. + * + * This class adds the method [accept] which allows binding a child expression + * into the current one. + * It manages the bound expressions internally, only giving the implementations + * access to them when [simplify] or [write] are called. + * + * @see Expression + */ +abstract class CompoundExpression( + codec: CodecRegistry, +) : Expression(codec) { + + // region Sub-expression binding + + private val children = ArrayList() + + /** + * Binds an arbitrary [expression] as a sub-expression of the receiver. + * + * ### Security and correctness + * + * This function makes no verification on the validity of the passed expression. + * It is added to this expression as-is. + * + * This function is only publicly available to allow users to add missing operators themselves + * by implementing [Expression] for their operator. + * + * **An incorrectly written expression may allow arbitrary code execution on the database, + * data corruption, or data leaks. Only call this function on expressions you are sure + * are implemented correctly!** + */ + @LowLevelApi + @KtMongoDsl + fun accept(expression: Expression) { + // println("Adding child expression ${expression.toString(simplified = false)}") //TODO remove + // RuntimeException().printStackTrace() + + children += expression + } + + // endregion + // region Simplifications + + /** + * See [Expression.simplify]. + * + * @param children The list of expressions that have been [bound][accept] into this + * expression. + */ + @LowLevelApi + protected open fun simplify(children: List): Expression = + this + + @LowLevelApi + final override fun simplify(): Expression = + simplify(children) + + // endregion + // region Writing + + /** + * See [Expression.write]. + * + * @param children The list of expressions that have been [bound][accept] into this + * expression. + */ + @LowLevelApi + protected open fun write(writer: AbstractBsonWriter, children: List) { + for (child in children) { + require(this !== child) { "Trying to write myself as my own child!" } + child.writeTo(writer) + } + } + + @LowLevelApi + final override fun write(writer: AbstractBsonWriter) { + write(writer, children) + } + + // endregion + + companion object +} + +/** + * Binds any arbitrary [expressions] as sub-expressions of the receiver. + * + * To learn more about the security implications, see [CompoundExpression.accept]. + */ +@LowLevelApi +@KtMongoDsl +fun CompoundExpression.acceptAll(expressions: Iterable) { + for (child in expressions) + accept(child) +} diff --git a/dsl/src/main/kotlin/expr/common/EmptyExpression.kt b/dsl/src/main/kotlin/expr/common/EmptyExpression.kt new file mode 100644 index 00000000..70a18fe1 --- /dev/null +++ b/dsl/src/main/kotlin/expr/common/EmptyExpression.kt @@ -0,0 +1,13 @@ +package fr.qsh.ktmongo.dsl.expr.common + +import fr.qsh.ktmongo.dsl.LowLevelApi +import org.bson.AbstractBsonWriter +import org.bson.codecs.configuration.CodecRegistry + +private class EmptyExpression(codec: CodecRegistry) : Expression(codec) { + @LowLevelApi + override fun write(writer: AbstractBsonWriter) {} +} + +fun Expression.Companion.empty(codec: CodecRegistry): Expression = + EmptyExpression(codec) diff --git a/dsl/src/main/kotlin/expr/common/Expression.kt b/dsl/src/main/kotlin/expr/common/Expression.kt new file mode 100644 index 00000000..a74ea778 --- /dev/null +++ b/dsl/src/main/kotlin/expr/common/Expression.kt @@ -0,0 +1,86 @@ +package fr.qsh.ktmongo.dsl.expr.common + +import fr.qsh.ktmongo.dsl.LowLevelApi +import org.bson.AbstractBsonWriter +import org.bson.BsonDocument +import org.bson.BsonDocumentWriter +import org.bson.codecs.configuration.CodecRegistry + +/** + * A node in the BSON AST. + * + * Each node knows how to [writeTo] itself into the expression. + * + * ### Security + * + * Implementing this interface allows to inject arbitrary BSON into a request. + * Be very careful not to allow request injections. + * + * ### Debugging notes + * + * Use [toString] to generate the JSON of this expression. + */ +abstract class Expression( + protected val codec: CodecRegistry, +) { + + /** + * Writes this expression into [writer] **exactly as it is described**. + * + * This function is not allowed to edit the expression in any way, + * in particular, simplifying it is not allowed. + * To modify the expression before writing it, implement [simplify]. + * + * **Implementations must be pure.** + */ + @LowLevelApi + protected abstract fun write(writer: AbstractBsonWriter) + + /** + * Allows the implementation to replace itself by another more appropriate representation. + * + * For example, if the current node is an `$and` operator with a single child, + * it may use this function to replace itself by that child. + * + * **Implementations must be pure.** + */ + @LowLevelApi + protected open fun simplify(): Expression = this + + /** + * Writes this expression into a [writer]. + * + * This function is guaranteed to be pure. + */ + @LowLevelApi + fun writeTo(writer: AbstractBsonWriter) { + this.simplify().write(writer) + } + + /** + * Returns a JSON representation of this node. + * + * If [simplified] is `true`, [simplifications][simplify] are executed before printing. + */ + fun toString(simplified: Boolean): String { + val document = BsonDocument() + + @OptIn(LowLevelApi::class) + BsonDocumentWriter(document).use { + if (simplified) + writeTo(it) + else + write(it) + } + + return document.toString() + } + + /** + * Returns a JSON representation of this node, generated using [writeTo]. + */ + final override fun toString(): String = + "NO STRING" + + companion object +} diff --git a/dsl/src/test/kotlin/expr/ExpressionTestUtils.kt b/dsl/src/test/kotlin/expr/ExpressionTestUtils.kt index 2324e3b8..c5a45847 100644 --- a/dsl/src/test/kotlin/expr/ExpressionTestUtils.kt +++ b/dsl/src/test/kotlin/expr/ExpressionTestUtils.kt @@ -1,9 +1,6 @@ package fr.qsh.ktmongo.dsl.expr -import fr.qsh.ktmongo.dsl.LowLevelApi import io.kotest.matchers.shouldBe -import org.bson.BsonDocument -import org.bson.BsonDocumentWriter import org.bson.codecs.* import org.bson.codecs.configuration.CodecRegistries import org.bson.codecs.configuration.CodecRegistry @@ -68,24 +65,6 @@ fun testCodec(): CodecRegistry = CodecRegistries.fromCodecs( UuidCodec(), ) -fun buildExpression(dsl: (BsonDocumentWriter, CodecRegistry) -> T, block: T.() -> Unit): String { - val document = BsonDocument() - - dsl(BsonDocumentWriter(document), testCodec()).apply(block) - - return document.toJson() -} - -@OptIn(LowLevelApi::class) -fun buildExpression(dsl: (CodecRegistry) -> E, block: E.() -> Unit): String { - val document = BsonDocument() - - val codec = testCodec() - dsl(codec).apply(block).simplifyAndWrite(BsonDocumentWriter(document), codec) - - return document.toJson() -} - infix fun String.shouldBeBson(expected: String) { this shouldBe expected .replace("\n", "") diff --git a/dsl/src/test/kotlin/expr/FilterExpressionTest.kt b/dsl/src/test/kotlin/expr/FilterExpressionTest.kt index 69640022..3142ac66 100644 --- a/dsl/src/test/kotlin/expr/FilterExpressionTest.kt +++ b/dsl/src/test/kotlin/expr/FilterExpressionTest.kt @@ -13,7 +13,7 @@ class FilterExpressionTest : FunSpec({ ) fun filter(block: FilterExpression.() -> Unit): String = - buildExpression(::FilterExpression, block) + FilterExpression(testCodec()).apply(block).toString(simplified = true) val eq = "\$eq" val and = "\$and" diff --git a/dsl/src/test/kotlin/expr/PredicateExpressionTest.kt b/dsl/src/test/kotlin/expr/PredicateExpressionTest.kt index ecc38948..3d34178a 100644 --- a/dsl/src/test/kotlin/expr/PredicateExpressionTest.kt +++ b/dsl/src/test/kotlin/expr/PredicateExpressionTest.kt @@ -6,7 +6,7 @@ import io.kotest.core.spec.style.FunSpec class PredicateExpressionTest : FunSpec({ fun predicate(block: PredicateExpression.() -> Unit): String = - buildExpression(::PredicateExpression, block) + PredicateExpression(testCodec()).apply(block).toString(simplified = true) val eq = "\$eq" -- 2.51.2 From 9790bf46bca9bbbfca4ff11874846d56c087a8d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Fri, 10 May 2024 17:03:37 +0200 Subject: [PATCH 05/18] refactor(dsl): Add more information to crashes of the BsonDocumentWriter state machine --- dsl/src/main/kotlin/DocumentWriter.kt | 38 +-- dsl/src/main/kotlin/expr/FilterExpression.kt | 36 ++- .../main/kotlin/expr/PredicateExpression.kt | 31 +- .../kotlin/expr/common/BsonWriterTracer.kt | 288 ++++++++++++++++++ .../kotlin/expr/common/CompoundExpression.kt | 6 +- .../kotlin/expr/common/EmptyExpression.kt | 4 +- dsl/src/main/kotlin/expr/common/Expression.kt | 21 +- .../kotlin/expr/PredicateExpressionTest.kt | 62 ++++ 8 files changed, 423 insertions(+), 63 deletions(-) create mode 100644 dsl/src/main/kotlin/expr/common/BsonWriterTracer.kt diff --git a/dsl/src/main/kotlin/DocumentWriter.kt b/dsl/src/main/kotlin/DocumentWriter.kt index aae34a4b..83ef6cff 100644 --- a/dsl/src/main/kotlin/DocumentWriter.kt +++ b/dsl/src/main/kotlin/DocumentWriter.kt @@ -1,22 +1,27 @@ package fr.qsh.ktmongo.dsl -import org.bson.AbstractBsonWriter +import org.bson.BsonWriter /** * Helper to start a document, ensuring it is closed. - * - * If [name] is not null, it is set as the document name. */ @LowLevelApi @PublishedApi -internal inline fun AbstractBsonWriter.buildDocument(name: String? = null, block: () -> Unit) { - try { - writeStartDocument() - name?.let(::writeName) - block() - } finally { - writeEndDocument() - } +internal inline fun BsonWriter.writeDocument(name: String, block: () -> Unit) { + writeStartDocument(name) + block() + writeEndDocument() +} + +/** + * Helper to start a document, ensuring it is closed. + */ +@LowLevelApi +@PublishedApi +internal inline fun BsonWriter.writeDocument(block: () -> Unit) { + writeStartDocument() + block() + writeEndDocument() } /** @@ -24,11 +29,8 @@ internal inline fun AbstractBsonWriter.buildDocument(name: String? = null, block */ @LowLevelApi @PublishedApi -internal inline fun AbstractBsonWriter.buildArray(block: () -> Unit) { - try { - writeStartArray() - block() - } finally { - writeEndArray() - } +internal inline fun BsonWriter.writeArray(block: () -> Unit) { + writeStartArray() + block() + writeEndArray() } diff --git a/dsl/src/main/kotlin/expr/FilterExpression.kt b/dsl/src/main/kotlin/expr/FilterExpression.kt index 6da743f3..3144c7b5 100644 --- a/dsl/src/main/kotlin/expr/FilterExpression.kt +++ b/dsl/src/main/kotlin/expr/FilterExpression.kt @@ -2,14 +2,14 @@ package fr.qsh.ktmongo.dsl.expr import fr.qsh.ktmongo.dsl.KtMongoDsl import fr.qsh.ktmongo.dsl.LowLevelApi -import fr.qsh.ktmongo.dsl.buildArray -import fr.qsh.ktmongo.dsl.buildDocument import fr.qsh.ktmongo.dsl.expr.common.CompoundExpression import fr.qsh.ktmongo.dsl.expr.common.Expression import fr.qsh.ktmongo.dsl.expr.common.empty import fr.qsh.ktmongo.dsl.path.path -import org.bson.AbstractBsonWriter +import fr.qsh.ktmongo.dsl.writeArray +import fr.qsh.ktmongo.dsl.writeDocument import org.bson.BsonType +import org.bson.BsonWriter import org.bson.codecs.configuration.CodecRegistry import javax.management.Query.and import kotlin.internal.OnlyInputTypes @@ -32,11 +32,12 @@ class FilterExpression( when (children.size) { 0 -> Expression.empty(codec) 1 -> this - // else -> { - // val expression = FilterExpression(codec) - // expression.and { acceptAll(children) } - // expression - // } + // else -> AndFilterExpressionNode( + // FilterExpression(codec).apply { + // acceptAll(children) + // }, + // codec, + // ) else -> this } @@ -83,9 +84,10 @@ class FilterExpression( codec: CodecRegistry, ) : FilterExpressionNode(codec) { - override fun write(writer: AbstractBsonWriter) { - writer.buildDocument("\$and") { - writer.buildArray { + override fun write(writer: BsonWriter) { + writer.writeDocument { + writer.writeName("\$and") + writer.writeArray { expression.writeTo(writer) } } @@ -131,9 +133,10 @@ class FilterExpression( codec: CodecRegistry, ) : FilterExpressionNode(codec) { - override fun write(writer: AbstractBsonWriter) { - writer.buildDocument("\$or") { - writer.buildArray { + override fun write(writer: BsonWriter) { + writer.writeDocument { + writer.writeName("\$or") + writer.writeArray { expression.writeTo(writer) } } @@ -180,8 +183,9 @@ class FilterExpression( codec: CodecRegistry, ) : FilterExpressionNode(codec) { - override fun write(writer: AbstractBsonWriter) { - writer.buildDocument(target) { + override fun write(writer: BsonWriter) { + writer.writeDocument { + writer.writeName(target) expression.writeTo(writer) } } diff --git a/dsl/src/main/kotlin/expr/PredicateExpression.kt b/dsl/src/main/kotlin/expr/PredicateExpression.kt index 0b2197ab..5d8698b5 100644 --- a/dsl/src/main/kotlin/expr/PredicateExpression.kt +++ b/dsl/src/main/kotlin/expr/PredicateExpression.kt @@ -2,11 +2,11 @@ package fr.qsh.ktmongo.dsl.expr import fr.qsh.ktmongo.dsl.KtMongoDsl import fr.qsh.ktmongo.dsl.LowLevelApi -import fr.qsh.ktmongo.dsl.buildDocument import fr.qsh.ktmongo.dsl.expr.common.CompoundExpression import fr.qsh.ktmongo.dsl.expr.common.Expression -import org.bson.AbstractBsonWriter +import fr.qsh.ktmongo.dsl.writeDocument import org.bson.BsonType +import org.bson.BsonWriter import org.bson.codecs.Encoder import org.bson.codecs.EncoderContext import org.bson.codecs.configuration.CodecRegistry @@ -63,11 +63,12 @@ class PredicateExpression( codec: CodecRegistry, ) : PredicateExpressionNode(codec) { - override fun write(writer: AbstractBsonWriter) { - writer.buildDocument("\$eq") { - if (value == null) { - writer.writeNull() - } else { + override fun write(writer: BsonWriter) { + writer.writeDocument { + if (value == null) + writer.writeNull("\$eq") + else { + writer.writeName("\$eq") @Suppress("UNNECESSARY_NOT_NULL_ASSERTION", "UNCHECKED_CAST") // Kotlin doesn't smart-cast here, but should, this is safe (codec.get(value!!::class.java) as Encoder) .encode(writer, value, EncoderContext.builder().build()) @@ -153,8 +154,9 @@ class PredicateExpression( codec: CodecRegistry, ) : PredicateExpressionNode(codec) { - override fun write(writer: AbstractBsonWriter) { - writer.buildDocument("\$exists") { + override fun write(writer: BsonWriter) { + writer.writeDocument { + writer.writeName("\$exists") writer.writeBoolean(exists) } } @@ -234,9 +236,9 @@ class PredicateExpression( codec: CodecRegistry, ) : PredicateExpressionNode(codec) { - override fun write(writer: AbstractBsonWriter) { - writer.buildDocument("\$type") { - writer.writeInt32(type.value) + override fun write(writer: BsonWriter) { + writer.writeDocument { + writer.writeInt32("\$type", type.value) } } } @@ -281,8 +283,9 @@ class PredicateExpression( codec: CodecRegistry, ) : PredicateExpressionNode(codec) { - override fun write(writer: AbstractBsonWriter) { - writer.buildDocument("\$not") { + override fun write(writer: BsonWriter) { + writer.writeDocument { + writer.writeName("\$not") expression.writeTo(writer) } } diff --git a/dsl/src/main/kotlin/expr/common/BsonWriterTracer.kt b/dsl/src/main/kotlin/expr/common/BsonWriterTracer.kt new file mode 100644 index 00000000..844c46fe --- /dev/null +++ b/dsl/src/main/kotlin/expr/common/BsonWriterTracer.kt @@ -0,0 +1,288 @@ +package fr.qsh.ktmongo.dsl.expr.common + +import org.bson.* +import org.bson.types.Decimal128 +import org.bson.types.ObjectId + +private class LoggingBsonWriter( + private val upstream: BsonWriter, +) : BsonWriter { + + // region Machinery + + private var indent = 0 + private val loggingBuffer = ArrayList() + + private fun addLine(text: String) { + loggingBuffer += buildString { + repeat(indent) { append('\t') } + append(text) + } + } + + private inline fun logStateOnException(block: () -> T) { + try { + block() + } catch (e: BsonInvalidOperationException) { + throw BsonInvalidOperationException("An error occurred while writing the BSON expression.\n$this ^ the exception happened while trying to write this line.", e) + } + } + + override fun toString() = buildString { + appendLine(" *** Expression writer ***") + appendLine("This is a debugger helper. The data represented here is a pseudo-representation of the current state of the writer. This isn't the real contents of the writer.") + + for (line in loggingBuffer) { + appendLine(line) + } + } + + // endregion + // region Methods + + override fun flush() = logStateOnException { + upstream.flush() + } + + override fun writeBinaryData(binary: BsonBinary?) = logStateOnException { + addLine("$binary (binary data)") + upstream.writeBinaryData(binary) + } + + override fun writeBinaryData(name: String?, binary: BsonBinary?) = logStateOnException { + addLine("$name: $binary (binary data)") + upstream.writeBinaryData(name, binary) + } + + override fun writeBoolean(value: Boolean) = logStateOnException { + addLine("$value (boolean)") + upstream.writeBoolean(value) + } + + override fun writeBoolean(name: String?, value: Boolean) = logStateOnException { + addLine("$name: $value (boolean)") + upstream.writeBoolean(name, value) + } + + override fun writeDateTime(value: Long) = logStateOnException { + addLine("$value (date time)") + upstream.writeDateTime(value) + } + + override fun writeDateTime(name: String?, value: Long) = logStateOnException { + addLine("$name: $value (date time)") + upstream.writeDateTime(name, value) + } + + override fun writeDBPointer(value: BsonDbPointer?) = logStateOnException { + addLine("$value (db pointer)") + upstream.writeDBPointer(value) + } + + override fun writeDBPointer(name: String?, value: BsonDbPointer?) = logStateOnException { + addLine("$name: $value (db pointer)") + upstream.writeDBPointer(name, value) + } + + override fun writeDouble(value: Double) = logStateOnException { + addLine("$value (double)") + upstream.writeDouble(value) + } + + override fun writeDouble(name: String?, value: Double) = logStateOnException { + addLine("$name: $value (double)") + upstream.writeDouble(name, value) + } + + override fun writeEndArray() = logStateOnException { + indent-- + addLine("]") + upstream.writeEndArray() + } + + override fun writeEndDocument() = logStateOnException { + indent-- + addLine("}") + upstream.writeEndDocument() + } + + override fun writeInt32(value: Int) = logStateOnException { + addLine("$value (int32)") + upstream.writeInt32(value) + } + + override fun writeInt32(name: String?, value: Int) = logStateOnException { + addLine("$name: $value (int32)") + upstream.writeInt32(name, value) + } + + override fun writeInt64(value: Long) = logStateOnException { + addLine("$value (int64)") + upstream.writeInt64(value) + } + + override fun writeInt64(name: String?, value: Long) = logStateOnException { + addLine("$name: $value (int64)") + upstream.writeInt64(name, value) + } + + override fun writeDecimal128(value: Decimal128?) = logStateOnException { + addLine("$value (decimal128)") + upstream.writeDecimal128(value) + } + + override fun writeDecimal128(name: String?, value: Decimal128?) = logStateOnException { + addLine("$name: $value (decimal128)") + upstream.writeDecimal128(name, value) + } + + override fun writeJavaScript(code: String?) = logStateOnException { + addLine("$code (JS)") + upstream.writeJavaScript(code) + } + + override fun writeJavaScript(name: String?, code: String?) = logStateOnException { + addLine("$name: $code (JS)") + upstream.writeJavaScript(name, code) + } + + override fun writeJavaScriptWithScope(code: String?) = logStateOnException { + addLine("$code (JS with scope)") + upstream.writeJavaScriptWithScope(code) + } + + override fun writeJavaScriptWithScope(name: String?, code: String?) = logStateOnException { + addLine("$name: $code (JS with scope)") + upstream.writeJavaScriptWithScope(name, code) + } + + override fun writeMaxKey() = logStateOnException { + addLine("(max key)") + upstream.writeMaxKey() + } + + override fun writeMaxKey(name: String?) = logStateOnException { + addLine("$name: (max key)") + upstream.writeMaxKey(name) + } + + override fun writeMinKey() = logStateOnException { + addLine("(min key)") + upstream.writeMinKey() + } + + override fun writeMinKey(name: String?) = logStateOnException { + addLine("(min key)") + upstream.writeMinKey(name) + } + + override fun writeName(name: String?) = logStateOnException { + addLine("$name:") + upstream.writeName(name) + } + + override fun writeNull() = logStateOnException { + addLine("null") + upstream.writeNull() + } + + override fun writeNull(name: String?) = logStateOnException { + addLine("$name: null") + upstream.writeNull(name) + } + + override fun writeObjectId(objectId: ObjectId?) = logStateOnException { + addLine("$objectId (ObjectID)") + upstream.writeObjectId(objectId) + } + + override fun writeObjectId(name: String?, objectId: ObjectId?) = logStateOnException { + addLine("$name: $objectId (ObjectId)") + upstream.writeObjectId(name, objectId) + } + + override fun writeRegularExpression(regularExpression: BsonRegularExpression?) = logStateOnException { + addLine("$regularExpression (RegExp)") + upstream.writeRegularExpression(regularExpression) + } + + override fun writeRegularExpression(name: String?, regularExpression: BsonRegularExpression?) = logStateOnException { + addLine("$name: $regularExpression (RegExp)") + upstream.writeRegularExpression(name, regularExpression) + } + + override fun writeStartArray() = logStateOnException { + addLine("[") + indent++ + upstream.writeStartArray() + } + + override fun writeStartArray(name: String?) = logStateOnException { + addLine("$name: [") + indent++ + upstream.writeStartArray(name) + } + + override fun writeStartDocument() = logStateOnException{ + addLine("{") + indent++ + upstream.writeStartDocument() + } + + override fun writeStartDocument(name: String?) = logStateOnException { + addLine("$name: {") + indent++ + upstream.writeStartDocument(name) + } + + override fun writeString(value: String?) = logStateOnException { + addLine("$value (string)") + upstream.writeString(value) + } + + override fun writeString(name: String?, value: String?) = logStateOnException { + addLine("$name: $value (string)") + upstream.writeString(name, value) + } + + override fun writeSymbol(value: String?) = logStateOnException { + addLine("$value (symbol)") + upstream.writeSymbol(value) + } + + override fun writeSymbol(name: String?, value: String?) = logStateOnException { + addLine("$name: $value (symbol)") + upstream.writeSymbol(name, value) + } + + override fun writeTimestamp(value: BsonTimestamp?) = logStateOnException { + addLine("$value (timestamp)") + upstream.writeTimestamp(value) + } + + override fun writeTimestamp(name: String?, value: BsonTimestamp?) = logStateOnException { + addLine("$name: $value (timestamp)") + upstream.writeTimestamp(name, value) + } + + override fun writeUndefined() = logStateOnException { + addLine("undefined") + upstream.writeUndefined() + } + + override fun writeUndefined(name: String?) = logStateOnException { + addLine("$name: undefined") + upstream.writeUndefined(name) + } + + override fun pipe(reader: BsonReader?) = logStateOnException { + addLine("Piping a reader…") + upstream.pipe(reader) + } + + // endregion +} + +fun BsonWriter.withLoggedContext(storeLogs: Boolean = true) = + if (storeLogs) LoggingBsonWriter(this) + else this diff --git a/dsl/src/main/kotlin/expr/common/CompoundExpression.kt b/dsl/src/main/kotlin/expr/common/CompoundExpression.kt index 8240cff7..fcec37a8 100644 --- a/dsl/src/main/kotlin/expr/common/CompoundExpression.kt +++ b/dsl/src/main/kotlin/expr/common/CompoundExpression.kt @@ -2,7 +2,7 @@ package fr.qsh.ktmongo.dsl.expr.common import fr.qsh.ktmongo.dsl.KtMongoDsl import fr.qsh.ktmongo.dsl.LowLevelApi -import org.bson.AbstractBsonWriter +import org.bson.BsonWriter import org.bson.codecs.configuration.CodecRegistry /** @@ -75,7 +75,7 @@ abstract class CompoundExpression( * expression. */ @LowLevelApi - protected open fun write(writer: AbstractBsonWriter, children: List) { + protected open fun write(writer: BsonWriter, children: List) { for (child in children) { require(this !== child) { "Trying to write myself as my own child!" } child.writeTo(writer) @@ -83,7 +83,7 @@ abstract class CompoundExpression( } @LowLevelApi - final override fun write(writer: AbstractBsonWriter) { + final override fun write(writer: BsonWriter) { write(writer, children) } diff --git a/dsl/src/main/kotlin/expr/common/EmptyExpression.kt b/dsl/src/main/kotlin/expr/common/EmptyExpression.kt index 70a18fe1..2181fd85 100644 --- a/dsl/src/main/kotlin/expr/common/EmptyExpression.kt +++ b/dsl/src/main/kotlin/expr/common/EmptyExpression.kt @@ -1,12 +1,12 @@ package fr.qsh.ktmongo.dsl.expr.common import fr.qsh.ktmongo.dsl.LowLevelApi -import org.bson.AbstractBsonWriter +import org.bson.BsonWriter import org.bson.codecs.configuration.CodecRegistry private class EmptyExpression(codec: CodecRegistry) : Expression(codec) { @LowLevelApi - override fun write(writer: AbstractBsonWriter) {} + override fun write(writer: BsonWriter) {} } fun Expression.Companion.empty(codec: CodecRegistry): Expression = diff --git a/dsl/src/main/kotlin/expr/common/Expression.kt b/dsl/src/main/kotlin/expr/common/Expression.kt index a74ea778..864a3c47 100644 --- a/dsl/src/main/kotlin/expr/common/Expression.kt +++ b/dsl/src/main/kotlin/expr/common/Expression.kt @@ -1,9 +1,9 @@ package fr.qsh.ktmongo.dsl.expr.common import fr.qsh.ktmongo.dsl.LowLevelApi -import org.bson.AbstractBsonWriter import org.bson.BsonDocument import org.bson.BsonDocumentWriter +import org.bson.BsonWriter import org.bson.codecs.configuration.CodecRegistry /** @@ -34,7 +34,7 @@ abstract class Expression( * **Implementations must be pure.** */ @LowLevelApi - protected abstract fun write(writer: AbstractBsonWriter) + protected abstract fun write(writer: BsonWriter) /** * Allows the implementation to replace itself by another more appropriate representation. @@ -53,7 +53,7 @@ abstract class Expression( * This function is guaranteed to be pure. */ @LowLevelApi - fun writeTo(writer: AbstractBsonWriter) { + fun writeTo(writer: BsonWriter) { this.simplify().write(writer) } @@ -65,13 +65,14 @@ abstract class Expression( fun toString(simplified: Boolean): String { val document = BsonDocument() + val writer = BsonDocumentWriter(document) + .withLoggedContext() + @OptIn(LowLevelApi::class) - BsonDocumentWriter(document).use { - if (simplified) - writeTo(it) - else - write(it) - } + if (simplified) + writeTo(writer) + else + write(writer) return document.toString() } @@ -80,7 +81,7 @@ abstract class Expression( * Returns a JSON representation of this node, generated using [writeTo]. */ final override fun toString(): String = - "NO STRING" + toString(simplified = true) companion object } diff --git a/dsl/src/test/kotlin/expr/PredicateExpressionTest.kt b/dsl/src/test/kotlin/expr/PredicateExpressionTest.kt index 3d34178a..24b96c6b 100644 --- a/dsl/src/test/kotlin/expr/PredicateExpressionTest.kt +++ b/dsl/src/test/kotlin/expr/PredicateExpressionTest.kt @@ -1,6 +1,7 @@ package fr.qsh.ktmongo.dsl.expr import io.kotest.core.spec.style.FunSpec +import org.bson.BsonType @Suppress("unused") class PredicateExpressionTest : FunSpec({ @@ -9,6 +10,9 @@ class PredicateExpressionTest : FunSpec({ PredicateExpression(testCodec()).apply(block).toString(simplified = true) val eq = "\$eq" + val exists = "\$exists" + val type = "\$type" + val not = "\$not" context("Operator \$eq") { test("Integer") { @@ -42,4 +46,62 @@ class PredicateExpressionTest : FunSpec({ } } + context("Operator $exists") { + test("Does exist") { + predicate { + exists() + } shouldBeBson """ + { + "$exists": true + } + """.trimIndent() + } + + test("Does not exist") { + predicate { + doesNotExist() + } shouldBeBson """ + { + "$exists": false + } + """.trimIndent() + } + } + + context("Operator $type") { + test("Has a given type") { + predicate { + hasType(BsonType.DOUBLE) + } shouldBeBson """ + { + "$type": 1 + } + """.trimIndent() + } + + test("Is null") { + predicate { + isNull() + } shouldBeBson """ + { + "$type": 10 + } + """.trimIndent() + } + } + + context("Operator $not") { + test("Is not null") { + predicate { + isNotNull() + } shouldBeBson """ + { + "$not": { + "$type": 10 + } + } + """.trimIndent() + } + } + }) -- 2.51.2 From d764584b37a8684912a94ef3143771eb3505c7f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Fri, 10 May 2024 17:08:45 +0200 Subject: [PATCH 06/18] refactor(dsl): Allow freezing expressions --- .../main/kotlin/expr/common/CompoundExpression.kt | 4 ++-- dsl/src/main/kotlin/expr/common/Expression.kt | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/dsl/src/main/kotlin/expr/common/CompoundExpression.kt b/dsl/src/main/kotlin/expr/common/CompoundExpression.kt index fcec37a8..fc1e4ee2 100644 --- a/dsl/src/main/kotlin/expr/common/CompoundExpression.kt +++ b/dsl/src/main/kotlin/expr/common/CompoundExpression.kt @@ -42,9 +42,9 @@ abstract class CompoundExpression( @LowLevelApi @KtMongoDsl fun accept(expression: Expression) { - // println("Adding child expression ${expression.toString(simplified = false)}") //TODO remove - // RuntimeException().printStackTrace() + require(!frozen) { "This expression has already been frozen, it cannot accept the child expression $expression" } + expression.freeze() children += expression } diff --git a/dsl/src/main/kotlin/expr/common/Expression.kt b/dsl/src/main/kotlin/expr/common/Expression.kt index 864a3c47..dd9edb3e 100644 --- a/dsl/src/main/kotlin/expr/common/Expression.kt +++ b/dsl/src/main/kotlin/expr/common/Expression.kt @@ -24,6 +24,20 @@ abstract class Expression( protected val codec: CodecRegistry, ) { + /** + * See [freeze]. + */ + protected var frozen: Boolean = false + private set + + /** + * Forbid further mutations to this expression. + */ + @LowLevelApi + fun freeze() { + frozen = true + } + /** * Writes this expression into [writer] **exactly as it is described**. * -- 2.51.2 From 23957d8f76871abde91141bc2155605cc0eaf4e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Fri, 10 May 2024 17:20:55 +0200 Subject: [PATCH 07/18] refactor(dsl): Simplify at bind-time instead of write-time, handle empty $not --- dsl/src/main/kotlin/expr/PredicateExpression.kt | 7 +++++++ .../kotlin/expr/common/CompoundExpression.kt | 16 +++++++++++++--- dsl/src/main/kotlin/expr/common/Expression.kt | 4 ++-- .../test/kotlin/expr/PredicateExpressionTest.kt | 9 +++++++++ 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/dsl/src/main/kotlin/expr/PredicateExpression.kt b/dsl/src/main/kotlin/expr/PredicateExpression.kt index 5d8698b5..c4e2a34e 100644 --- a/dsl/src/main/kotlin/expr/PredicateExpression.kt +++ b/dsl/src/main/kotlin/expr/PredicateExpression.kt @@ -283,6 +283,13 @@ class PredicateExpression( codec: CodecRegistry, ) : PredicateExpressionNode(codec) { + override fun simplify(): Expression? { + if (expression.children.isEmpty()) + return null + + return super.simplify() + } + override fun write(writer: BsonWriter) { writer.writeDocument { writer.writeName("\$not") diff --git a/dsl/src/main/kotlin/expr/common/CompoundExpression.kt b/dsl/src/main/kotlin/expr/common/CompoundExpression.kt index fc1e4ee2..4d0b7fc2 100644 --- a/dsl/src/main/kotlin/expr/common/CompoundExpression.kt +++ b/dsl/src/main/kotlin/expr/common/CompoundExpression.kt @@ -4,6 +4,7 @@ import fr.qsh.ktmongo.dsl.KtMongoDsl import fr.qsh.ktmongo.dsl.LowLevelApi import org.bson.BsonWriter import org.bson.codecs.configuration.CodecRegistry +import java.util.* /** * A compound node in the BSON AST. @@ -22,7 +23,11 @@ abstract class CompoundExpression( // region Sub-expression binding - private val children = ArrayList() + private val _children = ArrayList() + + @LowLevelApi + val children: List + get() = Collections.unmodifiableList(_children) /** * Binds an arbitrary [expression] as a sub-expression of the receiver. @@ -44,8 +49,12 @@ abstract class CompoundExpression( fun accept(expression: Expression) { require(!frozen) { "This expression has already been frozen, it cannot accept the child expression $expression" } - expression.freeze() - children += expression + val simplifiedExpression = expression.simplify() + + if (simplifiedExpression != null) { + _children += simplifiedExpression + .also { it.freeze() } + } } // endregion @@ -56,6 +65,7 @@ abstract class CompoundExpression( * * @param children The list of expressions that have been [bound][accept] into this * expression. + * **These children have already been simplified.** */ @LowLevelApi protected open fun simplify(children: List): Expression = diff --git a/dsl/src/main/kotlin/expr/common/Expression.kt b/dsl/src/main/kotlin/expr/common/Expression.kt index dd9edb3e..969b155e 100644 --- a/dsl/src/main/kotlin/expr/common/Expression.kt +++ b/dsl/src/main/kotlin/expr/common/Expression.kt @@ -59,7 +59,7 @@ abstract class Expression( * **Implementations must be pure.** */ @LowLevelApi - protected open fun simplify(): Expression = this + open fun simplify(): Expression? = this /** * Writes this expression into a [writer]. @@ -68,7 +68,7 @@ abstract class Expression( */ @LowLevelApi fun writeTo(writer: BsonWriter) { - this.simplify().write(writer) + this.simplify()?.write(writer) } /** diff --git a/dsl/src/test/kotlin/expr/PredicateExpressionTest.kt b/dsl/src/test/kotlin/expr/PredicateExpressionTest.kt index 24b96c6b..3de3a56f 100644 --- a/dsl/src/test/kotlin/expr/PredicateExpressionTest.kt +++ b/dsl/src/test/kotlin/expr/PredicateExpressionTest.kt @@ -102,6 +102,15 @@ class PredicateExpressionTest : FunSpec({ } """.trimIndent() } + + test("Empty $not is no-op and thus removed") { + predicate { + not { } + } shouldBeBson """ + { + } + """.trimIndent() + } } }) -- 2.51.2 From bcbb2d32cb72e1333d3e4764ea48ae8d125d0020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Fri, 10 May 2024 17:28:05 +0200 Subject: [PATCH 08/18] refactor(dsl): Handle empty and 1-arity $and and $or --- dsl/src/main/kotlin/expr/FilterExpression.kt | 20 ++++++++ .../test/kotlin/expr/FilterExpressionTest.kt | 46 +++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/dsl/src/main/kotlin/expr/FilterExpression.kt b/dsl/src/main/kotlin/expr/FilterExpression.kt index 3144c7b5..50c80b31 100644 --- a/dsl/src/main/kotlin/expr/FilterExpression.kt +++ b/dsl/src/main/kotlin/expr/FilterExpression.kt @@ -84,6 +84,16 @@ class FilterExpression( codec: CodecRegistry, ) : FilterExpressionNode(codec) { + override fun simplify(): Expression? { + if (expression.children.isEmpty()) + return null + + if (expression.children.size == 1) + return expression + + return super.simplify() + } + override fun write(writer: BsonWriter) { writer.writeDocument { writer.writeName("\$and") @@ -133,6 +143,16 @@ class FilterExpression( codec: CodecRegistry, ) : FilterExpressionNode(codec) { + override fun simplify(): Expression? { + if (expression.children.isEmpty()) + return null + + if (expression.children.size == 1) + return expression + + return super.simplify() + } + override fun write(writer: BsonWriter) { writer.writeDocument { writer.writeName("\$or") diff --git a/dsl/src/test/kotlin/expr/FilterExpressionTest.kt b/dsl/src/test/kotlin/expr/FilterExpressionTest.kt index 3142ac66..79f2dd08 100644 --- a/dsl/src/test/kotlin/expr/FilterExpressionTest.kt +++ b/dsl/src/test/kotlin/expr/FilterExpressionTest.kt @@ -177,6 +177,29 @@ class FilterExpressionTest : FunSpec({ """.trimIndent() } + test("Empty $and") { + filter { + and {} + } shouldBeBson """ + { + } + """.trimIndent() + } + + test("An $and with a single term is removed") { + filter { + and { + User::name eq "foo" + } + } shouldBeBson """ + { + "name": { + "$eq": "foo" + } + } + """.trimIndent() + } + test("An automatic $and is generated when multiple filters are given") { filter { // same example as the previous, but we didn't write the '$and' User::name eq "foo" @@ -222,5 +245,28 @@ class FilterExpressionTest : FunSpec({ } """.trimIndent() } + + test("Empty $or") { + filter { + or {} + } shouldBeBson """ + { + } + """.trimIndent() + } + + test("An $or with a single term is removed") { + filter { + or { + User::name eq "foo" + } + } shouldBeBson """ + { + "name": { + "$eq": "foo" + } + } + """.trimIndent() + } } }) -- 2.51.2 From 2ea3f9815c9622589469e70a1dccd60664c9aef5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Fri, 10 May 2024 17:35:04 +0200 Subject: [PATCH 09/18] refactor(dsl): Combine nested $and --- dsl/src/main/kotlin/expr/FilterExpression.kt | 21 +++++++++++- .../test/kotlin/expr/FilterExpressionTest.kt | 32 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/dsl/src/main/kotlin/expr/FilterExpression.kt b/dsl/src/main/kotlin/expr/FilterExpression.kt index 50c80b31..0b6e7113 100644 --- a/dsl/src/main/kotlin/expr/FilterExpression.kt +++ b/dsl/src/main/kotlin/expr/FilterExpression.kt @@ -4,6 +4,7 @@ import fr.qsh.ktmongo.dsl.KtMongoDsl import fr.qsh.ktmongo.dsl.LowLevelApi import fr.qsh.ktmongo.dsl.expr.common.CompoundExpression import fr.qsh.ktmongo.dsl.expr.common.Expression +import fr.qsh.ktmongo.dsl.expr.common.acceptAll import fr.qsh.ktmongo.dsl.expr.common.empty import fr.qsh.ktmongo.dsl.path.path import fr.qsh.ktmongo.dsl.writeArray @@ -91,7 +92,25 @@ class FilterExpression( if (expression.children.size == 1) return expression - return super.simplify() + // If there are nested $and operators, we combine them into the current one + val nestedChildren = ArrayList() + + for (child in expression.children) { + if (child is AndFilterExpressionNode<*>) { + for (nestedChild in child.expression.children) { + nestedChildren += nestedChild + } + } else { + nestedChildren += child + } + } + + return AndFilterExpressionNode( + FilterExpression(codec).apply { + acceptAll(nestedChildren) + }, + codec, + ) } override fun write(writer: BsonWriter) { diff --git a/dsl/src/test/kotlin/expr/FilterExpressionTest.kt b/dsl/src/test/kotlin/expr/FilterExpressionTest.kt index 79f2dd08..11364933 100644 --- a/dsl/src/test/kotlin/expr/FilterExpressionTest.kt +++ b/dsl/src/test/kotlin/expr/FilterExpressionTest.kt @@ -200,6 +200,38 @@ class FilterExpressionTest : FunSpec({ """.trimIndent() } + test("Combine nested $and") { + filter { + and { + User::name eq "foo" + and { + User::age eq 12 + User::id eq "abc" + } + } + } shouldBeBson """ + { + "$and": [ + { + "name": { + "$eq": "foo" + } + }, + { + "age": { + "$eq": 12 + } + }, + { + "id": { + "$eq": "abc" + } + } + ] + } + """.trimIndent() + } + test("An automatic $and is generated when multiple filters are given") { filter { // same example as the previous, but we didn't write the '$and' User::name eq "foo" -- 2.51.2 From 9a8fb9016e04ab912a62680da0e316f1c47c466e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Fri, 10 May 2024 17:54:57 +0200 Subject: [PATCH 10/18] fix(dsl): Correctly generate an implicit $and when a filter has multiple elements --- dsl/src/main/kotlin/expr/FilterExpression.kt | 53 ++++++++----------- .../kotlin/expr/common/CompoundExpression.kt | 6 +-- .../kotlin/expr/common/EmptyExpression.kt | 13 ----- 3 files changed, 25 insertions(+), 47 deletions(-) delete mode 100644 dsl/src/main/kotlin/expr/common/EmptyExpression.kt diff --git a/dsl/src/main/kotlin/expr/FilterExpression.kt b/dsl/src/main/kotlin/expr/FilterExpression.kt index 0b6e7113..43e8bcb9 100644 --- a/dsl/src/main/kotlin/expr/FilterExpression.kt +++ b/dsl/src/main/kotlin/expr/FilterExpression.kt @@ -4,8 +4,6 @@ import fr.qsh.ktmongo.dsl.KtMongoDsl import fr.qsh.ktmongo.dsl.LowLevelApi import fr.qsh.ktmongo.dsl.expr.common.CompoundExpression import fr.qsh.ktmongo.dsl.expr.common.Expression -import fr.qsh.ktmongo.dsl.expr.common.acceptAll -import fr.qsh.ktmongo.dsl.expr.common.empty import fr.qsh.ktmongo.dsl.path.path import fr.qsh.ktmongo.dsl.writeArray import fr.qsh.ktmongo.dsl.writeDocument @@ -29,17 +27,11 @@ class FilterExpression( // region Low-level operations @LowLevelApi - override fun simplify(children: List): Expression = + override fun simplify(children: List): Expression? = when (children.size) { - 0 -> Expression.empty(codec) + 0 -> null 1 -> this - // else -> AndFilterExpressionNode( - // FilterExpression(codec).apply { - // acceptAll(children) - // }, - // codec, - // ) - else -> this + else -> AndFilterExpressionNode(children, codec) } @LowLevelApi @@ -76,28 +68,28 @@ class FilterExpression( @OptIn(LowLevelApi::class) @KtMongoDsl fun and(block: FilterExpression.() -> Unit) { - accept(AndFilterExpressionNode(FilterExpression(codec).apply(block), codec)) + accept(AndFilterExpressionNode(FilterExpression(codec).apply(block).children, codec)) } @LowLevelApi private class AndFilterExpressionNode( - val expression: FilterExpression, + val declaredChildren: List, codec: CodecRegistry, ) : FilterExpressionNode(codec) { override fun simplify(): Expression? { - if (expression.children.isEmpty()) + if (declaredChildren.isEmpty()) return null - if (expression.children.size == 1) - return expression + if (declaredChildren.size == 1) + return FilterExpression(codec).apply { accept(declaredChildren.single()) } // If there are nested $and operators, we combine them into the current one val nestedChildren = ArrayList() - for (child in expression.children) { + for (child in declaredChildren) { if (child is AndFilterExpressionNode<*>) { - for (nestedChild in child.expression.children) { + for (nestedChild in child.declaredChildren) { nestedChildren += nestedChild } } else { @@ -105,19 +97,16 @@ class FilterExpression( } } - return AndFilterExpressionNode( - FilterExpression(codec).apply { - acceptAll(nestedChildren) - }, - codec, - ) + return AndFilterExpressionNode(nestedChildren, codec) } override fun write(writer: BsonWriter) { writer.writeDocument { writer.writeName("\$and") writer.writeArray { - expression.writeTo(writer) + for (child in declaredChildren) { + child.writeTo(writer) + } } } } @@ -153,21 +142,21 @@ class FilterExpression( @OptIn(LowLevelApi::class) @KtMongoDsl fun or(block: FilterExpression.() -> Unit) { - accept(OrFilterExpressionNode(FilterExpression(codec).apply(block), codec)) + accept(OrFilterExpressionNode(FilterExpression(codec).apply(block).children, codec)) } @LowLevelApi private class OrFilterExpressionNode( - val expression: FilterExpression, + val declaredChildren: List, codec: CodecRegistry, ) : FilterExpressionNode(codec) { override fun simplify(): Expression? { - if (expression.children.isEmpty()) + if (declaredChildren.isEmpty()) return null - if (expression.children.size == 1) - return expression + if (declaredChildren.size == 1) + return FilterExpression(codec).apply { accept(declaredChildren.single()) } return super.simplify() } @@ -176,7 +165,9 @@ class FilterExpression( writer.writeDocument { writer.writeName("\$or") writer.writeArray { - expression.writeTo(writer) + for (child in declaredChildren) { + child.writeTo(writer) + } } } } diff --git a/dsl/src/main/kotlin/expr/common/CompoundExpression.kt b/dsl/src/main/kotlin/expr/common/CompoundExpression.kt index 4d0b7fc2..98fa9c58 100644 --- a/dsl/src/main/kotlin/expr/common/CompoundExpression.kt +++ b/dsl/src/main/kotlin/expr/common/CompoundExpression.kt @@ -26,7 +26,7 @@ abstract class CompoundExpression( private val _children = ArrayList() @LowLevelApi - val children: List + protected val children: List get() = Collections.unmodifiableList(_children) /** @@ -68,11 +68,11 @@ abstract class CompoundExpression( * **These children have already been simplified.** */ @LowLevelApi - protected open fun simplify(children: List): Expression = + protected open fun simplify(children: List): Expression? = this @LowLevelApi - final override fun simplify(): Expression = + final override fun simplify(): Expression? = simplify(children) // endregion diff --git a/dsl/src/main/kotlin/expr/common/EmptyExpression.kt b/dsl/src/main/kotlin/expr/common/EmptyExpression.kt deleted file mode 100644 index 2181fd85..00000000 --- a/dsl/src/main/kotlin/expr/common/EmptyExpression.kt +++ /dev/null @@ -1,13 +0,0 @@ -package fr.qsh.ktmongo.dsl.expr.common - -import fr.qsh.ktmongo.dsl.LowLevelApi -import org.bson.BsonWriter -import org.bson.codecs.configuration.CodecRegistry - -private class EmptyExpression(codec: CodecRegistry) : Expression(codec) { - @LowLevelApi - override fun write(writer: BsonWriter) {} -} - -fun Expression.Companion.empty(codec: CodecRegistry): Expression = - EmptyExpression(codec) -- 2.51.2 From 720266b7961f810212139ca59e6e0a91560b8044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20=E2=80=9CCLOVIS=E2=80=9D=20Canet?= Date: Fri, 10 May 2024 17:55:17 +0200 Subject: [PATCH 11/18] build(idea): Update coding style --- .idea/codeStyles/Project.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml index bdee5c6e..6a603645 100644 --- a/.idea/codeStyles/Project.xml +++ b/.idea/codeStyles/Project.xml @@ -73,7 +73,7 @@ - +