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

Enabled the possibility of applying projections with different Conjoined Tenancy scopes for projections. #2497

Merged
merged 6 commits into from
Mar 24, 2023

Conversation

oskardudycz
Copy link
Collaborator

@oskardudycz oskardudycz commented Feb 23, 2023

Summary

  1. Enabled the possibility of applying projections with different Conjoined Tenancy scopes for projections.
  2. Enabled global projection for events with a conjoined tenancy style.
  3. Added possibility to specify that document can be single tenanted (SingleTenanted option) when the global convention is to have multi-tenanted.

Scenarios

SESSION SLICE STORAGE RESULT
DEFAULT DEFAULT SINGLE THE SAME
DEFAULT DEFAULT CONJOINED THE SAME
DEFAULT NON-DEFAULT SINGLE THE SAME
DEFAULT NON-DEFAULT CONJOINED NEW NON-DEFAULT
NON-DEFAULT DEFAULT SINGLE NEW DEFAULT
NON-DEFAULT DEFAULT CONJOINED THE SAME
NON-DEFAULT NON-DEFAULT SINGLE NEW DEFAULT
NON-DEFAULT NON-DEFAULT CONJOINED THE SAME

Plus permutations related to:

  • session.SessionOptions.AllowAnyTenant
  • session.Options.Advanced.DefaultTenantUsageEnabled
  • session.DocumentStore.Options.Tenancy.IsTenantStoredInCurrentDatabase

Fixes #2363

Note: Suggested review with whitespace hidden https://github.com/JasperFx/marten/pull/2497/files?diff=split&w=1.

@oskardudycz oskardudycz changed the title Added failing scenario for the STJ issue with tenanted session Enable possibility to apply projections with different Conjoined Tenancy scope for projections Feb 23, 2023
@jeremydmiller
Copy link
Member

@oskardudycz I think the approach is fine, but I'm still saying we should isolate that one method that determines how to get the session and test the crap out of the permutations. Maybe change the signature so you can "push" the DocumentMapping/IDocumentStorage, parent session, and EventSlice. Might even move the logic to EventSlice itself to clean things up a little bit.

@oskardudycz oskardudycz force-pushed the funky_stj branch 4 times, most recently from e922167 to 1da8a2e Compare March 9, 2023 12:54
@oskardudycz oskardudycz changed the title Enable possibility to apply projections with different Conjoined Tenancy scope for projections Enabled the possibility of applying projections with different Conjoined Tenancy scopes for projections. Mar 9, 2023
@oskardudycz oskardudycz marked this pull request as ready for review March 9, 2023 13:08
@mysticmind
Copy link
Member

mysticmind commented Mar 9, 2023

@oskardudycz there looks to be a failing test EventSourcingTests.EventStreamUnexpectedMaxEventIdExceptionTransformTest.throw_transformed_exception_with_details_available [FAIL], can you please check? possibly a blinking test.

@oskardudycz
Copy link
Collaborator Author

@oskardudycz there looks to be a failing test EventSourcingTests.EventStreamUnexpectedMaxEventIdExceptionTransformTest.throw_transformed_exception_with_details_available [FAIL], can you please check? possibly a blinking test.

Yes, it's a blinking test. Not related to the changes.

@oskardudycz
Copy link
Collaborator Author

@oskardudycz I think the approach is fine, but I'm still saying we should isolate that one method that determines how to get the session and test the crap out of the permutations. Maybe change the signature so you can "push" the DocumentMapping/IDocumentStorage, parent session, and EventSlice. Might even move the logic to EventSlice itself to clean things up a little bit.

@jeremydmiller, I applied your suggestion 👍

@jeremydmiller
Copy link
Member

@oskardudycz I'm reviewing this again finally

Copy link
Member

@jeremydmiller jeremydmiller left a comment

Choose a reason for hiding this comment

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

Thanks for taking that. That one was nasty.

@oskardudycz
Copy link
Collaborator Author

Thanks, @jeremydmiller, for reviewing it; indeed, it was tricky. I had to spend some time writing on paper all those permutations 😅

@oskardudycz oskardudycz merged commit 5d1b853 into master Mar 24, 2023
@oskardudycz oskardudycz deleted the funky_stj branch March 24, 2023 08:52
@oskardudycz oskardudycz added this to the 6.0.0 milestone Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants