opendev/gerrit
Code Issues Proposed changes

Remove ApprovalCategory and ApprovalCategoryValue

Browse Source 
These can be completely specified in the project config hierarchy and
the database tables are no longer required. Also remove the size limit
on patch_set_approvals.category_id, so new PatchSetApprovals can refer
to label names rather than IDs.

Migrate existing labels into project.config in All-Projects. When
migrating, also convert existing PatchSetApprovals to refer to label
names rather than IDs.

Label IDs are still written to PatchSetApprovals and still supported
in project.config. As of this change, update all code to match
PatchSetApproval's categoryId on either label ID or name.  This allows
for the possibility of database migration without downtime (e.g. for
gerrit-review.googlesource.com). The default schema migration code,
however, does not include label IDs in project.config, since this
schema migration is only intended to run offline.

Change-Id: I5df6f0c5665d0ae4ee6b5e2944f5954fa2f96b5c
This commit is contained in:
Dave Borowitz
2013-02-18 14:53:57 -08:00
parent 6445559a45
commit 8e5de82e56
42 changed files with 566 additions and 795 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
View File

@@ -416,7 +416,7 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
for (Map.Entry<String, Short> approval : approvals.entrySet()) {
LabelType lt = labelTypes.byLabel(approval.getKey());
if (lt != null) {
addArg(args, "--" + lt.getId(), Short.toString(approval.getValue()));
addArg(args, "--" + lt.getName(), Short.toString(approval.getValue()));
}
}

9
gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
View File

@@ -21,10 +21,10 @@ import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Account.Id;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.server.OrmException;
@@ -86,7 +86,7 @@ public class ApprovalsUtil {
List<PatchSetApproval> patchSetApprovals =
db.patchSetApprovals().byChange(dest.getParentKey()).toList();
for (PatchSetApproval a : patchSetApprovals) {
LabelType type = labelTypes.byId(a.getCategoryId().get());
LabelType type = labelTypes.byLabel(a.getLabelId());
if (type != null && a.getPatchSetId().equals(source) &&
type.isCopyMinScore() &&
type.isMaxNegative(a)) {
@@ -123,11 +123,10 @@ public class ApprovalsUtil {
need.removeAll(existingReviewers);
List<PatchSetApproval> cells = Lists.newArrayListWithCapacity(need.size());
ApprovalCategory.Id catId =
Iterables.getLast(allTypes).getApprovalCategoryId();
LabelId labelId = Iterables.getLast(allTypes).getLabelId();
for (Account.Id account : need) {
PatchSetApproval psa = new PatchSetApproval(
new PatchSetApproval.Key(ps.getId(), account, catId),
new PatchSetApproval.Key(ps.getId(), account, labelId),
(short) 0);
psa.cache(change);
cells.add(psa);

29
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
View File

@@ -45,7 +45,6 @@ import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -403,8 +402,7 @@ public class ChangeJson {
for (PatchSetApproval psa : cd.currentApprovals(db)) {
short val = psa.getValue();
if (val != 0 && min < val && val < max
&& psa.getCategoryId().get().equals(type.getId())) {
if (val != 0 && min < val && val < max && type.matches(psa)) {
if (0 < val) {
label.recommended = accountLoader.get(psa.getAccountId());
label.value = val != 1 ? val : null;
@@ -447,7 +445,7 @@ public class ChangeJson {
}
for (PatchSetApproval psa : current.get(accountId)) {
// TODO Support arbitrary labels placed by a reviewer.
LabelType lt = ctl.getLabelTypes().byId(psa.getCategoryId().get());
LabelType lt = ctl.getLabelTypes().byLabel(psa.getLabelId());
if (lt == null) {
continue;
}
@@ -471,11 +469,12 @@ public class ChangeJson {
allUsers.add(psa.getAccountId());
}
Set<ApprovalCategory.Id> categories = Sets.newHashSet();
Set<String> labelNames = Sets.newHashSet();
Multimap<Account.Id, PatchSetApproval> current = HashMultimap.create();
for (PatchSetApproval a : cd.currentApprovals(db)) {
if (a.getValue() != 0) {
categories.add(a.getCategoryId());
LabelType type = labelTypes.byLabel(a.getLabelId());
if (type != null && a.getValue() != 0) {
labelNames.add(type.getName());
current.put(a.getAccountId(), a);
}
}
@@ -486,15 +485,13 @@ public class ChangeJson {
// Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167.
Map<String, LabelInfo> labels =
new TreeMap<String, LabelInfo>(labelTypes.nameComparator());
for (ApprovalCategory.Id id : categories) {
LabelType type = labelTypes.byId(id.get());
if (type != null) {
LabelInfo li = new LabelInfo();
if (detailed) {
setLabelValues(type, li);
}
labels.put(type.getName(), li);
for (String name : labelNames) {
LabelType type = labelTypes.byLabel(name);
LabelInfo li = new LabelInfo();
if (detailed) {
setLabelValues(type, li);
}
labels.put(type.getName(), li);
}
for (Account.Id accountId : allUsers) {
@@ -509,7 +506,7 @@ public class ChangeJson {
}
}
for (PatchSetApproval psa : current.get(accountId)) {
LabelType type = labelTypes.byId(psa.getCategoryId().get());
LabelType type = labelTypes.byLabel(psa.getLabelId());
if (type == null) {
continue;
}

12
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
View File

@@ -28,7 +28,6 @@ import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.Patch;
@@ -180,8 +179,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
if (lt == null) {
if (strict) {
throw new BadRequestException(String.format(
"label \"%s\" is not a configured ApprovalCategory",
ent.getKey()));
"label \"%s\" is not a configured label", ent.getKey()));
} else {
itr.remove();
continue;
@@ -371,7 +369,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
c = new PatchSetApproval(new PatchSetApproval.Key(
rsrc.getPatchSet().getId(),
rsrc.getAccountId(),
lt.getApprovalCategoryId()),
lt.getLabelId()),
ent.getValue());
c.setGranted(timestamp);
c.cache(change);
@@ -400,7 +398,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
rsrc.getPatchSet().getId(),
rsrc.getAccountId(),
rsrc.getControl().getLabelTypes().getLabelTypes().get(0)
.getApprovalCategoryId()),
.getLabelId()),
(short) 0);
c.setGranted(timestamp);
c.cache(change);
@@ -424,11 +422,11 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
Map<String, PatchSetApproval> current = Maps.newHashMap();
for (PatchSetApproval a : db.patchSetApprovals().byPatchSetUser(
rsrc.getPatchSet().getId(), rsrc.getAccountId())) {
if (ApprovalCategory.SUBMIT_ID.equals(a.getCategoryId().get())) {
if (a.isSubmit()) {
continue;
}
LabelType lt = labelTypes.byId(a.getCategoryId().get());
LabelType lt = labelTypes.byLabel(a.getLabelId());
if (lt != null) {
current.put(lt.getName(), a);
} else {

6
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
View File

@@ -32,10 +32,10 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
@@ -274,8 +274,8 @@ public class PostReviewers implements RestModifyView<ChangeResource, Input> {
private PatchSetApproval dummyApproval(ChangeControl ctl,
PatchSet.Id patchSetId, Account.Id reviewerId) {
ApprovalCategory.Id id = new ApprovalCategory.Id(
Iterables.getLast(ctl.getLabelTypes().getLabelTypes()).getId());
LabelId id =
Iterables.getLast(ctl.getLabelTypes().getLabelTypes()).getLabelId();
PatchSetApproval dummyApproval = new PatchSetApproval(
new PatchSetApproval.Key(patchSetId, reviewerId, id), (short) 0);
dummyApproval.cache(ctl.getChange());

2
gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
View File

@@ -93,7 +93,7 @@ public class ReviewerJson {
for (PermissionRange pr : ctl.getLabelRanges()) {
if (!pr.isEmpty()) {
// TODO: Support arbitrary labels.
LabelType at = labelTypes.byId(ca.getCategoryId().get());
LabelType at = labelTypes.byLabel(ca.getLabelId());
if (at != null) {
out.approvals.put(at.getName(), formatValue(ca.getValue())); }
}

6
gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
View File

@@ -24,11 +24,11 @@ import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
@@ -187,7 +187,7 @@ public class Submit implements RestModifyView<RevisionResource, Input> {
new Predicate<PatchSetApproval>() {
@Override
public boolean apply(PatchSetApproval input) {
return ApprovalCategory.SUBMIT_ID.equals(input.getCategoryId().get());
return input.isSubmit();
}
}), null);
if (submit == null) {
@@ -195,7 +195,7 @@ public class Submit implements RestModifyView<RevisionResource, Input> {
new PatchSetApproval.Key(
rev.getId(),
caller.getAccountId(),
new ApprovalCategory.Id(ApprovalCategory.SUBMIT_ID)),
LabelId.SUBMIT),
(short) 1);
}
submit.setValue((short) 1);

3
gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
View File

@@ -19,7 +19,6 @@ import static com.google.inject.Scopes.SINGLETON;
import com.google.common.cache.Cache;
import com.google.gerrit.audit.AuditModule;
import com.google.gerrit.common.ChangeListener;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.events.NewProjectCreatedListener;
import com.google.gerrit.extensions.registration.DynamicItem;
@@ -142,8 +141,6 @@ public class GerritGlobalModule extends FactoryModule {
break;
}
bind(LabelTypes.class).toProvider(LabelTypesProvider.class).in(
SINGLETON);
bind(EmailExpander.class).toProvider(EmailExpanderProvider.class).in(
SINGLETON);

61
gerrit-server/src/main/java/com/google/gerrit/server/config/LabelTypesProvider.java
View File

@@ -1,61 +0,0 @@
// Copyright (C) 2009 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.config;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.ProvisionException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
public class LabelTypesProvider implements Provider<LabelTypes> {
private final SchemaFactory<ReviewDb> schema;
@Inject
LabelTypesProvider(final SchemaFactory<ReviewDb> sf) {
schema = sf;
}
@Override
public LabelTypes get() {
List<LabelType> types = new ArrayList<LabelType>(2);
try {
final ReviewDb db = schema.open();
try {
for (final ApprovalCategory c : db.approvalCategories().all()) {
final List<ApprovalCategoryValue> values =
db.approvalCategoryValues().byCategory(c.getId()).toList();
types.add(LabelType.fromApprovalCategory(c, values));
}
} finally {
db.close();
}
} catch (OrmException e) {
throw new ProvisionException("Cannot query label categories", e);
}
return new LabelTypes(Collections.unmodifiableList(types));
}
}

4
gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java
View File

@@ -456,12 +456,12 @@ public class EventFactory {
public ApprovalAttribute asApprovalAttribute(PatchSetApproval approval,
LabelTypes labelTypes) {
ApprovalAttribute a = new ApprovalAttribute();
a.type = approval.getCategoryId().get();
a.type = approval.getLabelId().get();
a.value = Short.toString(approval.getValue());
a.by = asAccountAttribute(approval.getAccountId());
a.grantedOn = approval.getGranted().getTime() / 1000L;
LabelType lt = labelTypes.byId(approval.getCategoryId().get());
LabelType lt = labelTypes.byLabel(approval.getLabelId());
if (lt != null) {
a.description = lt.getName();
}

6
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@@ -16,10 +16,10 @@ package com.google.gerrit.server.git;
import static com.google.gerrit.server.git.MergeUtil.getSubmitter;
import static java.util.concurrent.TimeUnit.DAYS;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.SECONDS;
import static java.util.concurrent.TimeUnit.DAYS;
import com.google.common.base.Objects;
import com.google.common.collect.ArrayListMultimap;
@@ -31,7 +31,6 @@ import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
@@ -938,8 +937,7 @@ public class MergeOp {
CategoryFunction.forType(lt).run(lt, fs);
}
for (PatchSetApproval a : approvals) {
if (a.getValue() > 0
&& ApprovalCategory.SUBMIT_ID.equals(a.getCategoryId().get())
if (a.getValue() > 0 && a.isSubmit()
&& a.getPatchSetId().equals(merged)) {
if (submitter == null
|| a.getGranted().compareTo(submitter.getGranted()) > 0) {

24
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
View File

@@ -16,10 +16,10 @@ package com.google.gerrit.server.git;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.config.CanonicalWebUrl;
@@ -82,9 +82,6 @@ public class MergeUtil {
private static final String R_HEADS_MASTER =
Constants.R_HEADS + Constants.MASTER;
private static final String CRVW = "CRVW";
private static final String VRIF = "VRIF";
private static final FooterKey REVIEWED_ON = new FooterKey("Reviewed-on");
private static final FooterKey CHANGE_ID = new FooterKey("Change-Id");
@@ -171,7 +168,7 @@ public class MergeUtil {
final List<PatchSetApproval> approvals =
reviewDb.patchSetApprovals().byPatchSet(c).toList();
for (PatchSetApproval a : approvals) {
if (a.getValue() > 0 && ApprovalCategory.isSubmit(a)) {
if (a.getValue() > 0 && a.isSubmit()) {
if (submitter == null
|| a.getGranted().compareTo(submitter.getGranted()) > 0) {
submitter = a;
@@ -257,7 +254,7 @@ public class MergeUtil {
continue;
}
if (ApprovalCategory.isSubmit(a)) {
if (a.isSubmit()) {
// Submit is treated specially, below (becomes committer)
//
if (submitAudit == null
@@ -294,13 +291,12 @@ public class MergeUtil {
}
final String tag;
if (CRVW.equals(a.getCategoryId().get())) {
if (isCodeReview(a.getLabelId())) {
tag = "Reviewed-by";
} else if (VRIF.equals(a.getCategoryId().get())) {
} else if (isVerified(a.getLabelId())) {
tag = "Tested-by";
} else {
final LabelType lt =
project.getLabelTypes().byId(a.getCategoryId().get());
final LabelType lt = project.getLabelTypes().byLabel(a.getLabelId());
if (lt == null) {
// TODO: Support arbitrary labels.
continue;
@@ -319,6 +315,14 @@ public class MergeUtil {
return msgbuf.toString();
}
private static boolean isCodeReview(LabelId id) {
return "CRVW".equals(id.get()) || "Code-Review".equalsIgnoreCase(id.get());
}
private static boolean isVerified(LabelId id) {
return "VRIF".equals(id.get()) || "Verified".equalsIgnoreCase(id.get());
}
public List<PatchSetApproval> getApprovalsForCommit(final CodeReviewCommit n) {
try {
List<PatchSetApproval> approvalList =

64
gerrit-server/src/main/java/com/google/gerrit/server/git/ProjectConfig.java
View File

@@ -22,6 +22,7 @@ import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.primitives.Shorts;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.ContributorAgreement;
@@ -225,8 +226,8 @@ public class ProjectConfig extends VersionedMetaData {
return notifySections.values();
}
public Collection<LabelType> getLabelSections() {
return labelSections.values();
public Map<String, LabelType> getLabelSections() {
return labelSections;
}
public GroupReference resolve(AccountGroup group) {
@@ -545,21 +546,14 @@ public class ProjectConfig extends VersionedMetaData {
}
LabelType label;
String id = rc.getString(LABEL, name, KEY_ID);
if (id == null || id.length() > 4) {
error(new ValidationError(PROJECT_CONFIG, String.format(
"Invalid label ID \"%s\" for label \"%s\": "
+ "Label ID may have at most 4 characters", id, name)));
continue;
}
try {
label = new LabelType(id, name, values);
label = new LabelType(name, values);
} catch (IllegalArgumentException badName) {
error(new ValidationError(PROJECT_CONFIG, String.format(
"Invalid label \"%s\"", name)));
continue;
}
label.setId(rc.getString(LABEL, name, KEY_ID));
String abbr = rc.getString(LABEL, name, KEY_ABBREVIATION);
if (abbr != null) {
label.setAbbreviatedName(abbr);
@@ -645,6 +639,7 @@ public class ProjectConfig extends VersionedMetaData {
saveAccessSections(rc, keepGroups);
saveNotifySections(rc, keepGroups);
groupsByUUID.keySet().retainAll(keepGroups);
saveLabelSections(rc);
saveConfig(PROJECT_CONFIG, rc);
saveGroupList();
@@ -814,6 +809,53 @@ public class ProjectConfig extends VersionedMetaData {
}
}
private void saveLabelSections(Config rc) {
List<String> existing = Lists.newArrayList(rc.getSubsections(LABEL));
if (!Lists.newArrayList(labelSections.keySet()).equals(existing)) {
// Order of sections changed, remove and rewrite them all.
for (String name : existing) {
rc.unsetSection(LABEL, name);
}
}
Set<String> toUnset = Sets.newHashSet(existing);
for (Map.Entry<String, LabelType> e : labelSections.entrySet()) {
String name = e.getKey();
LabelType label = e.getValue();
toUnset.remove(name);
rc.setString(LABEL, name, KEY_FUNCTION, label.getFunctionName());
if (!LabelType.defaultAbbreviation(name)
.equals(label.getAbbreviatedName())) {
rc.setString(
LABEL, name, KEY_ABBREVIATION, label.getAbbreviatedName());
} else {
rc.unset(LABEL, name, KEY_ABBREVIATION);
}
if (label.isCopyMinScore()) {
rc.setBoolean(LABEL, name, KEY_COPY_MIN_SCORE, true);
} else {
rc.unset(LABEL, name, KEY_COPY_MIN_SCORE);
}
if (!label.canOverride()) {
rc.setBoolean(LABEL, name, KEY_CAN_OVERRIDE, false);
} else {
rc.unset(LABEL, name, KEY_CAN_OVERRIDE);
}
List<String> values =
Lists.newArrayListWithCapacity(label.getValues().size());
for (LabelValue value : label.getValues()) {
values.add(value.format());
}
rc.setStringList(LABEL, name, KEY_VALUE, values);
}
for (String name : toUnset) {
rc.unsetSection(LABEL, name);
}
}
private void saveGroupList() throws IOException {
if (groupsByUUID.isEmpty()) {
saveFile(GROUP_LIST, null);

2
gerrit-server/src/main/java/com/google/gerrit/server/mail/MergedSender.java
View File

@@ -63,7 +63,7 @@ public class MergedSender extends ReplyToChangeSender {
Table<Account.Id, String, PatchSetApproval> neg = HashBasedTable.create();
for (PatchSetApproval ca : args.db.get().patchSetApprovals()
.byPatchSet(patchSet.getId())) {
LabelType lt = labelTypes.byId(ca.getCategoryId().get());
LabelType lt = labelTypes.byLabel(ca.getLabelId());
if (lt == null) {
continue;
}

20
gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectState.java
View File

@@ -69,7 +69,6 @@ public class ProjectState {
private final PrologEnvironment.Factory envFactory;
private final GitRepositoryManager gitMgr;
private final RulesCache rulesCache;
private final LabelTypes dbLabelTypes;
private final ProjectConfig config;
private final Set<AccountGroup.UUID> localOwners;
@@ -94,7 +93,6 @@ public class ProjectState {
final PrologEnvironment.Factory envFactory,
final GitRepositoryManager gitMgr,
final RulesCache rulesCache,
final LabelTypes labelTypes,
@Assisted final ProjectConfig config) {
this.projectCache = projectCache;
this.isAllProjects = config.getProject().getNameKey().equals(allProjectsName);
@@ -107,7 +105,6 @@ public class ProjectState {
this.capabilities = isAllProjects
? new CapabilityCollection(config.getAccessSection(AccessSection.GLOBAL_CAPABILITIES))
: null;
this.dbLabelTypes = labelTypes;
if (isAllProjects && !Permission.canBeOnAllProjects(AccessSection.ALL, Permission.OWNER)) {
localOwners = Collections.emptySet();
@@ -344,23 +341,16 @@ public class ProjectState {
});
}
private void putLabelType(Map<String, LabelType> types, LabelType type) {
LabelType old = types.get(type.getName());
if (old == null || old.canOverride()) {
types.put(type.getName(), type);
}
}
public LabelTypes getLabelTypes() {
Map<String, LabelType> types = Maps.newLinkedHashMap();
for (LabelType type : dbLabelTypes.getLabelTypes()) {
putLabelType(types, type);
}
List<ProjectState> projects = Lists.newArrayList(tree());
Collections.reverse(projects);
for (ProjectState s : projects) {
for (LabelType type : s.getConfig().getLabelSections()) {
putLabelType(types, type);
for (LabelType type : s.getConfig().getLabelSections().values()) {
LabelType old = types.get(type.getName());
if (old == null || !old.canOverride()) {
types.put(type.getName(), type);
}
}
}
List<LabelType> all = Lists.newArrayListWithCapacity(types.size());

8
gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
View File

@@ -64,10 +64,6 @@ class LabelPredicate extends OperatorPredicate<ChangeData> {
return types.byLabel(toFind);
}
if (types.byId(toFind) != null) {
return types.byId(toFind);
}
for (LabelType lt : types.getLabelTypes()) {
if (toFind.equalsIgnoreCase(lt.getName())) {
return lt;
@@ -80,7 +76,7 @@ class LabelPredicate extends OperatorPredicate<ChangeData> {
}
}
return LabelType.withDefaultValues(toFind.substring(0, 4), toFind);
return LabelType.withDefaultValues(toFind);
}
private static Test op(String op) {
@@ -162,7 +158,7 @@ class LabelPredicate extends OperatorPredicate<ChangeData> {
final Set<Account.Id> approversThatVotedInCategory = new HashSet<Account.Id>();
for (PatchSetApproval p : object.currentApprovals(dbProvider)) {
allApprovers.add(p.getAccountId());
if (p.getCategoryId().get().equals(labelType.getId())) {
if (labelType.matches(p)) {
approversThatVotedInCategory.add(p.getAccountId());
if (match(c, p.getValue(), p.getAccountId(), labelType)) {
return true;

72
gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaCreator.java
View File

@@ -14,15 +14,16 @@
package com.google.gerrit.server.schema;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Version;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupName;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.client.CurrentSchemaVersion;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.Project.InheritableBoolean;
@@ -51,7 +52,6 @@ import org.eclipse.jgit.lib.Repository;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
/** Creates the current database schema and populates initial code rows. */
@@ -109,15 +109,8 @@ public class SchemaCreator {
sVer.versionNbr = versionNbr;
db.schemaVersion().insert(Collections.singleton(sVer));
final SystemConfig sConfig = initSystemConfig(db);
initVerifiedCategory(db);
initCodeReviewCategory(db, sConfig);
if (mgr != null) {
// TODO This should never be null when initializing a site.
initWildCardProject();
}
initSystemConfig(db);
initWildCardProject();
dataSourceType.getIndexScript().run(db);
}
@@ -241,6 +234,9 @@ public class SchemaCreator {
metaReadPermission.setExclusiveGroup(true);
metaReadPermission.add(rule(config, owners));
initVerifiedCategory(config);
initCodeReviewCategory(config);
md.setMessage("Initialized Gerrit Code Review " + Version.getVersion());
config.commit(md);
} finally {
@@ -252,43 +248,23 @@ public class SchemaCreator {
return new PermissionRule(config.resolve(group));
}
private void initVerifiedCategory(final ReviewDb c) throws OrmException {
final ApprovalCategory cat;
final ArrayList<ApprovalCategoryValue> vals;
cat = new ApprovalCategory(new ApprovalCategory.Id("VRIF"), "Verified");
cat.setPosition((short) 0);
cat.setAbbreviatedName("V");
vals = new ArrayList<ApprovalCategoryValue>();
vals.add(value(cat, 1, "Verified"));
vals.add(value(cat, 0, "No score"));
vals.add(value(cat, -1, "Fails"));
c.approvalCategories().insert(Collections.singleton(cat));
c.approvalCategoryValues().insert(vals);
private void initVerifiedCategory(ProjectConfig c) {
LabelType type = new LabelType("Verified", ImmutableList.of(
new LabelValue((short) 1, "Verified"),
new LabelValue((short) 0, "No score"),
new LabelValue((short) -1, "Fails")));
c.getLabelSections().put(type.getName(), type);
}
private void initCodeReviewCategory(final ReviewDb c,
final SystemConfig sConfig) throws OrmException {
final ApprovalCategory cat;
final ArrayList<ApprovalCategoryValue> vals;
cat = new ApprovalCategory(new ApprovalCategory.Id("CRVW"), "Code Review");
cat.setPosition((short) 1);
cat.setAbbreviatedName("R");
cat.setCopyMinScore(true);
vals = new ArrayList<ApprovalCategoryValue>();
vals.add(value(cat, 2, "Looks good to me, approved"));
vals.add(value(cat, 1, "Looks good to me, but someone else must approve"));
vals.add(value(cat, 0, "No score"));
vals.add(value(cat, -1, "I would prefer that you didn't submit this"));
vals.add(value(cat, -2, "Do not submit"));
c.approvalCategories().insert(Collections.singleton(cat));
c.approvalCategoryValues().insert(vals);
}
private static ApprovalCategoryValue value(final ApprovalCategory cat,
final int value, final String name) {
return new ApprovalCategoryValue(new ApprovalCategoryValue.Id(cat.getId(),
(short) value), name);
private void initCodeReviewCategory(ProjectConfig c) {
LabelType type = new LabelType("Code-Review", ImmutableList.of(
new LabelValue((short) 2, "Looks good to me, approved"),
new LabelValue((short) 1, "Looks good to me, but someone else must approve"),
new LabelValue((short) 0, "No score"),
new LabelValue((short) -1, "I would prefer that you didn't submit this"),
new LabelValue((short) -2, "Do not submit")));
type.setAbbreviatedName("CR");
type.setCopyMinScore(true);
c.getLabelSections().put(type.getName(), type);
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
View File

@@ -32,7 +32,7 @@ import java.util.List;
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
public static final Class<Schema_76> C = Schema_76.class;
public static final Class<Schema_77> C = Schema_77.class;
public static class Module extends AbstractModule {
@Override

37
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_53.java
View File

@@ -28,10 +28,12 @@ import static com.google.gerrit.common.data.Permission.SUBMIT;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.Project.InheritableBoolean;
import com.google.gerrit.reviewdb.client.SystemConfig;
@@ -68,7 +70,7 @@ class Schema_53 extends SchemaVersion {
private SystemConfig systemConfig;
private Map<AccountGroup.Id, GroupReference> groupMap;
private Map<ApprovalCategory.Id, ApprovalCategory> categoryMap;
private LabelTypes labelTypes;
private GroupReference projectOwners;
private Map<Project.NameKey, Project.NameKey> parentsByProject;
@@ -93,7 +95,7 @@ class Schema_53 extends SchemaVersion {
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException,
SQLException {
systemConfig = db.systemConfig().get(new SystemConfig.Key());
categoryMap = db.approvalCategories().toMap(db.approvalCategories().all());
labelTypes = Schema_77.getLegacyTypes(db);
assignGroupUUIDs(db);
readOldRefRights(db);
@@ -104,13 +106,17 @@ class Schema_53 extends SchemaVersion {
}
private void deleteActionCategories(ReviewDb db) throws OrmException {
List<ApprovalCategory> delete = new ArrayList<ApprovalCategory>();
for (ApprovalCategory category : categoryMap.values()) {
if (category.getPosition() < 0) {
delete.add(category);
try {
Statement stmt = ((JdbcSchema) db).getConnection().createStatement();
try {
stmt.executeUpdate(
"DELETE FROM approval_categories WHERE position < 0");
} finally {
stmt.close();
}
} catch (SQLException e) {
throw new OrmException(e);
}
db.approvalCategories().delete(delete);
}
private void assignGroupUUIDs(ReviewDb db) throws OrmException {
@@ -365,8 +371,7 @@ class Schema_53 extends SchemaVersion {
if (3 <= old.max_value) {
add(section, FORGE_SERVER, old.exclusive, rule(group));
} else if (3 <= inheritedMax(config, old)) {
add(section, FORGE_SERVER, old.exclusive, deny(group));
} else if (3 <= inheritedMax(config, old)) { add(section, FORGE_SERVER, old.exclusive, deny(group));
}
} else {
@@ -375,7 +380,9 @@ class Schema_53 extends SchemaVersion {
if (old.min_value == 0 && old.max_value == 0) {
rule.setDeny();
}
add(section, LABEL + varNameOf(old.category), old.exclusive, rule);
LabelType type = labelTypes.byLabel(new LabelId(old.category));
String name = type != null ? type.getName() : old.category;
add(section, LABEL + name, old.exclusive, rule);
}
}
}
@@ -443,14 +450,6 @@ class Schema_53 extends SchemaVersion {
return max;
}
private String varNameOf(String id) {
ApprovalCategory category = categoryMap.get(new ApprovalCategory.Id(id));
if (category == null) {
category = new ApprovalCategory(new ApprovalCategory.Id(id), id);
}
return category.getLabelName();
}
private static void add(AccessSection section, String name,
boolean exclusive, PermissionRule rule) {
Permission p = section.getPermission(name, true);

219
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_77.java Normal file
View File

@@ -0,0 +1,219 @@
// Copyright (C) 2013 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.schema;
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gwtorm.jdbc.JdbcSchema;
import com.google.gwtorm.schema.sql.DialectH2;
import com.google.gwtorm.schema.sql.DialectMySQL;
import com.google.gwtorm.schema.sql.DialectPostgreSQL;
import com.google.gwtorm.schema.sql.SqlDialect;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import java.io.IOException;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.List;
import java.util.Map;
public class Schema_77 extends SchemaVersion {
private final GitRepositoryManager mgr;
private final AllProjectsName allProjects;
private final PersonIdent serverUser;
@Inject
Schema_77(Provider<Schema_76> prior, AllProjectsName allProjects,
GitRepositoryManager mgr, @GerritPersonIdent PersonIdent serverUser) {
super(prior);
this.allProjects = allProjects;
this.mgr = mgr;
this.serverUser = serverUser;
}
@Override
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
try {
LabelTypes labelTypes = getLegacyTypes(db);
SqlDialect dialect = ((JdbcSchema) db).getDialect();
if (dialect instanceof DialectH2) {
alterTable(db, "ALTER TABLE %s ALTER COLUMN %s varchar(255)");
} else if (dialect instanceof DialectPostgreSQL) {
alterTable(db, "ALTER TABLE %s ALTER %s TYPE varchar(255)");
} else if (dialect instanceof DialectMySQL) {
alterTable(db, "ALTER TABLE %s MODIFY %s varchar(255) BINARY");
} else {
alterTable(db, "ALTER TABLE %s MODIFY %s varchar(255)");
}
migratePatchSetApprovals(db, labelTypes);
migrateLabelsToAllProjects(db, labelTypes);
} catch (RepositoryNotFoundException e) {
throw new OrmException(e);
} catch (SQLException e) {
throw new OrmException(e);
} catch (IOException e) {
throw new OrmException(e);
} catch (ConfigInvalidException e) {
throw new OrmException(e);
}
ui.message(
"Migrated label types from database to All-Projects project.config");
}
private void alterTable(ReviewDb db, String sqlFormat) throws SQLException {
Statement stmt = ((JdbcSchema) db).getConnection().createStatement();
try {
stmt.executeUpdate(
String.format(sqlFormat, "patch_set_approvals", "category_id"));
} finally {
stmt.close();
}
}
private void migrateLabelsToAllProjects(ReviewDb db, LabelTypes labelTypes)
throws SQLException, RepositoryNotFoundException, IOException,
ConfigInvalidException {
Repository git = mgr.openRepository(allProjects);
try {
MetaDataUpdate md =
new MetaDataUpdate(GitReferenceUpdated.DISABLED, allProjects, git);
md.getCommitBuilder().setAuthor(serverUser);
md.getCommitBuilder().setCommitter(serverUser);
ProjectConfig config = ProjectConfig.read(md);
Map<String, LabelType> configTypes = config.getLabelSections();
List<LabelType> newTypes = Lists.newArrayList();
for (LabelType type : labelTypes.getLabelTypes()) {
// Don't include IDs for this migration, since we are also updating all
// existing PatchSetApprovals.
type.setId(null);
if (!configTypes.containsKey(type.getName())) {
newTypes.add(type);
}
}
newTypes.addAll(configTypes.values());
configTypes.clear();
for (LabelType type : newTypes) {
configTypes.put(type.getName(), type);
}
md.setMessage("Upgrade to Gerrit Code Review schema 77\n");
config.commit(md);
} finally {
git.close();
}
}
private void migratePatchSetApprovals(ReviewDb db, LabelTypes labelTypes)
throws SQLException {
PreparedStatement stmt = ((JdbcSchema) db).getConnection().prepareStatement(
"UPDATE patch_set_approvals SET category_id = ? WHERE category_id = ?");
try {
for (LabelType type : labelTypes.getLabelTypes()) {
stmt.setString(1, type.getName());
stmt.setString(2, type.getId());
stmt.addBatch();
}
stmt.executeBatch();
} finally {
stmt.close();
}
}
static LabelTypes getLegacyTypes(ReviewDb db) throws SQLException {
List<LabelType> types = Lists.newArrayListWithCapacity(2);
Statement catStmt = null;
PreparedStatement valStmt = null;
ResultSet catRs = null;
try {
catStmt = ((JdbcSchema) db).getConnection().createStatement();
catRs = catStmt.executeQuery(
"SELECT category_id, name, abbreviated_name, function_name, "
+ " copy_min_score"
+ " FROM approval_categories"
+ " ORDER BY position, name");
valStmt = ((JdbcSchema) db).getConnection().prepareStatement(
"SELECT value, name"
+ " FROM approval_category_values"
+ " WHERE category_id = ?");
while (catRs.next()) {
String id = catRs.getString("category_id");
valStmt.setString(1, id);
List<LabelValue> values = Lists.newArrayListWithCapacity(5);
ResultSet valRs = valStmt.executeQuery();
try {
while (valRs.next()) {
values.add(new LabelValue(
valRs.getShort("value"), valRs.getString("name")));
}
} finally {
valRs.close();
}
LabelType type =
new LabelType(getLabelName(catRs.getString("name")), values);
type.setId(id);
type.setAbbreviatedName(catRs.getString("abbreviated_name"));
type.setFunctionName(catRs.getString("function_name"));
type.setCopyMinScore("Y".equals(catRs.getString("copy_min_score")));
types.add(type);
}
} finally {
if (valStmt != null) {
valStmt.close();
}
if (catRs != null) {
catRs.close();
}
if (catStmt != null) {
catStmt.close();
}
}
return new LabelTypes(types);
}
private static String getLabelName(String name) {
StringBuilder r = new StringBuilder();
for (int i = 0; i < name.length(); i++) {
char c = name.charAt(i);
if (('0' <= c && c <= '9') //
|| ('a' <= c && c <= 'z') //
|| ('A' <= c && c <= 'Z') //
|| (c == '-')) {
r.append(c);
} else if (c == ' ') {
r.append('-');
}
}
return r.toString();
}
}

13
gerrit-server/src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java
View File

@@ -15,7 +15,6 @@
package com.google.gerrit.server.workflow;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import java.util.HashMap;
@@ -48,18 +47,6 @@ public abstract class CategoryFunction {
return r != null ? r : new NoOpFunction();
}
/**
* Locate a function by category.
*
* @param category the category the function is for.
* @return the function implementation; {@link NoOpFunction} if the function
* is not known to Gerrit and thus cannot be executed.
*/
public static CategoryFunction forCategory(final ApprovalCategory category) {
final CategoryFunction r = all.get(category.getFunctionName());
return r != null ? r : new NoOpFunction();
}
/**
* Normalize ChangeApprovals and set the valid flag for this category.
* <p>

19
gerrit-server/src/main/java/com/google/gerrit/server/workflow/FunctionState.java
View File

@@ -60,15 +60,14 @@ public class FunctionState {
for (final PatchSetApproval ca : all) {
if (psId.equals(ca.getPatchSetId())) {
Collection<PatchSetApproval> l =
approvals.get(ca.getCategoryId().get());
LabelType lt = c.getLabelTypes().byLabel(ca.getLabelId());
if (lt == null) {
continue;
}
Collection<PatchSetApproval> l = approvals.get(lt.getName());
if (l == null) {
l = new ArrayList<PatchSetApproval>();
LabelType lt = c.getLabelTypes().byId(ca.getCategoryId().get());
if (lt != null) {
// TODO: Support arbitrary labels
approvals.put(lt.getName(), l);
}
approvals.put(lt.getName(), l);
}
l.add(ca);
}
@@ -84,7 +83,7 @@ public class FunctionState {
}
public void valid(final LabelType lt, final boolean v) {
valid.put(id(lt), v);
valid.put(lt.getName(), v);
}
public boolean isValid(final LabelType lt) {
@@ -148,8 +147,4 @@ public class FunctionState {
applyTypeFloor(lt, ca);
applyRightFloor(lt, ca);
}
private static String id(final LabelType lt) {
return lt.getId();
}
}

1
gerrit-server/src/main/java/com/google/gerrit/server/workflow/MaxNoBlock.java
View File

@@ -15,7 +15,6 @@
package com.google.gerrit.server.workflow;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
/**

1
gerrit-server/src/main/java/com/google/gerrit/server/workflow/MaxWithBlock.java
View File

@@ -15,7 +15,6 @@
package com.google.gerrit.server.workflow;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
/**