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

MVELRuleDefinition should be public #206

Closed
sxcooler opened this issue Apr 15, 2019 · 10 comments
Closed

MVELRuleDefinition should be public #206

sxcooler opened this issue Apr 15, 2019 · 10 comments

Comments

@sxcooler
Copy link

sxcooler commented Apr 15, 2019

I wanna create rules directly from some in-memory data (like a Pojo or a Map) instead of a file, so I dig into your sources and I found this:
image
It's almost exactly what I want, but neither MVELRuleDefinitionReader nor MVELRuleDefinition is Public, so I could not use MVELRuleDefinition.create() to create my Rule.
So if this need is reasonable, please consider:

  1. expose these Classes
  2. provide a new method in MVELRuleFactory which take a Map as input.

I think 1 is more flexible so I could use my db as a rule store and provide a web UI to manage my rules, what about you guys?

@sxcooler
Copy link
Author

For now I just copy MVELRuleDefinition out and use the copy.

@fmbenhassine
Copy link
Member

I think you are referring to an old version of the code. After introducing SpEL support (See #204), the MVELRuleDefinition class has been refactored to RuleDefinition to be agnostic of any expression language. In the same way, a new RuleDefinitionReader API has been introduced as a strategy interface for rule definition readers (See JsonRuleDefinitionReader and YamlRuleDefinitionReader).

In your case, you can create a custom reader and use it with one of the rule factories (SpELRuleFactory or MVELRuleFactory). You can extend AbstractRuleDefinitionReader and override the protected RuleDefinition createRuleDefinition(Map<String, Object> map) method which is now protected.

Is this what you are looking for?

@sxcooler
Copy link
Author

I'm using 3.2.0 from maven, am I right?

@fmbenhassine
Copy link
Member

You are right, 3.2.0 is the current version. You are asking to make MVELRuleDefinition public. What I'm saying is that that class does not exist anymore in the upcoming release and is replaced with RuleDefinition which is now public.

The RuleDefinitionReader is also a public API so I think the upcoming version 3.3 actually already provides option 1) you mentioned: "expose these Classes".

Is that correct?

You can already try 3.3.0-SNAPSHOT and see if it works for you.

@sxcooler
Copy link
Author

Thank you, that's very kind.

@sxcooler
Copy link
Author

sxcooler commented Oct 31, 2019

I see this new RuleDefinition now...but it doesn't have a create() method.
And MVELRuleFactory's create methods (from RuleDefinition, not from a Reader) are still private.
image

Maybe I haven't explained my issue clearly last time.
What I demand is creating rules from various sources, e.g. a database (where I store my rule defs), not only files.
So plz consider making this createRule() method public, it could make easy-rules more flexable.

@sxcooler sxcooler reopened this Oct 31, 2019
@fmbenhassine
Copy link
Member

I see this new RuleDefinition now...but it doesn't have a create() method.

A rule definition is just the static definition of the rule, it does not have a create method. It is produced by a RuleDefinitionReader and consumed by rule factories to create rules (See its javadoc).

And MVELRuleFactory's create methods (from RuleDefinition, not from a Reader) are still private.

That's not true, you are still referring to an old version of the code. In v3.3, the method that creates a rule from a Reader is public, not private: https://www.javadoc.io/static/org.jeasy/easy-rules-mvel/3.3.0/org/jeasy/rules/mvel/MVELRuleFactory.html#createRule-java.io.Reader-

Here is the code from the v3.3.0 tag: https://github.com/j-easy/easy-rules/blob/easy-rules-3.3.0/easy-rules-mvel/src/main/java/org/jeasy/rules/mvel/MVELRuleFactory.java#L70

That said, you can use any Reader implementation (FileReader, StringReader, etc) to create rules from any resource.

@sxcooler
Copy link
Author

sxcooler commented Nov 6, 2019

I see this new RuleDefinition now...but it doesn't have a create() method.

A rule definition is just the static definition of the rule, it does not have a create method. It is produced by a RuleDefinitionReader and consumed by rule factories to create rules (See its javadoc).

And MVELRuleFactory's create methods (from RuleDefinition, not from a Reader) are still private.

That's not true, you are still referring to an old version of the code. In v3.3, the method that creates a rule from a Reader is public, not private: https://www.javadoc.io/static/org.jeasy/easy-rules-mvel/3.3.0/org/jeasy/rules/mvel/MVELRuleFactory.html#createRule-java.io.Reader-

Here is the code from the v3.3.0 tag: https://github.com/j-easy/easy-rules/blob/easy-rules-3.3.0/easy-rules-mvel/src/main/java/org/jeasy/rules/mvel/MVELRuleFactory.java#L70

That said, you can use any Reader implementation (FileReader, StringReader, etc) to create rules from any resource.

I'm sorry, is that My language isn't clear enough?
I mean these two method in my posted picture are private.
not those who take a Reader as param one.
Were you even reading?
Have u seen my picture? It's exactly 3.3 codes.

@sxcooler
Copy link
Author

sxcooler commented Nov 6, 2019

May I repeat in simple way:
I wanna create rules from definition directly!
I don't want to use a Reader!
Is my English too weird to understand?

@sxcooler
Copy link
Author

sxcooler commented Nov 6, 2019

I didn't make a PR because I haven't read all your codes, I'm not sure what change should be done to fit your whole design of this project (e.g. simply make these 2 method public).

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

No branches or pull requests

2 participants