Merge branch 'stable-2.8'

* stable-2.8:
  Copy reviewed flag for deleted files
  Refactor ChangesCollection to be injectable
  Documentation: Link from 'Server administration -> Plugins' to plugins
  Minor cleanups in plugin documentation
  Document 'Force Edit' flag of 'Edit Topic Name' access right

Documentation/access-control.txt

@@ -822,6 +822,10 @@ The change owner, branch owners, project owners, and site administrators
 can always edit the topic name (even without having the `Edit Topic Name`
 access right assigned).
 
+Whether the topic can be edited on closed changes can be controlled
+by the 'Force Edit' flag. If this flag is not set the topic can only be
+edited on open changes.
+
 
 Examples of typical roles in a project
 --------------------------------------

Documentation/dev-plugins.txt

@@ -673,16 +673,17 @@ this can be specified by setting `scope = CapabilityScope.CORE`:
 UI Extension
 ------------
 
-Plugins can contribute their own UI actions on core Gerrit pages.
-This is useful for workflow customization or exposing plugin functionality
-through the UI in addition to SSH commands and the REST API.
+Plugins can contribute UI actions on core Gerrit pages. This is useful
+for workflow customization or exposing plugin functionality through the
+UI in addition to SSH commands and the REST API.
 
-For instance a plugin to integrate Jira with Gerrit changes may contribute its
-own "File bug" button to allow filing a bug from the change page or plugins to
-integrate continuous integration systems may contribute a "Schedule" button to
-allow a CI build to be scheduled manually from the patch set panel.
+For instance a plugin to integrate Jira with Gerrit changes may
+contribute a "File bug" button to allow filing a bug from the change
+page or plugins to integrate continuous integration systems may
+contribute a "Schedule" button to allow a CI build to be scheduled
+manually from the patch set panel.
 
-Two different places on core Gerrit pages are currently supported:
+Two different places on core Gerrit pages are supported:
 
 * Change screen
 * Project info screen
@@ -791,7 +792,8 @@ public class Module extends AbstractModule {
 }
 ----
 
-The module above must be declared in pom.xml for Maven driven plugins:
+The module above must be declared in the `pom.xml` for Maven driven
+plugins:
 
 [source,xml]
 ----
@@ -800,7 +802,7 @@ The module above must be declared in pom.xml for Maven driven plugins:
 </manifestEntries>
 ----
 
-or in the BUCK configuration file for Buck driven plugins:
+or in the `BUCK` configuration file for Buck driven plugins:
 
 [source,python]
 ----
@@ -855,7 +857,8 @@ public class HttpModule extends HttpPluginModule {
 }
 ----
 
-The HTTP module above must be declared in pom.xml for Maven driven plugins:
+The HTTP module above must be declared in the `pom.xml` for Maven
+driven plugins:
 
 [source,xml]
 ----
@@ -864,7 +867,7 @@ The HTTP module above must be declared in pom.xml for Maven driven plugins:
 </manifestEntries>
 ----
 
-or in the BUCK configuration file for Buck driven plugins
+or in the `BUCK` configuration file for Buck driven plugins
 
 [source,python]
 ----
@@ -881,7 +884,7 @@ case.
 The following prerequisities must be met, to satisfy the capability check:
 
 * user is authenticated
-* user is a member of the Administrators group, or
+* user is a member of a group which has the `Administrate Server` capability, or
 * user is a member of a group which has the required capability
 
 The `apply` method is called when the button is clicked. If `UiAction` is
@@ -901,7 +904,7 @@ can be accessed from any REST client, i. e.:
 ====
 
 A special case is to bind an endpoint without a view name.  This is
-particularly useful for DELETE requests:
+particularly useful for `DELETE` requests:
 
 [source,java]
 ----

Documentation/index.txt

@@ -60,7 +60,7 @@ Index
 ... How to read stats from the JVM
 .. High availability
 .. Replication
-.. Plugins
+.. link:https://gerrit-review.googlesource.com/#/admin/projects/?filter=plugins%252F[Plugins]
 .. link:dev-design.html[System Design]
 .. link:config-contact.html[User Contact Information]
 .. link:config-reverseproxy.html[Reverse Proxy]

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

@@ -31,6 +31,9 @@ import static com.google.gerrit.common.changes.ListChangesOption.REVIEWED;
 
 import com.google.common.base.Joiner;
 import com.google.common.base.Objects;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
 import com.google.common.collect.HashBasedTable;
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableList;
@@ -64,6 +67,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.PatchSetInfo;
 import com.google.gerrit.reviewdb.client.PatchSetInfo.ParentInfo;
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.UserIdentity;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.AnonymousUser;
@@ -77,7 +81,8 @@ import com.google.gerrit.server.patch.PatchListNotAvailableException;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
-import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.ResultSet;
@@ -87,6 +92,7 @@ import com.google.inject.Provider;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.IOException;
 import java.sql.Timestamp;
 import java.util.Collection;
 import java.util.Collections;
@@ -97,6 +103,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
+import java.util.concurrent.ExecutionException;
 
 public class ChangeJson {
   private static final Logger log = LoggerFactory.getLogger(ChangeJson.class);
@@ -122,7 +129,7 @@ public class ChangeJson {
   private final Provider<CurrentUser> userProvider;
   private final AnonymousUser anonymous;
   private final IdentifiedUser.GenericFactory userFactory;
-  private final ChangeControl.GenericFactory changeControlGenericFactory;
+  private final ProjectControl.GenericFactory projectControlFactory;
   private final PatchSetInfoFactory patchSetInfoFactory;
   private final FileInfoJson fileInfoJson;
   private final AccountInfo.Loader.Factory accountLoaderFactory;
@@ -131,20 +138,20 @@ public class ChangeJson {
   private final DynamicMap<RestView<ChangeResource>> changes;
   private final Revisions revisions;
 
-  private ChangeControl.Factory changeControlUserFactory;
   private EnumSet<ListChangesOption> options;
   private AccountInfo.Loader accountLoader;
   private ChangeControl lastControl;
   private Set<Change.Id> reviewed;
+  private LoadingCache<Project.NameKey, ProjectControl> projectControls;
 
   @Inject
   ChangeJson(
       Provider<ReviewDb> db,
       LabelNormalizer ln,
-      Provider<CurrentUser> userProvider,
+      Provider<CurrentUser> user,
       AnonymousUser au,
       IdentifiedUser.GenericFactory uf,
-      ChangeControl.GenericFactory ccf,
+      ProjectControl.GenericFactory pcf,
       PatchSetInfoFactory psi,
       FileInfoJson fileInfoJson,
       AccountInfo.Loader.Factory ailf,
@@ -154,10 +161,10 @@ public class ChangeJson {
       Revisions revisions) {
     this.db = db;
     this.labelNormalizer = ln;
-    this.userProvider = userProvider;
+    this.userProvider = user;
     this.anonymous = au;
     this.userFactory = uf;
-    this.changeControlGenericFactory = ccf;
+    this.projectControlFactory = pcf;
     this.patchSetInfoFactory = psi;
     this.fileInfoJson = fileInfoJson;
     this.accountLoaderFactory = ailf;
@@ -167,6 +174,15 @@ public class ChangeJson {
     this.revisions = revisions;
 
     options = EnumSet.noneOf(ListChangesOption.class);
+    projectControls = CacheBuilder.newBuilder()
+      .concurrencyLevel(1)
+      .build(new CacheLoader<Project.NameKey, ProjectControl>() {
+        @Override
+        public ProjectControl load(Project.NameKey key)
+            throws NoSuchProjectException, IOException {
+          return projectControlFactory.controlFor(key, userProvider.get());
+        }
+      });
   }
 
   public ChangeJson addOption(ListChangesOption o) {
@@ -179,11 +195,6 @@ public class ChangeJson {
     return this;
   }
 
-  public ChangeJson setChangeControlFactory(ChangeControl.Factory cf) {
-    changeControlUserFactory = cf;
-    return this;
-  }
-
   public ChangeInfo format(ChangeResource rsrc) throws OrmException {
     return format(new ChangeData(rsrc.getControl()));
   }
@@ -321,13 +332,12 @@ public class ChangeJson {
     }
 
     try {
-      if (changeControlUserFactory != null) {
-        ctrl = changeControlUserFactory.controlFor(cd.change(db));
-      } else {
-        ctrl = changeControlGenericFactory.controlFor(cd.change(db),
-            userProvider.get());
+      Change change = cd.change(db);
+      if (change == null) {
+        return null;
       }
-    } catch (NoSuchChangeException e) {
+      ctrl = projectControls.get(change.getProject()).controlFor(change);
+    } catch (ExecutionException e) {
       return null;
     }
     lastControl = ctrl;

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangesCollection.java

@@ -23,6 +23,7 @@ import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.query.change.QueryChanges;
@@ -37,17 +38,20 @@ import java.util.List;
 public class ChangesCollection implements
     RestCollection<TopLevelResource, ChangeResource> {
   private final Provider<ReviewDb> db;
-  private final ChangeControl.Factory changeControlFactory;
+  private final Provider<CurrentUser> user;
+  private final ChangeControl.GenericFactory changeControlFactory;
   private final Provider<QueryChanges> queryFactory;
   private final DynamicMap<RestView<ChangeResource>> views;
 
   @Inject
   ChangesCollection(
       Provider<ReviewDb> dbProvider,
-      ChangeControl.Factory changeControlFactory,
+      Provider<CurrentUser> user,
+      ChangeControl.GenericFactory changeControlFactory,
       Provider<QueryChanges> queryFactory,
       DynamicMap<RestView<ChangeResource>> views) {
     this.db = dbProvider;
+    this.user = user;
     this.changeControlFactory = changeControlFactory;
     this.queryFactory = queryFactory;
     this.views = views;
@@ -74,7 +78,7 @@ public class ChangesCollection implements
 
     ChangeControl control;
     try {
-      control = changeControlFactory.validateFor(changes.get(0));
+      control = changeControlFactory.validateFor(changes.get(0), user.get());
     } catch (NoSuchChangeException e) {
       throw new ResourceNotFoundException(id);
     }

gerrit-server/src/main/java/com/google/gerrit/server/change/Files.java

@@ -37,7 +37,6 @@ import com.google.gerrit.server.change.FileInfoJson.FileInfo;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.patch.PatchList;
 import com.google.gerrit.server.patch.PatchListCache;
-import com.google.gerrit.server.patch.PatchListEntry;
 import com.google.gerrit.server.patch.PatchListNotAvailableException;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -45,9 +44,9 @@ import com.google.inject.Provider;
 
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.treewalk.TreeWalk;
+import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
 import org.kohsuke.args4j.Option;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -219,23 +218,49 @@ class Files implements ChildCollection<RevisionResource, FileResource> {
           List<String> pathList = Lists.newArrayListWithCapacity(sz);
 
           RevWalk rw = new RevWalk(reader);
-          RevTree o = rw.parseCommit(oldList.getNewId()).getTree();
-          RevTree c = rw.parseCommit(curList.getNewId()).getTree();
-          for (PatchListEntry p : curList.getPatches()) {
-            String path = p.getNewName();
-            if (!Patch.COMMIT_MSG.equals(path) && paths.contains(path)) {
-              TreeWalk tw = TreeWalk.forPath(reader, path, o, c);
-              if (tw != null
-                  && tw.getRawMode(0) != 0
-                  && tw.getRawMode(1) != 0
-                  && tw.idEqual(0, 1)) {
-                inserts.add(new AccountPatchReview(
-                    new Patch.Key(
-                        resource.getPatchSet().getId(),
-                        path),
-                      userId));
-                pathList.add(path);
-              }
+          TreeWalk tw = new TreeWalk(reader);
+          tw.setFilter(PathFilterGroup.createFromStrings(paths));
+          tw.setRecursive(true);
+          int o = tw.addTree(rw.parseCommit(oldList.getNewId()).getTree());
+          int c = tw.addTree(rw.parseCommit(curList.getNewId()).getTree());
+
+          int op = -1;
+          if (oldList.getOldId() != null) {
+            op = tw.addTree(rw.parseCommit(oldList.getOldId()).getTree());
+          }
+
+          int cp = -1;
+          if (curList.getOldId() != null) {
+            cp = tw.addTree(rw.parseCommit(curList.getOldId()).getTree());
+          }
+
+          while (tw.next()) {
+            String path = tw.getPathString();
+            if (tw.getRawMode(o) != 0 && tw.getRawMode(c) != 0
+                && tw.idEqual(o, c)
+                && paths.contains(path)) {
+              // File exists in previously reviewed oldList and in curList.
+              // File content is identical.
+              inserts.add(new AccountPatchReview(
+                  new Patch.Key(
+                      resource.getPatchSet().getId(),
+                      path),
+                    userId));
+              pathList.add(path);
+            } else if (op >= 0 && cp >= 0
+                && tw.getRawMode(o) == 0 && tw.getRawMode(c) == 0
+                && tw.getRawMode(op) != 0 && tw.getRawMode(cp) != 0
+                && tw.idEqual(op, cp)
+                && paths.contains(path)) {
+              // File was deleted in previously reviewed oldList and curList.
+              // File exists in ancestor of oldList and curList.
+              // File content is identical in ancestors.
+              inserts.add(new AccountPatchReview(
+                  new Patch.Key(
+                      resource.getPatchSet().getId(),
+                      path),
+                    userId));
+              pathList.add(path);
             }
           }
           db.get().accountPatchReviews().insert(inserts);

gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java

@@ -31,6 +31,7 @@ import com.google.gerrit.server.config.FactoryModule;
 public class Module extends RestApiModule {
   @Override
   protected void configure() {
+    bind(ChangesCollection.class);
     bind(Revisions.class);
     bind(Reviewers.class);
     bind(Drafts.class);

gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java

@@ -94,6 +94,15 @@ public class ChangeControl {
       return controlFor(change, user);
     }
 
+    public ChangeControl validateFor(Change change, CurrentUser user)
+        throws NoSuchChangeException, OrmException {
+      ChangeControl c = controlFor(change, user);
+      if (!c.isVisible(db.get())) {
+        throw new NoSuchChangeException(c.getChange().getId());
+      }
+      return c;
+    }
+
     public ChangeControl validateFor(Change.Id id, CurrentUser user)
         throws NoSuchChangeException, OrmException {
       ChangeControl c = controlFor(id, user);

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

@@ -24,7 +24,6 @@ import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.change.ChangeJson;
 import com.google.gerrit.server.change.ChangeJson.ChangeInfo;
-import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.query.QueryParseException;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -82,16 +81,12 @@ public class QueryChanges implements RestReadView<TopLevelResource> {
   }
 
   @Inject
-  QueryChanges(ChangeJson json,
-      QueryProcessor qp,
-      ChangeControl.Factory cf,
-      Provider<CurrentUser> user) {
+  QueryChanges(ChangeJson json, QueryProcessor qp, Provider<CurrentUser> user) {
     this.json = json;
     this.imp = qp;
     this.user = user;
 
     options = EnumSet.noneOf(ListChangesOption.class);
-    json.setChangeControlFactory(cf);
   }
 
   public void addQuery(String query) {

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

@@ -98,7 +98,8 @@ public class QueryProcessor {
   private final ChangeQueryRewriter queryRewriter;
   private final Provider<ReviewDb> db;
   private final GitRepositoryManager repoManager;
-  private final ChangeControl.Factory changeControlFactory;
+  private final ChangeControl.GenericFactory changeControlFactory;
+  private final CurrentUser user;
   private final int maxLimit;
 
   private OutputFormat outputFormat = OutputFormat.TEXT;
@@ -124,13 +125,14 @@ public class QueryProcessor {
       ChangeQueryBuilder.Factory queryBuilder, CurrentUser currentUser,
       ChangeQueryRewriter queryRewriter, Provider<ReviewDb> db,
       GitRepositoryManager repoManager,
-      ChangeControl.Factory changeControlFactory) {
+      ChangeControl.GenericFactory changeControlFactory) {
     this.eventFactory = eventFactory;
     this.queryBuilder = queryBuilder.create(currentUser);
     this.queryRewriter = queryRewriter;
     this.db = db;
     this.repoManager = repoManager;
     this.changeControlFactory = changeControlFactory;
+    this.user = currentUser;
     this.maxLimit = currentUser.getCapabilities()
       .getRange(GlobalCapability.QUERY_LIMIT)
       .getMax();
@@ -303,8 +305,11 @@ public class QueryProcessor {
         List<ChangeData> results = queryChanges(queryString);
         ChangeAttribute c = null;
         for (ChangeData d : results) {
-          LabelTypes labelTypes = changeControlFactory.controlFor(d.getChange())
-              .getLabelTypes();
+          ChangeControl cc = d.changeControl();
+          if (cc == null || cc.getCurrentUser() != user) {
+            cc = changeControlFactory.controlFor(d.change(db), user);
+          }
+          LabelTypes labelTypes = cc.getLabelTypes();
           c = eventFactory.asChangeAttribute(d.getChange());
           eventFactory.extend(c, d.getChange());
           eventFactory.addTrackingIds(c, d.trackingIds(db));
@@ -316,7 +321,7 @@ public class QueryProcessor {
           if (includeSubmitRecords) {
             PatchSet.Id psId = d.getChange().currentPatchSetId();
             PatchSet patchSet = db.get().patchSets().get(psId);
-            List<SubmitRecord> submitResult = d.changeControl().canSubmit( //
+            List<SubmitRecord> submitResult = cc.canSubmit( //
                 db.get(), patchSet, null, false, true, true);
             eventFactory.addSubmitRecords(c, submitResult);
           }