diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java index d80dcf3692..22b1aee3ef 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -17,6 +17,7 @@ package com.google.gerrit.lucene; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.gerrit.server.git.QueueProvider.QueueType.INTERACTIVE; +import static com.google.gerrit.server.index.ChangeField.CHANGE; import static com.google.gerrit.server.index.ChangeField.LEGACY_ID; import static com.google.gerrit.server.index.ChangeField.PROJECT; import static com.google.gerrit.server.index.IndexRewriter.CLOSED_STATUSES; @@ -431,18 +432,30 @@ public class LuceneChangeIndex implements ChangeIndex { } private Set fields(QueryOptions opts) { - if (schemaHasRequestedField(ChangeField.LEGACY_ID, opts.fields()) - || schemaHasRequestedField(ChangeField.CHANGE, opts.fields())) { - return opts.fields(); + // Ensure we request enough fields to construct a ChangeData. + Set fs = opts.fields(); + if (fs.contains(CHANGE.getName())) { + // A Change is always sufficient. + return fs; } - // Request the numeric ID field even if the caller did not request it, - // otherwise we can't actually construct a ChangeData. - return Sets.union(opts.fields(), ImmutableSet.of(LEGACY_ID.getName())); - } - private boolean schemaHasRequestedField(FieldDef field, - Set requested) { - return schema.hasField(field) && requested.contains(field.getName()); + if (!schema.hasField(PROJECT)) { + // Schema is not new enough to have project field. Ensure we have ID + // field, and call createOnlyWhenNotedbDisabled from toChangeData below. + if (fs.contains(LEGACY_ID.getName())) { + return fs; + } else { + return Sets.union(fs, ImmutableSet.of(LEGACY_ID.getName())); + } + } + + // New enough schema to have project field, so ensure that is requested. + if (fs.contains(PROJECT.getName()) && fs.contains(LEGACY_ID.getName())) { + return fs; + } else { + return Sets.union(fs, + ImmutableSet.of(LEGACY_ID.getName(), PROJECT.getName())); + } } private ChangeData toChangeData(Document doc, Set fields, @@ -455,10 +468,17 @@ public class LuceneChangeIndex implements ChangeIndex { cd = changeDataFactory.create(db.get(), ChangeProtoField.CODEC.decode(cb.bytes, cb.offset, cb.length)); } else { - int id = doc.getField(idFieldName).numericValue().intValue(); - Project.NameKey project = - new Project.NameKey(doc.getField(PROJECT.getName()).stringValue()); - cd = changeDataFactory.create(db.get(), project, new Change.Id(id)); + Change.Id id = + new Change.Id(doc.getField(idFieldName).numericValue().intValue()); + IndexableField project = doc.getField(PROJECT.getName()); + if (project == null) { + // Old schema without project field: we can safely assume notedb is + // disabled. + cd = changeDataFactory.createOnlyWhenNotedbDisabled(db.get(), id); + } else { + cd = changeDataFactory.create( + db.get(), new Project.NameKey(project.stringValue()), id); + } } if (fields.contains(PATCH_SET_FIELD)) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java index 8d1419ee0d..61a55d4d15 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java @@ -381,7 +381,7 @@ public class ChangeJson { // If any problems were fixed, the ChangeData needs to be reloaded. for (ProblemInfo p : out.problems) { if (p.status == ProblemInfo.Status.FIXED) { - cd = changeDataFactory.create(cd.db(), cd.getProject(), cd.getId()); + cd = changeDataFactory.create(cd.db(), cd.project(), cd.getId()); break; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java index 56fd25baff..77f65a1a6a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.notedb; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import static com.google.gerrit.server.notedb.ChangeNoteUtil.GERRIT_PLACEHOLDER_HOST; import com.google.common.annotations.VisibleForTesting; @@ -134,6 +135,16 @@ public class ChangeNotes extends AbstractChangeNotes { return new ChangeNotes(repoManager, migration, allUsersProvider, change.getProject(), change).load(); } + + // TODO(dborowitz): Remove when deleting index schemas <27. + public ChangeNotes createFromIdOnlyWhenNotedbDisabled( + ReviewDb db, Change.Id changeId) throws OrmException { + checkState(!migration.readChanges(), "do not call" + + " createFromIdOnlyWhenNotedbDisabled when notedb is enabled"); + Change change = db.changes().get(changeId); + return new ChangeNotes(repoManager, migration, allUsersProvider, + change.getProject(), change).load(); + } } private final Project.NameKey project; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java index c5bb532095..2bacafbbc3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.query.change; +import static com.google.common.base.Preconditions.checkState; import static com.google.gerrit.server.ApprovalsUtil.sortApprovals; import com.google.auto.value.AutoValue; @@ -275,6 +276,9 @@ public class ChangeData { ChangeData create(ReviewDb db, Project.NameKey project, Change.Id id); ChangeData create(ReviewDb db, Change c); ChangeData create(ReviewDb db, ChangeControl c); + + // TODO(dborowitz): Remove when deleting index schemas <27. + ChangeData createOnlyWhenNotedbDisabled(ReviewDb db, Change.Id id); } /** @@ -309,8 +313,8 @@ public class ChangeData { private final NotesMigration notesMigration; private final MergeabilityCache mergeabilityCache; private final Change.Id legacyId; - private final Project.NameKey project; private ChangeDataSource returnedBySource; + private Project.NameKey project; private Change change; private ChangeNotes notes; private String commitMessage; @@ -444,6 +448,43 @@ public class ChangeData { project = notes.getProjectName(); } + @AssistedInject + private ChangeData( + GitRepositoryManager repoManager, + ChangeControl.GenericFactory changeControlFactory, + IdentifiedUser.GenericFactory userFactory, + ProjectCache projectCache, + MergeUtil.Factory mergeUtilFactory, + ChangeNotes.Factory notesFactory, + ApprovalsUtil approvalsUtil, + ChangeMessagesUtil cmUtil, + PatchLineCommentsUtil plcUtil, + PatchSetUtil psUtil, + PatchListCache patchListCache, + NotesMigration notesMigration, + MergeabilityCache mergeabilityCache, + @Assisted ReviewDb db, + @Assisted Change.Id id) { + checkState(!notesMigration.readChanges(), + "do not call createOnlyWhenNotedbDisabled when notedb is enabled"); + this.db = db; + this.repoManager = repoManager; + this.changeControlFactory = changeControlFactory; + this.userFactory = userFactory; + this.projectCache = projectCache; + this.mergeUtilFactory = mergeUtilFactory; + this.notesFactory = notesFactory; + this.approvalsUtil = approvalsUtil; + this.cmUtil = cmUtil; + this.plcUtil = plcUtil; + this.psUtil = psUtil; + this.patchListCache = patchListCache; + this.notesMigration = notesMigration; + this.mergeabilityCache = mergeabilityCache; + this.legacyId = id; + this.project = null; + } + public ReviewDb db() { return db; } @@ -545,7 +586,12 @@ public class ChangeData { return legacyId; } - public Project.NameKey getProject() { + public Project.NameKey project() throws OrmException { + if (project == null) { + checkState(!notesMigration.readChanges(), "should not have created " + + " ChangeData without a project when notedb is enabled"); + project = change().getProject(); + } return project; } @@ -580,7 +626,7 @@ public class ChangeData { changeControl = changeControlFactory.controlFor(db, change, user); } else { changeControl = - changeControlFactory.controlFor(db, project, legacyId, user); + changeControlFactory.controlFor(db, project(), legacyId, user); } } catch (NoSuchChangeException e) { throw new OrmException(e); @@ -605,7 +651,11 @@ public class ChangeData { } public Change reloadChange() throws OrmException { - notes = notesFactory.create(db, project, legacyId); + if (project == null) { + notes = notesFactory.createFromIdOnlyWhenNotedbDisabled(db, legacyId); + } else { + notes = notesFactory.create(db, project, legacyId); + } change = notes.getChange(); if (change == null) { throw new OrmException("Unable to load change " + legacyId); @@ -615,7 +665,7 @@ public class ChangeData { public ChangeNotes notes() throws OrmException { if (notes == null) { - notes = notesFactory.create(db, project, legacyId); + notes = notesFactory.create(db, project(), legacyId); } return notes; } @@ -698,7 +748,7 @@ public class ChangeData { return false; } String sha1 = ps.getRevision().get(); - try (Repository repo = repoManager.openRepository(project); + try (Repository repo = repoManager.openRepository(project()); RevWalk walk = new RevWalk(repo)) { RevCommit c = walk.parseCommit(ObjectId.fromString(sha1)); commitMessage = c.getFullMessage(); @@ -821,7 +871,7 @@ public class ChangeData { if (ps == null || !changeControl().isPatchVisible(ps, db)) { return null; } - try (Repository repo = repoManager.openRepository(project)) { + try (Repository repo = repoManager.openRepository(project())) { Ref ref = repo.getRefDatabase().exactRef(c.getDest().get()); SubmitTypeRecord str = submitTypeRecord(); if (!str.isOk()) { @@ -830,7 +880,7 @@ public class ChangeData { return false; } String mergeStrategy = mergeUtilFactory - .create(projectCache.get(project)) + .create(projectCache.get(project())) .mergeStrategyName(); mergeable = mergeabilityCache.get( ObjectId.fromString(ps.getRevision().get()), @@ -851,7 +901,7 @@ public class ChangeData { } editsByUser = new HashSet<>(); Change.Id id = change.getId(); - try (Repository repo = repoManager.openRepository(project)) { + try (Repository repo = repoManager.openRepository(project())) { for (String ref : repo.getRefDatabase().getRefs(RefNames.REFS_USERS).keySet()) { if (Change.Id.fromEditRefPart(ref).equals(id)) {