Move PrologEnvironment creation out of ProjectState

The PrologMachineCopy field in ProjectState was an obtuse way of
upgrading the weak reference stored in the RulesCache to a strong
reference. Now that RulesCache stores strong references on its own, we
can just call into RulesCache at exactly the time it's needed, namely in
PrologRuleEvaluator.

Moving this out of ProjectState helps with untangling the cacheable and
non-cacheable pieces of this class; for context, see:
https://gerrit-review.googlesource.com/c/gerrit/+/167053

Change-Id: I4beda2c9ddcf0014c606c0bee01ec49dde419361

java/com/google/gerrit/server/project/ProjectState.java

@@ -47,14 +47,9 @@ import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.git.BranchOrderSection;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.rules.PrologEnvironment;
-import com.google.gerrit.server.rules.RulesCache;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
-import com.googlecode.prolog_cafe.exceptions.CompileException;
-import com.googlecode.prolog_cafe.lang.PrologMachineCopy;
 import java.io.IOException;
-import java.io.Reader;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -87,18 +82,13 @@ public class ProjectState {
   private final SitePaths sitePaths;
   private final AllProjectsName allProjectsName;
   private final ProjectCache projectCache;
-  private final PrologEnvironment.Factory envFactory;
   private final GitRepositoryManager gitMgr;
-  private final RulesCache rulesCache;
   private final List<CommentLinkInfo> commentLinks;
 
   private final ProjectConfig config;
   private final Map<String, ProjectLevelConfig> configs;
   private final Set<AccountGroup.UUID> localOwners;
 
-  /** Prolog rule state. */
-  private volatile PrologMachineCopy rulesMachine;
-
   /** Last system time the configuration's revision was examined. */
   private volatile long lastCheckGeneration;
 
@@ -120,9 +110,7 @@ public class ProjectState {
       final ProjectCache projectCache,
       final AllProjectsName allProjectsName,
       final AllUsersName allUsersName,
-      final PrologEnvironment.Factory envFactory,
       final GitRepositoryManager gitMgr,
-      final RulesCache rulesCache,
       final List<CommentLinkInfo> commentLinks,
       final CapabilityCollection.Factory limitsFactory,
       @Assisted final ProjectConfig config) {
@@ -131,9 +119,7 @@ public class ProjectState {
     this.isAllProjects = config.getProject().getNameKey().equals(allProjectsName);
     this.isAllUsers = config.getProject().getNameKey().equals(allUsersName);
     this.allProjectsName = allProjectsName;
-    this.envFactory = envFactory;
     this.gitMgr = gitMgr;
-    this.rulesCache = rulesCache;
     this.commentLinks = commentLinks;
     this.config = config;
     this.configs = new HashMap<>();
@@ -215,29 +201,6 @@ public class ProjectState {
         .anyMatch(Objects::nonNull);
   }
 
-  /** @return Construct a new PrologEnvironment for the calling thread. */
-  public PrologEnvironment newPrologEnvironment() throws CompileException {
-    PrologMachineCopy pmc = rulesMachine;
-    if (pmc == null) {
-      pmc = rulesCache.loadMachine(getNameKey(), config.getRulesId());
-      rulesMachine = pmc;
-    }
-    return envFactory.create(pmc);
-  }
-
-  /**
-   * Like {@link #newPrologEnvironment()} but instead of reading the rules.pl read the provided
-   * input stream.
-   *
-   * @param name a name of the input stream. Could be any name.
-   * @param in stream to read prolog rules from
-   * @throws CompileException
-   */
-  public PrologEnvironment newPrologEnvironment(String name, Reader in) throws CompileException {
-    PrologMachineCopy pmc = rulesCache.loadMachine(name, in);
-    return envFactory.create(pmc);
-  }
-
   public Project getProject() {
     return config.getProject();
   }

java/com/google/gerrit/server/rules/PrologRuleEvaluator.java

@@ -40,6 +40,7 @@ import com.googlecode.prolog_cafe.exceptions.ReductionLimitException;
 import com.googlecode.prolog_cafe.lang.IntegerTerm;
 import com.googlecode.prolog_cafe.lang.ListTerm;
 import com.googlecode.prolog_cafe.lang.Prolog;
+import com.googlecode.prolog_cafe.lang.PrologMachineCopy;
 import com.googlecode.prolog_cafe.lang.StructureTerm;
 import com.googlecode.prolog_cafe.lang.SymbolTerm;
 import com.googlecode.prolog_cafe.lang.Term;
@@ -79,6 +80,8 @@ public class PrologRuleEvaluator {
   private final AccountCache accountCache;
   private final Accounts accounts;
   private final Emails emails;
+  private final RulesCache rulesCache;
+  private final PrologEnvironment.Factory envFactory;
   private final ChangeData cd;
   private final ProjectState projectState;
   private final SubmitRuleOptions opts;
@@ -89,12 +92,16 @@ public class PrologRuleEvaluator {
       AccountCache accountCache,
       Accounts accounts,
       Emails emails,
+      RulesCache rulesCache,
+      PrologEnvironment.Factory envFactory,
       ProjectCache projectCache,
       @Assisted ChangeData cd,
       @Assisted SubmitRuleOptions options) {
     this.accountCache = accountCache;
     this.accounts = accounts;
     this.emails = emails;
+    this.rulesCache = rulesCache;
+    this.envFactory = envFactory;
     this.cd = cd;
     this.opts = options;
 
@@ -440,11 +447,15 @@ public class PrologRuleEvaluator {
   private PrologEnvironment getPrologEnvironment() throws RuleEvalException {
     PrologEnvironment env;
     try {
+      PrologMachineCopy pmc;
       if (opts.rule() == null) {
-        env = projectState.newPrologEnvironment();
+        pmc =
+            rulesCache.loadMachine(
+                projectState.getNameKey(), projectState.getConfig().getRulesId());
       } else {
-        env = projectState.newPrologEnvironment("stdin", new StringReader(opts.rule()));
+        pmc = rulesCache.loadMachine("stdin", new StringReader(opts.rule()));
       }
+      env = envFactory.create(pmc);
     } catch (CompileException err) {
       String msg;
       if (opts.rule() == null) {
@@ -477,7 +488,10 @@ public class PrologRuleEvaluator {
     for (ProjectState parentState : projectState.parents()) {
       PrologEnvironment parentEnv;
       try {
-        parentEnv = parentState.newPrologEnvironment();
+        parentEnv =
+            envFactory.create(
+                rulesCache.loadMachine(
+                    parentState.getNameKey(), parentState.getConfig().getRulesId()));
       } catch (CompileException err) {
         throw new RuleEvalException("Cannot consult rules.pl for " + parentState.getName(), err);
       }

javatests/com/google/gerrit/server/permissions/RefControlTest.java

@@ -61,8 +61,6 @@ import com.google.gerrit.server.project.ProjectConfig;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.project.RefPattern;
 import com.google.gerrit.server.project.testing.Util;
-import com.google.gerrit.server.rules.PrologEnvironment;
-import com.google.gerrit.server.rules.RulesCache;
 import com.google.gerrit.server.schema.SchemaCreator;
 import com.google.gerrit.server.util.RequestContext;
 import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -949,8 +947,6 @@ public class RefControlTest {
   }
 
   private InMemoryRepository add(ProjectConfig pc) {
-    PrologEnvironment.Factory envFactory = null;
-    RulesCache rulesCache = null;
     SitePaths sitePaths = null;
     List<CommentLinkInfo> commentLinks = null;
 
@@ -970,9 +966,7 @@ public class RefControlTest {
             projectCache,
             allProjectsName,
             allUsersName,
-            envFactory,
             repoManager,
-            rulesCache,
             commentLinks,
             capabilityCollectionFactory,
             pc));