Ensure All-Users:refs/meta/external-ids doesn't cause split-brains

The DESIRED policy was introduced in the early stages of the multi-site
to work around locking problems with the global ref-database, which have
later been fixed with the introduction of a locking mechanism[1].

Due to the concurrent nature of All-Users:refs/meta/external-ids, the
DESIRED policy had been applied by default to avoid causing continuous
client failures.

The DESIRED policy however, is obsolete and prone to cause spit-brains
due to the fact that nothing, really, is done to prevent them: this
should not be applied to a crucial core ref such as
refs/meta/external-ids.

Remove DESIRED policy (that was only used for
All-Users:refs/meta/external-ids) and instead make sure that it is
REQUIRED to be in sync with the global ref-database.

[1] I5395c22ffdf87fcc3247ea9a5d3afc57df60cfe9

Bug: Issue 13462
Change-Id: I6c8fb571e5467fd5596fb8ee1fcdf0179350a051
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/DefaultSharedRefEnforcement.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/DefaultSharedRefEnforcement.java
index 63bab09..01cd5c5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/DefaultSharedRefEnforcement.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/DefaultSharedRefEnforcement.java
@@ -14,22 +14,11 @@
 
 package com.googlesource.gerrit.plugins.multisite.validation.dfsrefdb;
 
-import com.google.common.base.MoreObjects;
-import com.google.common.collect.ImmutableMap;
-
 public class DefaultSharedRefEnforcement implements SharedRefEnforcement {
 
-  private static final ImmutableMap<String, EnforcePolicy> PREDEF_ENFORCEMENTS =
-      ImmutableMap.of("All-Users:refs/meta/external-ids", EnforcePolicy.DESIRED);
-
   @Override
   public EnforcePolicy getPolicy(String projectName, String refName) {
-    if (isRefToBeIgnoredBySharedRefDb(refName)) {
-      return EnforcePolicy.IGNORED;
-    }
-
-    return MoreObjects.firstNonNull(
-        PREDEF_ENFORCEMENTS.get(projectName + ":" + refName), EnforcePolicy.REQUIRED);
+    return isRefToBeIgnoredBySharedRefDb(refName) ? EnforcePolicy.IGNORED : EnforcePolicy.REQUIRED;
   }
 
   @Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/SharedRefEnforcement.java b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/SharedRefEnforcement.java
index 4de6dea..100def2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/SharedRefEnforcement.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/SharedRefEnforcement.java
@@ -18,7 +18,6 @@
 public interface SharedRefEnforcement {
   public enum EnforcePolicy {
     IGNORED,
-    DESIRED,
     REQUIRED;
   }
 
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 662f46e..67e942f 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -76,7 +76,7 @@
 
 ```ref-database.enforcementRules.<policy>```
 :   Level of consistency enforcement across sites on a project:refs basis.
-    Supports multiple values for enforcing the policy on multiple projects or refs.
+    Supports two values for enforcing the policy on multiple projects or refs.
     If the project or ref is omitted, apply the policy to all projects or all refs.
 
     The <policy> can be one of the following values:
@@ -86,18 +86,14 @@
     The user transaction is cancelled. The Gerrit GUI (or the Git client)
     receives an HTTP 500 - Internal Server Error.
 
-    2. DESIRED - Validate the git ref-update against the shared ref-database.
-    Any misaligned is logged in errors_log file but the user operation is allowed
-    to continue successfully.
-
-    3. IGNORED - Ignore any validation against the shared ref-database.
+    2. IGNORED - Ignore any validation against the shared ref-database.
 
     *Example:*
     ```
     [ref-database "enforcementRules"]
-       DESIRED = AProject:/refs/heads/feature
+       IGNORED = AProject:/refs/heads/feature
     ```
 
-    Relax the alignment with the shared ref-database for AProject on refs/heads/feature.
+    Ignore the alignment with the shared ref-database for AProject on refs/heads/feature.
 
     Defaults: No rules = All projects are REQUIRED to be consistent on all refs.
\ No newline at end of file
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/CustomSharedRefEnforcementByProjectTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/CustomSharedRefEnforcementByProjectTest.java
index 4ee3c85..f3008a2 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/CustomSharedRefEnforcementByProjectTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/CustomSharedRefEnforcementByProjectTest.java
@@ -35,16 +35,11 @@
     sharedRefDbConfig.setStringList(
         SharedRefDatabase.SECTION,
         SharedRefDatabase.SUBSECTION_ENFORCEMENT_RULES,
-        EnforcePolicy.DESIRED.name(),
+        EnforcePolicy.IGNORED.name(),
         Arrays.asList(
             "ProjectOne",
             "ProjectTwo:refs/heads/master/test",
             "ProjectTwo:refs/heads/master/test2"));
-    sharedRefDbConfig.setString(
-        SharedRefDatabase.SECTION,
-        SharedRefDatabase.SUBSECTION_ENFORCEMENT_RULES,
-        EnforcePolicy.IGNORED.name(),
-        ":refs/heads/master/test");
 
     refEnforcement = newCustomRefEnforcement(sharedRefDbConfig);
   }
@@ -53,32 +48,32 @@
   public void projectOneShouldReturnDesiredForAllRefs() {
     Ref aRef = newRef("refs/heads/master/2", AN_OBJECT_ID_1);
     assertThat(refEnforcement.getPolicy("ProjectOne", aRef.getName()))
-        .isEqualTo(EnforcePolicy.DESIRED);
+        .isEqualTo(EnforcePolicy.IGNORED);
   }
 
   @Test
   public void projectOneEnforcementShouldAlwaysPrevail() {
     Ref aRef = newRef("refs/heads/master/test", AN_OBJECT_ID_1);
     assertThat(refEnforcement.getPolicy("ProjectOne", aRef.getName()))
-        .isEqualTo(EnforcePolicy.DESIRED);
-  }
-
-  @Test
-  public void aNonListedProjectShouldIgnoreRefForMasterTest() {
-    Ref aRef = newRef("refs/heads/master/test", AN_OBJECT_ID_1);
-    assertThat(refEnforcement.getPolicy("NonListedProject", aRef.getName()))
         .isEqualTo(EnforcePolicy.IGNORED);
   }
 
   @Test
-  public void projectTwoSpecificRefShouldReturnDesiredPolicy() {
+  public void aNonListedProjectShouldRequireRefForMasterTest() {
+    Ref aRef = newRef("refs/heads/master/test", AN_OBJECT_ID_1);
+    assertThat(refEnforcement.getPolicy("NonListedProject", aRef.getName()))
+        .isEqualTo(EnforcePolicy.REQUIRED);
+  }
+
+  @Test
+  public void projectTwoSpecificRefShouldReturnIgnoredPolicy() {
     Ref refOne = newRef("refs/heads/master/test", AN_OBJECT_ID_1);
     Ref refTwo = newRef("refs/heads/master/test2", AN_OBJECT_ID_1);
 
     assertThat(refEnforcement.getPolicy("ProjectTwo", refOne.getName()))
-        .isEqualTo(EnforcePolicy.DESIRED);
+        .isEqualTo(EnforcePolicy.IGNORED);
     assertThat(refEnforcement.getPolicy("ProjectTwo", refTwo.getName()))
-        .isEqualTo(EnforcePolicy.DESIRED);
+        .isEqualTo(EnforcePolicy.IGNORED);
   }
 
   @Test
@@ -103,8 +98,8 @@
   }
 
   @Test
-  public void getProjectPolicyForProjectOneShouldRetrunDesired() {
-    assertThat(refEnforcement.getPolicy("ProjectOne")).isEqualTo(EnforcePolicy.DESIRED);
+  public void getProjectPolicyForProjectOneShouldReturnIgnored() {
+    assertThat(refEnforcement.getPolicy("ProjectOne")).isEqualTo(EnforcePolicy.IGNORED);
   }
 
   @Test
@@ -120,7 +115,7 @@
   @Test
   public void getProjectPolicyForNonListedProjectWhenSingleProject() {
     SharedRefEnforcement customEnforcement =
-        newCustomRefEnforcementWithValue(EnforcePolicy.DESIRED, ":refs/heads/master");
+        newCustomRefEnforcementWithValue(EnforcePolicy.IGNORED, ":refs/heads/master");
 
     assertThat(customEnforcement.getPolicy("NonListedProject")).isEqualTo(EnforcePolicy.REQUIRED);
   }
@@ -128,7 +123,7 @@
   @Test
   public void getANonListedProjectWhenOnlyOneProjectIsListedShouldReturnRequired() {
     SharedRefEnforcement customEnforcement =
-        newCustomRefEnforcementWithValue(EnforcePolicy.DESIRED, "AProject:");
+        newCustomRefEnforcementWithValue(EnforcePolicy.IGNORED, "AProject:");
     assertThat(customEnforcement.getPolicy("NonListedProject", "refs/heads/master"))
         .isEqualTo(EnforcePolicy.REQUIRED);
   }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java
index b462408..2964e15 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/multisite/validation/dfsrefdb/DefaultSharedRefEnforcementTest.java
@@ -66,6 +66,13 @@
         .isEqualTo(EnforcePolicy.REQUIRED);
   }
 
+  @Test
+  public void allUsersExternalIdsRefShouldBeRequired() {
+    Ref refOne = newRef("refs/meta/external-ids", AN_OBJECT_ID_1);
+    assertThat(refEnforcement.getPolicy("All-Users", refOne.getName()))
+        .isEqualTo(EnforcePolicy.REQUIRED);
+  }
+
   @Override
   public String testBranch() {
     return "fooBranch";