Merge changes from topic "notedb-2.15-prep" * changes: Rename dev-note-db.txt to note-db.txt dev-note-db.txt: Document trial mode Move non-migration-related NoteDb config options to regular docs Revamp NoteDb docs to describe supported migration process InitExperimental: Tone down NoteDb warnings MigrateToNoteDb: Default to non-trial mode Daemon: Unmark --migrate-to-note-db as experimental
diff --git a/Documentation/index.txt b/Documentation/index.txt index 0be93ff..da211b6 100644 --- a/Documentation/index.txt +++ b/Documentation/index.txt
@@ -12,7 +12,7 @@ == Guides . link:intro-user.html[User Guide] . link:intro-project-owner.html[Project Owner Guide] -. link:http://source.android.com/submit-patches/workflow[Default Android Workflow] (external) +. link:https://source.android.com/source/developing[Default Android Workflow] (external) == Tutorials . Web
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/conditions/BooleanCondition.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/conditions/BooleanCondition.java new file mode 100644 index 0000000..950365a --- /dev/null +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/conditions/BooleanCondition.java
@@ -0,0 +1,217 @@ +// Copyright (C) 2017 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.extensions.conditions; + +import com.google.common.collect.Iterables; +import java.util.Collections; + +/** Delayed evaluation of a boolean condition. */ +public abstract class BooleanCondition { + public static final BooleanCondition TRUE = new Value(true); + public static final BooleanCondition FALSE = new Value(false); + + public static BooleanCondition valueOf(boolean a) { + return a ? TRUE : FALSE; + } + + public static BooleanCondition and(BooleanCondition a, BooleanCondition b) { + return a == FALSE || b == FALSE ? FALSE : new And(a, b); + } + + public static BooleanCondition and(boolean a, BooleanCondition b) { + return and(valueOf(a), b); + } + + public static BooleanCondition or(BooleanCondition a, BooleanCondition b) { + return a == TRUE || b == TRUE ? TRUE : new Or(a, b); + } + + public static BooleanCondition or(boolean a, BooleanCondition b) { + return or(valueOf(a), b); + } + + public static BooleanCondition not(BooleanCondition bc) { + return bc == TRUE ? FALSE : bc == FALSE ? TRUE : new Not(bc); + } + + BooleanCondition() {} + + /** @return evaluate the condition and return its value. */ + public abstract boolean value(); + + /** + * Recursively collect all children of type {@code type}. + * + * @param type implementation type of the conditions to collect and return. + * @return non-null, unmodifiable iteration of children of type {@code type}. + */ + public abstract <T> Iterable<T> children(Class<T> type); + + private static final class And extends BooleanCondition { + private final BooleanCondition a; + private final BooleanCondition b; + + And(BooleanCondition a, BooleanCondition b) { + this.a = a; + this.b = b; + } + + @Override + public boolean value() { + return a.value() && b.value(); + } + + @Override + public <T> Iterable<T> children(Class<T> type) { + return Iterables.concat(a.children(type), b.children(type)); + } + + @Override + public int hashCode() { + return a.hashCode() * 31 + b.hashCode(); + } + + @Override + public boolean equals(Object other) { + if (other instanceof And) { + And o = (And) other; + return a.equals(o.a) && b.equals(o.b); + } + return false; + } + + @Override + public String toString() { + return "(" + maybeTrim(a, getClass()) + " && " + maybeTrim(a, getClass()) + ")"; + } + } + + private static final class Or extends BooleanCondition { + private final BooleanCondition a; + private final BooleanCondition b; + + Or(BooleanCondition a, BooleanCondition b) { + this.a = a; + this.b = b; + } + + @Override + public boolean value() { + return a.value() || b.value(); + } + + @Override + public <T> Iterable<T> children(Class<T> type) { + return Iterables.concat(a.children(type), b.children(type)); + } + + @Override + public int hashCode() { + return a.hashCode() * 31 + b.hashCode(); + } + + @Override + public boolean equals(Object other) { + if (other instanceof Or) { + Or o = (Or) other; + return a.equals(o.a) && b.equals(o.b); + } + return false; + } + + @Override + public String toString() { + return "(" + maybeTrim(a, getClass()) + " || " + maybeTrim(a, getClass()) + ")"; + } + } + + private static final class Not extends BooleanCondition { + private final BooleanCondition cond; + + Not(BooleanCondition bc) { + cond = bc; + } + + @Override + public boolean value() { + return !cond.value(); + } + + @Override + public <T> Iterable<T> children(Class<T> type) { + return cond.children(type); + } + + @Override + public int hashCode() { + return cond.hashCode() * 31; + } + + @Override + public boolean equals(Object other) { + return other instanceof Not ? cond.equals(((Not) other).cond) : false; + } + + @Override + public String toString() { + return "!" + cond; + } + } + + private static final class Value extends BooleanCondition { + private final boolean value; + + Value(boolean v) { + value = v; + } + + @Override + public boolean value() { + return value; + } + + @Override + public <T> Iterable<T> children(Class<T> type) { + return Collections.emptyList(); + } + + @Override + public int hashCode() { + return value ? 1 : 0; + } + + @Override + public boolean equals(Object other) { + return other instanceof Value ? value == ((Value) other).value : false; + } + + @Override + public String toString() { + return Boolean.toString(value); + } + } + + /** Remove leading '(' and trailing ')' if the type is the same as the parent. */ + static String maybeTrim(BooleanCondition cond, Class<? extends BooleanCondition> type) { + String s = cond.toString(); + if (cond.getClass() == type + && s.length() > 2 + && s.charAt(0) == '(' + && s.charAt(s.length() - 1) == ')') { + s = s.substring(1, s.length() - 1); + } + return s; + } +}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/conditions/PrivateInternals_BooleanCondition.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/conditions/PrivateInternals_BooleanCondition.java new file mode 100644 index 0000000..4fa932a --- /dev/null +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/conditions/PrivateInternals_BooleanCondition.java
@@ -0,0 +1,33 @@ +// Copyright (C) 2017 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.extensions.conditions; + +import java.util.Collections; + +/** <b>DO NOT USE</b> */ +public final class PrivateInternals_BooleanCondition { + private PrivateInternals_BooleanCondition() {} + + public abstract static class SubclassOnlyInCoreServer extends BooleanCondition { + @SuppressWarnings("unchecked") + @Override + public <T> Iterable<T> children(Class<T> type) { + if (type.isAssignableFrom(getClass())) { + return Collections.singleton((T) this); + } + return Collections.emptyList(); + } + } +}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/webui/UiAction.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/webui/UiAction.java index 62c074e..5f6dec3 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/webui/UiAction.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/webui/UiAction.java
@@ -14,6 +14,7 @@ package com.google.gerrit.extensions.webui; +import com.google.gerrit.extensions.conditions.BooleanCondition; import com.google.gerrit.extensions.restapi.RestResource; import com.google.gerrit.extensions.restapi.RestView; @@ -35,8 +36,8 @@ private String id; private String label; private String title; - private boolean visible = true; - private boolean enabled = true; + private BooleanCondition visible = BooleanCondition.TRUE; + private BooleanCondition enabled = BooleanCondition.TRUE; public String getMethod() { return method; @@ -77,6 +78,10 @@ } public boolean isVisible() { + return getVisibleCondition().value(); + } + + public BooleanCondition getVisibleCondition() { return visible; } @@ -85,16 +90,33 @@ * action description may not be sent to the client. */ public Description setVisible(boolean visible) { + return setVisible(BooleanCondition.valueOf(visible)); + } + + /** + * Set if the action's button is visible on screen for the current client. If not visible the + * action description may not be sent to the client. + */ + public Description setVisible(BooleanCondition visible) { this.visible = visible; return this; } public boolean isEnabled() { - return enabled && isVisible(); + return getEnabledCondition().value(); + } + + public BooleanCondition getEnabledCondition() { + return BooleanCondition.and(enabled, visible); } /** Set if the button should be invokable (true), or greyed out (false). */ public Description setEnabled(boolean enabled) { + return setEnabled(BooleanCondition.valueOf(enabled)); + } + + /** Set if the button should be invokable (true), or greyed out (false). */ + public Description setEnabled(BooleanCondition enabled) { this.enabled = enabled; return this; }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java index 19fdcfb..a1deb89 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ActionJson.java
@@ -16,7 +16,6 @@ import static com.google.common.base.Preconditions.checkNotNull; -import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -36,6 +35,7 @@ import com.google.inject.Inject; import com.google.inject.Singleton; import java.util.ArrayList; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -162,7 +162,7 @@ return out; } - FluentIterable<UiAction.Description> descs = + Iterable<UiAction.Description> descs = uiActions.from(changeViews, changeResourceFactory.create(ctl)); // The followup action is a client-side only operation that does not @@ -174,7 +174,7 @@ PrivateInternals_UiActionDescription.setMethod(descr, "POST"); descr.setTitle("Create follow-up change"); descr.setLabel("Follow-Up"); - descs = descs.append(descr); + descs = Iterables.concat(descs, Collections.singleton(descr)); } ACTION:
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java index 993148e..930cb8b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java
@@ -14,6 +14,8 @@ package com.google.gerrit.server.change; +import static com.google.gerrit.extensions.conditions.BooleanCondition.and; + import com.google.gerrit.extensions.api.changes.CherryPickInput; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.restapi.BadRequestException; @@ -106,10 +108,11 @@ .setLabel("Cherry Pick") .setTitle("Cherry pick change to a different branch") .setVisible( - rsrc.isCurrent() - && permissionBackend + and( + rsrc.isCurrent(), + permissionBackend .user(user) .project(rsrc.getProject()) - .testOrFalse(ProjectPermission.CREATE_CHANGE)); + .testCond(ProjectPermission.CREATE_CHANGE))); } }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/webui/UiActions.java b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/webui/UiActions.java index ae15cfd..79a5d4c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/extensions/webui/UiActions.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/extensions/webui/UiActions.java
@@ -14,23 +14,32 @@ package com.google.gerrit.server.extensions.webui; +import static com.google.gerrit.extensions.conditions.BooleanCondition.and; +import static com.google.gerrit.extensions.conditions.BooleanCondition.or; +import static java.util.stream.Collectors.toList; + import com.google.common.base.Predicate; -import com.google.common.collect.FluentIterable; +import com.google.common.collect.Streams; import com.google.gerrit.common.Nullable; import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission; +import com.google.gerrit.extensions.conditions.BooleanCondition; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.restapi.RestCollection; import com.google.gerrit.extensions.restapi.RestResource; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.webui.PrivateInternals_UiActionDescription; import com.google.gerrit.extensions.webui.UiAction; +import com.google.gerrit.extensions.webui.UiAction.Description; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.permissions.GlobalPermission; import com.google.gerrit.server.permissions.PermissionBackend; +import com.google.gerrit.server.permissions.PermissionBackendCondition; import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; +import java.util.Iterator; +import java.util.List; import java.util.Objects; import java.util.Set; import org.slf4j.Logger; @@ -53,16 +62,35 @@ this.userProvider = userProvider; } - public <R extends RestResource> FluentIterable<UiAction.Description> from( + public <R extends RestResource> Iterable<UiAction.Description> from( RestCollection<?, R> collection, R resource) { return from(collection.views(), resource); } - public <R extends RestResource> FluentIterable<UiAction.Description> from( + public <R extends RestResource> Iterable<UiAction.Description> from( DynamicMap<RestView<R>> views, R resource) { - return FluentIterable.from(views) - .transform((e) -> describe(e, resource)) - .filter(Objects::nonNull); + List<UiAction.Description> descs = + Streams.stream(views) + .map(e -> describe(e, resource)) + .filter(Objects::nonNull) + .collect(toList()); + + List<PermissionBackendCondition> conds = + Streams.concat( + descs.stream().flatMap(u -> Streams.stream(visibleCondition(u))), + descs.stream().flatMap(u -> Streams.stream(enabledCondition(u)))) + .collect(toList()); + permissionBackend.bulkEvaluateTest(conds); + + return descs.stream().filter(u -> u.isVisible()).collect(toList()); + } + + private static Iterable<PermissionBackendCondition> visibleCondition(Description u) { + return u.getVisibleCondition().children(PermissionBackendCondition.class); + } + + private static Iterable<PermissionBackendCondition> enabledCondition(Description u) { + return u.getEnabledCondition().children(PermissionBackendCondition.class); } @Nullable @@ -86,22 +114,27 @@ return null; } + UiAction.Description dsc = ((UiAction<R>) view).getDescription(resource); + if (dsc == null) { + return null; + } + + Set<GlobalOrPluginPermission> globalRequired; try { - Set<GlobalOrPluginPermission> need = - GlobalPermission.fromAnnotation(e.getPluginName(), view.getClass()); - if (!need.isEmpty() && permissionBackend.user(userProvider).test(need).isEmpty()) { - // A permission is required, but test returned no candidates. - return null; - } + globalRequired = GlobalPermission.fromAnnotation(e.getPluginName(), view.getClass()); } catch (PermissionBackendException err) { log.error( String.format("exception testing view %s.%s", e.getPluginName(), e.getExportName()), err); return null; } - - UiAction.Description dsc = ((UiAction<R>) view).getDescription(resource); - if (dsc == null || !dsc.isVisible()) { - return null; + if (!globalRequired.isEmpty()) { + PermissionBackend.WithUser withUser = permissionBackend.user(userProvider); + Iterator<GlobalOrPluginPermission> i = globalRequired.iterator(); + BooleanCondition p = withUser.testCond(i.next()); + while (i.hasNext()) { + p = or(p, withUser.testCond(i.next())); + } + dsc.setVisible(and(p, dsc.getVisibleCondition())); } String name = e.getExportName().substring(d + 1);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java index 522eccb..5c0cf44 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -20,6 +20,7 @@ import com.google.common.collect.Sets; import com.google.gerrit.common.data.LabelType; import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission; +import com.google.gerrit.extensions.conditions.BooleanCondition; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Project; @@ -78,7 +79,7 @@ * public UiAction.Description getDescription(ChangeResource rsrc) { * return new UiAction.Description() * .setLabel("Submit") - * .setVisible(rsrc.permissions().testOrFalse(ChangePermission.SUBMIT)); + * .setVisible(rsrc.permissions().testCond(ChangePermission.SUBMIT)); * } * </pre> */ @@ -94,6 +95,24 @@ return user(checkNotNull(user, "Provider<CurrentUser>").get()); } + /** + * Bulk evaluate a collection of {@link PermissionBackendCondition} for view handling. + * + * <p>Overridden implementations should call {@link PermissionBackendCondition#set(boolean)} to + * cache the result of {@code testOrFalse} in the condition for later evaluation. Caching the + * result will bypass the usual invocation of {@code testOrFalse}. + * + * <p>{@code conds} may contain duplicate entries (such as same user, resource, permission + * triplet). When duplicates exist, implementations should set a result into all instances to + * ensure {@code testOrFalse} does not get invoked during evaluation of the containing condition. + * + * @param conds conditions to consider. + */ + public void bulkEvaluateTest(Collection<PermissionBackendCondition> conds) { + // Do nothing by default. The default implementation of PermissionBackendCondition + // delegates to the appropriate testOrFalse method in PermissionBackend. + } + /** PermissionBackend with an optional per-request ReviewDb handle. */ public abstract static class AcceptsReviewDb<T> { protected Provider<ReviewDb> db; @@ -198,6 +217,10 @@ } } + public BooleanCondition testCond(GlobalOrPluginPermission perm) { + return new PermissionBackendCondition.WithUser(this, perm); + } + /** * Filter a set of projects using {@code check(perm)}. * @@ -265,6 +288,10 @@ return false; } } + + public BooleanCondition testCond(ProjectPermission perm) { + return new PermissionBackendCondition.ForProject(this, perm); + } } /** PermissionBackend scoped to a user, project and reference. */ @@ -313,6 +340,10 @@ return false; } } + + public BooleanCondition testCond(RefPermission perm) { + return new PermissionBackendCondition.ForRef(this, perm); + } } /** PermissionBackend scoped to a user, project, reference and change. */ @@ -354,6 +385,10 @@ } } + public BooleanCondition testCond(ChangePermissionOrLabel perm) { + return new PermissionBackendCondition.ForChange(this, perm); + } + /** * Test which values of a label the user may be able to set. *
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java new file mode 100644 index 0000000..8d66e50 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackendCondition.java
@@ -0,0 +1,152 @@ +// Copyright (C) 2017 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.permissions; + +import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission; +import com.google.gerrit.extensions.conditions.BooleanCondition; +import com.google.gerrit.extensions.conditions.PrivateInternals_BooleanCondition; + +/** {@link BooleanCondition} to evaluate a permission. */ +public abstract class PermissionBackendCondition + extends PrivateInternals_BooleanCondition.SubclassOnlyInCoreServer { + Boolean value; + + /** + * Assign a specific {@code testOrFalse} result to this condition. + * + * <p>By setting the condition to a specific value the condition will bypass calling {@link + * PermissionBackend} during {@code value()}, and immediately return the set value instead. + * + * @param val value to return from {@code value()}. + */ + public void set(boolean val) { + value = val; + } + + @Override + public abstract String toString(); + + public static class WithUser extends PermissionBackendCondition { + private final PermissionBackend.WithUser impl; + private final GlobalOrPluginPermission perm; + + WithUser(PermissionBackend.WithUser impl, GlobalOrPluginPermission perm) { + this.impl = impl; + this.perm = perm; + } + + public PermissionBackend.WithUser withUser() { + return impl; + } + + public GlobalOrPluginPermission permission() { + return perm; + } + + @Override + public boolean value() { + return value != null ? value : impl.testOrFalse(perm); + } + + @Override + public String toString() { + return "PermissionBackendCondition.WithUser(" + perm + ")"; + } + } + + public static class ForProject extends PermissionBackendCondition { + private final PermissionBackend.ForProject impl; + private final ProjectPermission perm; + + ForProject(PermissionBackend.ForProject impl, ProjectPermission perm) { + this.impl = impl; + this.perm = perm; + } + + public PermissionBackend.ForProject project() { + return impl; + } + + public ProjectPermission permission() { + return perm; + } + + @Override + public boolean value() { + return value != null ? value : impl.testOrFalse(perm); + } + + @Override + public String toString() { + return "PermissionBackendCondition.ForProject(" + perm + ")"; + } + } + + public static class ForRef extends PermissionBackendCondition { + private final PermissionBackend.ForRef impl; + private final RefPermission perm; + + ForRef(PermissionBackend.ForRef impl, RefPermission perm) { + this.impl = impl; + this.perm = perm; + } + + public PermissionBackend.ForRef ref() { + return impl; + } + + public RefPermission permission() { + return perm; + } + + @Override + public boolean value() { + return value != null ? value : impl.testOrFalse(perm); + } + + @Override + public String toString() { + return "PermissionBackendCondition.ForRef(" + perm + ")"; + } + } + + public static class ForChange extends PermissionBackendCondition { + private final PermissionBackend.ForChange impl; + private final ChangePermissionOrLabel perm; + + ForChange(PermissionBackend.ForChange impl, ChangePermissionOrLabel perm) { + this.impl = impl; + this.perm = perm; + } + + public PermissionBackend.ForChange change() { + return impl; + } + + public ChangePermissionOrLabel permission() { + return perm; + } + + @Override + public boolean value() { + return value != null ? value : impl.testOrFalse(perm); + } + + @Override + public String toString() { + return "PermissionBackendCondition.ForChange(" + perm + ")"; + } + } +}