Merge "FileEdits: Simplify instance creation"
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index bc859f3..6272cda 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -318,7 +318,7 @@
&& !r.getName().startsWith(RefNames.REFS_TAGS)
&& !r.isSymbolic()
&& !r.getName().equals(RefNames.REFS_CONFIG))
- .collect(toCollection(() -> new ArrayList<>(allRefs.size())));
+ .collect(Collectors.toList());
} catch (IOException e) {
throw new PermissionBackendException(e);
}
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index b27cb9b..7f434ca 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -533,6 +533,9 @@
// Multiply the timeout by the number of projects we're actually attempting to
// submit.
.defaultTimeoutMultiplier(cs.projects().size())
+ // By default, we only retry lock failures. Here it's better to also retry unexpected
+ // runtime exceptions.
+ .retryOn(t -> t instanceof RuntimeException)
.call();
submissionExecutor.afterExecutions(orm);
diff --git a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
index 7f8add8..02916c7 100644
--- a/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/TraceIT.java
@@ -43,7 +43,6 @@
import com.google.gerrit.extensions.events.ChangeIndexedListener;
import com.google.gerrit.httpd.restapi.ParameterParser;
import com.google.gerrit.httpd.restapi.RestApiServlet;
-import com.google.gerrit.server.ExceptionHook;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.git.validators.CommitValidationException;
@@ -713,7 +712,7 @@
@Test
@GerritConfig(name = "retry.retryWithTraceOnFailure", value = "true")
- public void autoRetryWithTrace() throws Exception {
+ public void noAutoRetryIfExceptionCausesNormalRetrying() throws Exception {
String changeId = createChange().getChangeId();
approve(changeId);
@@ -723,49 +722,6 @@
RestResponse response = adminRestSession.post("/changes/" + changeId + "/submit");
assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).startsWith("retry-on-failure-");
- assertThat(traceSubmitRule.traceId).startsWith("retry-on-failure-");
- assertThat(traceSubmitRule.isLoggingForced).isTrue();
- }
- }
-
- @Test
- @GerritConfig(name = "retry.retryWithTraceOnFailure", value = "true")
- public void noAutoRetryIfExceptionCausesNormalRetrying() throws Exception {
- String changeId = createChange().getChangeId();
- approve(changeId);
-
- TraceSubmitRule traceSubmitRule = new TraceSubmitRule();
- traceSubmitRule.failAlways = true;
- try (Registration registration =
- extensionRegistry
- .newRegistration()
- .add(traceSubmitRule)
- .add(
- new ExceptionHook() {
- @Override
- public boolean shouldRetry(String actionType, String actionName, Throwable t) {
- return true;
- }
- })) {
- RestResponse response = adminRestSession.post("/changes/" + changeId + "/submit");
- assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
- assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull();
- assertThat(traceSubmitRule.traceId).isNull();
- assertThat(traceSubmitRule.isLoggingForced).isFalse();
- }
- }
-
- @Test
- public void noAutoRetryWithTraceIfDisabled() throws Exception {
- String changeId = createChange().getChangeId();
- approve(changeId);
-
- TraceSubmitRule traceSubmitRule = new TraceSubmitRule();
- traceSubmitRule.failOnce = true;
- try (Registration registration = extensionRegistry.newRegistration().add(traceSubmitRule)) {
- RestResponse response = adminRestSession.post("/changes/" + changeId + "/submit");
- assertThat(response.getStatusCode()).isEqualTo(SC_INTERNAL_SERVER_ERROR);
- assertThat(response.getHeader(RestApiServlet.X_GERRIT_TRACE)).isNull();
assertThat(traceSubmitRule.traceId).isNull();
assertThat(traceSubmitRule.isLoggingForced).isFalse();
}
diff --git a/javatests/com/google/gerrit/entities/converter/ChangeMessageProtoConverterTest.java b/javatests/com/google/gerrit/entities/converter/ChangeMessageProtoConverterTest.java
index 933ffb4..b185558 100644
--- a/javatests/com/google/gerrit/entities/converter/ChangeMessageProtoConverterTest.java
+++ b/javatests/com/google/gerrit/entities/converter/ChangeMessageProtoConverterTest.java
@@ -25,7 +25,6 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.proto.Entities;
import com.google.gerrit.proto.testing.SerializedClassSubject;
-import com.google.protobuf.Parser;
import java.lang.reflect.Type;
import java.sql.Timestamp;
import org.junit.Test;
@@ -174,23 +173,6 @@
assertThat(convertedChangeMessage).isEqualTo(changeMessage);
}
- @Test
- public void protoCanBeParsedFromBytes() throws Exception {
- Entities.ChangeMessage proto =
- Entities.ChangeMessage.newBuilder()
- .setKey(
- Entities.ChangeMessage_Key.newBuilder()
- .setChangeId(Entities.Change_Id.newBuilder().setId(543))
- .setUuid("change-message-21"))
- .build();
- byte[] bytes = proto.toByteArray();
-
- Parser<Entities.ChangeMessage> parser = changeMessageProtoConverter.getParser();
- Entities.ChangeMessage parsedProto = parser.parseFrom(bytes);
-
- assertThat(parsedProto).isEqualTo(proto);
- }
-
/** See {@link SerializedClassSubject} for background and what to do if this test fails. */
@Test
public void fieldsExistAsExpected() {
diff --git a/javatests/com/google/gerrit/entities/converter/PatchSetApprovalProtoConverterTest.java b/javatests/com/google/gerrit/entities/converter/PatchSetApprovalProtoConverterTest.java
index bca5eea..bf39ff8 100644
--- a/javatests/com/google/gerrit/entities/converter/PatchSetApprovalProtoConverterTest.java
+++ b/javatests/com/google/gerrit/entities/converter/PatchSetApprovalProtoConverterTest.java
@@ -27,7 +27,6 @@
import com.google.gerrit.proto.Entities;
import com.google.gerrit.proto.testing.SerializedClassSubject;
import com.google.inject.TypeLiteral;
-import com.google.protobuf.Parser;
import java.lang.reflect.Type;
import java.sql.Timestamp;
import java.util.Date;
@@ -165,29 +164,6 @@
assertThat(patchSetApproval.postSubmit()).isEqualTo(false);
}
- @Test
- public void protoCanBeParsedFromBytes() throws Exception {
- Entities.PatchSetApproval proto =
- Entities.PatchSetApproval.newBuilder()
- .setKey(
- Entities.PatchSetApproval_Key.newBuilder()
- .setPatchSetId(
- Entities.PatchSet_Id.newBuilder()
- .setChangeId(Entities.Change_Id.newBuilder().setId(42))
- .setId(14))
- .setAccountId(Entities.Account_Id.newBuilder().setId(100013))
- .setLabelId(Entities.LabelId.newBuilder().setId("label-8")))
- .setValue(456)
- .setGranted(987654L)
- .build();
- byte[] bytes = proto.toByteArray();
-
- Parser<Entities.PatchSetApproval> parser = protoConverter.getParser();
- Entities.PatchSetApproval parsedProto = parser.parseFrom(bytes);
-
- assertThat(parsedProto).isEqualTo(proto);
- }
-
/** See {@link SerializedClassSubject} for background and what to do if this test fails. */
@Test
public void fieldsExistAsExpected() {
diff --git a/javatests/com/google/gerrit/entities/converter/PatchSetProtoConverterTest.java b/javatests/com/google/gerrit/entities/converter/PatchSetProtoConverterTest.java
index 2519e75..efeb24f 100644
--- a/javatests/com/google/gerrit/entities/converter/PatchSetProtoConverterTest.java
+++ b/javatests/com/google/gerrit/entities/converter/PatchSetProtoConverterTest.java
@@ -26,7 +26,6 @@
import com.google.gerrit.proto.Entities;
import com.google.gerrit.proto.testing.SerializedClassSubject;
import com.google.inject.TypeLiteral;
-import com.google.protobuf.Parser;
import java.lang.reflect.Type;
import java.sql.Timestamp;
import java.util.Optional;
@@ -148,23 +147,6 @@
.build());
}
- @Test
- public void protoCanBeParsedFromBytes() throws Exception {
- Entities.PatchSet proto =
- Entities.PatchSet.newBuilder()
- .setId(
- Entities.PatchSet_Id.newBuilder()
- .setChangeId(Entities.Change_Id.newBuilder().setId(103))
- .setId(73))
- .build();
- byte[] bytes = proto.toByteArray();
-
- Parser<Entities.PatchSet> parser = patchSetProtoConverter.getParser();
- Entities.PatchSet parsedProto = parser.parseFrom(bytes);
-
- assertThat(parsedProto).isEqualTo(proto);
- }
-
/** See {@link SerializedClassSubject} for background and what to do if this test fails. */
@Test
public void fieldsExistAsExpected() {
diff --git a/plugins/singleusergroup b/plugins/singleusergroup
index a0c53c6..3548ec8 160000
--- a/plugins/singleusergroup
+++ b/plugins/singleusergroup
@@ -1 +1 @@
-Subproject commit a0c53c6c5ad1ba8f8967ed6d2bcb18995f734cad
+Subproject commit 3548ec83c0c271a8768a6b03b0c28711521ed6cf
diff --git a/polygerrit-ui/app/api/core.ts b/polygerrit-ui/app/api/core.ts
index 91c2b7b..5820139 100644
--- a/polygerrit-ui/app/api/core.ts
+++ b/polygerrit-ui/app/api/core.ts
@@ -32,7 +32,7 @@
* however a range with end_line set to 5 and end_character equal to 0 will not
* include any characters on line 5.
*/
-export interface CommentRange {
+export declare interface CommentRange {
/** The start line number of the range. (1-based) */
start_line: number;
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index 496653f..b5624c6 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -219,7 +219,7 @@
end_line: number;
}
-export interface CoverageRange {
+export declare interface CoverageRange {
type: CoverageType;
side: Side;
code_range: LineRange;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 59ce443..e361f9f 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -164,6 +164,7 @@
import {takeUntil} from 'rxjs/operators';
import {aPluginHasRegistered} from '../../../services/checks/checks-model';
import {Subject} from 'rxjs';
+import {GrRelatedChangesListExperimental} from '../gr-related-changes-list-experimental/gr-related-changes-list-experimental';
const CHANGE_ID_ERROR = {
MISMATCH: 'mismatch',
@@ -2296,6 +2297,9 @@
this._editingCommitMessage = false;
const relatedChangesLoaded = coreDataPromise.then(() => {
this.getRelatedChangesList()?.reload();
+ if (this._isNewChangeSummaryUiEnabled) {
+ this.getRelatedChangesListExperimental()?.reload();
+ }
});
allDataPromises.push(relatedChangesLoaded);
}
@@ -2819,6 +2823,12 @@
'#relatedChanges'
);
}
+
+ getRelatedChangesListExperimental() {
+ return this.shadowRoot!.querySelector<GrRelatedChangesListExperimental>(
+ '#relatedChangesExperimental'
+ );
+ }
}
declare global {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index 06a1939..ed33916 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -594,11 +594,14 @@
</gr-endpoint-param>
</gr-endpoint-decorator>
</div>
- <template is="dom-if" if="[[_isNewChangeSummaryUiEnabled]]">
- <gr-related-changes-list-experimental></gr-related-changes-list-experimental>
- </template>
- <template is="dom-if" if="[[!_isNewChangeSummaryUiEnabled]]">
- <div class="relatedChanges">
+ <div class="relatedChanges">
+ <template is="dom-if" if="[[_isNewChangeSummaryUiEnabled]]">
+ <gr-related-changes-list-experimental
+ change="[[_change]]"
+ id="relatedChangesExperimental"
+ ></gr-related-changes-list-experimental>
+ </template>
+ <template is="dom-if" if="[[!_isNewChangeSummaryUiEnabled]]">
<gr-related-changes-list
id="relatedChanges"
class$="[[_computeRelatedChangesClass(_relatedChangesCollapsed)]]"
@@ -620,8 +623,8 @@
[[_computeCollapseText(_relatedChangesCollapsed)]]
</gr-button>
</div>
- </div>
- </template>
+ </template>
+ </div>
</div>
</div>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts
index ab7566f..a66bb04 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list-experimental/gr-related-changes-list-experimental.ts
@@ -16,22 +16,217 @@
*/
import {html} from 'lit-html';
import {GrLitElement} from '../../lit/gr-lit-element';
-import {customElement} from 'lit-element';
+import {customElement, property, css} from 'lit-element';
import {sharedStyles} from '../../../styles/shared-styles';
+import {
+ SubmittedTogetherInfo,
+ ChangeInfo,
+ RelatedChangeAndCommitInfo,
+} from '../../../types/common';
+import {appContext} from '../../../services/app-context';
+import {ParsedChangeInfo} from '../../../types/types';
+import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {pluralize} from '../../../utils/string-util';
+import {ChangeStatus} from '../../../constants/constants';
+function isChangeInfo(
+ x: ChangeInfo | RelatedChangeAndCommitInfo | ParsedChangeInfo
+): x is ChangeInfo | ParsedChangeInfo {
+ return (x as ChangeInfo)._number !== undefined;
+}
@customElement('gr-related-changes-list-experimental')
export class GrRelatedChangesListExperimental extends GrLitElement {
+ @property()
+ change?: ParsedChangeInfo;
+
+ @property()
+ _submittedTogether?: SubmittedTogetherInfo = {
+ changes: [],
+ non_visible_changes: 0,
+ };
+
+ private readonly restApiService = appContext.restApiService;
+
static get styles() {
- return [sharedStyles];
+ return [
+ sharedStyles,
+ css`
+ .title {
+ font-weight: var(--font-weight-bold);
+ color: var(--deemphasized-text-color);
+ padding-left: var(--metadata-horizontal-padding);
+ }
+ h4,
+ section gr-related-change {
+ display: flex;
+ }
+ h4:before,
+ section gr-related-change:before {
+ content: ' ';
+ flex-shrink: 0;
+ width: 1.2em;
+ }
+ .note {
+ color: var(--error-text-color);
+ }
+ `,
+ ];
}
render() {
- return html``;
+ const submittedTogetherChanges = this._submittedTogether?.changes ?? [];
+ const countNonVisibleChanges =
+ this._submittedTogether?.non_visible_changes ?? 0;
+ return html` <section
+ id="submittedTogether"
+ ?hidden=${!submittedTogetherChanges?.length &&
+ !this._submittedTogether?.non_visible_changes}
+ >
+ <h4 class="title">Submitted together</h4>
+ ${submittedTogetherChanges.map(
+ relatedChange =>
+ html`<gr-related-change
+ .currentChange="${this._changesEqual(relatedChange, this.change)}"
+ .change="${relatedChange}"
+ ></gr-related-change>`
+ )}
+ <div class="note" ?hidden=${!countNonVisibleChanges}>
+ (+ ${pluralize(countNonVisibleChanges, 'non-visible change')})
+ </div>
+ </section>`;
+ }
+
+ reload() {
+ if (!this.change) return Promise.reject(new Error('change missing'));
+ return this.restApiService
+ .getChangesSubmittedTogether(this.change._number)
+ .then(response => {
+ this._submittedTogether = response;
+ });
+ }
+
+ /**
+ * Do the given objects describe the same change? Compares the changes by
+ * their numbers.
+ */
+ _changesEqual(
+ a?: ChangeInfo | RelatedChangeAndCommitInfo,
+ b?: ChangeInfo | ParsedChangeInfo | RelatedChangeAndCommitInfo
+ ) {
+ const aNum = this._getChangeNumber(a);
+ const bNum = this._getChangeNumber(b);
+ return aNum === bNum;
+ }
+
+ /**
+ * Get the change number from either a ChangeInfo (such as those included in
+ * SubmittedTogetherInfo responses) or get the change number from a
+ * RelatedChangeAndCommitInfo (such as those included in a
+ * RelatedChangesInfo response).
+ */
+ _getChangeNumber(
+ change?: ChangeInfo | ParsedChangeInfo | RelatedChangeAndCommitInfo
+ ) {
+ // Default to 0 if change property is not defined.
+ if (!change) return 0;
+
+ if (isChangeInfo(change)) {
+ return change._number;
+ }
+ return change._change_number;
+ }
+}
+
+@customElement('gr-related-change')
+export class GrRelatedChange extends GrLitElement {
+ @property()
+ change?: ChangeInfo;
+
+ @property()
+ currentChange = false;
+
+ static get styles() {
+ return [
+ sharedStyles,
+ css`
+ a {
+ display: block;
+ }
+ .changeContainer,
+ a {
+ max-width: 100%;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ white-space: nowrap;
+ }
+ .changeContainer {
+ display: flex;
+ }
+ .strikethrough {
+ color: var(--deemphasized-text-color);
+ text-decoration: line-through;
+ }
+ .submittableCheck {
+ padding-left: var(--spacing-s);
+ color: var(--positive-green-text-color);
+ display: none;
+ }
+ .submittableCheck.submittable {
+ display: inline;
+ }
+ .arrowToCurrentChange {
+ position: absolute;
+ }
+ `,
+ ];
+ }
+
+ render() {
+ const change = this.change;
+ if (!change) throw new Error('Missing change');
+ const linkClass = this._computeLinkClass(change);
+ return html`<span
+ role="img"
+ class="arrowToCurrentChange"
+ aria-label="Arrow marking current change"
+ ?hidden=${!this.currentChange}
+ >➔</span
+ >
+ <div class="changeContainer">
+ <a
+ href="${GerritNav.getUrlForChangeById(
+ change._number,
+ change.project
+ )}"
+ class="${linkClass}"
+ >${change.project}: ${change.branch}: ${change.subject}</a
+ >
+ <span
+ tabindex="-1"
+ title="Submittable"
+ class="submittableCheck ${linkClass}"
+ role="img"
+ aria-label="Submittable"
+ >✓</span
+ >
+ </div> `;
+ }
+
+ _computeLinkClass(change: ChangeInfo) {
+ const statuses = [];
+ if (change.status === ChangeStatus.ABANDONED) {
+ statuses.push('strikethrough');
+ }
+ if (change.submittable) {
+ statuses.push('submittable');
+ }
+ return statuses.join(' ');
}
}
declare global {
interface HTMLElementTagNameMap {
'gr-related-changes-list-experimental': GrRelatedChangesListExperimental;
+ 'gr-related-change': GrRelatedChange;
}
}
