Merge changes from topic 'old-index-schemas'

* changes:
  Remove special-casing of index schemas prior to PROJECT
  Remove index schemas earlier than v32

gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java

@@ -130,8 +130,6 @@ public class LuceneChangeIndex implements ChangeIndex {
   private static final String HASHTAG_FIELD =
       ChangeField.HASHTAG_CASE_AWARE.getName();
   private static final String STAR_FIELD = ChangeField.STAR.getName();
-  @Deprecated
-  private static final String STARREDBY_FIELD = ChangeField.STARREDBY.getName();
 
   static Term idTerm(ChangeData cd) {
     return QueryBuilder.intTerm(LEGACY_ID.getName(), cd.getId().get());
@@ -410,23 +408,14 @@ public class LuceneChangeIndex implements ChangeIndex {
   }
 
   private Set<String> fields(QueryOptions opts) {
-    // Ensure we request enough fields to construct a ChangeData.
+    // Ensure we request enough fields to construct a ChangeData. We need both
+    // change ID and project, which can either come via the Change field or
+    // separate fields.
     Set<String> fs = opts.fields();
     if (fs.contains(CHANGE.getName())) {
       // A Change is always sufficient.
       return fs;
     }
-
-    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;
-      }
-      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;
     }
@@ -492,9 +481,6 @@ public class LuceneChangeIndex implements ChangeIndex {
     if (fields.contains(HASHTAG_FIELD)) {
       decodeHashtags(doc, cd);
     }
-    if (fields.contains(STARREDBY_FIELD)) {
-      decodeStarredBy(doc, cd);
-    }
     if (fields.contains(STAR_FIELD)) {
       decodeStar(doc, cd);
     }
@@ -584,17 +570,6 @@ public class LuceneChangeIndex implements ChangeIndex {
     cd.setHashtags(hashtags);
   }
 
-  @Deprecated
-  private void decodeStarredBy(Multimap<String, IndexableField> doc, ChangeData cd) {
-    Collection<IndexableField> starredBy = doc.get(STARREDBY_FIELD);
-    Set<Account.Id> accounts =
-        Sets.newHashSetWithExpectedSize(starredBy.size());
-    for (IndexableField r : starredBy) {
-      accounts.add(new Account.Id(r.numericValue().intValue()));
-    }
-    cd.setStarredBy(accounts);
-  }
-
   private void decodeStar(Multimap<String, IndexableField> doc, ChangeData cd) {
     Collection<IndexableField> star = doc.get(STAR_FIELD);
     Multimap<Account.Id, String> stars = ArrayListMultimap.create();

gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java

@@ -293,29 +293,6 @@ public class ChangeField {
         }
       };
 
-  /** Reviewer(s) associated with the change. */
-  @Deprecated
-  public static final FieldDef<ChangeData, Iterable<Integer>> LEGACY_REVIEWER =
-      new FieldDef.Repeatable<ChangeData, Integer>(
-          ChangeQueryBuilder.FIELD_REVIEWER, FieldType.INTEGER, false) {
-        @Override
-        public Iterable<Integer> get(ChangeData input, FillArgs args)
-            throws OrmException {
-          Change c = input.change();
-          if (c == null) {
-            return ImmutableSet.of();
-          }
-          Set<Integer> r = new HashSet<>();
-          if (!args.allowsDrafts && c.getStatus() == Change.Status.DRAFT) {
-            return r;
-          }
-          for (PatchSetApproval a : input.approvals().values()) {
-            r.add(a.getAccountId().get());
-          }
-          return r;
-        }
-      };
-
   /** The user assigned to the change. */
   public static final FieldDef<ChangeData, Integer> ASSIGNEE =
       new FieldDef.Single<ChangeData, Integer>(
@@ -682,18 +659,6 @@ public class ChangeField {
         }
       };
 
-  /** Users who have starred this change. */
-  @Deprecated
-  public static final FieldDef<ChangeData, Iterable<Integer>> STARREDBY =
-      new FieldDef.Repeatable<ChangeData, Integer>(
-          ChangeQueryBuilder.FIELD_STARREDBY, FieldType.INTEGER, true) {
-        @Override
-        public Iterable<Integer> get(ChangeData input, FillArgs args)
-            throws OrmException {
-          return Iterables.transform(input.starredBy(), Account.Id::get);
-        }
-      };
-
   /**
    * Star labels on this change in the format: &lt;account-id&gt;:&lt;label&gt;
    */

gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java

@@ -22,7 +22,7 @@ import com.google.gerrit.server.query.change.ChangeData;
 
 public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
   @Deprecated
-  static final Schema<ChangeData> V25 = schema(
+  static final Schema<ChangeData> V32 = schema(
       ChangeField.LEGACY_ID,
       ChangeField.ID,
       ChangeField.STATUS,
@@ -35,7 +35,6 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
       ChangeField.FILE_PART,
       ChangeField.PATH,
       ChangeField.OWNER,
-      ChangeField.LEGACY_REVIEWER,
       ChangeField.COMMIT,
       ChangeField.TR,
       ChangeField.LABEL,
@@ -56,37 +55,12 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
       ChangeField.REVIEWEDBY,
       ChangeField.EXACT_COMMIT,
       ChangeField.AUTHOR,
-      ChangeField.COMMITTER);
+      ChangeField.COMMITTER,
-
+      ChangeField.DRAFTBY,
-  @Deprecated
+      ChangeField.HASHTAG_CASE_AWARE,
-  static final Schema<ChangeData> V26 = schema(V25, ChangeField.DRAFTBY);
+      ChangeField.STAR,
-
+      ChangeField.STARBY,
-  @Deprecated
+      ChangeField.REVIEWER);
-  static final Schema<ChangeData> V27 = schema(V26.getFields().values());
-
-  @Deprecated
-  static final Schema<ChangeData> V28 = schema(V27, ChangeField.STARREDBY);
-
-  @Deprecated
-  static final Schema<ChangeData> V29 =
-      schema(V28, ChangeField.HASHTAG_CASE_AWARE);
-
-  @Deprecated
-  static final Schema<ChangeData> V30 =
-      schema(V29, ChangeField.STAR, ChangeField.STARBY);
-
-  @Deprecated
-  static final Schema<ChangeData> V31 = new Schema.Builder<ChangeData>()
-      .add(V30)
-      .remove(ChangeField.STARREDBY)
-      .build();
-
-  @Deprecated
-  static final Schema<ChangeData> V32 = new Schema.Builder<ChangeData>()
-      .add(V31)
-      .remove(ChangeField.LEGACY_REVIEWER)
-      .add(ChangeField.REVIEWER)
-      .build();
 
   @Deprecated
   static final Schema<ChangeData> V33 =

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java

@@ -143,6 +143,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
     private Change loadChangeFromDb(ReviewDb db, Project.NameKey project,
         Change.Id changeId) throws OrmException {
       Change change = ReviewDbUtil.unwrapDb(db).changes().get(changeId);
+      checkArgument(project != null, "project is required");
       checkNotNull(change,
           "change %s not found in ReviewDb", changeId);
       checkArgument(change.getProject().equals(project),
@@ -182,17 +183,6 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
       return new ChangeNotes(args, change, false, null).load();
     }
 
-    // TODO(dborowitz): Remove when deleting index schemas <27.
-    public ChangeNotes createFromIdOnlyWhenNoteDbDisabled(
-        ReviewDb db, Change.Id changeId) throws OrmException {
-      checkState(!args.migration.readChanges(), "do not call"
-          + " createFromIdOnlyWhenNoteDbDisabled when NoteDb is enabled");
-      Change change = ReviewDbUtil.unwrapDb(db).changes().get(changeId);
-      checkNotNull(change,
-          "change %s not found in ReviewDb", changeId);
-      return new ChangeNotes(args, change).load();
-    }
-
     public ChangeNotes createWithAutoRebuildingDisabled(Change change,
         RefCache refs) throws OrmException {
       return new ChangeNotes(args, change, false, refs).load();

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java

@@ -766,11 +766,7 @@ public class ChangeData {
   }
 
   public Change reloadChange() throws OrmException {
-    if (project == null) {
+    notes = notesFactory.create(db, project, legacyId);
-      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);

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java

@@ -683,8 +683,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
     return starredby(parseAccount(who));
   }
 
-  private Predicate<ChangeData> starredby(Set<Account.Id> who)
+  private Predicate<ChangeData> starredby(Set<Account.Id> who) {
-      throws QueryParseException {
     List<Predicate<ChangeData>> p = Lists.newArrayListWithCapacity(who.size());
     for (Account.Id id : who) {
       p.add(starredby(id));
@@ -692,25 +691,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
     return Predicate.or(p);
   }
 
-  @SuppressWarnings("deprecation")
+  private Predicate<ChangeData> starredby(Account.Id who) {
-  private Predicate<ChangeData> starredby(Account.Id who)
+    return new StarPredicate(who, StarredChangesUtil.DEFAULT_LABEL);
-      throws QueryParseException {
-    if (args.getSchema().hasField(ChangeField.STAR)) {
-      return new StarPredicate(who, StarredChangesUtil.DEFAULT_LABEL);
-    }
-
-    if (args.getSchema().hasField(ChangeField.STARREDBY)) {
-      return new IsStarredByPredicate(who);
-    }
-
-    try {
-      // starred changes are not contained in the index, we must read them from
-      // git
-      return new IsStarredByLegacyPredicate(who, args.starredChangesUtil
-          .byAccount(who, StarredChangesUtil.DEFAULT_LABEL));
-    } catch (OrmException e) {
-      throw new QueryParseException("Failed to query starred changes.", e);
-    }
   }
 
   @Operator
@@ -749,11 +731,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
     return Predicate.or(p);
   }
 
-  @SuppressWarnings("deprecation")
   private Predicate<ChangeData> draftby(Account.Id who) {
-    return args.getSchema().hasField(ChangeField.DRAFTBY)
+    return new HasDraftByPredicate(who);
-        ? new HasDraftByPredicate(who)
-        : new HasDraftByLegacyPredicate(args, who);
   }
 
   @Operator

gerrit-server/src/main/java/com/google/gerrit/server/query/change/HasDraftByLegacyPredicate.java

@@ -1,80 +0,0 @@
-// Copyright (C) 2010 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.query.change;
-
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.query.change.ChangeQueryBuilder.Arguments;
-import com.google.gwtorm.server.ListResultSet;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
-
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
-@Deprecated
-class HasDraftByLegacyPredicate extends ChangeOperatorPredicate
-    implements ChangeDataSource {
-  private final Arguments args;
-  private final Account.Id accountId;
-
-  HasDraftByLegacyPredicate(Arguments args,
-      Account.Id accountId) {
-    super(ChangeQueryBuilder.FIELD_DRAFTBY, accountId.toString());
-    this.args = args;
-    this.accountId = accountId;
-  }
-
-  @Override
-  public boolean match(final ChangeData object) throws OrmException {
-    return !args.commentsUtil
-        .draftByChangeAuthor(args.db.get(), object.notes(), accountId)
-        .isEmpty();
-  }
-
-  @Override
-  public ResultSet<ChangeData> read() throws OrmException {
-    Set<Change.Id> ids = new HashSet<>();
-    for (Change.Id changeId : args.commentsUtil
-        .changesWithDraftsByAuthor(args.db.get(), accountId)) {
-      ids.add(changeId);
-    }
-
-    List<ChangeData> r = new ArrayList<>(ids.size());
-    // TODO Don't load the changes directly from the database, but provide
-    // project name + change ID to changeDataFactory, or delete this predicate.
-    for (Change c : args.db.get().changes().get(ids)) {
-      r.add(args.changeDataFactory.create(args.db.get(), c));
-    }
-    return new ListResultSet<>(r);
-  }
-
-  @Override
-  public boolean hasChange() {
-    return false;
-  }
-
-  @Override
-  public int getCardinality() {
-    return 20;
-  }
-
-  @Override
-  public int getCost() {
-    return 0;
-  }
-}

gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.server.query.change;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.gerrit.server.index.change.ChangeField.SUBMISSIONID;
 import static com.google.gerrit.server.query.Predicate.and;
 import static com.google.gerrit.server.query.Predicate.not;
 import static com.google.gerrit.server.query.Predicate.or;
@@ -25,7 +24,6 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
-import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
@@ -269,7 +267,7 @@ public class InternalChangeQuery extends InternalQuery<ChangeData> {
   }
 
   public List<ChangeData> bySubmissionId(String cs) throws OrmException {
-    if (Strings.isNullOrEmpty(cs) || !schema().hasField(SUBMISSIONID)) {
+    if (Strings.isNullOrEmpty(cs)) {
       return Collections.emptyList();
     }
     return query(new SubmissionIdPredicate(cs));
@@ -283,9 +281,4 @@ public class InternalChangeQuery extends InternalQuery<ChangeData> {
     }
     return query(and(project(project), or(groupPredicates)));
   }
-
-  @SuppressWarnings("deprecation")
-  public List<ChangeData> byIsStarred(Account.Id id) throws OrmException {
-    return query(new IsStarredByPredicate(id));
-  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByLegacyPredicate.java

@@ -1,60 +0,0 @@
-// Copyright (C) 2010 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.query.change;
-
-import com.google.common.collect.Lists;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.query.OrPredicate;
-import com.google.gerrit.server.query.Predicate;
-
-import java.util.List;
-import java.util.Set;
-
-@Deprecated
-class IsStarredByLegacyPredicate extends OrPredicate<ChangeData> {
-  private static List<Predicate<ChangeData>> predicates(Set<Change.Id> ids) {
-    List<Predicate<ChangeData>> r = Lists.newArrayListWithCapacity(ids.size());
-    for (Change.Id id : ids) {
-      r.add(new LegacyChangeIdPredicate(id));
-    }
-    return r;
-  }
-
-  private final Account.Id accountId;
-  private final Set<Change.Id> starredChanges;
-
-  IsStarredByLegacyPredicate(Account.Id accountId,
-      Set<Change.Id> starredChanges) {
-    super(predicates(starredChanges));
-    this.accountId = accountId;
-    this.starredChanges = starredChanges;
-  }
-
-  @Override
-  public boolean match(final ChangeData object) {
-    return starredChanges.contains(object.getId());
-  }
-
-  @Override
-  public int getCost() {
-    return 0;
-  }
-
-  @Override
-  public String toString() {
-    return ChangeQueryBuilder.FIELD_STARREDBY + ":" + accountId.toString();
-  }
-}

gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsStarredByPredicate.java

@@ -1,44 +0,0 @@
-// Copyright (C) 2010 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.query.change;
-
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.server.index.change.ChangeField;
-import com.google.gwtorm.server.OrmException;
-
-@Deprecated
-class IsStarredByPredicate extends ChangeIndexPredicate {
-  private final Account.Id accountId;
-
-  IsStarredByPredicate(Account.Id accountId) {
-    super(ChangeField.STARREDBY, accountId.toString());
-    this.accountId = accountId;
-  }
-
-  @Override
-  public boolean match(ChangeData cd) throws OrmException {
-    return cd.starredBy().contains(accountId);
-  }
-
-  @Override
-  public int getCost() {
-    return 1;
-  }
-
-  @Override
-  public String toString() {
-    return ChangeQueryBuilder.FIELD_STARREDBY + ":" + accountId;
-  }
-}

gerrit-server/src/main/java/com/google/gerrit/server/query/change/LegacyReviewerPredicate.java

@@ -1,43 +0,0 @@
-// Copyright (C) 2016 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.query.change;
-
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.server.index.change.ChangeField;
-import com.google.gwtorm.server.OrmException;
-
-@Deprecated
-class LegacyReviewerPredicate extends ChangeIndexPredicate {
-  private final Account.Id id;
-
-  LegacyReviewerPredicate(Account.Id id) {
-    super(ChangeField.LEGACY_REVIEWER, id.toString());
-    this.id = id;
-  }
-
-  Account.Id getAccountId() {
-    return id;
-  }
-
-  @Override
-  public boolean match(ChangeData object) throws OrmException {
-    return object.reviewers().all().contains(id);
-  }
-
-  @Override
-  public int getCost() {
-    return 1;
-  }
-}

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ReviewerPredicate.java

@@ -26,21 +26,16 @@ import java.util.ArrayList;
 import java.util.List;
 
 class ReviewerPredicate extends ChangeIndexPredicate {
-  @SuppressWarnings("deprecation")
   static Predicate<ChangeData> create(Arguments args, Account.Id id) {
     List<Predicate<ChangeData>> and = new ArrayList<>(2);
-    if (args.getSchema().hasField(ChangeField.REVIEWER)) {
+    ReviewerStateInternal[] states = ReviewerStateInternal.values();
-      ReviewerStateInternal[] states = ReviewerStateInternal.values();
+    List<Predicate<ChangeData>> or = new ArrayList<>(states.length - 1);
-      List<Predicate<ChangeData>> or = new ArrayList<>(states.length - 1);
+    for (ReviewerStateInternal state : states) {
-      for (ReviewerStateInternal state : states) {
+      if (state != ReviewerStateInternal.REMOVED) {
-        if (state != ReviewerStateInternal.REMOVED) {
+        or.add(new ReviewerPredicate(state, id));
-          or.add(new ReviewerPredicate(state, id));
-        }
       }
-      and.add(Predicate.or(or));
-    } else {
-      and.add(new LegacyReviewerPredicate(id));
     }
+    and.add(Predicate.or(or));
 
     // TODO(dborowitz): This really belongs much higher up e.g. QueryProcessor.
     if (!args.allowsDrafts) {