diff --git a/src/commands.zig b/src/commands.zig index 3b7fa36..2146436 100644 --- a/src/commands.zig +++ b/src/commands.zig @@ -4272,6 +4272,42 @@ fn update_refusal(reply: *wire.Reply, err: anyerror, diag: update.Diagnostic) !v .{ if (std.mem.eql(u8, diag.segment, "$inc")) "increment" else "multiply", diag.path }, ), ), + error.NotAnArrayField => return bad_value(reply, try std.fmt.allocPrint( + arena, + "Cannot apply {s} to non-array field. Field named '{s}' has non-array type", + .{ diag.segment, diag.path }, + )), + error.NotAnArrayPathElement => return reply.put_error( + @intFromEnum(ErrorCode.type_mismatch), + "TypeMismatch", + try std.fmt.allocPrint( + arena, + "Path '{s}' contains an element of non-array type", + .{diag.path}, + ), + ), + // mongod has two sentences here -- "$pop expects 1 or -1, found: 2" + // and "Expected a number in: t: \"x\"" -- both code 9, and both about + // an argument that is not one of the two values `$pop` takes. One + // sentence covering both says the same thing without rendering the + // operand, which no formatter here does. + error.BadPopArgument => return failed_to_parse(reply, try std.fmt.allocPrint( + arena, + "$pop expects 1 or -1, at field '{s}'", + .{diag.path}, + )), + error.PullAllNeedsArray => return bad_value(reply, try std.fmt.allocPrint( + arena, + "$pullAll requires an array argument but was given a {s}", + .{diag.other}, + )), + // `$push` and `$addToSet` disagree about the code for the identical + // mistake: 2 and 14. Measured on both, and not derivable from either. + error.BadEach => if (std.mem.eql(u8, diag.segment, "$addToSet")) return reply.put_error( + @intFromEnum(ErrorCode.type_mismatch), + "TypeMismatch", + "The argument to $each in $addToSet must be an array", + ) else return bad_value(reply, "The argument to $each in $push must be an array"), error.PathNotViable => return reply.put_error( @intFromEnum(ErrorCode.path_not_viable), "PathNotViable", diff --git a/src/update.zig b/src/update.zig index dca3b54..6b3ddce 100644 --- a/src/update.zig +++ b/src/update.zig @@ -22,6 +22,18 @@ pub const UpdateError = error{ /// sentence, so they are two errors. NotNumericField, NotNumericOperand, + /// `$addToSet` or `$pullAll` against a field that is not an array. + /// `BadValue` (2) on mongod, where `$pop` against one is `TypeMismatch`. + NotAnArrayField, + /// `$pop` against a field that is not an array. Same mistake as + /// `NotAnArrayField`, different code -- measured, not derived. + NotAnArrayPathElement, + /// `$pop` by anything that is not 1 or -1. `FailedToParse` (9). + BadPopArgument, + /// `$pullAll` by something that is not an array of values. + PullAllNeedsArray, + /// `$each` that is not an array. + BadEach, /// A path segment that is not a field to create: a non-numeric name /// applied to an array, or any name applied to a scalar element a /// positional segment selected. mongod's `PathNotViable`. @@ -627,6 +639,9 @@ fn apply_operator( if (std.mem.eql(u8, op, "$max")) return op_extremum(arena, pairs, ops, opts, .gt); if (std.mem.eql(u8, op, "$push")) return op_push(arena, pairs, ops, opts); if (std.mem.eql(u8, op, "$pull")) return op_pull(arena, pairs, ops, opts); + if (std.mem.eql(u8, op, "$addToSet")) return op_add_to_set(arena, pairs, ops, opts); + if (std.mem.eql(u8, op, "$pop")) return op_pop(arena, pairs, ops, opts); + if (std.mem.eql(u8, op, "$pullAll")) return op_pull_all(arena, pairs, ops, opts); if (std.mem.eql(u8, op, "$rename")) return op_rename(arena, pairs, ops, opts); return error.InvalidUpdate; } @@ -774,6 +789,139 @@ fn op_pull( } } +/// `$addToSet`: append what the array does not already hold. +/// +/// "Already hold" is `bson.compare` equality, which is exactly mongod's: an +/// int32 `2` and a double `2.0` are one value, and two documents with the same +/// fields in a different order are two -- because `compare_docs` walks the +/// pairs positionally and tie-breaks on the key. +fn op_add_to_set( + arena: std.mem.Allocator, + pairs: *std.ArrayListUnmanaged(bson.Pair), + ops: []const bson.Pair, + opts: Options, +) UpdateError!void { + for (ops) |p| { + // `$each` is a modifier only here and under `$push`; anywhere else a + // document operand is the value being added. + const candidates: []const bson.Value = if (each_of(p.value)) |each| switch (each) { + .array => |a| a, + else => { + note(opts.diag, p.key, "$addToSet"); + return error.BadEach; + }, + } else &.{p.value}; + + for (try resolve(arena, pairs.items, p.key, opts)) |segs| { + var items: std.ArrayListUnmanaged(bson.Value) = .empty; + defer items.deinit(arena); + if (get_value(pairs.items, segs)) |current| switch (current) { + .array => |arr| try items.appendSlice(arena, arr), + .null => {}, + else => { + note(opts.diag, p.key, "$addToSet"); + return error.NotAnArrayField; + }, + }; + for (candidates) |c| { + if (holds(items.items, c)) continue; + try items.append(arena, try bson.copy_value(arena, c)); + } + try set_path(arena, pairs, segs, .{ .array = try items.toOwnedSlice(arena) }, p.key, opts.diag); + } + } +} + +fn holds(items: []const bson.Value, v: bson.Value) bool { + for (items) |item| if (bson.compare(item, v) == .eq) return true; + return false; +} + +/// The `$each` of a modifier document, or null when the operand is a value. +/// +/// Presence of `$each` is what makes an operand a modifier document at all -- +/// measured: `{$push: {t: {$slice: 1}}}` pushes `{$slice: 1}` as a value. +fn each_of(v: bson.Value) ?bson.Value { + const doc = doc_pairs(v) orelse return null; + return bson.get_pair(doc, "$each"); +} + +/// `$pop`: remove one element from an end. `1` is the last, `-1` the first. +/// +/// An empty array and an absent field are both no-ops rather than errors, so +/// the only refusals are the argument and the field's type. +fn op_pop( + arena: std.mem.Allocator, + pairs: *std.ArrayListUnmanaged(bson.Pair), + ops: []const bson.Pair, + opts: Options, +) UpdateError!void { + for (ops) |p| { + if (!p.value.is_number()) { + note(opts.diag, p.key, ""); + return error.BadPopArgument; + } + const n = p.value.as_f128(); + if (n != 1 and n != -1) { + note(opts.diag, p.key, ""); + return error.BadPopArgument; + } + for (try resolve(arena, pairs.items, p.key, opts)) |segs| { + const current = get_value(pairs.items, segs) orelse continue; + const arr = switch (current) { + .array => |a| a, + else => { + note(opts.diag, p.key, "$pop"); + return error.NotAnArrayPathElement; + }, + }; + if (arr.len == 0) continue; + const kept = if (n == 1) arr[0 .. arr.len - 1] else arr[1..]; + try set_path(arena, pairs, segs, .{ .array = try arena.dupe(bson.Value, kept) }, p.key, opts.diag); + } + } +} + +/// `$pullAll`: remove every element equal to any of the listed values. +/// +/// The difference from `$pull` is the whole of it: `$pull` takes a *predicate* +/// and `$pullAll` takes values, compared whole. `{$pull: {t: {a: 1}}}` matches +/// elements having `a: 1`; `{$pullAll: {t: [{a: 1}]}}` matches elements that +/// *are* `{a: 1}`. +fn op_pull_all( + arena: std.mem.Allocator, + pairs: *std.ArrayListUnmanaged(bson.Pair), + ops: []const bson.Pair, + opts: Options, +) UpdateError!void { + for (ops) |p| { + const wanted = switch (p.value) { + .array => |a| a, + else => { + if (opts.diag) |d| d.* = .{ .path = p.key, .other = p.value.type_name() }; + return error.PullAllNeedsArray; + }, + }; + for (try resolve(arena, pairs.items, p.key, opts)) |segs| { + const current = get_value(pairs.items, segs) orelse continue; + const arr = switch (current) { + .array => |a| a, + else => { + note(opts.diag, p.key, "$pullAll"); + return error.NotAnArrayField; + }, + }; + var items: std.ArrayListUnmanaged(bson.Value) = .empty; + defer items.deinit(arena); + for (arr) |elem| { + if (holds(wanted, elem)) continue; + try items.append(arena, elem); + } + try set_path(arena, pairs, segs, .{ .array = try items.toOwnedSlice(arena) }, p.key, opts.diag); + } + } +} + /// `$rename` alone keeps the plain split: `validate` has already refused a /// positional path on either end of it, which is what mongod does too. fn op_rename( @@ -1864,3 +2012,141 @@ test "$mul reaches every element a positional segment names" { try testing.expectEqual(@as(i32, 20), t[0].doc[0].value.int32); try testing.expectEqual(@as(i32, 30), t[1].doc[0].value.int32); } + +test "$addToSet appends only what the array does not hold" { + var doc = try doc_with(testing.allocator, &.{ + .{ .key = "t", .value = .{ .array = &.{ .{ .int32 = 1 }, .{ .int32 = 2 } } } }, + }); + defer doc.arena.deinit(); + try apply(&doc, &doc_of(&.{ + .{ .key = "$addToSet", .value = .{ .doc = &.{ + .{ .key = "t", .value = .{ .doc = &.{ + .{ .key = "$each", .value = .{ .array = &.{ .{ .int32 = 2 }, .{ .int32 = 3 }, .{ .int32 = 3 } } } }, + } } }, + .{ .key = "gone", .value = .{ .int32 = 1 } }, + } } }, + }), .{}); + // 2 is already there, and the two 3s in one `$each` are one value: the + // candidates are checked against the array *as it grows*. + const t = doc.get("t").?.array; + try testing.expectEqual(@as(usize, 3), t.len); + try testing.expectEqual(@as(i32, 3), t[2].int32); + // An absent field becomes a one-element array rather than an error. + try testing.expectEqual(@as(usize, 1), doc.get("gone").?.array.len); +} + +test "$addToSet identity is BSON equality, field order included" { + // Two rows, opposite answers, one comparator. Mutation check: compare with + // anything that ignores key order and the second half goes red -- mongod + // stores both spellings of the same document. + var doc = try doc_with(testing.allocator, &.{ + .{ .key = "n", .value = .{ .array = &.{.{ .int32 = 2 }} } }, + .{ .key = "d", .value = .{ .array = &.{.{ .doc = &.{ + .{ .key = "a", .value = .{ .int32 = 1 } }, + .{ .key = "b", .value = .{ .int32 = 2 } }, + } }} } }, + }); + defer doc.arena.deinit(); + try apply(&doc, &doc_of(&.{ + .{ .key = "$addToSet", .value = .{ .doc = &.{ + // An int32 2 and a double 2.0 are one value. + .{ .key = "n", .value = .{ .double = 2.0 } }, + // The same fields in the other order are two. + .{ .key = "d", .value = .{ .doc = &.{ + .{ .key = "b", .value = .{ .int32 = 2 } }, + .{ .key = "a", .value = .{ .int32 = 1 } }, + } } }, + } } }, + }), .{}); + try testing.expectEqual(@as(usize, 1), doc.get("n").?.array.len); + try testing.expectEqual(@as(usize, 2), doc.get("d").?.array.len); +} + +test "$pop takes one element off an end, and is quiet when there is none" { + var doc = try doc_with(testing.allocator, &.{ + .{ .key = "a", .value = .{ .array = &.{ .{ .int32 = 1 }, .{ .int32 = 2 }, .{ .int32 = 3 } } } }, + .{ .key = "b", .value = .{ .array = &.{ .{ .int32 = 1 }, .{ .int32 = 2 } } } }, + .{ .key = "e", .value = .{ .array = &.{} } }, + }); + defer doc.arena.deinit(); + try apply(&doc, &doc_of(&.{ + .{ .key = "$pop", .value = .{ .doc = &.{ + .{ .key = "a", .value = .{ .int32 = 1 } }, // the last + .{ .key = "b", .value = .{ .double = -1.0 } }, // the first, and -1.0 is -1 + .{ .key = "e", .value = .{ .int32 = 1 } }, // empty: no-op + .{ .key = "gone", .value = .{ .int32 = 1 } }, // absent: no-op + } } }, + }), .{}); + try testing.expectEqual(@as(usize, 2), doc.get("a").?.array.len); + try testing.expectEqual(@as(i32, 2), doc.get("a").?.array[1].int32); + try testing.expectEqual(@as(usize, 1), doc.get("b").?.array.len); + try testing.expectEqual(@as(i32, 2), doc.get("b").?.array[0].int32); + try testing.expectEqual(@as(usize, 0), doc.get("e").?.array.len); + try testing.expect(doc.get("gone") == null); +} + +test "$pop refuses an argument that is not one of its two values" { + var doc = try doc_with(testing.allocator, &.{ + .{ .key = "t", .value = .{ .array = &.{.{ .int32 = 1 }} } }, + }); + defer doc.arena.deinit(); + for ([_]bson.Value{ .{ .int32 = 2 }, .{ .int32 = 0 }, .{ .string = "x" } }) |bad| { + try testing.expectError(error.BadPopArgument, apply(&doc, &doc_of(&.{ + .{ .key = "$pop", .value = .{ .doc = &.{.{ .key = "t", .value = bad }} } }, + }), .{})); + } + try testing.expectEqual(@as(usize, 1), doc.get("t").?.array.len); +} + +test "$pullAll removes values, where $pull removes matches" { + // The whole difference between the two, in one document: `{a: 1}` as a + // `$pull` argument is a predicate and as a `$pullAll` element is a value. + // Mutation check: route `$pullAll` through `pull_matches` and the second + // half goes red -- `{a: 1, b: 2}` would be pulled by a predicate too. + var doc = try doc_with(testing.allocator, &.{ + .{ .key = "n", .value = .{ .array = &.{ + .{ .int32 = 1 }, .{ .int32 = 2 }, .{ .int32 = 3 }, .{ .int32 = 2 }, + } } }, + .{ .key = "d", .value = .{ .array = &.{ + .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} }, + .{ .doc = &.{ + .{ .key = "a", .value = .{ .int32 = 1 } }, + .{ .key = "b", .value = .{ .int32 = 2 } }, + } }, + } } }, + }); + defer doc.arena.deinit(); + try apply(&doc, &doc_of(&.{ + .{ .key = "$pullAll", .value = .{ .doc = &.{ + .{ .key = "n", .value = .{ .array = &.{.{ .int32 = 2 }} } }, + .{ .key = "d", .value = .{ .array = &.{ + .{ .doc = &.{.{ .key = "a", .value = .{ .int32 = 1 } }} }, + } } }, + .{ .key = "gone", .value = .{ .array = &.{.{ .int32 = 1 }} } }, + } } }, + }), .{}); + try testing.expectEqual(@as(usize, 2), doc.get("n").?.array.len); + // Only the element that *is* `{a: 1}` went; the one that merely matches + // that predicate stayed. + try testing.expectEqual(@as(usize, 1), doc.get("d").?.array.len); + try testing.expectEqual(@as(usize, 2), doc.get("d").?.array[0].doc.len); + try testing.expect(doc.get("gone") == null); +} + +test "the array operators refuse a field that is not an array" { + var doc = try doc_with(testing.allocator, &.{.{ .key = "t", .value = .{ .int32 = 5 } }}); + defer doc.arena.deinit(); + try testing.expectError(error.NotAnArrayField, apply(&doc, &doc_of(&.{ + .{ .key = "$addToSet", .value = .{ .doc = &.{.{ .key = "t", .value = .{ .int32 = 1 } }} } }, + }), .{})); + try testing.expectError(error.NotAnArrayPathElement, apply(&doc, &doc_of(&.{ + .{ .key = "$pop", .value = .{ .doc = &.{.{ .key = "t", .value = .{ .int32 = 1 } }} } }, + }), .{})); + try testing.expectError(error.NotAnArrayField, apply(&doc, &doc_of(&.{ + .{ .key = "$pullAll", .value = .{ .doc = &.{.{ .key = "t", .value = .{ .array = &.{.{ .int32 = 1 }} } }} } }, + }), .{})); + try testing.expectError(error.PullAllNeedsArray, apply(&doc, &doc_of(&.{ + .{ .key = "$pullAll", .value = .{ .doc = &.{.{ .key = "t", .value = .{ .int32 = 1 } }} } }, + }), .{})); + try testing.expectEqual(@as(i32, 5), doc.get("t").?.int32); +}