Running the Javascript rules
Jesus Rodriguez
jesusr at redhat.com
Tue Mar 16 13:54:28 UTC 2010
On Mon, Mar 15, 2010 at 07:24:37PM -0400, Adam Young wrote:
> I added a new Constructor to JavascriptEnforcer:
>
>
> public JavascriptEnforcer(DateSource dateSource, Reader rulesReader,
> PreEntHelper preHelper, PostEntHelper postHelper,
> ProductServiceAdapter prodAdapter, ScriptEngine jsEngine)
I like it with the Reader. +1
> I also left the old one, but changed its implementation to call the new one:
> public JavascriptEnforcer(DateSource dateSource, RulesCurator
> rulesCurator,
> PreEntHelper preHelper, PostEntHelper postHelper,
> ProductServiceAdapter prodAdapter) {
>
> this(dateSource, new
> StringReader(rulesCurator.getRules().getRules()),
> preHelper, postHelper, prodAdapter, new ScriptEngineManager()
> .getEngineByName("JavaScript"));
> }
Why can't we just whack this ctor? Is it still needed?
> However, this type of construction code is really no appropriate a class
> like this. Jesus mentioned that he feels the same way about
> ProductServiceAdapter, which I will look at in a little bit.
+1
> With those changes, I can make a stand alone unit test that runs without
> any Hibernate support, at least for calling
> public Pool selectBestPool(Consumer consumer, String productId,
> List<Pool> pools);
Very nice.
> The next piece of trickiness is loading in the Javascript file. For
> static deployments, the usual way to find a text file like this is:
>
> URL url =
> this.getClass().getClassLoader().getResource("rules/default-rules.js");
> InputStreamReader inputStreamReader = new
> InputStreamReader(url.openStream());
>
> Which works fine so long as the resource is on the classpath. Thus for
> the Junit test, I added a file CLASSPATH extension which was
> "target/test/resources". Since it looks like we are regenerating the
> .project file from git sources, this might not live across check-ins.
IIRC I removed .project from the checkin altogether, it is now
generated and no longer committed.
> I'm thinkin that if our goal is to make the Javascript something that
> the end use is responsible for editing and deploying, we need to provide
> the user with a tool to go through the edit-deploy-debug cycle. Are we
> planning on doing that as part of the web app? Are we going to allow
> the user to deploy multiple .js rules files, and indicate which is the
> "live" one?
Not 100% sure we've decided the .js files would be user editable. I
guess from an OSS project consumer they'd want to create their own
rules and deploy them, so those folks will need this level of support.
> Both patch and html diff are are attached.
>
[snip remove html diff] comments inline
> >From 510de733bd92039cff551140b7b1060b828f58de Mon Sep 17 00:00:00 2001
> From: Adam Young <ayoung at redhat.com>
> Date: Mon, 15 Mar 2010 19:23:16 -0400
> Subject: [PATCH] Added unit test for default rules, and modified classes the unit test to run outside of the container.
>
> ---
> .../candlepin/controller/Entitler.java | 58 ++++++++------
> .../candlepin/policy/js/JavascriptEnforcer.java | 84 +++++++++++--------
> proxy/src/main/resources/rules/default-rules.js | 34 ++++++--
> .../candlepin/policy/test/DefaultRulesTest.java | 83 +++++++++++++++++++
> 4 files changed, 192 insertions(+), 67 deletions(-)
> create mode 100644 proxy/src/test/java/org/fedoraproject/candlepin/policy/test/DefaultRulesTest.java
>
> diff --git a/proxy/src/main/java/org/fedoraproject/candlepin/controller/Entitler.java b/proxy/src/main/java/org/fedoraproject/candlepin/controller/Entitler.java
> index 5f5a060..19264b3 100644
> --- a/proxy/src/main/java/org/fedoraproject/candlepin/controller/Entitler.java
> +++ b/proxy/src/main/java/org/fedoraproject/candlepin/controller/Entitler.java
> @@ -37,17 +37,17 @@ import java.util.Date;
> * Entitler
> */
> public class Entitler {
This one looked ok to me.
[body snipped]
> diff --git a/proxy/src/main/java/org/fedoraproject/candlepin/policy/js/JavascriptEnforcer.java b/proxy/src/main/java/org/fedoraproject/candlepin/policy/js/JavascriptEnforcer.java
> index a00e0a3..123ee74 100644
> --- a/proxy/src/main/java/org/fedoraproject/candlepin/policy/js/JavascriptEnforcer.java
> +++ b/proxy/src/main/java/org/fedoraproject/candlepin/policy/js/JavascriptEnforcer.java
> @@ -14,7 +14,6 @@
> */
> package org.fedoraproject.candlepin.policy.js;
>
> -
> import org.fedoraproject.candlepin.model.Consumer;
> import org.fedoraproject.candlepin.model.Entitlement;
> import org.fedoraproject.candlepin.model.Pool;
> @@ -45,7 +44,7 @@ public class JavascriptEnforcer implements Enforcer {
>
> private static Logger log = Logger.getLogger(JavascriptEnforcer.class);
> private DateSource dateSource;
> - private RulesCurator rulesCurator;
> +
> private ProductServiceAdapter prodAdapter;
> private PreEntHelper preHelper;
> private PostEntHelper postHelper;
> @@ -55,29 +54,36 @@ public class JavascriptEnforcer implements Enforcer {
> private static final String PRE_PREFIX = "pre_";
> private static final String POST_PREFIX = "post_";
> private static final String SELECT_POOL_PREFIX = "select_pool_";
> - private static final String GLOBAL_SELECT_POOL_FUNCTION = SELECT_POOL_PREFIX + "global";
> + private static final String GLOBAL_SELECT_POOL_FUNCTION = SELECT_POOL_PREFIX +
> + "global";
> private static final String GLOBAL_PRE_FUNCTION = PRE_PREFIX + "global";
> private static final String GLOBAL_POST_FUNCTION = POST_PREFIX + "global";
>
> @Inject
> - public JavascriptEnforcer(DateSource dateSource,
> - RulesCurator rulesCurator, PreEntHelper preHelper,
> - PostEntHelper postHelper, ProductServiceAdapter prodAdapter) {
> + public JavascriptEnforcer(DateSource dateSource, RulesCurator rulesCurator,
> + PreEntHelper preHelper, PostEntHelper postHelper,
> + ProductServiceAdapter prodAdapter) {
> + this(dateSource, new StringReader(rulesCurator.getRules().getRules()),
> + preHelper, postHelper, prodAdapter, new ScriptEngineManager()
> + .getEngineByName("JavaScript"));
> + }
I'd get rid of the above ctor altogether.
> +
> + @Inject
> + public JavascriptEnforcer(DateSource dateSource, Reader rulesReader,
> + PreEntHelper preHelper, PostEntHelper postHelper,
> + ProductServiceAdapter prodAdapter, ScriptEngine jsEngine) {
> this.dateSource = dateSource;
> - this.rulesCurator = rulesCurator;
> this.preHelper = preHelper;
> this.postHelper = postHelper;
> this.prodAdapter = prodAdapter;
> -
> - ScriptEngineManager mgr = new ScriptEngineManager();
> - jsEngine = mgr.getEngineByName("JavaScript");
> + this.jsEngine = jsEngine;
Why do we pass in the ScriptEngine? Do we intend to have the JavascriptEnforcer
use a different engine aside from JavaScript? Seems odd to pass in say
a Jython ScriptEngine.
I guess for "testing" you could supply a mock ScriptEngine, but this is where
I find coding for testing to be bad, because we are now stating that
the JavascriptEnforcer can use ANY ScriptEngine which I don't think is
true.
> +
> if (jsEngine == null) {
> - throw new RuntimeException("No Javascript engine found");
> + throw new RuntimeException("No Javascript engine");
> }
>
> try {
> - Reader reader = new StringReader(this.rulesCurator.getRules().getRules());
> - jsEngine.eval(reader);
> + this.jsEngine.eval(rulesReader);
> }
> catch (ScriptException ex) {
> throw new RuleParseException(ex);
> @@ -90,23 +96,24 @@ public class JavascriptEnforcer implements Enforcer {
> runPre(preHelper, consumer, entitlementPool);
>
> if (entitlementPool.isExpired(dateSource)) {
> - preHelper.getResult().addError(new ValidationError("Entitlements for " +
> - entitlementPool.getProductId() +
> - " expired on: " + entitlementPool.getEndDate()));
> + preHelper.getResult().addError(
> + new ValidationError("Entitlements for " +
> + entitlementPool.getProductId() + " expired on: " +
> + entitlementPool.getEndDate()));
> return preHelper;
> }
>
> return preHelper;
> }
>
> - private void runPre(PreEntHelper preHelper, Consumer consumer,
> - Pool pool) {
> + private void runPre(PreEntHelper preHelper, Consumer consumer, Pool pool) {
> Invocable inv = (Invocable) jsEngine;
> String productId = pool.getProductId();
>
> // Provide objects for the script:
> jsEngine.put("consumer", new ReadOnlyConsumer(consumer));
> - jsEngine.put("product", new ReadOnlyProduct(prodAdapter.getProductById(productId)));
> + jsEngine.put("product", new ReadOnlyProduct(prodAdapter
> + .getProductById(productId)));
> jsEngine.put("pool", new ReadOnlyEntitlementPool(pool));
> jsEngine.put("pre", preHelper);
>
> @@ -151,12 +158,13 @@ public class JavascriptEnforcer implements Enforcer {
>
> // Provide objects for the script:
> jsEngine.put("consumer", new ReadOnlyConsumer(c));
> - jsEngine.put("product", new ReadOnlyProduct(prodAdapter.getProductById(productId)));
> + jsEngine.put("product", new ReadOnlyProduct(prodAdapter
> + .getProductById(productId)));
> jsEngine.put("post", postHelper);
> jsEngine.put("entitlement", new ReadOnlyEntitlement(ent));
>
> - log.debug("Running post-entitlement rules for: " + c.getUuid() + " product: " +
> - pool.getProductId());
> + log.debug("Running post-entitlement rules for: " + c.getUuid() +
> + " product: " + pool.getProductId());
>
> try {
> inv.invokeFunction(POST_PREFIX + productId);
> @@ -182,13 +190,13 @@ public class JavascriptEnforcer implements Enforcer {
> }
> }
>
> - public Pool selectBestPool(Consumer consumer, String productId, List<Pool> pools) {
> + public Pool selectBestPool(Consumer consumer, String productId,
> + List<Pool> pools) {
>
> Invocable inv = (Invocable) jsEngine;
>
> log.info("Selecting best entitlement pool for product: " + productId);
> - List<ReadOnlyEntitlementPool> readOnlyPools =
> - new LinkedList<ReadOnlyEntitlementPool>();
> + List<ReadOnlyEntitlementPool> readOnlyPools = new LinkedList<ReadOnlyEntitlementPool>();
> for (Pool p : pools) {
> log.info(" " + p);
> readOnlyPools.add(new ReadOnlyEntitlementPool(p));
> @@ -199,20 +207,23 @@ public class JavascriptEnforcer implements Enforcer {
>
> ReadOnlyEntitlementPool result = null;
> try {
> - result = (ReadOnlyEntitlementPool) inv.invokeFunction(
> - SELECT_POOL_PREFIX + productId);
> - log.info("Excuted javascript rule: " + SELECT_POOL_PREFIX + productId);
> + result = (ReadOnlyEntitlementPool) inv
> + .invokeFunction(SELECT_POOL_PREFIX + productId);
> + log.info("Excuted javascript rule: " + SELECT_POOL_PREFIX +
> + productId);
> }
> catch (NoSuchMethodException e) {
> // No method for this product, try to find a global function, if
> // neither exists this is ok and we'll just carry on.
> try {
> - result = (ReadOnlyEntitlementPool) inv.invokeFunction(
> + result = (ReadOnlyEntitlementPool) inv
> + .invokeFunction(GLOBAL_SELECT_POOL_FUNCTION);
> + log.info("Excuted javascript rule: " +
> GLOBAL_SELECT_POOL_FUNCTION);
> - log.info("Excuted javascript rule: " + GLOBAL_SELECT_POOL_FUNCTION);
> }
> catch (NoSuchMethodException ex) {
> - log.warn("No default rule found: " + GLOBAL_SELECT_POOL_FUNCTION);
> + log.warn("No default rule found: " +
> + GLOBAL_SELECT_POOL_FUNCTION);
> log.warn("Resorting to default pool selection behavior.");
> return selectBestPoolDefault(pools);
> }
> @@ -225,8 +236,8 @@ public class JavascriptEnforcer implements Enforcer {
> }
>
> if (pools.size() > 0 && result == null) {
> - throw new RuleExecutionException("Rule did not select a pool for product: " +
> - productId);
> + throw new RuleExecutionException(
> + "Rule did not select a pool for product: " + productId);
> }
>
> for (Pool p : pools) {
> @@ -240,8 +251,11 @@ public class JavascriptEnforcer implements Enforcer {
> }
>
> /**
> - * Default behavior if no product specific and no global pool select rules exist.
> - * @param pools Pools to choose from.
> + * Default behavior if no product specific and no global pool select rules
> + * exist.
> + *
> + * @param pools
> + * Pools to choose from.
> * @return First pool in the list. (default behavior)
> */
> private Pool selectBestPoolDefault(List<Pool> pools) {
> diff --git a/proxy/src/main/resources/rules/default-rules.js b/proxy/src/main/resources/rules/default-rules.js
> index 71de8f2..7caf73b 100644
> --- a/proxy/src/main/resources/rules/default-rules.js
> +++ b/proxy/src/main/resources/rules/default-rules.js
> @@ -12,7 +12,8 @@ function virtualization_common() {
> return;
> }
>
> - // Host must not have any guests currently (could be changed but for simplicities sake):
> + // Host must not have any guests currently (could be changed but for
> + // simplicities sake):
> if (parseInt(consumer.getFact("total_guests")) > 0) {
> pre.addError("rulefailed.host.already.has.guests");
> }
> @@ -36,16 +37,16 @@ function post_virtualization_host_platform() {
>
> function pre_global() {
>
> - // Support free entitlements for guests, if their parent has virt host or platform,
> + // Support free entitlements for guests, if their parent has virt host or
> + // platform,
> // and is entitled to the product the guest is requesting:
> if (consumer.getType() == "virt_system" && consumer.getParent() != null) {
> - if ((consumer.getParent().hasEntitlement("virtualization_host") ||
> - consumer.getParent().hasEntitlement("virtualization_host_platform")) &&
> - consumer.getParent().hasEntitlement(product.getLabel())) {
> + if ((consumer.getParent().hasEntitlement("virtualization_host") || consumer
> + .getParent().hasEntitlement("virtualization_host_platform"))
> + && consumer.getParent().hasEntitlement(product.getLabel())) {
> pre.grantFreeEntitlement();
> }
> - }
> - else {
> + } else {
> pre.checkQuantity(pool);
> }
> }
> @@ -53,3 +54,22 @@ function pre_global() {
> function post_global() {
>
> }
> +
> +function select_pool_global() {
> + return pools.getFirst();
> +}
> +
> +function select_pool_monitoring() {
> + var pool;
> + var poolItr;
> +
> + for (poolItr = pools.iterator(); poolItr.hasNext();) {
> + pool = poolItr.next();
> + if (pool.productId == "monitoring") {
> + return pool;
> + }
> + }
> +
> + return pools.getFirst();
> +
> +}
> diff --git a/proxy/src/test/java/org/fedoraproject/candlepin/policy/test/DefaultRulesTest.java b/proxy/src/test/java/org/fedoraproject/candlepin/policy/test/DefaultRulesTest.java
> new file mode 100644
> index 0000000..1af7eb8
> --- /dev/null
> +++ b/proxy/src/test/java/org/fedoraproject/candlepin/policy/test/DefaultRulesTest.java
> @@ -0,0 +1,83 @@
> +/**
> + * Copyright (c) 2009 Red Hat, Inc.
> + *
> + * This software is licensed to you under the GNU General Public License,
> + * version 2 (GPLv2). There is NO WARRANTY for this software, express or
> + * implied, including the implied warranties of MERCHANTABILITY or FITNESS
> + * FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
> + * along with this software; if not, see
> + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
> + *
> + * Red Hat trademarks are not licensed under GPLv2. No permission is
> + * granted to use or replicate Red Hat trademarks that are incorporated
> + * in this software or its documentation.
> + */
> +package org.fedoraproject.candlepin.policy.test;
> +
> +import java.io.File;
> +import java.io.FileNotFoundException;
> +import java.io.FileReader;
> +import java.io.IOException;
> +import java.io.InputStream;
> +import java.io.InputStreamReader;
> +import java.io.Reader;
> +import java.net.URL;
> +import java.util.ArrayList;
> +import java.util.Date;
> +import java.util.List;
> +
> +import javax.script.ScriptEngineManager;
> +
> +import junit.framework.Assert;
> +
> +import org.fedoraproject.candlepin.model.Consumer;
> +import org.fedoraproject.candlepin.model.Pool;
> +import org.fedoraproject.candlepin.model.RulesCurator;
> +import org.fedoraproject.candlepin.policy.Enforcer;
> +import org.fedoraproject.candlepin.policy.js.JavascriptEnforcer;
> +import org.fedoraproject.candlepin.policy.js.PostEntHelper;
> +import org.fedoraproject.candlepin.policy.js.PreEntHelper;
> +import org.fedoraproject.candlepin.service.ProductServiceAdapter;
> +import org.fedoraproject.candlepin.util.DateSource;
> +import org.junit.Before;
> +import org.junit.Test;
> +
> +import com.sun.jersey.spi.StringReader;
why are we using this StringReader?
> +
> +/**
> + * DefaultRulesTest
> + */
> +public class DefaultRulesTest {
> + private Enforcer enforcer;
> + private DateSource dateSource;
> + private PreEntHelper preHelper;
> + private PostEntHelper postHelper;
> + private ProductServiceAdapter prodAdapter;
> +
> + @Before
> + public void createEnforcer() throws IOException {
> + // InputStream inStream = this
> + URL url = this.getClass().getClassLoader().getResource("rules/default-rules.js");
> +
> + InputStreamReader inputStreamReader = new InputStreamReader(url
> + .openStream());
> +
> + enforcer = new JavascriptEnforcer(dateSource, inputStreamReader,
> + preHelper, postHelper, prodAdapter, new ScriptEngineManager()
> + .getEngineByName("JavaScript"));
> + }
> +
> + @Test
> + public void runDefaultRuels() {
> + Consumer consumer = new Consumer();
> + String productId = "Shampoo";
> + Pool pool = new Pool();
> + pool.setId(new Long(0));
> + pool.setProductId("default");
> + List<Pool> pools = new ArrayList<Pool>();
> + pools.add(pool);
> + Pool selected = enforcer.selectBestPool(consumer, productId, pools);
> + Assert.assertNotNull(selected);
> + }
> +
> +}
> --
> 1.6.6.1
>
--
jesus m. rodriguez | jesusr at redhat.com
principal software engineer | irc: zeus
red hat systems management | 919.754.4413 (w)
rhce # 805008586930012 | 919.623.0080 (c)
+---------------------------------------------+
| "Those who cannot remember the past |
| are condemned to repeat it." |
| -- George Santayana |
+---------------------------------------------+
More information about the candlepin
mailing list