diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 89cc724242..10a88529e2 100644 --- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -1257,7 +1257,7 @@ public abstract class AbstractDaemonTest { protected GroupReference groupRef(AccountGroup.UUID groupUuid) { GroupDescription.Basic groupDescription = groupBackend.get(groupUuid); - return new GroupReference(groupDescription.getGroupUUID(), groupDescription.getName()); + return GroupReference.create(groupDescription.getGroupUUID(), groupDescription.getName()); } protected InternalGroup group(String groupName) { @@ -1269,7 +1269,7 @@ public abstract class AbstractDaemonTest { protected GroupReference groupRef(String groupName) { InternalGroup group = groupCache.get(AccountGroup.nameKey(groupName)).orElse(null); assertThat(group).isNotNull(); - return new GroupReference(group.getGroupUUID(), group.getName()); + return GroupReference.create(group.getGroupUUID(), group.getName()); } protected AccountGroup.UUID groupUuid(String groupName) { diff --git a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java index 21bfcd1b8a..de83cff1c4 100644 --- a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java +++ b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java @@ -154,7 +154,7 @@ public class ProjectOperationsImpl implements ProjectOperations { Permission permission = projectConfig.getAccessSection(p.section(), true).getPermission(p.name(), true); if (p.group().isPresent()) { - GroupReference group = new GroupReference(p.group().get(), p.group().get().get()); + GroupReference group = GroupReference.create(p.group().get(), p.group().get().get()); group = projectConfig.resolve(group); permission.removeRule(group); } else { @@ -325,7 +325,7 @@ public class ProjectOperationsImpl implements ProjectOperations { } private static PermissionRule newRule(ProjectConfig project, AccountGroup.UUID groupUUID) { - GroupReference group = new GroupReference(groupUUID, groupUUID.get()); + GroupReference group = GroupReference.create(groupUUID, groupUUID.get()); group = project.resolve(group); return new PermissionRule(group); } diff --git a/java/com/google/gerrit/common/data/GroupReference.java b/java/com/google/gerrit/common/data/GroupReference.java index 0af088ede9..2620138f61 100644 --- a/java/com/google/gerrit/common/data/GroupReference.java +++ b/java/com/google/gerrit/common/data/GroupReference.java @@ -16,16 +16,18 @@ package com.google.gerrit.common.data; import static java.util.Objects.requireNonNull; +import com.google.auto.value.AutoValue; import com.google.gerrit.common.Nullable; import com.google.gerrit.entities.AccountGroup; /** Describes a group within a projects {@link AccessSection}s. */ -public class GroupReference implements Comparable { +@AutoValue +public abstract class GroupReference implements Comparable { private static final String PREFIX = "group "; public static GroupReference forGroup(GroupDescription.Basic group) { - return new GroupReference(group.getGroupUUID(), group.getName()); + return GroupReference.create(group.getGroupUUID(), group.getName()); } public static boolean isGroupReference(String configValue) { @@ -40,10 +42,10 @@ public class GroupReference implements Comparable { return configValue.substring(PREFIX.length()).trim(); } - protected String uuid; - protected String name; + @Nullable + public abstract AccountGroup.UUID getUUID(); - protected GroupReference() {} + public abstract String getName(); /** * Create a group reference. @@ -51,9 +53,8 @@ public class GroupReference implements Comparable { * @param uuid UUID of the group, must not be {@code null} * @param name the group name, must not be {@code null} */ - public GroupReference(AccountGroup.UUID uuid, String name) { - setUUID(requireNonNull(uuid)); - setName(name); + public static GroupReference create(AccountGroup.UUID uuid, String name) { + return new AutoValue_GroupReference(requireNonNull(uuid), requireNonNull(name)); } /** @@ -61,33 +62,12 @@ public class GroupReference implements Comparable { * * @param name the group name, must not be {@code null} */ - public GroupReference(String name) { - setUUID(null); - setName(name); - } - - @Nullable - public AccountGroup.UUID getUUID() { - return uuid != null ? AccountGroup.uuid(uuid) : null; - } - - public void setUUID(@Nullable AccountGroup.UUID newUUID) { - uuid = newUUID != null ? newUUID.get() : null; - } - - public String getName() { - return name; - } - - public void setName(String newName) { - if (newName == null) { - throw new NullPointerException(); - } - this.name = newName; + public static GroupReference create(String name) { + return new AutoValue_GroupReference(null, name); } @Override - public int compareTo(GroupReference o) { + public final int compareTo(GroupReference o) { return uuid(this).compareTo(uuid(o)); } @@ -100,21 +80,21 @@ public class GroupReference implements Comparable { } @Override - public int hashCode() { + public final int hashCode() { return uuid(this).hashCode(); } @Override - public boolean equals(Object o) { + public final boolean equals(Object o) { return o instanceof GroupReference && compareTo((GroupReference) o) == 0; } - public String toConfigValue() { - return PREFIX + name; - } - @Override - public String toString() { + public final String toString() { return "Group[" + getName() + " / " + getUUID() + "]"; } + + public String toConfigValue() { + return PREFIX + getName(); + } } diff --git a/java/com/google/gerrit/common/data/PermissionRule.java b/java/com/google/gerrit/common/data/PermissionRule.java index 8ab0a55e7f..ce946955de 100644 --- a/java/com/google/gerrit/common/data/PermissionRule.java +++ b/java/com/google/gerrit/common/data/PermissionRule.java @@ -255,8 +255,7 @@ public class PermissionRule implements Comparable { String groupName = GroupReference.extractGroupName(src); if (groupName != null) { - GroupReference group = new GroupReference(); - group.setName(groupName); + GroupReference group = GroupReference.create(groupName); rule.setGroup(group); } else { throw new IllegalArgumentException("Rule must include group: " + orig); diff --git a/java/com/google/gerrit/common/data/SubscribeSection.java b/java/com/google/gerrit/common/data/SubscribeSection.java index 6ac4695bce..533a2f0a15 100644 --- a/java/com/google/gerrit/common/data/SubscribeSection.java +++ b/java/com/google/gerrit/common/data/SubscribeSection.java @@ -14,43 +14,58 @@ package com.google.gerrit.common.data; +import static com.google.common.collect.ImmutableList.toImmutableList; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.entities.BranchNameKey; import com.google.gerrit.entities.Project; -import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; -import java.util.List; +import java.util.HashSet; +import java.util.Set; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.transport.RefSpec; /** Portion of a {@link Project} describing superproject subscription rules. */ -public class SubscribeSection { +@AutoValue +public abstract class SubscribeSection { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final List multiMatchRefSpecs; - private final List matchingRefSpecs; - private final Project.NameKey project; + public abstract Project.NameKey project(); - public SubscribeSection(Project.NameKey p) { - project = p; - matchingRefSpecs = new ArrayList<>(); - multiMatchRefSpecs = new ArrayList<>(); + protected abstract ImmutableList matchingRefSpecs(); + + protected abstract ImmutableList multiMatchRefSpecs(); + + public static Builder builder(Project.NameKey project) { + return new AutoValue_SubscribeSection.Builder().project(project); } - public void addMatchingRefSpec(RefSpec spec) { - matchingRefSpecs.add(spec); - } + public abstract Builder toBuilder(); - public void addMatchingRefSpec(String spec) { - RefSpec r = new RefSpec(spec); - matchingRefSpecs.add(r); - } + @AutoValue.Builder + public abstract static class Builder { + public abstract Builder project(Project.NameKey project); - public void addMultiMatchRefSpec(String spec) { - RefSpec r = new RefSpec(spec, RefSpec.WildcardMode.ALLOW_MISMATCH); - multiMatchRefSpecs.add(r); - } + abstract ImmutableList.Builder matchingRefSpecsBuilder(); - public Project.NameKey getProject() { - return project; + abstract ImmutableList.Builder multiMatchRefSpecsBuilder(); + + public Builder addMatchingRefSpec(String matchingRefSpec) { + matchingRefSpecsBuilder() + .add(new RefSpec(matchingRefSpec, RefSpec.WildcardMode.REQUIRE_MATCH)); + return this; + } + + public Builder addMultiMatchRefSpec(String multiMatchRefSpec) { + multiMatchRefSpecsBuilder() + .add(new RefSpec(multiMatchRefSpec, RefSpec.WildcardMode.ALLOW_MISMATCH)); + return this; + } + + public abstract SubscribeSection build(); } /** @@ -61,12 +76,12 @@ public class SubscribeSection { * @return if the branch could trigger a superproject update */ public boolean appliesTo(BranchNameKey branch) { - for (RefSpec r : matchingRefSpecs) { + for (RefSpec r : matchingRefSpecs()) { if (r.matchSource(branch.branch())) { return true; } } - for (RefSpec r : multiMatchRefSpecs) { + for (RefSpec r : multiMatchRefSpecs()) { if (r.matchSource(branch.branch())) { return true; } @@ -74,29 +89,71 @@ public class SubscribeSection { return false; } - public Collection getMatchingRefSpecs() { - return Collections.unmodifiableCollection(matchingRefSpecs); + public Collection matchingRefSpecsAsString() { + return matchingRefSpecs().stream().map(RefSpec::toString).collect(toImmutableList()); } - public Collection getMultiMatchRefSpecs() { - return Collections.unmodifiableCollection(multiMatchRefSpecs); + public Collection multiMatchRefSpecsAsString() { + return multiMatchRefSpecs().stream().map(RefSpec::toString).collect(toImmutableList()); + } + + /** Evaluates what the destination branches for the subscription are. */ + public ImmutableSet getDestinationBranches( + BranchNameKey src, Collection allRefsInRefsHeads) { + Set ret = new HashSet<>(); + logger.atFine().log("Inspecting SubscribeSection %s", this); + for (RefSpec r : matchingRefSpecs()) { + logger.atFine().log("Inspecting [matching] ref %s", r); + if (!r.matchSource(src.branch())) { + continue; + } + if (r.isWildcard()) { + // refs/heads/*[:refs/somewhere/*] + ret.add(BranchNameKey.create(project(), r.expandFromSource(src.branch()).getDestination())); + } else { + // e.g. refs/heads/master[:refs/heads/stable] + String dest = r.getDestination(); + if (dest == null) { + dest = r.getSource(); + } + ret.add(BranchNameKey.create(project(), dest)); + } + } + + for (RefSpec r : multiMatchRefSpecs()) { + logger.atFine().log("Inspecting [all] ref %s", r); + if (!r.matchSource(src.branch())) { + continue; + } + for (Ref ref : allRefsInRefsHeads) { + if (r.getDestination() != null && !r.matchDestination(ref.getName())) { + continue; + } + BranchNameKey b = BranchNameKey.create(project(), ref.getName()); + if (!ret.contains(b)) { + ret.add(b); + } + } + } + logger.atFine().log("Returning possible branches: %s for project %s", ret, project()); + return ImmutableSet.copyOf(ret); } @Override - public String toString() { + public final String toString() { StringBuilder ret = new StringBuilder(); ret.append("[SubscribeSection, project="); - ret.append(project); - if (!matchingRefSpecs.isEmpty()) { + ret.append(project()); + if (!matchingRefSpecs().isEmpty()) { ret.append(", matching=["); - for (RefSpec r : matchingRefSpecs) { + for (RefSpec r : matchingRefSpecs()) { ret.append(r.toString()); ret.append(", "); } } - if (!multiMatchRefSpecs.isEmpty()) { + if (!multiMatchRefSpecs().isEmpty()) { ret.append(", all=["); - for (RefSpec r : multiMatchRefSpecs) { + for (RefSpec r : multiMatchRefSpecs()) { ret.append(r.toString()); ret.append(", "); } diff --git a/java/com/google/gerrit/entities/Project.java b/java/com/google/gerrit/entities/Project.java index 867b14db79..1300b9d2fe 100644 --- a/java/com/google/gerrit/entities/Project.java +++ b/java/com/google/gerrit/entities/Project.java @@ -16,6 +16,7 @@ package com.google.gerrit.entities; import static java.util.Objects.requireNonNull; +import com.google.errorprone.annotations.Immutable; import com.google.gerrit.extensions.client.InheritableBoolean; import com.google.gerrit.extensions.client.ProjectState; import com.google.gerrit.extensions.client.SubmitType; @@ -47,7 +48,10 @@ public final class Project { *

Because of this unusual subclassing behavior, this class is not an {@code @AutoValue}, * unlike other key types in this package. However, this is strictly an implementation detail; its * interface and semantics are otherwise analogous to the {@code @AutoValue} types. + * + *

This class is immutable and thread safe. */ + @Immutable public static class NameKey implements Serializable, Comparable { private static final long serialVersionUID = 1L; @@ -72,25 +76,25 @@ public final class Project { @Override public final int hashCode() { - return get().hashCode(); + return name.hashCode(); } @Override public final boolean equals(Object b) { if (b instanceof NameKey) { - return get().equals(((NameKey) b).get()); + return name.equals(((NameKey) b).get()); } return false; } @Override public final int compareTo(NameKey o) { - return get().compareTo(o.get()); + return name.compareTo(o.get()); } @Override public final String toString() { - return KeyUtil.encode(get()); + return KeyUtil.encode(name); } } diff --git a/java/com/google/gerrit/server/account/ProjectWatches.java b/java/com/google/gerrit/server/account/ProjectWatches.java index cf63346a54..6d84f2071e 100644 --- a/java/com/google/gerrit/server/account/ProjectWatches.java +++ b/java/com/google/gerrit/server/account/ProjectWatches.java @@ -219,7 +219,7 @@ public class ProjectWatches { int i = notifyValue.lastIndexOf('['); if (i < 0 || notifyValue.charAt(notifyValue.length() - 1) != ']') { validationErrorSink.error( - new ValidationError( + ValidationError.create( WATCH_CONFIG, String.format( "Invalid project watch of account %d for project %s: %s", @@ -240,7 +240,7 @@ public class ProjectWatches { NotifyType notifyType = Enums.getIfPresent(NotifyType.class, nt).orNull(); if (notifyType == null) { validationErrorSink.error( - new ValidationError( + ValidationError.create( WATCH_CONFIG, String.format( "Invalid notify type %s in project watch " diff --git a/java/com/google/gerrit/server/account/StoredPreferences.java b/java/com/google/gerrit/server/account/StoredPreferences.java index 573c61928a..79be9e5845 100644 --- a/java/com/google/gerrit/server/account/StoredPreferences.java +++ b/java/com/google/gerrit/server/account/StoredPreferences.java @@ -183,7 +183,7 @@ public class StoredPreferences { return PreferencesParserUtil.parseGeneralPreferences(cfg, defaultCfg, input); } catch (ConfigInvalidException e) { validationErrorSink.error( - new ValidationError( + ValidationError.create( PREFERENCES_CONFIG, String.format( "Invalid general preferences for account %d: %s", @@ -197,7 +197,7 @@ public class StoredPreferences { return PreferencesParserUtil.parseDiffPreferences(cfg, defaultCfg, input); } catch (ConfigInvalidException e) { validationErrorSink.error( - new ValidationError( + ValidationError.create( PREFERENCES_CONFIG, String.format( "Invalid diff preferences for account %d: %s", accountId.get(), e.getMessage()))); @@ -210,7 +210,7 @@ public class StoredPreferences { return PreferencesParserUtil.parseEditPreferences(cfg, defaultCfg, input); } catch (ConfigInvalidException e) { validationErrorSink.error( - new ValidationError( + ValidationError.create( PREFERENCES_CONFIG, String.format( "Invalid edit preferences for account %d: %s", accountId.get(), e.getMessage()))); diff --git a/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java b/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java index 1d85a5eb7a..e86439a188 100644 --- a/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java +++ b/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java @@ -91,7 +91,7 @@ public class LdapGroupBackend implements GroupBackend { private static GroupReference groupReference(ParameterizedString p, LdapQuery.Result res) throws NamingException { - return new GroupReference( + return GroupReference.create( AccountGroup.uuid(LDAP_UUID + res.getDN()), LDAP_NAME + LdapRealm.apply(p, res)); } diff --git a/java/com/google/gerrit/server/config/AllProjectsName.java b/java/com/google/gerrit/server/config/AllProjectsName.java index 6d5525c96f..3a13a58bcd 100644 --- a/java/com/google/gerrit/server/config/AllProjectsName.java +++ b/java/com/google/gerrit/server/config/AllProjectsName.java @@ -14,9 +14,15 @@ package com.google.gerrit.server.config; +import com.google.errorprone.annotations.Immutable; import com.google.gerrit.entities.Project; -/** Special name of the project that all projects derive from. */ +/** + * Special name of the project that all projects derive from. + * + *

This class is immutable and thread safe. + */ +@Immutable public class AllProjectsName extends Project.NameKey { private static final long serialVersionUID = 1L; diff --git a/java/com/google/gerrit/server/config/AllUsersName.java b/java/com/google/gerrit/server/config/AllUsersName.java index aa92db899d..393fb6be40 100644 --- a/java/com/google/gerrit/server/config/AllUsersName.java +++ b/java/com/google/gerrit/server/config/AllUsersName.java @@ -14,9 +14,15 @@ package com.google.gerrit.server.config; +import com.google.errorprone.annotations.Immutable; import com.google.gerrit.entities.Project; -/** Special name of the project in which meta data for all users is stored. */ +/** + * Special name of the project in which meta data for all users is stored. + * + *

This class is immutable and thread safe. + */ +@Immutable public class AllUsersName extends Project.NameKey { private static final long serialVersionUID = 1L; diff --git a/java/com/google/gerrit/server/git/BranchOrderSection.java b/java/com/google/gerrit/server/git/BranchOrderSection.java index 02666558be..826067f1b0 100644 --- a/java/com/google/gerrit/server/git/BranchOrderSection.java +++ b/java/com/google/gerrit/server/git/BranchOrderSection.java @@ -14,9 +14,12 @@ package com.google.gerrit.server.git; +import static com.google.common.collect.ImmutableList.toImmutableList; + +import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; import com.google.gerrit.entities.RefNames; -import java.util.List; +import java.util.Collection; /** * An ordering of branches by stability. @@ -25,33 +28,36 @@ import java.util.List; * into stable branches. This is configured by the {@code branchOrder.branch} project setting. This * class represents the ordered list of branches, by increasing stability. */ -public class BranchOrderSection { +@AutoValue +public abstract class BranchOrderSection { /** * Branch names ordered from least to the most stable. * *

Typically the order will be like: master, stable-M.N, stable-M.N-1, ... + * + *

Ref names in this list are exactly as they appear in {@code project.config} */ - private final ImmutableList order; + public abstract ImmutableList order(); - public BranchOrderSection(String[] order) { - if (order.length == 0) { - this.order = ImmutableList.of(); - } else { - ImmutableList.Builder builder = ImmutableList.builder(); - for (String b : order) { - builder.add(RefNames.fullName(b)); - } - this.order = builder.build(); - } + public static BranchOrderSection create(Collection order) { + // Do not mutate the given list as this will be written back to disk when ProjectConfig is + // stored. + return new AutoValue_BranchOrderSection(ImmutableList.copyOf(order)); } - public String[] getMoreStable(String branch) { - int i = order.indexOf(RefNames.fullName(branch)); + /** + * Returns the tail list of branches that are more stable - so lower in the entire list ordered by + * priority compared to the provided branch. Always returns a fully qualified ref name (including + * the refs/heads/ prefix). + */ + public ImmutableList getMoreStable(String branch) { + ImmutableList fullyQualifiedOrder = + order().stream().map(RefNames::fullName).collect(toImmutableList()); + int i = fullyQualifiedOrder.indexOf(RefNames.fullName(branch)); if (0 <= i) { - List r = order.subList(i + 1, order.size()); - return r.toArray(new String[r.size()]); + return fullyQualifiedOrder.subList(i + 1, fullyQualifiedOrder.size()); } - return new String[] {}; + return ImmutableList.of(); } } diff --git a/java/com/google/gerrit/server/git/ValidationError.java b/java/com/google/gerrit/server/git/ValidationError.java index 28d5171945..3606c4294b 100644 --- a/java/com/google/gerrit/server/git/ValidationError.java +++ b/java/com/google/gerrit/server/git/ValidationError.java @@ -14,51 +14,26 @@ package com.google.gerrit.server.git; -import java.util.Objects; +import com.google.auto.value.AutoValue; /** Indicates a problem with Git based data. */ -public class ValidationError { - private final String message; +@AutoValue +public abstract class ValidationError { + public abstract String getMessage(); - public ValidationError(String file, String message) { - this(file + ": " + message); + public static ValidationError create(String file, String message) { + return create(file + ": " + message); } - public ValidationError(String file, int line, String message) { - this(file + ":" + line + ": " + message); + public static ValidationError create(String file, int line, String message) { + return create(file + ":" + line + ": " + message); } - public ValidationError(String message) { - this.message = message; - } - - public String getMessage() { - return message; - } - - @Override - public String toString() { - return "ValidationError[" + message + "]"; + public static ValidationError create(String message) { + return new AutoValue_ValidationError(message); } public interface Sink { void error(ValidationError error); } - - @Override - public boolean equals(Object o) { - if (o == this) { - return true; - } - if (o instanceof ValidationError) { - ValidationError that = (ValidationError) o; - return Objects.equals(this.message, that.message); - } - return false; - } - - @Override - public int hashCode() { - return Objects.hashCode(message); - } } diff --git a/java/com/google/gerrit/server/git/meta/TabFile.java b/java/com/google/gerrit/server/git/meta/TabFile.java index c9a8e7771c..80570a563c 100644 --- a/java/com/google/gerrit/server/git/meta/TabFile.java +++ b/java/com/google/gerrit/server/git/meta/TabFile.java @@ -59,7 +59,7 @@ public class TabFile { int tab = s.indexOf('\t'); if (tab < 0) { - errors.error(new ValidationError(filename, lineNumber, "missing tab delimiter")); + errors.error(ValidationError.create(filename, lineNumber, "missing tab delimiter")); continue; } diff --git a/java/com/google/gerrit/server/group/SystemGroupBackend.java b/java/com/google/gerrit/server/group/SystemGroupBackend.java index 7821a01d67..a44671811e 100644 --- a/java/com/google/gerrit/server/group/SystemGroupBackend.java +++ b/java/com/google/gerrit/server/group/SystemGroupBackend.java @@ -104,7 +104,7 @@ public class SystemGroupBackend extends AbstractGroupBackend { reservedNamesBuilder.add(defaultName); String configuredName = cfg.getString("groups", uuid.get(), "name"); GroupReference ref = - new GroupReference(uuid, MoreObjects.firstNonNull(configuredName, defaultName)); + GroupReference.create(uuid, MoreObjects.firstNonNull(configuredName, defaultName)); n.put(ref.getName().toLowerCase(Locale.US), ref); u.put(ref.getUUID(), ref); } diff --git a/java/com/google/gerrit/server/group/db/GroupNameNotes.java b/java/com/google/gerrit/server/group/db/GroupNameNotes.java index 70d7a1aad3..b75670d654 100644 --- a/java/com/google/gerrit/server/group/db/GroupNameNotes.java +++ b/java/com/google/gerrit/server/group/db/GroupNameNotes.java @@ -443,7 +443,7 @@ public class GroupNameNotes extends VersionedMetaData { throw new ConfigInvalidException(String.format("UUID for group '%s' must be defined", name)); } - return new GroupReference(AccountGroup.uuid(uuid), name); + return GroupReference.create(AccountGroup.uuid(uuid), name); } private String getCommitMessage() { diff --git a/java/com/google/gerrit/server/group/db/RenameGroupOp.java b/java/com/google/gerrit/server/group/db/RenameGroupOp.java index 420dd33eba..4cc6138652 100644 --- a/java/com/google/gerrit/server/group/db/RenameGroupOp.java +++ b/java/com/google/gerrit/server/group/db/RenameGroupOp.java @@ -125,7 +125,7 @@ class RenameGroupOp extends DefaultQueueOp { return; } - ref.setName(newName); + config.renameGroup(uuid, newName); md.getCommitBuilder().setAuthor(author); md.setMessage("Rename group " + oldName + " to " + newName + "\n"); try { diff --git a/java/com/google/gerrit/server/project/CommentLinkInfoImpl.java b/java/com/google/gerrit/server/project/CommentLinkInfoImpl.java deleted file mode 100644 index 35de96343e..0000000000 --- a/java/com/google/gerrit/server/project/CommentLinkInfoImpl.java +++ /dev/null @@ -1,85 +0,0 @@ -// Copyright (C) 2012 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.project; - -import static com.google.common.base.Preconditions.checkArgument; - -import com.google.common.base.Strings; -import com.google.gerrit.extensions.api.projects.CommentLinkInfo; - -/** Info about a single commentlink section in a config. */ -public class CommentLinkInfoImpl extends CommentLinkInfo { - public static class Enabled extends CommentLinkInfoImpl { - public Enabled(String name) { - super(name, true); - } - - @Override - boolean isOverrideOnly() { - return true; - } - } - - public static class Disabled extends CommentLinkInfoImpl { - public Disabled(String name) { - super(name, false); - } - - @Override - boolean isOverrideOnly() { - return true; - } - } - - public CommentLinkInfoImpl(String name, String match, String link, String html, Boolean enabled) { - checkArgument(name != null, "invalid commentlink.name"); - checkArgument(!Strings.isNullOrEmpty(match), "invalid commentlink.%s.match", name); - link = Strings.emptyToNull(link); - html = Strings.emptyToNull(html); - checkArgument( - (link != null && html == null) || (link == null && html != null), - "commentlink.%s must have either link or html", - name); - this.name = name; - this.match = match; - this.link = link; - this.html = html; - this.enabled = enabled; - } - - private CommentLinkInfoImpl(CommentLinkInfo src, boolean enabled) { - this.name = src.name; - this.match = src.match; - this.link = src.link; - this.html = src.html; - this.enabled = enabled; - } - - private CommentLinkInfoImpl(String name, boolean enabled) { - this.name = name; - this.match = null; - this.link = null; - this.html = null; - this.enabled = enabled; - } - - boolean isOverrideOnly() { - return false; - } - - CommentLinkInfo inherit(CommentLinkInfo src) { - return new CommentLinkInfoImpl(src, enabled); - } -} diff --git a/java/com/google/gerrit/server/project/CommentLinkProvider.java b/java/com/google/gerrit/server/project/CommentLinkProvider.java index 4987d0019f..500e1633ff 100644 --- a/java/com/google/gerrit/server/project/CommentLinkProvider.java +++ b/java/com/google/gerrit/server/project/CommentLinkProvider.java @@ -47,12 +47,12 @@ public class CommentLinkProvider implements Provider>, Ger List cls = Lists.newArrayListWithCapacity(subsections.size()); for (String name : subsections) { try { - CommentLinkInfoImpl cl = ProjectConfig.buildCommentLink(cfg, name, true); - if (cl.isOverrideOnly()) { + StoredCommentLinkInfo cl = ProjectConfig.buildCommentLink(cfg, name, true); + if (cl.getOverrideOnly()) { logger.atWarning().log("commentlink %s empty except for \"enabled\"", name); continue; } - cls.add(cl); + cls.add(cl.toInfo()); } catch (IllegalArgumentException e) { logger.atWarning().log("invalid commentlink: %s", e.getMessage()); } diff --git a/java/com/google/gerrit/server/project/ConfiguredMimeTypes.java b/java/com/google/gerrit/server/project/ConfiguredMimeTypes.java index a6661f7160..0447edb940 100644 --- a/java/com/google/gerrit/server/project/ConfiguredMimeTypes.java +++ b/java/com/google/gerrit/server/project/ConfiguredMimeTypes.java @@ -14,35 +14,38 @@ package com.google.gerrit.server.project; +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; import com.google.common.flogger.FluentLogger; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; import java.util.Set; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; +import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.errors.InvalidPatternException; import org.eclipse.jgit.fnmatch.FileNameMatcher; import org.eclipse.jgit.lib.Config; -public class ConfiguredMimeTypes { +@AutoValue +public abstract class ConfiguredMimeTypes { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final String MIMETYPE = "mimetype"; private static final String KEY_PATH = "path"; - private final List matchers; + protected abstract ImmutableList matchers(); - ConfiguredMimeTypes(String projectName, Config rc) { + static ConfiguredMimeTypes create(String projectName, Config rc) { Set types = rc.getSubsections(MIMETYPE); - if (types.isEmpty()) { - matchers = Collections.emptyList(); - } else { - matchers = new ArrayList<>(); + ImmutableList.Builder matchers = ImmutableList.builder(); + if (!types.isEmpty()) { for (String typeName : types) { for (String path : rc.getStringList(MIMETYPE, typeName, KEY_PATH)) { try { - add(typeName, path); + if (path.startsWith("^")) { + matchers.add(new ReType(typeName, path)); + } else { + matchers.add(new FnType(typeName, path)); + } } catch (PatternSyntaxException | InvalidPatternException e) { logger.atWarning().log( "Ignoring invalid %s.%s.%s = %s in project %s: %s", @@ -51,19 +54,12 @@ public class ConfiguredMimeTypes { } } } + return new AutoValue_ConfiguredMimeTypes(matchers.build()); } - private void add(String typeName, String path) - throws PatternSyntaxException, InvalidPatternException { - if (path.startsWith("^")) { - matchers.add(new ReType(typeName, path)); - } else { - matchers.add(new FnType(typeName, path)); - } - } - + @Nullable public String getMimeType(String path) { - for (TypeMatcher m : matchers) { + for (TypeMatcher m : matchers()) { if (m.matches(path)) { return m.type; } @@ -71,42 +67,42 @@ public class ConfiguredMimeTypes { return null; } - private abstract static class TypeMatcher { - final String type; + protected abstract static class TypeMatcher { + private final String type; - TypeMatcher(String type) { + private TypeMatcher(String type) { this.type = type; } - abstract boolean matches(String path); + protected abstract boolean matches(String path); } - private static class FnType extends TypeMatcher { + protected static class FnType extends TypeMatcher { private final FileNameMatcher matcher; - FnType(String type, String pattern) throws InvalidPatternException { + private FnType(String type, String pattern) throws InvalidPatternException { super(type); this.matcher = new FileNameMatcher(pattern, null); } @Override - boolean matches(String input) { + protected boolean matches(String input) { FileNameMatcher m = new FileNameMatcher(matcher); m.append(input); return m.isMatch(); } } - private static class ReType extends TypeMatcher { + protected static class ReType extends TypeMatcher { private final Pattern re; - ReType(String type, String pattern) throws PatternSyntaxException { + private ReType(String type, String pattern) throws PatternSyntaxException { super(type); this.re = Pattern.compile(pattern); } @Override - boolean matches(String input) { + protected boolean matches(String input) { return re.matcher(input).matches(); } } diff --git a/java/com/google/gerrit/server/project/GroupList.java b/java/com/google/gerrit/server/project/GroupList.java index ba7dc95c62..7237bb63e6 100644 --- a/java/com/google/gerrit/server/project/GroupList.java +++ b/java/com/google/gerrit/server/project/GroupList.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.project; import com.google.common.flogger.FluentLogger; +import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.entities.AccountGroup; import com.google.gerrit.entities.Project; @@ -56,7 +57,7 @@ public class GroupList extends TabFile { } AccountGroup.UUID uuid = AccountGroup.uuid(row.left); String name = row.right; - GroupReference ref = new GroupReference(uuid, name); + GroupReference ref = GroupReference.create(uuid, name); groupsByUUID.put(uuid, ref); } @@ -64,10 +65,26 @@ public class GroupList extends TabFile { return new GroupList(groupsByUUID); } + @Nullable public GroupReference byUUID(AccountGroup.UUID uuid) { return byUUID.get(uuid); } + @Nullable + public GroupReference byName(String name) { + return byUUID.entrySet().stream() + .map(Map.Entry::getValue) + .filter(groupReference -> name.equals(groupReference.getName())) + .findAny() + .orElse(null); + } + + /** + * Returns the {@link GroupReference} instance that {@link GroupList} holds on to that has the + * same {@link com.google.gerrit.entities.AccountGroup.UUID} as the argument. Will store the + * argument internally, if no group with this {@link com.google.gerrit.entities.AccountGroup.UUID} + * was stored previously. + */ public GroupReference resolve(GroupReference group) { if (group != null) { if (group.getUUID() == null || group.getUUID().get() == null) { @@ -86,6 +103,10 @@ public class GroupList extends TabFile { return group; } + public void renameGroup(AccountGroup.UUID uuid, String name) { + byUUID.replace(uuid, GroupReference.create(uuid, name)); + } + public Collection references() { return byUUID.values(); } diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java index 4ab583d5f4..2d4928aa91 100644 --- a/java/com/google/gerrit/server/project/ProjectConfig.java +++ b/java/com/google/gerrit/server/project/ProjectConfig.java @@ -91,7 +91,6 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.StoredConfig; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.storage.file.FileBasedConfig; -import org.eclipse.jgit.transport.RefSpec; import org.eclipse.jgit.util.FS; public class ProjectConfig extends VersionedMetaData implements ValidationError.Sink { @@ -241,7 +240,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. private Map labelSections; private ConfiguredMimeTypes mimeTypes; private Map subscribeSections; - private Map commentLinkSections; + private Map commentLinkSections; private List validationErrors; private ObjectId rulesId; private long maxObjectSizeLimit; @@ -250,9 +249,8 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. private Set sectionsWithUnknownPermissions; private boolean hasLegacyPermissions; private Map> extensionPanelSections; - private Map groupsByName; - public static CommentLinkInfoImpl buildCommentLink(Config cfg, String name, boolean allowRaw) + public static StoredCommentLinkInfo buildCommentLink(Config cfg, String name, boolean allowRaw) throws IllegalArgumentException { String match = cfg.getString(COMMENTLINK, name, KEY_MATCH); if (match != null) { @@ -282,15 +280,21 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. && !hasHtml && enabled != null) { if (enabled) { - return new CommentLinkInfoImpl.Enabled(name); + return StoredCommentLinkInfo.enabled(name); } - return new CommentLinkInfoImpl.Disabled(name); + return StoredCommentLinkInfo.disabled(name); } - return new CommentLinkInfoImpl(name, match, link, html, enabled); + return StoredCommentLinkInfo.builder(name) + .setMatch(match) + .setLink(link) + .setHtml(html) + .setEnabled(enabled) + .setOverrideOnly(false) + .build(); } - public void addCommentLinkSection(CommentLinkInfoImpl commentLink) { - commentLinkSections.put(commentLink.name, commentLink); + public void addCommentLinkSection(StoredCommentLinkInfo commentLink) { + commentLinkSections.put(commentLink.getName(), commentLink); } public void removeCommentLinkSection(String name) { @@ -358,6 +362,10 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. return branchOrderSection; } + public void setBranchOrderSection(BranchOrderSection branchOrderSection) { + this.branchOrderSection = branchOrderSection; + } + public Map getSubscribeSections() { return subscribeSections; } @@ -373,7 +381,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. } public void addSubscribeSection(SubscribeSection s) { - subscribeSections.put(s.getProject(), s); + subscribeSections.put(s.project(), s); } public void remove(AccessSection section) { @@ -476,7 +484,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. return labelSections; } - public Collection getCommentLinkSections() { + public Collection getCommentLinkSections() { return commentLinkSections.values(); } @@ -485,13 +493,11 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. } public GroupReference resolve(GroupReference group) { - GroupReference groupRef = groupList.resolve(group); - if (groupRef != null - && groupRef.getUUID() != null - && !groupsByName.containsKey(groupRef.getName())) { - groupsByName.put(groupRef.getName(), groupRef); - } - return groupRef; + return groupList.resolve(group); + } + + public void renameGroup(AccountGroup.UUID uuid, String newName) { + groupList.renameGroup(uuid, newName); } /** @return the group reference, if the group is used by at least one rule. */ @@ -504,7 +510,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. * at least one rule or plugin value. */ public GroupReference getGroup(String groupName) { - return groupsByName.get(groupName); + return groupList.byName(groupName); } /** @return set of all groups used by this configuration. */ @@ -541,7 +547,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. GroupDescription.Basic g = groupBackend.get(ref.getUUID()); if (g != null && !g.getName().equals(ref.getName())) { dirty = true; - ref.setName(g.getName()); + groupList.renameGroup(ref.getUUID(), g.getName()); } } return dirty; @@ -570,7 +576,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. baseConfig.load(); } readGroupList(); - groupsByName = mapGroupReferences(); rulesId = getObjectId("rules.pl"); Config rc = readConfig(PROJECT_CONFIG, baseConfig); @@ -588,7 +593,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. if (rc.getStringList(ACCESS, null, KEY_INHERIT_FROM).length > 1) { // The config must not contain more than one parent to inherit from // as there is no guarantee which of the parents would be used then. - error(new ValidationError(PROJECT_CONFIG, "Cannot inherit from multiple projects")); + error(ValidationError.create(PROJECT_CONFIG, "Cannot inherit from multiple projects")); } p.setParentName(rc.getString(ACCESS, null, KEY_INHERIT_FROM)); @@ -619,7 +624,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. loadLabelSections(rc); loadCommentLinkSections(rc); loadSubscribeSections(rc); - mimeTypes = new ConfiguredMimeTypes(projectName.get(), rc); + mimeTypes = ConfiguredMimeTypes.create(projectName.get(), rc); loadPluginSections(rc); loadReceiveSection(rc); loadExtensionPanelSections(rc); @@ -628,7 +633,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. private void loadAccountsSection(Config rc) { accountsSection = new AccountsSection(); accountsSection.setSameGroupVisibility( - loadPermissionRules(rc, ACCOUNTS, null, KEY_SAME_GROUP_VISIBILITY, groupsByName, false)); + loadPermissionRules(rc, ACCOUNTS, null, KEY_SAME_GROUP_VISIBILITY, false)); } private void loadExtensionPanelSections(Config rc) { @@ -638,7 +643,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. String lower = name.toLowerCase(); if (lowerNames.containsKey(lower)) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, String.format( "Extension Panels \"%s\" conflicts with \"%s\"", name, lowerNames.get(lower)))); @@ -656,20 +661,18 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. ContributorAgreement ca = getContributorAgreement(name, true); ca.setDescription(rc.getString(CONTRIBUTOR_AGREEMENT, name, KEY_DESCRIPTION)); ca.setAgreementUrl(rc.getString(CONTRIBUTOR_AGREEMENT, name, KEY_AGREEMENT_URL)); - ca.setAccepted( - loadPermissionRules(rc, CONTRIBUTOR_AGREEMENT, name, KEY_ACCEPTED, groupsByName, false)); + ca.setAccepted(loadPermissionRules(rc, CONTRIBUTOR_AGREEMENT, name, KEY_ACCEPTED, false)); ca.setExcludeProjectsRegexes( loadPatterns(rc, CONTRIBUTOR_AGREEMENT, name, KEY_EXCLUDE_PROJECTS)); ca.setMatchProjectsRegexes(loadPatterns(rc, CONTRIBUTOR_AGREEMENT, name, KEY_MATCH_PROJECTS)); List rules = - loadPermissionRules( - rc, CONTRIBUTOR_AGREEMENT, name, KEY_AUTO_VERIFY, groupsByName, false); + loadPermissionRules(rc, CONTRIBUTOR_AGREEMENT, name, KEY_AUTO_VERIFY, false); if (rules.isEmpty()) { ca.setAutoVerify(null); } else if (rules.size() > 1) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, "Invalid rule in " + CONTRIBUTOR_AGREEMENT @@ -680,7 +683,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. + ": at most one group may be set")); } else if (rules.get(0).getAction() != Action.ALLOW) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, "Invalid rule in " + CONTRIBUTOR_AGREEMENT @@ -728,27 +731,26 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. for (String dst : rc.getStringList(NOTIFY, sectionName, KEY_EMAIL)) { String groupName = GroupReference.extractGroupName(dst); if (groupName != null) { - GroupReference ref = groupsByName.get(groupName); + GroupReference ref = groupList.byName(groupName); if (ref == null) { - ref = new GroupReference(groupName); - groupsByName.put(ref.getName(), ref); + ref = groupList.resolve(GroupReference.create(groupName)); } if (ref.getUUID() != null) { n.addEmail(ref); } else { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, "group \"" + ref.getName() + "\" not in " + GroupList.FILE_NAME)); } } else if (dst.startsWith("user ")) { - error(new ValidationError(PROJECT_CONFIG, dst + " not supported")); + error(ValidationError.create(PROJECT_CONFIG, dst + " not supported")); } else { try { n.addEmail(Address.parse(dst)); } catch (IllegalArgumentException err) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, "notify section \"" + sectionName + "\" has invalid email \"" + dst + "\"")); } @@ -779,13 +781,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. if (isCoreOrPluginPermission(convertedName)) { Permission perm = as.getPermission(convertedName, true); loadPermissionRules( - rc, - ACCESS, - refName, - varName, - groupsByName, - perm, - Permission.hasRange(convertedName)); + rc, ACCESS, refName, varName, perm, Permission.hasRange(convertedName)); } else { sectionsWithUnknownPermissions.add(as.getName()); } @@ -800,8 +796,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. accessSections.put(AccessSection.GLOBAL_CAPABILITIES, capability); } Permission perm = capability.getPermission(varName, true); - loadPermissionRules( - rc, CAPABILITY, null, varName, groupsByName, perm, GlobalCapability.hasRange(varName)); + loadPermissionRules(rc, CAPABILITY, null, varName, perm, GlobalCapability.hasRange(varName)); } } @@ -815,7 +810,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. try { RefPattern.validateRegExp(refPattern); } catch (InvalidNameException e) { - error(new ValidationError(PROJECT_CONFIG, "Invalid ref name: " + e.getMessage())); + error(ValidationError.create(PROJECT_CONFIG, "Invalid ref name: " + e.getMessage())); return false; } return true; @@ -823,7 +818,14 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. private void loadBranchOrderSection(Config rc) { if (rc.getSections().contains(BRANCH_ORDER)) { - branchOrderSection = new BranchOrderSection(rc.getStringList(BRANCH_ORDER, null, BRANCH)); + branchOrderSection = + BranchOrderSection.create(Arrays.asList(rc.getStringList(BRANCH_ORDER, null, BRANCH))); + } + } + + private void saveBranchOrderSection(Config rc) { + if (branchOrderSection != null) { + rc.setStringList(BRANCH_ORDER, null, BRANCH, branchOrderSection.order()); } } @@ -836,7 +838,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. // to fail fast if any of the patterns are invalid. patterns.add(Pattern.compile(patternString).pattern()); } catch (PatternSyntaxException e) { - error(new ValidationError(PROJECT_CONFIG, "Invalid regular expression: " + e.getMessage())); + error( + ValidationError.create( + PROJECT_CONFIG, "Invalid regular expression: " + e.getMessage())); continue; } } @@ -844,14 +848,9 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. } private ImmutableList loadPermissionRules( - Config rc, - String section, - String subsection, - String varName, - Map groupsByName, - boolean useRange) { + Config rc, String section, String subsection, String varName, boolean useRange) { Permission perm = new Permission(varName); - loadPermissionRules(rc, section, subsection, varName, groupsByName, perm, useRange); + loadPermissionRules(rc, section, subsection, varName, perm, useRange); return ImmutableList.copyOf(perm.getRules()); } @@ -860,7 +859,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. String section, String subsection, String varName, - Map groupsByName, Permission perm, boolean useRange) { for (String ruleString : rc.getStringList(section, subsection, varName)) { @@ -869,7 +867,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. rule = PermissionRule.fromString(ruleString, useRange); } catch (IllegalArgumentException notRule) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, "Invalid rule in " + section @@ -881,16 +879,15 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. continue; } - GroupReference ref = groupsByName.get(rule.getGroup().getName()); + GroupReference ref = groupList.byName(rule.getGroup().getName()); if (ref == null) { // The group wasn't mentioned in the groups table, so there is // no valid UUID for it. Pool the reference anyway so at least // all rules in the same file share the same GroupReference. // - ref = rule.getGroup(); - groupsByName.put(ref.getName(), ref); + ref = groupList.resolve(rule.getGroup()); error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, "group \"" + ref.getName() + "\" not in " + GroupList.FILE_NAME)); } @@ -917,7 +914,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. String lower = name.toLowerCase(); if (lowerNames.containsKey(lower)) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, String.format("Label \"%s\" conflicts with \"%s\"", name, lowerNames.get(lower)))); } @@ -932,13 +929,13 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. values.add(labelValue); } else { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, String.format("Duplicate %s \"%s\" for label \"%s\"", KEY_VALUE, value, name))); } } catch (IllegalArgumentException notValue) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, String.format( "Invalid %s \"%s\" for label \"%s\": %s", @@ -950,7 +947,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. try { label = new LabelType(name, values); } catch (IllegalArgumentException badName) { - error(new ValidationError(PROJECT_CONFIG, String.format("Invalid label \"%s\"", name))); + error(ValidationError.create(PROJECT_CONFIG, String.format("Invalid label \"%s\"", name))); continue; } @@ -961,7 +958,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. : Optional.of(LabelFunction.MAX_WITH_BLOCK); if (!function.isPresent()) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, String.format( "Invalid %s for label \"%s\". Valid names are: %s", @@ -975,7 +972,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. label.setDefaultValue(dv); } else { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, String.format( "Invalid %s \"%s\" for label \"%s\"", KEY_DEFAULT_VALUE, dv, name))); @@ -1021,14 +1018,14 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. short copyValue = Shorts.checkedCast(PermissionRule.parseInt(value)); if (!copyValues.add(copyValue)) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, String.format( "Duplicate %s \"%s\" for label \"%s\"", KEY_COPY_VALUE, value, name))); } } catch (IllegalArgumentException notValue) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, String.format( "Invalid %s \"%s\" for label \"%s\": %s", @@ -1066,14 +1063,14 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. commentLinkSections.put(name, buildCommentLink(rc, name, false)); } catch (PatternSyntaxException e) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, String.format( "Invalid pattern \"%s\" in commentlink.%s.match: %s", rc.getString(COMMENTLINK, name, KEY_MATCH), name, e.getMessage()))); } catch (IllegalArgumentException e) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, String.format( "Error in pattern \"%s\" in commentlink.%s.match: %s", @@ -1088,7 +1085,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. try { for (String projectName : subsections) { Project.NameKey p = Project.nameKey(projectName); - SubscribeSection ss = new SubscribeSection(p); + SubscribeSection.Builder ss = SubscribeSection.builder(p); for (String s : rc.getStringList(SUBSCRIBE_SECTION, projectName, SUBSCRIBE_MULTI_MATCH_REFS)) { ss.addMultiMatchRefSpec(s); @@ -1096,7 +1093,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. for (String s : rc.getStringList(SUBSCRIBE_SECTION, projectName, SUBSCRIBE_MATCH_REFS)) { ss.addMatchingRefSpec(s); } - subscribeSections.put(p, ss); + subscribeSections.put(p, ss.build()); } } catch (IllegalArgumentException e) { throw new ConfigInvalidException(e.getMessage()); @@ -1117,10 +1114,10 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. String value = rc.getString(PLUGIN, plugin, name); String groupName = GroupReference.extractGroupName(value); if (groupName != null) { - GroupReference ref = groupsByName.get(groupName); + GroupReference ref = groupList.byName(groupName); if (ref == null) { error( - new ValidationError( + ValidationError.create( PROJECT_CONFIG, "group \"" + groupName + "\" not in " + GroupList.FILE_NAME)); } rc.setString(PLUGIN, plugin, name, value); @@ -1144,16 +1141,6 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. groupList = GroupList.parse(projectName, readUTF8(GroupList.FILE_NAME), this); } - private Map mapGroupReferences() { - Collection references = groupList.references(); - Map result = new HashMap<>(references.size()); - for (GroupReference ref : references) { - result.put(ref.getName(), ref); - } - - return result; - } - @Override protected boolean onSave(CommitBuilder commit) throws IOException, ConfigInvalidException { if (commit.getMessage() == null || "".equals(commit.getMessage())) { @@ -1204,6 +1191,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. saveLabelSections(rc); saveCommentLinkSections(rc); saveSubscribeSections(rc); + saveBranchOrderSection(rc); saveConfig(PROJECT_CONFIG, rc); saveGroupList(); @@ -1252,16 +1240,16 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. private void saveCommentLinkSections(Config rc) { unsetSection(rc, COMMENTLINK); if (commentLinkSections != null) { - for (CommentLinkInfoImpl cm : commentLinkSections.values()) { - rc.setString(COMMENTLINK, cm.name, KEY_MATCH, cm.match); - if (!Strings.isNullOrEmpty(cm.html)) { - rc.setString(COMMENTLINK, cm.name, KEY_HTML, cm.html); + for (StoredCommentLinkInfo cm : commentLinkSections.values()) { + rc.setString(COMMENTLINK, cm.getName(), KEY_MATCH, cm.getMatch()); + if (!Strings.isNullOrEmpty(cm.getHtml())) { + rc.setString(COMMENTLINK, cm.getName(), KEY_HTML, cm.getHtml()); } - if (!Strings.isNullOrEmpty(cm.link)) { - rc.setString(COMMENTLINK, cm.name, KEY_LINK, cm.link); + if (!Strings.isNullOrEmpty(cm.getLink())) { + rc.setString(COMMENTLINK, cm.getName(), KEY_LINK, cm.getLink()); } - if (cm.enabled != null && !cm.enabled) { - rc.setBoolean(COMMENTLINK, cm.name, KEY_ENABLED, cm.enabled); + if (cm.getEnabled() != null && !cm.getEnabled()) { + rc.setBoolean(COMMENTLINK, cm.getName(), KEY_ENABLED, cm.getEnabled()); } } } @@ -1558,7 +1546,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. String value = pluginConfig.getString(PLUGIN, plugin, name); String groupName = GroupReference.extractGroupName(value); if (groupName != null) { - GroupReference ref = groupsByName.get(groupName); + GroupReference ref = groupList.byName(groupName); if (ref != null && ref.getUUID() != null) { keepGroups.add(ref.getUUID()); pluginConfig.setString(PLUGIN, plugin, name, "group " + ref.getName()); @@ -1578,14 +1566,14 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. for (Project.NameKey p : subscribeSections.keySet()) { SubscribeSection s = subscribeSections.get(p); List matchings = new ArrayList<>(); - for (RefSpec r : s.getMatchingRefSpecs()) { - matchings.add(r.toString()); + for (String r : s.matchingRefSpecsAsString()) { + matchings.add(r); } rc.setStringList(SUBSCRIBE_SECTION, p.get(), SUBSCRIBE_MATCH_REFS, matchings); List multimatchs = new ArrayList<>(); - for (RefSpec r : s.getMultiMatchRefSpecs()) { - multimatchs.add(r.toString()); + for (String r : s.multiMatchRefSpecsAsString()) { + multimatchs.add(r); } rc.setStringList(SUBSCRIBE_SECTION, p.get(), SUBSCRIBE_MULTI_MATCH_REFS, multimatchs); } @@ -1603,7 +1591,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError. try { return rc.getEnum(section, subsection, name, defaultValue); } catch (IllegalArgumentException err) { - error(new ValidationError(PROJECT_CONFIG, err.getMessage())); + error(ValidationError.create(PROJECT_CONFIG, err.getMessage())); return defaultValue; } } diff --git a/java/com/google/gerrit/server/project/ProjectState.java b/java/com/google/gerrit/server/project/ProjectState.java index efadcc8863..1c7c7d6a45 100644 --- a/java/com/google/gerrit/server/project/ProjectState.java +++ b/java/com/google/gerrit/server/project/ProjectState.java @@ -457,16 +457,16 @@ public class ProjectState { cls.put(cl.name.toLowerCase(), cl); } for (ProjectState s : treeInOrder()) { - for (CommentLinkInfoImpl cl : s.getConfig().getCommentLinkSections()) { - String name = cl.name.toLowerCase(); - if (cl.isOverrideOnly()) { + for (StoredCommentLinkInfo cl : s.getConfig().getCommentLinkSections()) { + String name = cl.getName().toLowerCase(); + if (cl.getOverrideOnly()) { CommentLinkInfo parent = cls.get(name); if (parent == null) { continue; // Ignore invalid overrides. } - cls.put(name, cl.inherit(parent)); + cls.put(name, StoredCommentLinkInfo.fromInfo(parent, cl.getEnabled()).toInfo()); } else { - cls.put(name, cl); + cls.put(name, cl.toInfo()); } } } diff --git a/java/com/google/gerrit/server/project/StoredCommentLinkInfo.java b/java/com/google/gerrit/server/project/StoredCommentLinkInfo.java new file mode 100644 index 0000000000..4e311b83d8 --- /dev/null +++ b/java/com/google/gerrit/server/project/StoredCommentLinkInfo.java @@ -0,0 +1,133 @@ +// Copyright (C) 2012 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.project; + +import static com.google.common.base.Preconditions.checkArgument; + +import com.google.auto.value.AutoValue; +import com.google.common.base.Strings; +import com.google.gerrit.common.Nullable; +import com.google.gerrit.extensions.api.projects.CommentLinkInfo; + +/** Info about a single commentlink section in a config. */ +@AutoValue +public abstract class StoredCommentLinkInfo { + public abstract String getName(); + + /** A regular expression to match for the commentlink to apply. */ + @Nullable + public abstract String getMatch(); + + /** The link to replace the match with. This can only be set if html is {@code null}. */ + @Nullable + public abstract String getLink(); + + /** The html to replace the match with. This can only be set if link is {@code null}. */ + @Nullable + public abstract String getHtml(); + + /** Weather this comment link is active. {@code null} means true. */ + @Nullable + public abstract Boolean getEnabled(); + + /** If set, {@link StoredCommentLinkInfo} has to be overriden to take any effect. */ + public abstract boolean getOverrideOnly(); + + /** + * Creates an enabled {@link StoredCommentLinkInfo} that can be overriden but doesn't do anything + * on its own. + */ + public static StoredCommentLinkInfo enabled(String name) { + return builder(name).setOverrideOnly(true).build(); + } + + /** + * Creates a disabled {@link StoredCommentLinkInfo} that can be overriden but doesn't do anything + * on it's own. + */ + public static StoredCommentLinkInfo disabled(String name) { + return builder(name).setOverrideOnly(true).build(); + } + + /** Creates and returns a new {@link StoredCommentLinkInfo.Builder} instance. */ + public static Builder builder(String name) { + checkArgument(name != null, "invalid commentlink.name"); + return new AutoValue_StoredCommentLinkInfo.Builder().setName(name).setOverrideOnly(false); + } + + /** Creates and returns a new {@link StoredCommentLinkInfo} instance with the same values. */ + static StoredCommentLinkInfo fromInfo(CommentLinkInfo src, boolean enabled) { + return builder(src.name) + .setMatch(src.match) + .setLink(src.link) + .setHtml(src.html) + .setEnabled(enabled) + .setOverrideOnly(false) + .build(); + } + + /** Returns an {@link CommentLinkInfo} instance with the same values. */ + CommentLinkInfo toInfo() { + CommentLinkInfo info = new CommentLinkInfo(); + info.name = getName(); + info.match = getMatch(); + info.link = getLink(); + info.html = getHtml(); + info.enabled = getEnabled(); + return info; + } + + @AutoValue.Builder + public abstract static class Builder { + public abstract Builder setName(String value); + + public abstract Builder setMatch(@Nullable String value); + + public abstract Builder setLink(@Nullable String value); + + public abstract Builder setHtml(@Nullable String value); + + public abstract Builder setEnabled(@Nullable Boolean value); + + public abstract Builder setOverrideOnly(boolean value); + + public StoredCommentLinkInfo build() { + checkArgument(getName() != null, "invalid commentlink.name"); + setLink(Strings.emptyToNull(getLink())); + setHtml(Strings.emptyToNull(getHtml())); + if (!getOverrideOnly()) { + checkArgument( + !Strings.isNullOrEmpty(getMatch()), "invalid commentlink.%s.match", getName()); + checkArgument( + (getLink() != null && getHtml() == null) || (getLink() == null && getHtml() != null), + "commentlink.%s must have either link or html", + getName()); + } + return autoBuild(); + } + + protected abstract StoredCommentLinkInfo autoBuild(); + + protected abstract String getName(); + + protected abstract String getMatch(); + + protected abstract String getLink(); + + protected abstract String getHtml(); + + protected abstract boolean getOverrideOnly(); + } +} diff --git a/java/com/google/gerrit/server/restapi/change/Mergeable.java b/java/com/google/gerrit/server/restapi/change/Mergeable.java index b84b5e315b..6fccdd184f 100644 --- a/java/com/google/gerrit/server/restapi/change/Mergeable.java +++ b/java/com/google/gerrit/server/restapi/change/Mergeable.java @@ -41,6 +41,7 @@ import com.google.gerrit.server.query.change.ChangeData; import com.google.inject.Inject; import java.io.IOException; import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.concurrent.Future; @@ -118,8 +119,9 @@ public class Mergeable implements RestReadView { BranchOrderSection branchOrder = projectState.getBranchOrderSection(); if (branchOrder != null) { int prefixLen = Constants.R_HEADS.length(); - String[] names = branchOrder.getMoreStable(ref.getName()); - Map refs = git.getRefDatabase().exactRef(names); + List names = branchOrder.getMoreStable(ref.getName()); + Map refs = + git.getRefDatabase().exactRef(names.toArray(new String[names.size()])); for (String n : names) { Ref other = refs.get(n); if (other == null) { diff --git a/java/com/google/gerrit/server/schema/SchemaCreatorImpl.java b/java/com/google/gerrit/server/schema/SchemaCreatorImpl.java index 78fa5bd6bf..1279218b00 100644 --- a/java/com/google/gerrit/server/schema/SchemaCreatorImpl.java +++ b/java/com/google/gerrit/server/schema/SchemaCreatorImpl.java @@ -212,7 +212,7 @@ public class SchemaCreatorImpl implements SchemaCreator { private GroupReference createGroupReference(String name) { AccountGroup.UUID groupUuid = GroupUuid.make(name, serverUser); - return new GroupReference(groupUuid, name); + return GroupReference.create(groupUuid, name); } private InternalGroupCreation getGroupCreation(Sequences seqs, GroupReference groupReference) { diff --git a/java/com/google/gerrit/server/submit/SubscriptionGraph.java b/java/com/google/gerrit/server/submit/SubscriptionGraph.java index 406d878a4b..eac6d2cd03 100644 --- a/java/com/google/gerrit/server/submit/SubscriptionGraph.java +++ b/java/com/google/gerrit/server/submit/SubscriptionGraph.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.submit; import static com.google.gerrit.server.project.ProjectCache.illegalState; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.MultimapBuilder; @@ -38,11 +39,11 @@ import java.util.Deque; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.Set; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; -import org.eclipse.jgit.transport.RefSpec; /** * A container which stores subscription relationship. A SubscriptionGraph is calculated every time @@ -270,55 +271,18 @@ public class SubscriptionGraph { private Collection getDestinationBranches(BranchNameKey src, SubscribeSection s) throws IOException { - Collection ret = new HashSet<>(); - logger.atFine().log("Inspecting SubscribeSection %s", s); - for (RefSpec r : s.getMatchingRefSpecs()) { - logger.atFine().log("Inspecting [matching] ref %s", r); - if (!r.matchSource(src.branch())) { - continue; - } - if (r.isWildcard()) { - // refs/heads/*[:refs/somewhere/*] - ret.add( - BranchNameKey.create( - s.getProject(), r.expandFromSource(src.branch()).getDestination())); - } else { - // e.g. refs/heads/master[:refs/heads/stable] - String dest = r.getDestination(); - if (dest == null) { - dest = r.getSource(); - } - ret.add(BranchNameKey.create(s.getProject(), dest)); - } + OpenRepo or; + try { + or = orm.getRepo(s.project()); + } catch (NoSuchProjectException e) { + // A project listed a non existent project to be allowed + // to subscribe to it. Allow this for now, i.e. no exception is + // thrown. + return s.getDestinationBranches(src, ImmutableList.of()); } - for (RefSpec r : s.getMultiMatchRefSpecs()) { - logger.atFine().log("Inspecting [all] ref %s", r); - if (!r.matchSource(src.branch())) { - continue; - } - OpenRepo or; - try { - or = orm.getRepo(s.getProject()); - } catch (NoSuchProjectException e) { - // A project listed a non existent project to be allowed - // to subscribe to it. Allow this for now, i.e. no exception is - // thrown. - continue; - } - - for (Ref ref : or.repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_HEADS)) { - if (r.getDestination() != null && !r.matchDestination(ref.getName())) { - continue; - } - BranchNameKey b = BranchNameKey.create(s.getProject(), ref.getName()); - if (!ret.contains(b)) { - ret.add(b); - } - } - } - logger.atFine().log("Returning possible branches: %s for project %s", ret, s.getProject()); - return ret; + List refs = or.repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_HEADS); + return s.getDestinationBranches(src, refs); } private Collection superProjectSubscriptionsForSubmoduleBranch( diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java index 11ca391d03..11cc82f895 100644 --- a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java +++ b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java @@ -81,7 +81,7 @@ public class AgreementsIT extends AbstractDaemonTest { GroupApi groupApi = gApi.groups().id(g.get()); groupApi.description("CLA test group"); InternalGroup caGroup = group(AccountGroup.uuid(groupApi.detail().id)); - GroupReference groupRef = new GroupReference(caGroup.getGroupUUID(), caGroup.getName()); + GroupReference groupRef = GroupReference.create(caGroup.getGroupUUID(), caGroup.getName()); PermissionRule rule = new PermissionRule(groupRef); rule.setAction(PermissionRule.Action.ALLOW); if (autoVerify) { diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java index 8dc76dd78b..cb2284990b 100644 --- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java +++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java @@ -64,7 +64,6 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.UnprocessableEntityException; import com.google.gerrit.server.config.ProjectConfigEntry; import com.google.gerrit.server.group.SystemGroupBackend; -import com.google.gerrit.server.project.CommentLinkInfoImpl; import com.google.gerrit.server.project.ProjectConfig; import com.google.inject.AbstractModule; import com.google.inject.Inject; @@ -916,7 +915,11 @@ public class ProjectIT extends AbstractDaemonTest { } private CommentLinkInfo commentLinkInfo(String name, String match, String link) { - return new CommentLinkInfoImpl(name, match, link, null /*html*/, null /*enabled*/); + CommentLinkInfo info = new CommentLinkInfo(); + info.name = name; + info.match = match; + info.link = link; + return info; } private void assertCommentLinks(ConfigInfo actual, Map expected) { diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java index dd13643ed8..da9238144b 100644 --- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java @@ -93,6 +93,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.UnprocessableEntityException; import com.google.gerrit.extensions.webui.PatchSetWebLink; import com.google.gerrit.server.change.RevisionResource; +import com.google.gerrit.server.git.BranchOrderSection; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.restapi.change.GetRevisionActions; import com.google.inject.Inject; @@ -1296,8 +1297,25 @@ public class RevisionIT extends AbstractDaemonTest { assertThat(changes).hasSize(1); assertThat(changes.get(0).changeId).isEqualTo(r2.getChangeId()); assertThat(changes.get(0).mergeable).isEqualTo(Boolean.TRUE); + } - // TODO(dborowitz): Test for other-branches. + @Test + public void mergeableOtherBranches() throws Exception { + String head = getHead(repo(), HEAD).name(); + createBranchWithRevision(BranchNameKey.create(project, "mergeable-other-branch"), head); + createBranchWithRevision(BranchNameKey.create(project, "ignored"), head); + PushOneCommit.Result change1 = createChange(); + try (ProjectConfigUpdate u = updateProject(project)) { + u.getConfig() + .setBranchOrderSection( + BranchOrderSection.create( + ImmutableList.of("master", "nonexistent", "mergeable-other-branch"))); + u.save(); + } + + MergeableInfo mergeableInfo = + gApi.changes().id(change1.getChangeId()).current().mergeableOtherBranches(); + assertThat(mergeableInfo.mergeableInto).containsExactly("mergeable-other-branch"); } @Test diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java index a0725c3034..df21625a58 100644 --- a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java +++ b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java @@ -204,12 +204,12 @@ public abstract class AbstractSubmoduleSubscription extends AbstractDaemonTest { throws Exception { try (MetaDataUpdate md = metaDataUpdateFactory.create(submodule)) { md.setMessage("Added superproject subscription"); - SubscribeSection s; + SubscribeSection.Builder s; ProjectConfig pc = projectConfigFactory.read(md); if (pc.getSubscribeSections().containsKey(superproject)) { - s = pc.getSubscribeSections().get(superproject); + s = pc.getSubscribeSections().get(superproject).toBuilder(); } else { - s = new SubscribeSection(superproject); + s = SubscribeSection.builder(superproject); } String refspec; if (superBranch == null) { @@ -222,7 +222,7 @@ public abstract class AbstractSubmoduleSubscription extends AbstractDaemonTest { } else { s.addMultiMatchRefSpec(refspec); } - pc.addSubscribeSection(s); + pc.addSubscribeSection(s.build()); ObjectId oldId = pc.getRevision(); ObjectId newId = pc.commit(md); assertThat(newId).isNotEqualTo(oldId); diff --git a/javatests/com/google/gerrit/common/data/GroupReferenceTest.java b/javatests/com/google/gerrit/common/data/GroupReferenceTest.java index 25b55c79bb..113bd77165 100644 --- a/javatests/com/google/gerrit/common/data/GroupReferenceTest.java +++ b/javatests/com/google/gerrit/common/data/GroupReferenceTest.java @@ -58,7 +58,7 @@ public class GroupReferenceTest { public void create() { AccountGroup.UUID uuid = AccountGroup.uuid("uuid"); String name = "foo"; - GroupReference groupReference = new GroupReference(uuid, name); + GroupReference groupReference = GroupReference.create(uuid, name); assertThat(groupReference.getUUID()).isEqualTo(uuid); assertThat(groupReference.getName()).isEqualTo(name); } @@ -68,7 +68,7 @@ public class GroupReferenceTest { // GroupReferences where the UUID is null are used to represent groups from project.config that // cannot be resolved. String name = "foo"; - GroupReference groupReference = new GroupReference(name); + GroupReference groupReference = GroupReference.create(name); assertThat(groupReference.getUUID()).isNull(); assertThat(groupReference.getName()).isEqualTo(name); } @@ -76,7 +76,7 @@ public class GroupReferenceTest { @Test public void cannotCreateWithoutName() { assertThrows( - NullPointerException.class, () -> new GroupReference(AccountGroup.uuid("uuid"), null)); + NullPointerException.class, () -> GroupReference.create(AccountGroup.uuid("uuid"), null)); } @Test @@ -97,41 +97,10 @@ public class GroupReferenceTest { assertThat(GroupReference.extractGroupName("group foo bar")).isEqualTo("foo bar"); } - @Test - public void getAndSetUuid() { - AccountGroup.UUID uuid = AccountGroup.uuid("uuid-foo"); - String name = "foo"; - GroupReference groupReference = new GroupReference(uuid, name); - assertThat(groupReference.getUUID()).isEqualTo(uuid); - - AccountGroup.UUID uuid2 = AccountGroup.uuid("uuid-bar"); - groupReference.setUUID(uuid2); - assertThat(groupReference.getUUID()).isEqualTo(uuid2); - - // GroupReferences where the UUID is null are used to represent groups from project.config that - // cannot be resolved. - groupReference.setUUID(null); - assertThat(groupReference.getUUID()).isNull(); - } - - @Test - public void getAndSetName() { - AccountGroup.UUID uuid = AccountGroup.uuid("uuid-foo"); - String name = "foo"; - GroupReference groupReference = new GroupReference(uuid, name); - assertThat(groupReference.getName()).isEqualTo(name); - - String name2 = "bar"; - groupReference.setName(name2); - assertThat(groupReference.getName()).isEqualTo(name2); - - assertThrows(NullPointerException.class, () -> groupReference.setName(null)); - } - @Test public void toConfigValue() { String name = "foo"; - GroupReference groupReference = new GroupReference(AccountGroup.uuid("uuid-foo"), name); + GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-foo"), name); assertThat(groupReference.toConfigValue()).isEqualTo("group " + name); } @@ -142,9 +111,9 @@ public class GroupReferenceTest { String name1 = "foo"; String name2 = "bar"; - GroupReference groupReference1 = new GroupReference(uuid1, name1); - GroupReference groupReference2 = new GroupReference(uuid1, name2); - GroupReference groupReference3 = new GroupReference(uuid2, name1); + GroupReference groupReference1 = GroupReference.create(uuid1, name1); + GroupReference groupReference2 = GroupReference.create(uuid1, name2); + GroupReference groupReference3 = GroupReference.create(uuid2, name1); assertThat(groupReference1.equals(groupReference2)).isTrue(); assertThat(groupReference1.equals(groupReference3)).isFalse(); @@ -154,10 +123,10 @@ public class GroupReferenceTest { @Test public void testHashcode() { AccountGroup.UUID uuid1 = AccountGroup.uuid("uuid1"); - assertThat(new GroupReference(uuid1, "foo").hashCode()) - .isEqualTo(new GroupReference(uuid1, "bar").hashCode()); + assertThat(GroupReference.create(uuid1, "foo").hashCode()) + .isEqualTo(GroupReference.create(uuid1, "bar").hashCode()); // Check that the following calls don't fail with an exception. - new GroupReference("bar").hashCode(); + GroupReference.create("bar").hashCode(); } } diff --git a/javatests/com/google/gerrit/common/data/PermissionRuleTest.java b/javatests/com/google/gerrit/common/data/PermissionRuleTest.java index d815dbc2e5..6dc357c218 100644 --- a/javatests/com/google/gerrit/common/data/PermissionRuleTest.java +++ b/javatests/com/google/gerrit/common/data/PermissionRuleTest.java @@ -28,7 +28,7 @@ public class PermissionRuleTest { @Before public void setup() { - this.groupReference = new GroupReference(AccountGroup.uuid("uuid"), "group"); + this.groupReference = GroupReference.create(AccountGroup.uuid("uuid"), "group"); this.permissionRule = new PermissionRule(groupReference); } @@ -130,7 +130,7 @@ public class PermissionRuleTest { @Test public void setGroup() { - GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2"); + GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2"); assertThat(groupReference2).isNotEqualTo(groupReference); assertThat(permissionRule.getGroup()).isEqualTo(groupReference); @@ -141,10 +141,10 @@ public class PermissionRuleTest { @Test public void mergeFromAnyBlock() { - GroupReference groupReference1 = new GroupReference(AccountGroup.uuid("uuid1"), "group1"); + GroupReference groupReference1 = GroupReference.create(AccountGroup.uuid("uuid1"), "group1"); PermissionRule permissionRule1 = new PermissionRule(groupReference1); - GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2"); + GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2"); PermissionRule permissionRule2 = new PermissionRule(groupReference2); permissionRule1.mergeFrom(permissionRule2); @@ -169,10 +169,10 @@ public class PermissionRuleTest { @Test public void mergeFromAnyDeny() { - GroupReference groupReference1 = new GroupReference(AccountGroup.uuid("uuid1"), "group1"); + GroupReference groupReference1 = GroupReference.create(AccountGroup.uuid("uuid1"), "group1"); PermissionRule permissionRule1 = new PermissionRule(groupReference1); - GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2"); + GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2"); PermissionRule permissionRule2 = new PermissionRule(groupReference2); permissionRule1.mergeFrom(permissionRule2); @@ -192,10 +192,10 @@ public class PermissionRuleTest { @Test public void mergeFromAnyBatch() { - GroupReference groupReference1 = new GroupReference(AccountGroup.uuid("uuid1"), "group1"); + GroupReference groupReference1 = GroupReference.create(AccountGroup.uuid("uuid1"), "group1"); PermissionRule permissionRule1 = new PermissionRule(groupReference1); - GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2"); + GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2"); PermissionRule permissionRule2 = new PermissionRule(groupReference2); permissionRule1.mergeFrom(permissionRule2); @@ -215,10 +215,10 @@ public class PermissionRuleTest { @Test public void mergeFromAnyForce() { - GroupReference groupReference1 = new GroupReference(AccountGroup.uuid("uuid1"), "group1"); + GroupReference groupReference1 = GroupReference.create(AccountGroup.uuid("uuid1"), "group1"); PermissionRule permissionRule1 = new PermissionRule(groupReference1); - GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2"); + GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2"); PermissionRule permissionRule2 = new PermissionRule(groupReference2); permissionRule1.mergeFrom(permissionRule2); @@ -238,11 +238,11 @@ public class PermissionRuleTest { @Test public void mergeFromMergeRange() { - GroupReference groupReference1 = new GroupReference(AccountGroup.uuid("uuid1"), "group1"); + GroupReference groupReference1 = GroupReference.create(AccountGroup.uuid("uuid1"), "group1"); PermissionRule permissionRule1 = new PermissionRule(groupReference1); permissionRule1.setRange(-1, 2); - GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2"); + GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2"); PermissionRule permissionRule2 = new PermissionRule(groupReference2); permissionRule2.setRange(-2, 1); @@ -255,10 +255,10 @@ public class PermissionRuleTest { @Test public void mergeFromGroupNotChanged() { - GroupReference groupReference1 = new GroupReference(AccountGroup.uuid("uuid1"), "group1"); + GroupReference groupReference1 = GroupReference.create(AccountGroup.uuid("uuid1"), "group1"); PermissionRule permissionRule1 = new PermissionRule(groupReference1); - GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2"); + GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2"); PermissionRule permissionRule2 = new PermissionRule(groupReference2); permissionRule1.mergeFrom(permissionRule2); @@ -347,7 +347,7 @@ public class PermissionRuleTest { @Test public void testEquals() { - GroupReference groupReference2 = new GroupReference(AccountGroup.uuid("uuid2"), "group2"); + GroupReference groupReference2 = GroupReference.create(AccountGroup.uuid("uuid2"), "group2"); PermissionRule permissionRuleOther = new PermissionRule(groupReference2); assertThat(permissionRule.equals(permissionRuleOther)).isFalse(); diff --git a/javatests/com/google/gerrit/common/data/PermissionTest.java b/javatests/com/google/gerrit/common/data/PermissionTest.java index 1012eff795..ef36ad9e95 100644 --- a/javatests/com/google/gerrit/common/data/PermissionTest.java +++ b/javatests/com/google/gerrit/common/data/PermissionTest.java @@ -154,14 +154,14 @@ public class PermissionTest { @Test public void setAndGetRules() { PermissionRule permissionRule1 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1")); PermissionRule permissionRule2 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2")); permission.setRules(ImmutableList.of(permissionRule1, permissionRule2)); assertThat(permission.getRules()).containsExactly(permissionRule1, permissionRule2).inOrder(); PermissionRule permissionRule3 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-3"), "group3")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-3"), "group3")); permission.setRules(ImmutableList.of(permissionRule3)); assertThat(permission.getRules()).containsExactly(permissionRule3); } @@ -169,10 +169,10 @@ public class PermissionTest { @Test public void cannotAddPermissionByModifyingListThatWasProvidedToAccessSection() { PermissionRule permissionRule1 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1")); PermissionRule permissionRule2 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2")); - GroupReference groupReference3 = new GroupReference(AccountGroup.uuid("uuid-3"), "group3"); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2")); + GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3"); List rules = new ArrayList<>(); rules.add(permissionRule1); @@ -187,14 +187,14 @@ public class PermissionTest { @Test public void getNonExistingRule() { - GroupReference groupReference = new GroupReference(AccountGroup.uuid("uuid-1"), "group1"); + GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"); assertThat(permission.getRule(groupReference)).isNull(); assertThat(permission.getRule(groupReference, false)).isNull(); } @Test public void getRule() { - GroupReference groupReference = new GroupReference(AccountGroup.uuid("uuid-1"), "group1"); + GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"); PermissionRule permissionRule = new PermissionRule(groupReference); permission.setRules(ImmutableList.of(permissionRule)); assertThat(permission.getRule(groupReference)).isEqualTo(permissionRule); @@ -202,7 +202,7 @@ public class PermissionTest { @Test public void createMissingRuleOnGet() { - GroupReference groupReference = new GroupReference(AccountGroup.uuid("uuid-1"), "group1"); + GroupReference groupReference = GroupReference.create(AccountGroup.uuid("uuid-1"), "group1"); assertThat(permission.getRule(groupReference)).isNull(); assertThat(permission.getRule(groupReference, true)) @@ -212,11 +212,11 @@ public class PermissionTest { @Test public void addRule() { PermissionRule permissionRule1 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1")); PermissionRule permissionRule2 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2")); permission.setRules(ImmutableList.of(permissionRule1, permissionRule2)); - GroupReference groupReference3 = new GroupReference(AccountGroup.uuid("uuid-3"), "group3"); + GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3"); assertThat(permission.getRule(groupReference3)).isNull(); PermissionRule permissionRule3 = new PermissionRule(groupReference3); @@ -230,10 +230,10 @@ public class PermissionTest { @Test public void removeRule() { PermissionRule permissionRule1 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1")); PermissionRule permissionRule2 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2")); - GroupReference groupReference3 = new GroupReference(AccountGroup.uuid("uuid-3"), "group3"); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2")); + GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3"); PermissionRule permissionRule3 = new PermissionRule(groupReference3); permission.setRules(ImmutableList.of(permissionRule1, permissionRule2, permissionRule3)); @@ -247,10 +247,10 @@ public class PermissionTest { @Test public void removeRuleByGroupReference() { PermissionRule permissionRule1 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1")); PermissionRule permissionRule2 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2")); - GroupReference groupReference3 = new GroupReference(AccountGroup.uuid("uuid-3"), "group3"); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2")); + GroupReference groupReference3 = GroupReference.create(AccountGroup.uuid("uuid-3"), "group3"); PermissionRule permissionRule3 = new PermissionRule(groupReference3); permission.setRules(ImmutableList.of(permissionRule1, permissionRule2, permissionRule3)); @@ -264,9 +264,9 @@ public class PermissionTest { @Test public void clearRules() { PermissionRule permissionRule1 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1")); PermissionRule permissionRule2 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2")); permission.setRules(ImmutableList.of(permissionRule1, permissionRule2)); assertThat(permission.getRules()).isNotEmpty(); @@ -278,11 +278,11 @@ public class PermissionTest { @Test public void mergePermissions() { PermissionRule permissionRule1 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1")); PermissionRule permissionRule2 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2")); PermissionRule permissionRule3 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-3"), "group3")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-3"), "group3")); Permission permission1 = new Permission("foo"); permission1.setRules(ImmutableList.of(permissionRule1, permissionRule2)); @@ -299,9 +299,9 @@ public class PermissionTest { @Test public void testEquals() { PermissionRule permissionRule1 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-1"), "group1")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-1"), "group1")); PermissionRule permissionRule2 = - new PermissionRule(new GroupReference(AccountGroup.uuid("uuid-2"), "group2")); + new PermissionRule(GroupReference.create(AccountGroup.uuid("uuid-2"), "group2")); permission.setRules(ImmutableList.of(permissionRule1, permissionRule2)); diff --git a/javatests/com/google/gerrit/server/account/DestinationListTest.java b/javatests/com/google/gerrit/server/account/DestinationListTest.java index 4188f39df7..6fcf75caac 100644 --- a/javatests/com/google/gerrit/server/account/DestinationListTest.java +++ b/javatests/com/google/gerrit/server/account/DestinationListTest.java @@ -132,7 +132,7 @@ public class DestinationListTest { List errors = new ArrayList<>(); new DestinationList().parseLabel(LABEL, L_BAD, errors::add); assertThat(errors) - .containsExactly(new ValidationError("destinationslabel", 1, "missing tab delimiter")); + .containsExactly(ValidationError.create("destinationslabel", 1, "missing tab delimiter")); } @Test diff --git a/javatests/com/google/gerrit/server/account/QueryListTest.java b/javatests/com/google/gerrit/server/account/QueryListTest.java index 7d491c96e8..74ce907d4c 100644 --- a/javatests/com/google/gerrit/server/account/QueryListTest.java +++ b/javatests/com/google/gerrit/server/account/QueryListTest.java @@ -101,7 +101,8 @@ public class QueryListTest { public void testParseBad() throws Exception { List errors = new ArrayList<>(); assertThat(QueryList.parse(L_BAD, errors::add).asText()).isNull(); - assertThat(errors).containsExactly(new ValidationError("queries", 1, "missing tab delimiter")); + assertThat(errors) + .containsExactly(ValidationError.create("queries", 1, "missing tab delimiter")); } @Test diff --git a/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java b/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java index df97e8809d..278f6179e4 100644 --- a/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java +++ b/javatests/com/google/gerrit/server/group/db/GroupNameNotesTest.java @@ -393,8 +393,8 @@ public class GroupNameNotesTest { ImmutableList allGroups = GroupNameNotes.loadAllGroups(repo); - GroupReference group1 = new GroupReference(groupUuid1, groupName1.get()); - GroupReference group2 = new GroupReference(groupUuid2, groupName2.get()); + GroupReference group1 = GroupReference.create(groupUuid1, groupName1.get()); + GroupReference group2 = GroupReference.create(groupUuid2, groupName2.get()); assertThat(allGroups).containsExactly(group1, group2); } @@ -406,8 +406,8 @@ public class GroupNameNotesTest { ImmutableList allGroups = GroupNameNotes.loadAllGroups(repo); - GroupReference group1 = new GroupReference(groupUuid, groupName.get()); - GroupReference group2 = new GroupReference(groupUuid, anotherGroupName.get()); + GroupReference group1 = GroupReference.create(groupUuid, groupName.get()); + GroupReference group2 = GroupReference.create(groupUuid, anotherGroupName.get()); assertThat(allGroups).containsExactly(group1, group2); } @@ -498,14 +498,14 @@ public class GroupNameNotesTest { @Test public void updateGroupNamesRejectsNonOneToOneGroupReferences() throws Exception { assertIllegalArgument( - new GroupReference(AccountGroup.uuid("uuid1"), "name1"), - new GroupReference(AccountGroup.uuid("uuid1"), "name2")); + GroupReference.create(AccountGroup.uuid("uuid1"), "name1"), + GroupReference.create(AccountGroup.uuid("uuid1"), "name2")); assertIllegalArgument( - new GroupReference(AccountGroup.uuid("uuid1"), "name1"), - new GroupReference(AccountGroup.uuid("uuid2"), "name1")); + GroupReference.create(AccountGroup.uuid("uuid1"), "name1"), + GroupReference.create(AccountGroup.uuid("uuid2"), "name1")); assertIllegalArgument( - new GroupReference(AccountGroup.uuid("uuid1"), "name1"), - new GroupReference(AccountGroup.uuid("uuid1"), "name1")); + GroupReference.create(AccountGroup.uuid("uuid1"), "name1"), + GroupReference.create(AccountGroup.uuid("uuid1"), "name1")); } @Test @@ -554,7 +554,7 @@ public class GroupNameNotesTest { private GroupReference newGroup(String name) { int id = idCounter.incrementAndGet(); - return new GroupReference(AccountGroup.uuid(name + "-" + id), name); + return GroupReference.create(AccountGroup.uuid(name + "-" + id), name); } private static PersonIdent newPersonIdent() { diff --git a/javatests/com/google/gerrit/server/project/GroupListTest.java b/javatests/com/google/gerrit/server/project/GroupListTest.java index 518f85dc48..18e1631418 100644 --- a/javatests/com/google/gerrit/server/project/GroupListTest.java +++ b/javatests/com/google/gerrit/server/project/GroupListTest.java @@ -63,7 +63,7 @@ public class GroupListTest { @Test public void put() { AccountGroup.UUID uuid = AccountGroup.uuid("abc"); - GroupReference groupReference = new GroupReference(uuid, "Hutzliputz"); + GroupReference groupReference = GroupReference.create(uuid, "Hutzliputz"); groupList.put(uuid, groupReference); @@ -78,7 +78,7 @@ public class GroupListTest { assertEquals(2, result.size()); AccountGroup.UUID uuid = AccountGroup.uuid("ebe31c01aec2c9ac3b3c03e87a47450829ff4310"); - GroupReference expected = new GroupReference(uuid, "Administrators"); + GroupReference expected = GroupReference.create(uuid, "Administrators"); assertTrue(result.contains(expected)); } diff --git a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java index 0dd643627b..214aae7a12 100644 --- a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java +++ b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java @@ -34,6 +34,7 @@ import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.PluginConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; +import com.google.gerrit.server.git.BranchOrderSection; import com.google.gerrit.server.git.ValidationError; import com.google.gerrit.server.git.meta.MetaDataUpdate; import com.google.gerrit.server.project.testing.TestLabels; @@ -90,8 +91,8 @@ public class ProjectConfigTest { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); private final GroupReference developers = - new GroupReference(AccountGroup.uuid("X"), "Developers"); - private final GroupReference staff = new GroupReference(AccountGroup.uuid("Y"), "Staff"); + GroupReference.create(AccountGroup.uuid("X"), "Developers"); + private final GroupReference staff = GroupReference.create(AccountGroup.uuid("Y"), "Staff"); private SitePaths sitePaths; private ProjectConfig.Factory factory; @@ -360,13 +361,44 @@ public class ProjectConfigTest { + "\tvalue = +1 Positive\n"); } + @Test + public void readExistingBranchOrder() throws Exception { + RevCommit rev = + tr.commit() + .add("project.config", "[branchOrder]\n" + "\tbranch = foo\n" + "\tbranch = bar\n") + .create(); + update(rev); + + ProjectConfig cfg = read(rev); + assertThat(cfg.getBranchOrderSection()) + .isEqualTo(BranchOrderSection.create(ImmutableList.of("foo", "bar"))); + } + + @Test + public void editBranchOrder() throws Exception { + RevCommit rev = tr.commit().create(); + update(rev); + + ProjectConfig cfg = read(rev); + cfg.setBranchOrderSection(BranchOrderSection.create(ImmutableList.of("foo", "bar"))); + rev = commit(cfg); + assertThat(text(rev, "project.config")) + .isEqualTo("[branchOrder]\n" + "\tbranch = foo\n" + "\tbranch = bar\n"); + } + @Test public void addCommentLink() throws Exception { RevCommit rev = tr.commit().create(); update(rev); ProjectConfig cfg = read(rev); - CommentLinkInfoImpl cm = new CommentLinkInfoImpl("Test", "abc.*", null, "link", true); + StoredCommentLinkInfo cm = + StoredCommentLinkInfo.builder("Test") + .setMatch("abc.*") + .setHtml("link") + .setEnabled(true) + .setOverrideOnly(false) + .build(); cfg.addCommentLinkSection(cm); rev = commit(cfg); assertThat(text(rev, "project.config")) @@ -529,12 +561,11 @@ public class ProjectConfigTest { ProjectConfig cfg = read(rev); assertThat(cfg.getCommentLinkSections()) .containsExactly( - new CommentLinkInfoImpl( - "bugzilla", - "(bug\\s+#?)(\\d+)", - "http://bugs.example.com/show_bug.cgi?id=$2", - null, - null)); + StoredCommentLinkInfo.builder("bugzilla") + .setMatch("(bug\\s+#?)(\\d+)") + .setLink("http://bugs.example.com/show_bug.cgi?id=$2") + .setOverrideOnly(false) + .build()); } @Test @@ -543,7 +574,7 @@ public class ProjectConfigTest { tr.commit().add("project.config", "[commentlink \"bugzilla\"]\n \tenabled = true").create(); ProjectConfig cfg = read(rev); assertThat(cfg.getCommentLinkSections()) - .containsExactly(new CommentLinkInfoImpl.Enabled("bugzilla")); + .containsExactly(StoredCommentLinkInfo.enabled("bugzilla")); } @Test @@ -554,7 +585,7 @@ public class ProjectConfigTest { .create(); ProjectConfig cfg = read(rev); assertThat(cfg.getCommentLinkSections()) - .containsExactly(new CommentLinkInfoImpl.Disabled("bugzilla")); + .containsExactly(StoredCommentLinkInfo.disabled("bugzilla")); } @Test @@ -571,7 +602,7 @@ public class ProjectConfigTest { assertThat(cfg.getCommentLinkSections()).isEmpty(); assertThat(cfg.getValidationErrors()) .containsExactly( - new ValidationError( + ValidationError.create( "project.config: Invalid pattern \"(bugs{+#?)(d+)\" in commentlink.bugzilla.match: " + "Illegal repetition near index 4\n" + "(bugs{+#?)(d+)\n" @@ -592,7 +623,7 @@ public class ProjectConfigTest { assertThat(cfg.getCommentLinkSections()).isEmpty(); assertThat(cfg.getValidationErrors()) .containsExactly( - new ValidationError( + ValidationError.create( "project.config: Error in pattern \"(bugs#?)(d+)\" in commentlink.bugzilla.match: " + "Raw html replacement not allowed")); } @@ -607,7 +638,7 @@ public class ProjectConfigTest { assertThat(cfg.getCommentLinkSections()).isEmpty(); assertThat(cfg.getValidationErrors()) .containsExactly( - new ValidationError( + ValidationError.create( "project.config: Error in pattern \"(bugs#?)(d+)\" in commentlink.bugzilla.match: " + "commentlink.bugzilla must have either link or html")); } diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java index dbcac0d8a2..331bb4c45f 100644 --- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java +++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java @@ -1861,7 +1861,7 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests { ProjectConfig config = projectConfigFactory.read(md); AccessSection s = config.getAccessSection(ref, true); Permission p = s.getPermission(permission, true); - PermissionRule rule = new PermissionRule(new GroupReference(groupUUID, groupUUID.get())); + PermissionRule rule = new PermissionRule(GroupReference.create(groupUUID, groupUUID.get())); rule.setForce(force); p.add(rule); config.commit(md); diff --git a/javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java b/javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java index b65f4d24c9..eceec8b91e 100644 --- a/javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java +++ b/javatests/com/google/gerrit/server/schema/AllProjectsCreatorTest.java @@ -102,7 +102,7 @@ public class AllProjectsCreatorTest { private GroupReference createGroupReference(String name) { AccountGroup.UUID groupUuid = GroupUuid.make(name, serverUser); - return new GroupReference(groupUuid, name); + return GroupReference.create(groupUuid, name); } @Test diff --git a/javatests/com/google/gerrit/server/submit/SubscriptionGraphTest.java b/javatests/com/google/gerrit/server/submit/SubscriptionGraphTest.java index dbcc20952e..489e1b6ab4 100644 --- a/javatests/com/google/gerrit/server/submit/SubscriptionGraphTest.java +++ b/javatests/com/google/gerrit/server/submit/SubscriptionGraphTest.java @@ -193,9 +193,9 @@ public class SubscriptionGraphTest { } private void allowSubscription(BranchNameKey branch) { - SubscribeSection s = new SubscribeSection(branch.project()); + SubscribeSection.Builder s = SubscribeSection.builder(branch.project()); s.addMultiMatchRefSpec("refs/heads/*:refs/heads/*"); - when(mockProjectState.getSubscribeSections(branch)).thenReturn(ImmutableSet.of(s)); + when(mockProjectState.getSubscribeSections(branch)).thenReturn(ImmutableSet.of(s.build())); } private void setSubscription( diff --git a/plugins/singleusergroup b/plugins/singleusergroup index d04c4c33ad..9eb63345a1 160000 --- a/plugins/singleusergroup +++ b/plugins/singleusergroup @@ -1 +1 @@ -Subproject commit d04c4c33ad36e2e11ccc8b798357dd1e4e979a1a +Subproject commit 9eb63345a129533aa88235af3ba9308c53cee1d2