Fix Packet size mismatch! error on collection drop

[gedaiu]

fix #2812

[gedaiu]

@WebFreak001 @s-ludwig any thoughts on this pr?

[s-ludwig]

The change looks good, but the code in the unittest needs to be moved to a test in "/tests/mongodb", as the regular CI runs do not have a MongoDB server available. It can probably just be moved to "/tests/mongodb/_connection/source/app.d"

[WebFreak001]

sounds good but please don't put it in the fundamental _connection test, but rather in some (perhaps new) collection management test

[gedaiu]

@s-ludwig @WebFreak001 ready for another look

[WebFreak001]

is this test only testing dropping non-existent collections?

Should probably drop it twice just in case if it randomly existed beforehand on CI. It should behave the same both times after all.

Otherwise it might make sense to also create a collection here.

[gedaiu]

From what I tested, the issue happens even when the collection exists. So it does not matter if I do it once or twice.

…

On Mon, Jan 27, 2025 at 9:03 PM Jan Jurzitza ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In tests/mongodb/collection/source/app.d <#2813 (comment)>: > +import vibe.core.core; +import vibe.core.log; +import vibe.data.bson; +import vibe.db.mongo.mongo; + +import std.algorithm : all, canFind, equal, map, sort; +import std.conv : to; +import std.encoding : sanitize; +import std.exception : assertThrown; + + +void runTest(ushort port) +{ + MongoClient client = connectMongoDB("127.0.0.1", port); + + /// Drop a collection is this test only testing dropping non-existent collections? Should probably drop it twice just in case if it randomly existed beforehand on CI. It should behave the same both times after all. Otherwise it might make sense to also create a collection here. — Reply to this email directly, view it on GitHub <#2813 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABAFRTKKFZYMOQTAYXHBSRT2M2GJHAVCNFSM6AAAAABU766R5KVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKNZWGQZTONZTG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[s-ludwig]

I've just tested locally and for me the test passes when I insert just a single {_id: ...} entry into the collection beforehand. So I'd say let's just add a second drop to make it safe.

[WebFreak001]

it does not matter if I do it once or twice.

prove it in the test then? :D

Right now the CI fails with command failed: ns not found in vibe.db.mongo.collection.MongoCollection.drop (../../../mongodb/vibe/db/mongo/collection.d:1238) on all MongoDB versions prior to 7.0

So it looks like in older MongoDB versions this didn't work.

Now we could either ignore this error on the vibe.d side to make it behave the same with all MongoDB versions (and perhaps return a boolean if we can extract it in >=7 as well and make it part of the API) or just say double free has unspecified behavior.