Merge "Merge branch 'stable-2.16'"

java/com/google/gerrit/extensions/api/changes/FileApi.java

@@ -39,6 +39,9 @@ public interface FileApi {
    */
   DiffRequest diffRequest() throws RestApiException;
 
+  /** Set the file reviewed or not reviewed */
+  void setReviewed(boolean reviewed) throws RestApiException;
+
   abstract class DiffRequest {
     private String base;
     private Integer context;
@@ -123,5 +126,10 @@ public interface FileApi {
     public DiffRequest diffRequest() throws RestApiException {
       throw new NotImplementedException();
     }
+
+    @Override
+    public void setReviewed(boolean reviewed) throws RestApiException {
+      throw new NotImplementedException();
+    }
   }
 }

java/com/google/gerrit/server/api/changes/FileApiImpl.java

@@ -18,11 +18,13 @@ import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
 
 import com.google.gerrit.extensions.api.changes.FileApi;
 import com.google.gerrit.extensions.common.DiffInfo;
+import com.google.gerrit.extensions.common.Input;
 import com.google.gerrit.extensions.restapi.BinaryResult;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.server.change.FileResource;
 import com.google.gerrit.server.restapi.change.GetContent;
 import com.google.gerrit.server.restapi.change.GetDiff;
+import com.google.gerrit.server.restapi.change.Reviewed;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 
@@ -33,12 +35,21 @@ class FileApiImpl implements FileApi {
 
   private final GetContent getContent;
   private final GetDiff getDiff;
+  private final Reviewed.PutReviewed putReviewed;
+  private final Reviewed.DeleteReviewed deleteReviewed;
   private final FileResource file;
 
   @Inject
-  FileApiImpl(GetContent getContent, GetDiff getDiff, @Assisted FileResource file) {
+  FileApiImpl(
+      GetContent getContent,
+      GetDiff getDiff,
+      Reviewed.PutReviewed putReviewed,
+      Reviewed.DeleteReviewed deleteReviewed,
+      @Assisted FileResource file) {
     this.getContent = getContent;
     this.getDiff = getDiff;
+    this.putReviewed = putReviewed;
+    this.deleteReviewed = deleteReviewed;
     this.file = file;
   }
 
@@ -88,6 +99,19 @@ class FileApiImpl implements FileApi {
     };
   }
 
+  @Override
+  public void setReviewed(boolean reviewed) throws RestApiException {
+    try {
+      if (reviewed) {
+        putReviewed.apply(file, new Input());
+      } else {
+        deleteReviewed.apply(file, new Input());
+      }
+    } catch (Exception e) {
+      throw asRestApiException(String.format("Cannot set %sreviewed", reviewed ? "" : "un"), e);
+    }
+  }
+
   private DiffInfo get(DiffRequest r) throws RestApiException {
     if (r.getBase() != null) {
       getDiff.setBase(r.getBase());

java/com/google/gerrit/server/project/ProjectCacheImpl.java

@@ -16,6 +16,7 @@ package com.google.gerrit.server.project;
 
 import static java.util.stream.Collectors.toSet;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Throwables;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
@@ -329,4 +330,14 @@ public class ProjectCacheImpl implements ProjectCache {
       }
     }
   }
+
+  @VisibleForTesting
+  public void evictAllByName() {
+    byName.invalidateAll();
+  }
+
+  @VisibleForTesting
+  public long sizeAllByName() {
+    return byName.size();
+  }
 }

java/com/google/gerrit/server/restapi/project/ListProjects.java

@@ -16,8 +16,6 @@ package com.google.gerrit.server.restapi.project;
 
 import static com.google.gerrit.extensions.client.ProjectState.HIDDEN;
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.Objects.requireNonNull;
-import static java.util.stream.Collectors.toList;
 
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
@@ -62,18 +60,19 @@ import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.SortedMap;
 import java.util.SortedSet;
 import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Ref;
@@ -335,9 +334,10 @@ public class ListProjects implements RestReadView<TopLevelResource> {
     PermissionBackend.WithUser perm = permissionBackend.user(currentUser);
     final TreeMap<Project.NameKey, ProjectNode> treeMap = new TreeMap<>();
     try {
-      for (Project.NameKey projectName : filter(perm)) {
-        final ProjectState e = projectCache.get(projectName);
-        if (e == null || (e.getProject().getState() == HIDDEN && !all && state != HIDDEN)) {
+      Iterable<ProjectState> projectStatesIt = filter(perm)::iterator;
+      for (ProjectState e : projectStatesIt) {
+        Project.NameKey projectName = e.getNameKey();
+        if (e.getProject().getState() == HIDDEN && !all && state != HIDDEN) {
           // If we can't get it from the cache, pretend it's not present.
           // If all wasn't selected, and it's HIDDEN, pretend it's not present.
           // If state HIDDEN wasn't selected, and it's HIDDEN, pretend it's not present.
@@ -487,31 +487,21 @@ public class ListProjects implements RestReadView<TopLevelResource> {
     }
   }
 
-  private Collection<Project.NameKey> filter(PermissionBackend.WithUser perm)
-      throws BadRequestException, PermissionBackendException {
-    Stream<Project.NameKey> matches = scan();
-
-    List<Project.NameKey> results = new ArrayList<>();
-    List<Project.NameKey> projectNameKeys = matches.sorted().collect(toList());
-    for (Project.NameKey nameKey : projectNameKeys) {
-      ProjectState state = projectCache.get(nameKey);
-      requireNonNull(state, () -> String.format("Failed to load project %s", nameKey));
+  private Stream<ProjectState> filter(PermissionBackend.WithUser perm) throws BadRequestException {
+    return StreamSupport.stream(scan().spliterator(), false)
+        .map(projectCache::get)
+        .filter(Objects::nonNull)
+        .filter(p -> permissionCheck(p, perm));
+  }
 
+  private boolean permissionCheck(ProjectState state, PermissionBackend.WithUser perm) {
     // Hidden projects(permitsRead = false) should only be accessible by the project owners.
     // READ_CONFIG is checked here because it's only allowed to project owners(ACCESS may also
     // be allowed for other users). Allowing project owners to access here will help them to view
     // and update the config of hidden projects easily.
-      ProjectPermission permissionToCheck =
-          state.statePermitsRead() ? ProjectPermission.ACCESS : ProjectPermission.READ_CONFIG;
-      try {
-        perm.project(nameKey).check(permissionToCheck);
-        results.add(nameKey);
-      } catch (AuthException e) {
-        // Not added to results.
-      }
-    }
-
-    return results;
+    return perm.project(state.getNameKey())
+        .testOrFalse(
+            state.statePermitsRead() ? ProjectPermission.ACCESS : ProjectPermission.READ_CONFIG);
   }
 
   private boolean isParentAccessible(

javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java

@@ -953,6 +953,21 @@ public class RevisionIT extends AbstractDaemonTest {
     assertThat(gApi.changes().id(r.getChangeId()).current().reviewed()).isEmpty();
   }
 
+  @Test
+  public void setUnsetReviewedFlagByFileApi() throws Exception {
+    PushOneCommit push = pushFactory.create(admin.getIdent(), testRepo);
+    PushOneCommit.Result r = push.to("refs/for/master");
+
+    gApi.changes().id(r.getChangeId()).current().file(PushOneCommit.FILE_NAME).setReviewed(true);
+
+    assertThat(Iterables.getOnlyElement(gApi.changes().id(r.getChangeId()).current().reviewed()))
+        .isEqualTo(PushOneCommit.FILE_NAME);
+
+    gApi.changes().id(r.getChangeId()).current().file(PushOneCommit.FILE_NAME).setReviewed(false);
+
+    assertThat(gApi.changes().id(r.getChangeId()).current().reviewed()).isEmpty();
+  }
+
   @Test
   public void mergeable() throws Exception {
     ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();

javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java

@@ -34,6 +34,7 @@ import com.google.gerrit.extensions.client.ProjectState;
 import com.google.gerrit.extensions.common.ProjectInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.project.ProjectCacheImpl;
 import com.google.gerrit.server.project.testing.Util;
 import com.google.inject.Inject;
 import java.util.List;
@@ -92,15 +93,19 @@ public class ListProjectsIT extends AbstractDaemonTest {
 
   @Test
   public void listProjectsWithLimit() throws Exception {
+    ProjectCacheImpl projectCacheImpl = (ProjectCacheImpl) projectCache;
     String pre = "lpwl-someProject";
     int n = 6;
     for (int i = 0; i < n; i++) {
       projectOperations.newProject().name(pre + i).create();
     }
 
+    projectCacheImpl.evictAllByName();
     for (int i = 1; i <= n + 2; i++) {
       assertThatNameList(gApi.projects().list().withPrefix(pre).withLimit(i).get())
           .hasSize(Math.min(i, n));
+      assertThat(projectCacheImpl.sizeAllByName())
+          .isAtMost((long) (i + 2)); // 2 = AllProjects + AllUsers
     }
   }
 

polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html

@@ -99,57 +99,6 @@ limitations under the License.
           element.serverConfig), 'https://link-url');
     });
 
-    test('use gitweb when available', () => {
-      const router = document.createElement('gr-router');
-      element.serverConfig = {gitweb: {
-        url: 'url-base/',
-        type: {revision: 'xx ${project} xx ${commit} xx'},
-      }};
-      sandbox.stub(Gerrit.Nav, '_generateWeblinks',
-          router._generateWeblinks.bind(router));
-
-      element.commitInfo = {commit: 'commit-sha'};
-      element.change = {
-        project: 'project-name',
-        labels: [],
-        current_revision: element.commitInfo.commit,
-      };
-
-      assert.isOk(element._computeShowWebLink(element.change,
-          element.commitInfo, element.serverConfig));
-
-      assert.equal(element._computeWebLink(element.change, element.commitInfo,
-          element.serverConfig), 'url-base/xx project-name xx commit-sha xx');
-    });
-
-    test('prefer gitweb when both are available', () => {
-      const router = document.createElement('gr-router');
-      element.serverConfig = {gitweb: {
-        url: 'url-base/',
-        type: {revision: 'xx ${project} xx ${commit} xx'},
-      }};
-      sandbox.stub(Gerrit.Nav, '_generateWeblinks',
-          router._generateWeblinks.bind(router));
-
-      element.commitInfo = {
-        commit: 'commit-sha',
-        web_links: [{url: 'link-url'}],
-      };
-      element.change = {
-        project: 'project-name',
-        labels: [],
-        current_revision: element.commitInfo.commit,
-      };
-
-      assert.isOk(element._computeShowWebLink(element.change,
-          element.commitInfo, element.serverConfig));
-
-      const link = element._computeWebLink(element.change, element.commitInfo,
-          element.serverConfig);
-
-      assert.equal(link, 'url-base/xx project-name xx commit-sha xx');
-      assert.notEqual(link, '../../link-url');
-    });
 
     test('ignore web links that are neither gitweb nor gitiles', () => {
       const router = document.createElement('gr-router');

polygerrit-ui/app/elements/core/gr-router/gr-router.js

@@ -280,16 +280,9 @@
     },
 
     _getPatchSetWeblink(params) {
-      const {repo, commit, options} = params;
-      const {weblinks, config} = options || {};
+      const {commit, options} = params;
+      const {weblinks} = options || {};
       const name = commit && commit.slice(0, 7);
-      const gitwebConfigUrl = this._configBasedCommitUrl(repo, commit, config);
-      if (gitwebConfigUrl) {
-        return {
-          name,
-          url: gitwebConfigUrl,
-        };
-      }
       const url = this._getSupportedWeblinkUrl(weblinks);
       if (!url) {
         return {name};
@@ -298,15 +291,6 @@
       }
     },
 
-    _configBasedCommitUrl(repo, commit, config) {
-      if (config && config.gitweb && config.gitweb.url &&
-          config.gitweb.type && config.gitweb.type.revision) {
-        return config.gitweb.url + config.gitweb.type.revision
-            .replace('${project}', repo)
-            .replace('${commit}', commit);
-      }
-    },
-
     _isDirectCommit(link) {
       // This is a whitelist of web link types that provide direct links to
       // the commit in the url property.