Migrate refs/for after all sections and permissions are loaded.

Avoid deleting a section by deleting the last permission, before
all permissions have been loaded.

Change-Id: I626d3eb8a55952d0280caeaa4c331d8daca47e85

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

@@ -312,6 +312,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
       remove(section);
     } else if (section != null) {
       AccessSection a = accessSections.get(section.getName());
+      if (a == null) {
+        System.out.println(a);
+      }
       a.remove(permission);
       if (a.getPermissions().isEmpty()) {
         remove(a);
@@ -703,19 +706,13 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
                 groupsByName,
                 perm,
                 Permission.hasRange(convertedName));
-            if (migrateRefsFor(as, perm)) {
-              if (isRefsForExclusively(refName)) {
-                // Since the ref only applies on refs/for/* and no other
-                // namespaces, we can remove the old permission.
-                remove(as, perm);
-              }
-            }
           } else {
             sectionsWithUnknownPermissions.add(as.getName());
           }
         }
       }
     }
+    migrateRefsFor();
 
     AccessSection capability = null;
     for (String varName : rc.getNames(CAPABILITY)) {
@@ -729,6 +726,20 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
     }
   }
 
+  private void migrateRefsFor() {
+    for (AccessSection as : ImmutableList.copyOf(accessSections.values())) {
+      for (Permission p : ImmutableList.copyOf(as.getPermissions())) {
+        if (migrateRefsFor(as, p)) {
+          if (isRefsForExclusively(as.getName())) {
+            // Since the ref only applies on refs/for/* and no other
+            // namespaces, we can remove the old permission.
+            remove(as, p);
+          }
+        }
+      }
+    }
+  }
+
   private boolean isRefsForExclusively(String refName) {
     return refName.startsWith("refs/for/");
   }

javatests/com/google/gerrit/server/project/ProjectConfigTest.java

@@ -693,7 +693,7 @@ public class ProjectConfigTest extends GerritBaseTests {
   }
 
   @Test
-  public void readConfigAddPushMergeRefsForStarIsMigrated() throws Exception {
+  public void readConfigPushMergeRefsForStarIsMigrated() throws Exception {
     RevCommit rev =
         tr.commit()
             .add("groups", group(developers))
@@ -710,6 +710,29 @@ public class ProjectConfigTest extends GerritBaseTests {
         .isEqualTo(Lists.newArrayList(rule));
   }
 
+  @Test
+  public void readConfigMultiplePermissionsOnRefsForStarIsMigrated() throws Exception {
+    RevCommit rev =
+        tr.commit()
+            .add("groups", group(developers))
+            .add(
+                "project.config",
+                "[access \"refs/for/*\"]\n"
+                    + "  pushMerge = group Developers\n"
+                    + "  addPatchSet = group Developers\n")
+            .create();
+
+    ProjectConfig cfg = read(rev);
+    AccessSection as = cfg.getAccessSection("refs/for/*");
+    assertThat(as).isNull();
+    as = cfg.getAccessSection("refs/*");
+    assertThat(as).isNotNull();
+    assertThat(as.getPermission(Permission.PUSH_MERGE, false).getRules())
+        .isEqualTo(Lists.newArrayList(new PermissionRule(developers)));
+    assertThat(as.getPermission(Permission.ADD_PATCH_SET, false).getRules())
+        .isEqualTo(Lists.newArrayList(new PermissionRule(developers)));
+  }
+
   @Test
   public void readCommentLinkMatchButNoHtmlOrLink() throws Exception {
     RevCommit rev =