search

deliveredtechnologies / rulebook

100% Java, Lambda Enabled, Lightweight Rules Engine with a Simple and Intuitive DSL
http://www.deliveredtechnologies.com
Apache License 2.0
717 stars 124 forks source link

Added Spring context awareness support #133

Closed rizjoj closed 6 years ago

rizjoj commented 6 years ago

Hi @Clayton7510 - PR is more for discussion than merging/approval. Let me know what you think.

rizjoj commented 6 years ago

Usage: Everything else stays the same (follow documentation sections 5.3 and 5.4), except annotate just the rules you want spring aware with @Component (you don't have to annotate all rules), and within the rules now @Autowire injection, etc. works. No longer need to introduce your spring beans in the factMap and fetch them via @Given.

I've tested it for my use-cases and also tested backward compatibility and they work. I haven't added unit tests though. I thought I'd run it by you first.

Update: just came across the discussions in issues #111 and #119. I didn't realize SpringRuleBook was meant to be used with @RuleBean. I'll try it - maybe this PR is moot then, which is fine. (probably an update in documentation would help)

Clayton7510 commented 6 years ago

I do like the idea of being able to support @Autowire inside of POJO beans. This has been discussed a bit. The only issue I have is that I wouldn't want it bound to all RuleBookRunner's, like what would happen if you add context awareness to AbstractRuleBookRunner.

Instead, perhaps a different RuleBookRunner (i.e. SpringAwareRuleBookRunner) in rulebook-spring could be added since it would be specific to Spring. This also has the benefit of not tying rulebook-core to spring dependencies, which I think is important. Some people do use rulebook without spring (e.g. JavaEE and Guice).

Aside from those couple of items, I think where you are going is great!

Clayton7510 commented 6 years ago

Oh, and as far as @RuleBean goes... originally RuleBook was not thread safe and RuleBookRunner wired up individual rules. So, @RuleBean made POJO rules Spring @Component's and it also made them prototype scoped. Now, RuleBook is thread safe (facts are no longer state bound to Rules or RuleBooks), so RuleBean is a bit less useful. But it could be updated to be used for this purpose. I think probably the only thing necessary would be to remove the prototype scope, since forcing a prototype scope is really no longer needed.

rizjoj commented 6 years ago

Superceeded by PR #134

lanegoolsby commented 6 years ago

I was able to get @Autowire to work by adding both @Rule and @RuleBean annotations to my POJO classes. I couldn't get Autowire to work with POJO classes in a Spring Boot app until I did that. I may be oversimplfying (or setting myself up for disaster) but it seems the reason @Rule POJO's don't work with Spring injection is because the reflective call to find the POJO objects only searches for objects that implement @Rule. This all might be solvable by just making @RuleBean implement @Rule.

Clayton7510 commented 6 years ago

@lanegoolsby, @RuleBean does extend @Rule. The reason why you needed @Rule and @RuleBean annotations to make @Autowire work was because @RuleBean also extends @Component, which is what Spring needs to @Autowire inject beans. The reason why it doesn’t just work with @RuleBean is partly because of how annotations are resolved via Reflection and partly because @RuleBean hasn’t really been touched in a while.

The plan is to fix it so that you would only need @RuleBean for Spring aware POJO rules.

Thanks for the comment. Look for this to be updated within the next week.