Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store multiple documents with StoreDoc bug #1162

Closed
davidst opened this issue Dec 9, 2024 · 5 comments · Fixed by #1158
Closed

Store multiple documents with StoreDoc bug #1162

davidst opened this issue Dec 9, 2024 · 5 comments · Fixed by #1158

Comments

@davidst
Copy link

davidst commented Dec 9, 2024

Describe the bug
In the Wolverine V3.4.0 StoreDoc<T> now stores an object[] with one element of type T[].

To Reproduce

public recod Foo;

public void MartenOps_store_test()
{
    var docs = new[] { new Foo(), new Foo() };

    var storeOp = MartenOps.Store<Foo>(docs);

    storeOp.Documents.ShouldBe(docs);
}

Expected behavior
StoreDoc<T> should store an object[] with elements of type T.

Additional context

  • The bug was introduced in this commit.
  • The initial change from this commit introduced a breaking change in a minor version.
@haefele
Copy link
Contributor

haefele commented Dec 9, 2024

Fix is already prepared in #1158

@davidst
Copy link
Author

davidst commented Dec 10, 2024

The bug is still there in the public StoreManyDocs(params T[] documents) : base(documents) constructor calling the base constructor, because there is no implicit conversion from T[] to object[].
A quick fix would be to first cast the T elements to object then pass the array to the base constructor public StoreManyDocs(params T[] documents) : base(documents.Cast<Object>().ToArray()).

@jay-zahiri
Copy link
Contributor

jay-zahiri commented Dec 10, 2024

I missed this issue while reorganizing the code; it's fixed now. Here's a quick demo

@davidst
Copy link
Author

davidst commented Dec 10, 2024

Yes, that fixes the issue, but it's missing the params keyword which would make the usage nicer.

@jeremydmiller
Copy link
Member

Fixes coming late today / early tomorrow

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

Successfully merging a pull request may close this issue.

4 participants