Skip to content

Added Tests for Query with inner query - #1107

Merged
jeremydmiller merged 11 commits into
JasperFx:masterfrom
oskardudycz:1105-addedTestsForInnerQueries
May 7, 2019
Merged

jeremydmiller merged 11 commits into
JasperFx:masterfrom
oskardudycz:1105-addedTestsForInnerQueries

Conversation

@oskardudycz

Copy link
Copy Markdown
Collaborator

Added tests for issue #1105

var serializer = new JsonNetSerializer();
serializer.Customize(s =>
{
s.TypeNameHandling = Newtonsoft.Json.TypeNameHandling.None;

@mysticmind mysticmind Oct 8, 2018 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the same can be applied as an attribute on the IEnumerable properties on the class to achieve the same rather than customizing the serializer at the store level. Thoughts?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mysticmind good catch, I'll add next case checking that

@robert-skarzycki

Copy link
Copy Markdown

@oskardudycz, what about one more level down, i.e. having creation of test data looking like this:

 public static TypeWithInnerCollections Create(params string[] array)
        {
            return new TypeWithInnerCollections()
            {
                Id = Guid.NewGuid(),
                Array = array,
                Flatten = array.Aggregate((i, j) => i + j),
                Enumerable = array.AsEnumerable(),
                IEnumerableFromArray = array,
                IEnumerbaleFromList = array.ToList(),
                List = array.ToList(),
                IList = array.ToList(),
                IReadonlyCollection = array.ToList(),
                IReadonlyCollectionOfInnerClasses = new List<TypeWithInnerCollections>
                    {
                        new TypeWithInnerCollections()
                        {
                            Id = Guid.NewGuid(),
                            Array = array,
                            Flatten = array.Aggregate((i, j) => i + j),
                            Enumerable = array.AsEnumerable(),
                            IEnumerableFromArray = array,
                            IEnumerbaleFromList = array.ToList(),
                            List = array.ToList(),
                            IList = array.ToList(),
                            IReadonlyCollection = array.ToList(),
                            IReadonlyCollectionOfInnerClasses = new List<TypeWithInnerCollections>
                            {
                                new TypeWithInnerCollections()
                                {
                                    Id = Guid.NewGuid(),
                                    Array = array,
                                    Flatten = array.Aggregate((i, j) => i + j),
                                    Enumerable = array.AsEnumerable(),
                                    IEnumerableFromArray = array,
                                    IEnumerbaleFromList = array.ToList(),
                                    List = array.ToList(),
                                    IList = array.ToList(),
                                    IReadonlyCollection = array.ToList(),
                                }
                            }
                        }
                    }
            };

?

Then having a query over this extra-level leaves would fail:

await query.Query<TypeWithInnerCollections>()
                .Where(x => x
                .IReadonlyCollectionOfInnerClasses
                .Any(y => y.IReadonlyCollectionOfInnerClasses
                .Where(e => e.Flatten == "onetwo")
                .Any())
                || x
                .IReadonlyCollectionOfInnerClasses
                .Any(y => y.IReadonlyCollectionOfInnerClasses
                .Where(e => e.Flatten == "twothree").Any())
                )
                .ToListAsync();

with exception stating that only Contains is allowed inside Any.

P.S. That's what I meant by asking about subqueries on Gitter: https://gitter.im/JasperFx/marten?at=5c66d8b64bafd97ca3f8f457

@oskardudycz

oskardudycz commented Feb 18, 2019 •

Copy link
Copy Markdown
Collaborator Author

@robert-skarzycki I agree, current support for InnerCollection queries is far from being ideal. I'll try to revisit that after I finish my work on NodaTime support. But the fix won't be quick, probably no sooner than in version 3.6...
For now as a workaround I would suggest to use getters with flattened strucures. It's workaround as this will make data denormalized but it should work for most of the cases.

@mysticmind

mysticmind commented Apr 27, 2019 •

Copy link
Copy Markdown
Member

@oskardudycz I have added CollectionToArrayJsonConverter<T> under Marten.Util and updated the unit tests to apply attribute [JsonConverter(typeof(Marten.Util.CollectionToArrayJsonConverter<TypeWithInnerCollections>))] on collection properties. All tests are green.

With this fix, we can close #1105 and #1192. We will also need to add some documentation on this.

@oskardudycz

Copy link
Copy Markdown
Collaborator Author

@mysticmind I made few additional improvements:

  • Refactored CollectionToArrayJsonConverter to not need generic parameter,
  • added JsonNetContractResolver to allow global setting of storing collections as arrays

Could you have a look and share your thoughts? 😄

@mysticmind

mysticmind commented Apr 28, 2019 •

Copy link
Copy Markdown
Member

@oskardudycz Looks like the tests are failing with the recent changes, can you please have a look?

@oskardudycz

Copy link
Copy Markdown
Collaborator Author

@mysticmind should be good now. Too long document class name ;)

@mysticmind mysticmind left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oskardudycz I have added few review comments. Please have a look.

@oskardudycz

Copy link
Copy Markdown
Collaborator Author

@mysticmind comments applied ;)

@mysticmind mysticmind left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me now.

@oskardudycz oskardudycz added this to the 3.6.0 milestone Apr 28, 2019
@jeremydmiller
jeremydmiller merged commit c7b86b3 into JasperFx:master May 7, 2019
@oskardudycz
oskardudycz deleted the 1105-addedTestsForInnerQueries branch May 11, 2019 10:53
elexisvenator pushed a commit to elexisvenator/marten that referenced this pull request Nov 2, 2026
Carries the correlation id a slice's events agree on onto its side effects
(JasperFx#1101, JasperFx#1105), correlation and causation on cloned events
(JasperFx#1107), SubscriptionDescriptor.PublishedTypes and the descriptor fixes
(JasperFx#1102, JasperFx#1106, JasperFx#1108), and the 2.97.1 fixes that never reached NuGet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants