From 3531ca4b8fe9c45deb8c30ff8192efd64fe26403 Mon Sep 17 00:00:00 2001
From: Sasa Zivkov <sasa.zivkov@sap.com>
Date: Wed, 10 Oct 2012 14:05:03 +0200
Subject: [PATCH] Use locate_helper to replace locate_* predicates

locate_submit_rule and locate_submit_filter are now refactored into
locate_helper. To make that easier, a noop_filter is introduced in order
to have a default filter implementation (same like default_submit is a
default implementation of submit rule) and avoid need to handle the case
of non-existing filter.

Also removed checks for non-null return values from locate_* predicates.
If a locate_* predicate returns null then this is bug in its
implementation.

Change-Id: I9cef2b6a01ac0288874154a898ddead2ea9446c3
Signed-off-by: Sasa Zivkov <sasa.zivkov@sap.com>
---
 .../server/project/SubmitRuleEvaluator.java   | 48 ++++++++-----------
 .../src/main/prolog/gerrit_common.pl          | 44 +++++++++--------
 2 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
index 41ed75b071..c18896e864 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
@@ -116,10 +116,6 @@ public class SubmitRuleEvaluator {
       env.set(StoredValues.CHANGE_CONTROL, changeControl);
 
       submitRule = env.once("gerrit", userRuleLocatorName, new VariableTerm());
-      if (submitRule == null) {
-        throw new RuleEvalException(userRuleLocatorName + " returned null");
-      }
-
       if (fastEvalLabels) {
         env.once("gerrit", "assume_range_from_label");
       }
@@ -160,30 +156,28 @@ public class SubmitRuleEvaluator {
         parentEnv.copyStoredValues(childEnv);
         Term filterRule =
             parentEnv.once("gerrit", filterRuleLocatorName, new VariableTerm());
-        if (filterRule != null) {
-          try {
-            if (fastEvalLabels) {
-              env.once("gerrit", "assume_range_from_label");
-            }
-
-            Term resultsTerm = toListTerm(results);
-            results.clear();
-            Term[] template =
-                parentEnv.once("gerrit", filterRuleWrapperName, filterRule,
-                    resultsTerm, new VariableTerm());
-            @SuppressWarnings("unchecked")
-            final List<? extends Term> termList =
-                ((ListTerm) template[2]).toJava();
-            results.addAll(termList);
-          } catch (PrologException err) {
-            throw new RuleEvalException("Exception calling " + filterRule
-                + " on change " + change.getId() + " of "
-                + parentState.getProject().getName(), err);
-          } catch (RuntimeException err) {
-            throw new RuleEvalException("Exception calling " + filterRule
-                + " on change " + change.getId() + " of "
-                + parentState.getProject().getName(), err);
+        try {
+          if (fastEvalLabels) {
+            env.once("gerrit", "assume_range_from_label");
           }
+
+          Term resultsTerm = toListTerm(results);
+          results.clear();
+          Term[] template =
+              parentEnv.once("gerrit", filterRuleWrapperName, filterRule,
+                  resultsTerm, new VariableTerm());
+          @SuppressWarnings("unchecked")
+          final List<? extends Term> termList =
+              ((ListTerm) template[2]).toJava();
+          results.addAll(termList);
+        } catch (PrologException err) {
+          throw new RuleEvalException("Exception calling " + filterRule
+              + " on change " + change.getId() + " of "
+              + parentState.getProject().getName(), err);
+        } catch (RuntimeException err) {
+          throw new RuleEvalException("Exception calling " + filterRule
+              + " on change " + change.getId() + " of "
+              + parentState.getProject().getName(), err);
         }
 
         parentState = parentState.getParentState();
diff --git a/gerrit-server/src/main/prolog/gerrit_common.pl b/gerrit-server/src/main/prolog/gerrit_common.pl
index a75acc0199..5ee08ea61b 100644
--- a/gerrit-server/src/main/prolog/gerrit_common.pl
+++ b/gerrit-server/src/main/prolog/gerrit_common.pl
@@ -153,6 +153,21 @@ is_all_ok([label(_, may(__)) | Ls]) :- is_all_ok(Ls).
 is_all_ok(_) :- fail.
 
 
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+%%
+%% locate_helper
+%%
+%%   Returns user:Func if it exists otherwise returns gerrit:Default
+
+locate_helper(Func, Default, Arity, user:Func) :-
+    '$compiled_predicate'(user, Func, Arity), !.
+locate_helper(Func, Default, Arity, user:Func) :-
+    listN(Arity, P), C =.. [Func | P], clause(user:C, _), !.
+locate_helper(Func, Default, _, gerrit:Default).
+
+listN(0, []).
+listN(N, [_|T]) :- N > 0, N1 is N - 1, listN(N1, T).
+
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %%
 %% locate_submit_rule/1:
@@ -164,17 +179,7 @@ is_all_ok(_) :- fail.
 %%
 
 locate_submit_rule(RuleName) :-
-  '$compiled_predicate'(user, submit_rule, 1),
-  !,
-  RuleName = user:submit_rule
-  .
-locate_submit_rule(RuleName) :-
-  clause(user:submit_rule(_), _),
-  !,
-  RuleName = user:submit_rule
-  .
-locate_submit_rule(RuleName) :-
-  RuleName = gerrit:default_submit.
+  locate_helper(submit_rule, default_submit, 1, RuleName).
 
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
@@ -316,14 +321,15 @@ call_submit_filter(X, R, S) :- F =.. [X, R, S], F.
 :- public locate_submit_filter/1.
 %%
 locate_submit_filter(FilterName) :-
-  '$compiled_predicate'(user, submit_filter, 2),
-  !,
-  FilterName = user:submit_filter
-  .
-locate_submit_filter(FilterName) :-
-  clause(user:submit_filter(_,_), _),
-  FilterName = user:submit_filter
-  .
+  locate_helper(submit_filter, noop_filter, 2, FilterName).
+
+%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
+%%
+%% noop_filter/2:
+%%
+:- public noop_filter/2.
+%%
+noop_filter(In, In).
 
 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%