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

Fixes of event type sorting for pattern matching, encapsulating logic in EventTypePatternMatchFrame #2760

Merged
merged 2 commits into from
Nov 8, 2023

Conversation

svrx
Copy link
Contributor

@svrx svrx commented Oct 22, 2023

After upgrade to version 6 some issue have been identified with compilation of generated code.
When a projection depends on events from different levels of the same inheritance chain, the sorting of methods performed during code generation has been found to be non-deterministic.

See more in #2749.

Current PR provides fixes and tests for such scenario.

… in `EventTypePatternMatchFrame`

commit 794bd44
Author: Sergio Amaral <sergio.a.amaral@gmail.com>
Date:   Mon Oct 23 00:33:20 2023 +0100

    Updates tests to include interfaces scenario

commit d116879
Author: Sergio Amaral <samaral@spencerstuart.com>
Date:   Fri Oct 20 17:04:33 2023 +0100

    Updates order to take into consideration type and interface hierarchies

commit 45f416a
Author: Sergio Amaral <samaral@spencerstuart.com>
Date:   Thu Oct 19 17:12:34 2023 +0100

    Implements of EventTypeComparer to version that pass all combination and permutation tests

    Note: Only consider event type hierarchy, overlooking any interfaces implemented by event types.

commit fc68200
Author: Sergio Amaral <samaral@spencerstuart.com>
Date:   Thu Oct 19 16:49:25 2023 +0100

    Adds expectation that sort has same elements as original collection

commit 06ab305
Author: Jeremy D. Miller <jeremydmiller@yahoo.com>
Date:   Thu Oct 19 08:40:13 2023 -0500

    adjustment to the new event handler frame ordering fix

commit 35d2a5c
Author: Sergio Amaral <samaral@spencerstuart.com>
Date:   Mon Oct 16 11:01:32 2023 +0100

    Fixes minor repetition in test class name

commit dc8578c
Author: Sergio Amaral <samaral@spencerstuart.com>
Date:   Mon Oct 16 10:57:51 2023 +0100

    Simplifies of event type sorting for pattern matching by encapsulating logic in `EventTypePatternMatchFrame`

    Adds support for having event types as interfaces of that is ever a valid scenario.
    Adjust tests in accordance to functional changes.

commit b72a431
Author: Sergio Amaral <samaral@spencerstuart.com>
Date:   Fri Oct 13 18:17:18 2023 +0100

    Fixes `EventTypeComparer` implementation to reliably sort types for codegen and implements extensive sorting tests

    Disclaimer: Comparer implementation may not work properly if interface types are used in projections.
    Current implementation relies on using entire type hierarchy for sorting, starting from base type, within same hierarchy level type name sort is used.

commit 83eb9e2
Author: Sergio Amaral <samaral@spencerstuart.com>
Date:   Wed Oct 11 15:20:16 2023 +0100

    Adds test scenario to repoduce non-deterministic code generation issue (will only fail sometimes)
@svrx
Copy link
Contributor Author

svrx commented Oct 22, 2023

After discussion with @jeremydmiller, still pending providing an integration test for the scenario, but submitting here for review of implementation and potential consideration for the upcoming 6.3.0 release.

…es in projections

Confirms that order of generated code is correct, processing the most specific registered type to the event generated.
@svrx
Copy link
Contributor Author

svrx commented Oct 23, 2023

Integration test covering this scenario has been added, please let me know if it meets the expectations.

@jeremydmiller jeremydmiller merged commit 854f307 into JasperFx:master Nov 8, 2023
13 checks passed
@svrx
Copy link
Contributor Author

svrx commented Nov 8, 2023

Thanks for merging this one. When can I expect a release with it included?

@oskardudycz
Copy link
Collaborator

@svrx, I think that it should be available in 6.4.1

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.

None yet

3 participants