opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-3.2'

Browse Source 
* stable-3.2:
  Set version to 3.2.3-SNAPSHOT
  Set version to 3.2.2
  Set version to 3.1.8-SNAPSHOT
  Set version to 3.1.7
  Use Mockito instead of EasyMock for X-Frame-Options header tests
  Set version to 3.0.12-SNAPSHOT
  Set version to 3.0.11
  Set X-Frame-Options header to avoid clickjacking
  PG: Skip unsupported global capabilities
  Revert "Remove documentation of obsolete gerrit.canLoadInIFrame"
  Fix typos in note-db.txt
  Document skipping of reindexing step for offline NoteDB migration
  Report end of NoteDB migration when skipping reindexing
  Clarify that index.batchThreads is relevant for offline reindexing
  Add project to output when reindexing changes in verbose mode
  Auto-flush SiteIndexer's PrintWriters
  Allow to re-index in verbose mode during NoteDB migration
  Avoid closing System.out after All-Users GC in NoteDB migration
  Honor project watches also for changes created via cherry-pick
  Report the index state after re-indexing
  Remove documentation of obsolete gerrit.canLoadInIFrame
  Remove obsolete HostPage.html
  IndexHtmlUtil: Don't log full stack trace when user is not authenticated
  Schema: Show only a single log for inexistent commits
  Schema: Refactor lambda in buildFields to a separate function
  Don't render gr-comment-list when collapsed initially
  ProjectJson: Use merge function for label value rendering
  Simplify Init for Elasticsearch
  Refactor reindexProjects in Init to be general
  Avoid auto-reindex of projects during init when unneeded
  Schema: Show only a single log for inexistent commits
  Schema: Refactor lambda in buildFields to a separate function

Change-Id: Iec0946ea9957a1f5e0898deac9ce2316bc308322
This commit is contained in:
Luca Milanesio
2020-06-18 00:33:13 +01:00
parent 0a4a19398a e98f3fa3ce
commit d29dedbc49
14 changed files with 287 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/com/google/gerrit/extensions/api/changes/CherryPickInput.java
View File

@@ -24,7 +24,7 @@ public class CherryPickInput {
public String base;
public Integer parent;
public NotifyHandling notify = NotifyHandling.NONE;
public NotifyHandling notify = NotifyHandling.ALL;
public Map<RecipientType, NotifyInfo> notifyDetails;
public boolean keepReviewers;

8
java/com/google/gerrit/httpd/AllRequestFilter.java
View File

@@ -18,6 +18,8 @@ import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.plugins.Plugin;
import com.google.gerrit.server.plugins.StopPluginListener;
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.Scopes;
import com.google.inject.Singleton;
import com.google.inject.internal.UniqueAnnotations;
import com.google.inject.servlet.ServletModule;
@@ -32,11 +34,15 @@ import javax.servlet.ServletResponse;
/** Filters all HTTP requests passing through the server. */
public abstract class AllRequestFilter implements Filter {
public static ServletModule module() {
public static Module module() {
return new ServletModule() {
@Override
protected void configureServlets() {
DynamicSet.setOf(binder(), AllRequestFilter.class);
DynamicSet.bind(binder(), AllRequestFilter.class)
.to(AllowRenderInFrameFilter.class)
.in(Scopes.SINGLETON);
filter("/*").through(FilterProxy.class);
bind(StopPluginListener.class)

59
java/com/google/gerrit/httpd/AllowRenderInFrameFilter.java Normal file
View File

@@ -0,0 +1,59 @@
// Copyright (C) 2020 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.httpd;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import java.io.IOException;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jgit.lib.Config;
public class AllowRenderInFrameFilter extends AllRequestFilter {
static final String X_FRAME_OPTIONS_HEADER_NAME = "X-Frame-Options";
public static enum XFrameOption {
ALLOW,
SAMEORIGIN;
}
private final String xframeOptionString;
private final boolean skipXFrameOption;
@Inject
public AllowRenderInFrameFilter(@GerritServerConfig Config cfg) {
XFrameOption xframeOption =
cfg.getEnum("gerrit", null, "xframeOption", XFrameOption.SAMEORIGIN);
boolean canLoadInIFrame = cfg.getBoolean("gerrit", "canLoadInIFrame", false);
xframeOptionString = canLoadInIFrame ? xframeOption.name() : "DENY";
skipXFrameOption = xframeOption.equals(XFrameOption.ALLOW) && canLoadInIFrame;
}
@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
throws IOException, ServletException {
if (skipXFrameOption) {
chain.doFilter(request, response);
} else {
HttpServletResponse httpResponse = (HttpServletResponse) response;
httpResponse.addHeader(X_FRAME_OPTIONS_HEADER_NAME, xframeOptionString);
chain.doFilter(request, httpResponse);
}
}
}

3
java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
View File

@@ -153,8 +153,7 @@ public class IndexHtmlUtil {
serializeObject(GSON, accountApi.getEditPreferences()));
data.put("userIsAuthenticated", true);
} catch (AuthException e) {
logger.atFine().withCause(e).log(
"Can't inline account-related data because user is unauthenticated");
logger.atFine().log("Can't inline account-related data because user is unauthenticated");
// Don't render data
}

61
java/com/google/gerrit/index/Schema.java
View File

@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.exceptions.StorageException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -176,6 +177,33 @@ public class Schema<T> {
return true;
}
private Values<T> fieldValues(T obj, FieldDef<T, ?> f, ImmutableSet<String> skipFields) {
if (skipFields.contains(f.getName())) {
return null;
}
Object v;
try {
v = f.get(obj);
} catch (StorageException e) {
// StorageException is thrown when the object is not found. On this case,
// it is pointless to make further attempts for each field, so propagate
// the exception to return an empty list.
logger.atSevere().withCause(e).log("error getting field %s of %s", f.getName(), obj);
throw e;
} catch (RuntimeException e) {
logger.atSevere().withCause(e).log("error getting field %s of %s", f.getName(), obj);
return null;
}
if (v == null) {
return null;
} else if (f.isRepeatable()) {
return new Values<>(f, (Iterable<?>) v);
} else {
return new Values<>(f, Collections.singleton(v));
}
}
/**
* Build all fields in the schema from an input object.
*
@@ -186,31 +214,14 @@ public class Schema<T> {
* @return all non-null field values from the object.
*/
public final Iterable<Values<T>> buildFields(T obj, ImmutableSet<String> skipFields) {
return fields.values().stream()
.map(
f -> {
if (skipFields.contains(f.getName())) {
return null;
}
Object v;
try {
v = f.get(obj);
} catch (RuntimeException e) {
logger.atSevere().withCause(e).log(
"error getting field %s of %s", f.getName(), obj);
return null;
}
if (v == null) {
return null;
} else if (f.isRepeatable()) {
return new Values<>(f, (Iterable<?>) v);
} else {
return new Values<>(f, Collections.singleton(v));
}
})
.filter(Objects::nonNull)
.collect(toImmutableList());
try {
return fields.values().stream()
.map(f -> fieldValues(obj, f, skipFields))
.filter(Objects::nonNull)
.collect(toImmutableList());
} catch (StorageException e) {
return ImmutableList.of();
}
}
@Override

2
java/com/google/gerrit/index/SiteIndexer.java
View File

@@ -83,7 +83,7 @@ public abstract class SiteIndexer<K, V, I extends Index<K, V>> {
}
protected PrintWriter newPrintWriter(OutputStream out) {
return new PrintWriter(new OutputStreamWriter(out, UTF_8));
return new PrintWriter(new OutputStreamWriter(out, UTF_8), true);
}
private static class ErrorListener implements Runnable {

3
java/com/google/gerrit/pgm/Reindex.java
View File

@@ -202,6 +202,9 @@ public class Reindex extends SiteProgram {
if (result.success()) {
index.markReady(true);
}
System.out.format(
"Index %s in version %d is %sready\n",
def.getName(), index.getSchema().getVersion(), result.success() ? "" : "NOT ");
return result.success();
}
}

3
java/com/google/gerrit/server/index/change/AllChangesIndexer.java
View File

@@ -244,7 +244,8 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
try {
indexer.index(changeDataFactory.create(r.notes()));
done.update(1);
verboseWriter.println("Reindexed change " + r.id());
verboseWriter.format(
"Reindexed change %d (project: %s)\n", r.id().get(), r.notes().getProjectName().get());
} catch (RejectedExecutionException e) {
// Server shutdown, don't spam the logs.
failSilently();

14
java/com/google/gerrit/server/project/ProjectJson.java
View File

@@ -18,6 +18,7 @@ import static java.util.stream.Collectors.toMap;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelValue;
import com.google.gerrit.entities.Project;
@@ -34,6 +35,7 @@ import java.util.HashMap;
/** Collection of routines to populate {@link ProjectInfo}. */
@Singleton
public class ProjectJson {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final AllProjectsName allProjects;
private final WebLinks webLinks;
@@ -50,7 +52,17 @@ public class ProjectJson {
for (LabelType t : projectState.getLabelTypes().getLabelTypes()) {
LabelTypeInfo labelInfo = new LabelTypeInfo();
labelInfo.values =
t.getValues().stream().collect(toMap(LabelValue::formatValue, LabelValue::getText));
t.getValues().stream()
.collect(
toMap(
LabelValue::formatValue,
LabelValue::getText,
(v1, v2) -> {
logger.atSevere().log(
"Duplicate values for project: %s, label: %s found: '%s':'%s'",
projectState.getName(), t.getName(), v1, v2);
return v1;
}));
labelInfo.defaultValue = t.getDefaultValue();
info.labels.put(t.getName(), labelInfo);
}