Do not check groupsMigration status in GroupsNoteDbConsistencyChecker
During a test bulk migration, the real groupsMigration status is not known or relevant. Change-Id: Ia077547ec3011a45deba49d0c29b536a366a0b85
This commit is contained in:
		| @@ -15,7 +15,6 @@ | |||||||
| package com.google.gerrit.server.group.db; | package com.google.gerrit.server.group.db; | ||||||
|  |  | ||||||
| import static com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo.error; | import static com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo.error; | ||||||
| import static com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo.warning; |  | ||||||
|  |  | ||||||
| import com.google.common.collect.BiMap; | import com.google.common.collect.BiMap; | ||||||
| import com.google.common.collect.HashBiMap; | import com.google.common.collect.HashBiMap; | ||||||
| @@ -25,7 +24,6 @@ import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyP | |||||||
| import com.google.gerrit.reviewdb.client.AccountGroup; | import com.google.gerrit.reviewdb.client.AccountGroup; | ||||||
| import com.google.gerrit.reviewdb.client.RefNames; | import com.google.gerrit.reviewdb.client.RefNames; | ||||||
| import com.google.gerrit.server.group.InternalGroup; | import com.google.gerrit.server.group.InternalGroup; | ||||||
| import com.google.gerrit.server.notedb.GroupsMigration; |  | ||||||
| import java.io.IOException; | import java.io.IOException; | ||||||
| import java.nio.charset.StandardCharsets; | import java.nio.charset.StandardCharsets; | ||||||
| import java.util.ArrayList; | import java.util.ArrayList; | ||||||
| @@ -48,7 +46,6 @@ import org.eclipse.jgit.revwalk.RevWalk; | |||||||
| /** Check the referential integrity of NoteDb group storage. */ | /** Check the referential integrity of NoteDb group storage. */ | ||||||
| @Singleton | @Singleton | ||||||
| public class GroupsNoteDbConsistencyChecker { | public class GroupsNoteDbConsistencyChecker { | ||||||
|   private final GroupsMigration groupsMigration; |  | ||||||
|  |  | ||||||
|   /** |   /** | ||||||
|    * The result of a consistency check. The UUID map is only non-null if no problems were detected. |    * The result of a consistency check. The UUID map is only non-null if no problems were detected. | ||||||
| @@ -60,20 +57,10 @@ public class GroupsNoteDbConsistencyChecker { | |||||||
|   } |   } | ||||||
|  |  | ||||||
|   @Inject |   @Inject | ||||||
|   GroupsNoteDbConsistencyChecker(GroupsMigration groupsMigration) { |   GroupsNoteDbConsistencyChecker() {} | ||||||
|     this.groupsMigration = groupsMigration; |  | ||||||
|   } |  | ||||||
|  |  | ||||||
|   /** |   /** Checks for problems with the given All-Users repo. */ | ||||||
|    * Checks for problems with the given All-Users repo. Returns null if we are not writing to |  | ||||||
|    * NoteDb. |  | ||||||
|    */ |  | ||||||
|   @Nullable |  | ||||||
|   public Result check(Repository repo) throws IOException { |   public Result check(Repository repo) throws IOException { | ||||||
|     if (!groupsMigration.writeToNoteDb()) { |  | ||||||
|       return null; |  | ||||||
|     } |  | ||||||
|  |  | ||||||
|     Result r = doCheck(repo); |     Result r = doCheck(repo); | ||||||
|     if (!r.problems.isEmpty()) { |     if (!r.problems.isEmpty()) { | ||||||
|       r.uuidToGroupMap = null; |       r.uuidToGroupMap = null; | ||||||
| @@ -81,7 +68,6 @@ public class GroupsNoteDbConsistencyChecker { | |||||||
|     return r; |     return r; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   @Nullable |  | ||||||
|   private Result doCheck(Repository repo) throws IOException { |   private Result doCheck(Repository repo) throws IOException { | ||||||
|     Result result = new Result(); |     Result result = new Result(); | ||||||
|     result.problems = new ArrayList<>(); |     result.problems = new ArrayList<>(); | ||||||
| @@ -136,7 +122,7 @@ public class GroupsNoteDbConsistencyChecker { | |||||||
|     Ref ref = refs.get(RefNames.REFS_GROUPNAMES); |     Ref ref = refs.get(RefNames.REFS_GROUPNAMES); | ||||||
|     if (ref == null) { |     if (ref == null) { | ||||||
|       String msg = String.format("ref %s does not exist", RefNames.REFS_GROUPNAMES); |       String msg = String.format("ref %s does not exist", RefNames.REFS_GROUPNAMES); | ||||||
|       result.problems.add(groupsMigration.readFromNoteDb() ? error(msg) : warning(msg)); |       result.problems.add(error(msg)); | ||||||
|       return; |       return; | ||||||
|     } |     } | ||||||
|  |  | ||||||
|   | |||||||
| @@ -108,11 +108,7 @@ public class GroupsConsistencyIT extends AbstractDaemonTest { | |||||||
|       assertThat(result).isEqualTo(Result.FORCED); |       assertThat(result).isEqualTo(Result.FORCED); | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     assertConsistency( |     assertError("refs/meta/group-names does not exist"); | ||||||
|         "refs/meta/group-names does not exist", |  | ||||||
|         groupsMigration.readFromNoteDb() |  | ||||||
|             ? ConsistencyProblemInfo.Status.ERROR |  | ||||||
|             : ConsistencyProblemInfo.Status.WARNING); |  | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   @Test |   @Test | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Han-Wen Nienhuys
					Han-Wen Nienhuys