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

Rename content type to ruleset #171

Merged
merged 2 commits into from
Oct 5, 2024

Conversation

luispfgarces
Copy link
Contributor

Description

Renames content type to ruleset and condition type to condition:

  • TContentType generic references renamed to TRuleset.
  • TConditionType generic references renamed to TCondition.
  • Method names referencing content type concept renamed to refer to the ruleset concept.
  • Method names referencing condition type concept renamed to refer to the condition concept.
  • Parameter names fixed to refer to new concepts.

Note

The concept of Ruleset is introduced to replace the content type concept. Functionally, it works the same way, but presents a simpler concept to new library users: a ruleset is a collection of related rules which are managed and evaluated together.

Warning

BREAKING CHANGE: the rule builder API was refactored to align with the Rule Query Language specification (being developed on a separate branch).

Take as example the RQL sentence:

create rule "Test rule" on ruleset "TexasHoldemPokerSingleCombinations"
set content "{\"BasePoints\":0,\"Combination\":\"Test combination\"}"
since $2023-01-01Z$ until $2023-12-31Z$
apply when @NumberOfAces >= 5
set priority top;

It would be written on rule builder API as:

var rule = Rule.Create("Test rule")
    .OnRuleset("TexasHoldemPokerSingleCombinations")
    .SetContent("{\"BasePoints\":0,\"Combination\":\"Test combination\"}")
    .Since(DateTime.Parse("2023-01-01Z"))
    .Until(DateTime.Parse("2023-12-31Z"))
    .ApplyWhen("NumberOfAces", Operators.GreaterOrEqualThan, 5)
    .Build().Rule;

var operationResult = await rulesEngine.AddRuleAsync(rule, RuleAddPriorityOption.AtTop);

Change checklist

  • Code follows the code rules guidelines of this project
  • Commit messages follow the commit rules of this project
  • I have self-reviewed my changes before submitting this pull request
  • I have covered new/changed code with new tests and/or adjusted existent ones
  • I have made changes necessary to update the documentation accordingly

Please also check the I want to contribute guidelines and make sure you have done accordingly.

Disclaimer

By sending us your contributions, you are agreeing that your contribution is made subject to the terms of our Contributor Ownership Statement

@luispfgarces luispfgarces added this to the Release 3.0.0 milestone Sep 1, 2024
@luispfgarces luispfgarces changed the title Draft: Rename content type to ruleset Rename content type to ruleset Sep 1, 2024
@luispfgarces luispfgarces marked this pull request as draft September 6, 2024 14:10
@luispfgarces luispfgarces mentioned this pull request Sep 24, 2024
5 tasks
@luispfgarces luispfgarces force-pushed the rename_content_type_to_ruleset branch 2 times, most recently from bbfecac to e3bbb2e Compare September 24, 2024 23:05
@luispfgarces luispfgarces marked this pull request as ready for review September 24, 2024 23:05
@luispfgarces luispfgarces force-pushed the rename_content_type_to_ruleset branch from e3bbb2e to 4bbd83b Compare September 25, 2024 12:05
.WithDatesInterval(dateBegin, dateEnd)
bool isActive = true) => Rule.Create<RulesetNames, ConditionNames>($"Multi rule for test {ruleset} {value}")
.OnRuleset(ruleset)
.SetContent(new { Value = value })
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a race condition in this method if a thread changes the object that was created, on the next match the object changed will be given to the new thread. Perhaps we can allow the dev to have a function to be able to give the thread a new object whenever the rule matches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but I will address that on a separate PR 👍

@luispfgarces luispfgarces merged commit 5555311 into release-3.0.0 Oct 5, 2024
1 check passed
@luispfgarces luispfgarces deleted the rename_content_type_to_ruleset branch October 5, 2024 18:17
@luispfgarces luispfgarces mentioned this pull request Oct 6, 2024
5 tasks
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 this pull request may close these issues.

3 participants