Merge "Plugin endpoint parameters"
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/InternalGroupBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/InternalGroupBackend.java
index 9f5bb48..3f4fee9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/InternalGroupBackend.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/InternalGroupBackend.java
@@ -26,8 +26,8 @@
import com.google.gerrit.server.group.InternalGroupDescription;
import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.util.Collection;
import org.eclipse.jgit.lib.ObjectId;
@@ -38,7 +38,7 @@
private final GroupControl.Factory groupControlFactory;
private final GroupCache groupCache;
private final Groups groups;
- private final Provider<ReviewDb> db;
+ private final SchemaFactory<ReviewDb> schema;
private final IncludingGroupMembership.Factory groupMembershipFactory;
@Inject
@@ -46,12 +46,12 @@
GroupControl.Factory groupControlFactory,
GroupCache groupCache,
Groups groups,
- Provider<ReviewDb> db,
+ SchemaFactory<ReviewDb> schema,
IncludingGroupMembership.Factory groupMembershipFactory) {
this.groupControlFactory = groupControlFactory;
this.groupCache = groupCache;
this.groups = groups;
- this.db = db;
+ this.schema = schema;
this.groupMembershipFactory = groupMembershipFactory;
}
@@ -72,9 +72,9 @@
@Override
public Collection<GroupReference> suggest(String name, ProjectState project) {
- try {
+ try (ReviewDb db = schema.open()) {
return groups
- .getAll(db.get())
+ .getAll(db)
.filter(group -> startsWithIgnoreCase(group, name))
.filter(this::isVisible)
.map(GroupReference::forGroup)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/SystemGroupBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/SystemGroupBackend.java
index cea0d25..5d60790 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/group/SystemGroupBackend.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/SystemGroupBackend.java
@@ -213,10 +213,10 @@
}
Optional<AccountGroup> conflictingGroup;
- try {
+ try (ReviewDb db = schema.open()) {
conflictingGroup =
groups
- .getAll(schema.open())
+ .getAll(db)
.filter(group -> hasConfiguredName(byLowerCaseConfiguredName, group))
.findAny();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 144ec1d..c103c89 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -88,7 +88,7 @@
@Deprecated static final Schema<ChangeData> V45 = schema(V44, ChangeField.REVERT_OF);
- static final Schema<ChangeData> V46 = schema(V45);
+ @Deprecated static final Schema<ChangeData> V46 = schema(V45);
// Removal of draft change workflow requires reindexing
static final Schema<ChangeData> V47 = schema(V46);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
index ad74108..c9a28f4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -167,22 +167,18 @@
checkArgument(project != null, "project is required");
Change change = readOneReviewDbChange(db, changeId);
- if (change == null && args.migration.readChanges()) {
- // Change isn't in ReviewDb, but its primary storage might be in NoteDb.
- // Prepopulate the change exists with proper noteDbState field.
- change = newNoteDbOnlyChange(project, changeId);
- } else {
- checkNotNull(change, "change %s not found in ReviewDb", changeId);
- checkArgument(
- change.getProject().equals(project),
- "passed project %s when creating ChangeNotes for %s, but actual project is %s",
- project,
- changeId,
- change.getProject());
+ if (change == null) {
+ if (args.migration.readChanges()) {
+ return newNoteDbOnlyChange(project, changeId);
+ }
+ throw new NoSuchChangeException(changeId);
}
-
- // TODO: Throw NoSuchChangeException when the change is not found in the
- // database
+ checkArgument(
+ change.getProject().equals(project),
+ "passed project %s when creating ChangeNotes for %s, but actual project is %s",
+ project,
+ changeId,
+ change.getProject());
return change;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
index 19190510..e92b003 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
@@ -35,7 +35,7 @@
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
- public static final Class<Schema_159> C = Schema_159.class;
+ public static final Class<Schema_160> C = Schema_160.class;
public static int getBinaryVersion() {
return guessVersion(C);
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 3464fea..10f4b27 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -37,6 +37,7 @@
display: block;
}
.row {
+ align-items: center;
border-top: 1px solid #eee;
display: flex;
padding: .1em .25em;
@@ -44,6 +45,9 @@
:host(.loading) .row {
opacity: .5;
};
+ :host(.editLoaded) .hideOnEdit {
+ display: none;
+ }
.reviewed,
.status {
align-items: center;
@@ -107,14 +111,13 @@
font-family: var(--font-family-bold);
}
.show-hide {
- margin-left: .4em;
+ width: 1em;
}
.fileListButton {
margin: .5em;
}
.totalChanges {
justify-content: flex-end;
- padding-right: 2.6em;
text-align: right;
}
.warning {
@@ -129,7 +132,6 @@
display: block;
font-size: .8em;
min-width: 2em;
- margin-top: .1em;
}
gr-diff {
box-shadow: 0 1px 3px rgba(0, 0, 0, .3);
@@ -147,12 +149,39 @@
.mobile {
display: none;
}
- #container.editLoaded .hideOnEdit {
- display: none;
- }
.expandInline {
padding-right: .25em;
}
+ .reviewed {
+ margin-left: 2em;
+ width: 15em;
+ }
+ .reviewed label {
+ color: #2A66D9;
+ opacity: 0;
+ justify-content: flex-end;
+ width: 100%;
+ }
+ .reviewed label:hover {
+ cursor: pointer;
+ opacity: 100;
+ }
+ .row:hover .reviewed label,
+ .row:focus .reviewed label {
+ opacity: 100;
+ }
+ .reviewed input {
+ display: none;
+ }
+ .reviewedLabel {
+ color: rgba(0, 0, 0, .54);
+ margin-right: 1em;
+ opacity: 0;
+ }
+ .reviewedLabel.isReviewed {
+ display: initial;
+ opacity: 100;
+ }
@media screen and (max-width: 50em) {
.desktop {
display: none;
@@ -185,7 +214,6 @@
</style>
<div
id="container"
- class$="[[_computeContainerClass(editLoaded)]]"
on-tap="_handleFileListTap">
<template is="dom-repeat"
items="[[_shownFiles]]"
@@ -194,13 +222,14 @@
initial-count="[[fileListIncrement]]"
target-framerate="1">
<div class="file-row row" data-path$="[[file.__path]]" tabindex="-1">
- <div class="reviewed hideOnEdit" hidden$="[[!_loggedIn]]" hidden>
- <input
- type="checkbox"
- checked="[[file.isReviewed]]"
- class="reviewed"
- aria-label="Reviewed checkbox"
- title="Mark as reviewed (shortcut: r)">
+ <div class="show-hide" hidden$="[[_userPrefs.expand_inline_diffs]]">
+ <label class="show-hide" data-path$="[[file.__path]]"
+ data-expand=true>
+ <input type="checkbox" class="show-hide"
+ checked$="[[_isFileExpanded(file.__path, _expandedFilePaths.*)]]"
+ data-path$="[[file.__path]]" data-expand=true>
+ [[_computeShowHideText(file.__path, _expandedFilePaths.*)]]
+ </label>
</div>
<div class$="[[_computeClass('status', file.__path)]]"
tabindex="0"
@@ -261,13 +290,11 @@
[[_formatPercentage(file.size, file.size_delta)]]
</span>
</div>
- <div class="show-hide" hidden$="[[_userPrefs.expand_inline_diffs]]">
- <label class="show-hide" data-path$="[[file.__path]]"
- data-expand=true>
- <input type="checkbox" class="show-hide"
- checked$="[[_isFileExpanded(file.__path, _expandedFilePaths.*)]]"
- data-path$="[[file.__path]]" data-expand=true>
- [[_computeShowHideText(file.__path, _expandedFilePaths.*)]]
+ <div class="reviewed hideOnEdit" hidden$="[[!_loggedIn]]" hidden>
+ <span class$="reviewedLabel [[_computeReviewedClass(file.isReviewed)]]">Reviewed</span>
+ <label>
+ <input class="reviewed" type="checkbox" checked="[[file.isReviewed]]">
+ <span class="markReviewed" title="Mark as reviewed (shortcut: r)">[[_computeReviewedText(file.isReviewed)]]</span>
</label>
</div>
</div>
@@ -307,6 +334,8 @@
-[[_patchChange.deleted]]
</span>
</div>
+ <!-- Empty div here exists to keep spacing in sync with file rows. -->
+ <div class="reviewed hideOnEdit" hidden$="[[!_loggedIn]]" hidden></div>
</div>
<div
class$="row totalChanges [[_computeExpandInlineClass(_userPrefs)]]"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 100bdee..7807f5e 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -59,7 +59,10 @@
notify: true,
observer: '_updateDiffPreferences',
},
- editLoaded: Boolean,
+ editLoaded: {
+ type: Boolean,
+ observer: '_editLoadedChanged',
+ },
_files: {
type: Array,
observer: '_filesChanged',
@@ -424,9 +427,8 @@
row = row.parentElement;
}
const path = row.dataset.path;
-
// Handle checkbox mark as reviewed.
- if (e.target.classList.contains('reviewed')) {
+ if (e.target.classList.contains('markReviewed')) {
return this._reviewFile(path);
}
@@ -720,7 +722,7 @@
},
_computeShowHideText(path, expandedFilesRecord) {
- return this._isFileExpanded(path, expandedFilesRecord) ? '▼' : '◀';
+ return this._isFileExpanded(path, expandedFilesRecord) ? '▼' : '▶';
},
_computeFilesShown(numFilesShown, files) {
@@ -910,8 +912,16 @@
}, LOADING_DEBOUNCE_INTERVAL);
},
- _computeContainerClass(editLoaded) {
- return editLoaded ? 'editLoaded' : '';
+ _editLoadedChanged(editLoaded) {
+ this.classList.toggle('editLoaded', editLoaded);
+ },
+
+ _computeReviewedClass(isReviewed) {
+ return isReviewed ? 'isReviewed' : '';
+ },
+
+ _computeReviewedText(isReviewed) {
+ return isReviewed ? 'MARK UNREVIEWED' : 'MARK REVIEWED';
},
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 4b407c4..e77ad2c 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -609,21 +609,29 @@
flushAsynchronousOperations();
const fileRows =
Polymer.dom(element.root).querySelectorAll('.row:not(.header)');
- const commitMsg = fileRows[0].querySelector(
- 'input.reviewed[type="checkbox"]');
- const fileAdded = fileRows[1].querySelector(
- 'input.reviewed[type="checkbox"]');
- const myFile = fileRows[2].querySelector(
- 'input.reviewed[type="checkbox"]');
+ const checkSelector = 'input.reviewed[type="checkbox"]';
+ const commitMsg = fileRows[0].querySelector(checkSelector);
+ const fileAdded = fileRows[1].querySelector(checkSelector);
+ const myFile = fileRows[2].querySelector(checkSelector);
assert.isTrue(commitMsg.checked);
assert.isFalse(fileAdded.checked);
assert.isTrue(myFile.checked);
- MockInteractions.tap(commitMsg);
+ const commitReviewLabel = fileRows[0].querySelector('.reviewedLabel');
+ const markReviewLabel = commitMsg.nextElementSibling;
+ assert.isTrue(commitReviewLabel.classList.contains('isReviewed'));
+ assert.equal(markReviewLabel.textContent, 'MARK UNREVIEWED');
+
+ MockInteractions.tap(markReviewLabel);
assert.isTrue(saveStub.lastCall.calledWithExactly('/COMMIT_MSG', false));
- MockInteractions.tap(commitMsg);
+ assert.isFalse(commitReviewLabel.classList.contains('isReviewed'));
+ assert.equal(markReviewLabel.textContent, 'MARK REVIEWED');
+
+ MockInteractions.tap(markReviewLabel);
assert.isTrue(saveStub.lastCall.calledWithExactly('/COMMIT_MSG', true));
+ assert.isTrue(commitReviewLabel.classList.contains('isReviewed'));
+ assert.equal(markReviewLabel.textContent, 'MARK UNREVIEWED');
});
test('patch set from revisions', () => {
@@ -1207,12 +1215,15 @@
test('reviewed checkbox', () => {
const alertStub = sandbox.stub();
+ const saveReviewStub = sandbox.stub(element, '_saveReviewedState');
+
element.addEventListener('show-alert', alertStub);
element.editLoaded = false;
// Reviewed checkbox should be shown.
assert.isTrue(isVisible(element.$$('.reviewed')));
MockInteractions.pressAndReleaseKeyOn(element, 82, null, 'r');
assert.isFalse(alertStub.called);
+ assert.isTrue(saveReviewStub.calledOnce);
element.editLoaded = true;
flushAsynchronousOperations();
@@ -1220,6 +1231,7 @@
assert.isFalse(isVisible(element.$$('.reviewed')));
MockInteractions.pressAndReleaseKeyOn(element, 82, null, 'r');
assert.isTrue(alertStub.called);
+ assert.isTrue(saveReviewStub.calledOnce);
});
test('_getReviewedFiles does not call API', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-lib-loader/gr-syntax-lib-loader.js b/polygerrit-ui/app/elements/diff/gr-syntax-lib-loader/gr-syntax-lib-loader.js
index 75b00e8..bfd8e90 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-lib-loader/gr-syntax-lib-loader.js
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-lib-loader/gr-syntax-lib-loader.js
@@ -26,7 +26,6 @@
// NOTE: intended singleton.
value: {
- loaded: false,
loading: false,
callbacks: [],
},
@@ -34,9 +33,9 @@
},
get() {
- return new Promise(resolve => {
+ return new Promise((resolve, reject) => {
// If the lib is totally loaded, resolve immediately.
- if (this._state.loaded) {
+ if (this._getHighlightLib()) {
resolve(this._getHighlightLib());
return;
}
@@ -44,7 +43,7 @@
// If the library is not currently being loaded, then start loading it.
if (!this._state.loading) {
this._state.loading = true;
- this._loadHLJS().then(this._onLibLoaded.bind(this));
+ this._loadHLJS().then(this._onLibLoaded.bind(this)).catch(reject);
}
this._state.callbacks.push(resolve);
@@ -53,7 +52,6 @@
_onLibLoaded() {
const lib = this._getHighlightLib();
- this._state.loaded = true;
this._state.loading = false;
for (const cb of this._state.callbacks) {
cb(lib);
@@ -73,17 +71,28 @@
_getLibRoot() {
if (this._cachedLibRoot) { return this._cachedLibRoot; }
- return this._cachedLibRoot = document.head
- .querySelector('link[rel=import][href$="gr-app.html"]')
+ const appLink = document.head
+ .querySelector('link[rel=import][href$="gr-app.html"]');
+
+ if (!appLink) { return null; }
+
+ return this._cachedLibRoot = appLink
.href
.match(LIB_ROOT_PATTERN)[1];
},
_cachedLibRoot: null,
_loadHLJS() {
- return new Promise(resolve => {
+ return new Promise((resolve, reject) => {
const script = document.createElement('script');
- script.src = this._getLibRoot() + HLJS_PATH;
+ const src = this._getHLJSUrl();
+
+ if (!src) {
+ reject(new Error('Unable to load blank HLJS url.'));
+ return;
+ }
+
+ script.src = src;
script.onload = function() {
this._configureHighlightLib();
resolve();
@@ -91,5 +100,11 @@
Polymer.dom(document.head).appendChild(script);
});
},
+
+ _getHLJSUrl() {
+ const root = this._getLibRoot();
+ if (!root) { return null; }
+ return root + HLJS_PATH;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-lib-loader/gr-syntax-lib-loader_test.html b/polygerrit-ui/app/elements/diff/gr-syntax-lib-loader/gr-syntax-lib-loader_test.html
index b46910a..6ddde46 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-lib-loader/gr-syntax-lib-loader_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-lib-loader/gr-syntax-lib-loader_test.html
@@ -45,7 +45,6 @@
);
// Assert preconditions:
- assert.isFalse(element._state.loaded);
assert.isFalse(element._state.loading);
});
@@ -57,7 +56,6 @@
// Because the element state is a singleton, clean it up.
element._state.loading = false;
- element._state.loaded = false;
element._state.callbacks = [];
});
@@ -68,7 +66,6 @@
// It should now be in the loading state.
assert.isTrue(loadStub.called);
assert.isTrue(element._state.loading);
- assert.isFalse(element._state.loaded);
assert.isFalse(firstCallHandler.called);
const secondCallHandler = sinon.stub();
@@ -76,7 +73,6 @@
// No change in state.
assert.isTrue(element._state.loading);
- assert.isFalse(element._state.loaded);
assert.isFalse(firstCallHandler.called);
assert.isFalse(secondCallHandler.called);
@@ -85,11 +81,55 @@
flush(() => {
// The state should be loaded and both handlers called.
assert.isFalse(element._state.loading);
- assert.isTrue(element._state.loaded);
assert.isTrue(firstCallHandler.called);
assert.isTrue(secondCallHandler.called);
done();
});
});
+
+ suite('preloaded', () => {
+ setup(() => {
+ window.hljs = 'test-object';
+ });
+
+ teardown(() => {
+ delete window.hljs;
+ });
+
+ test('returns hljs', done => {
+ const firstCallHandler = sinon.stub();
+ element.get().then(firstCallHandler);
+ flush(() => {
+ assert.isTrue(firstCallHandler.called);
+ assert.isTrue(firstCallHandler.calledWith('test-object'));
+ done();
+ });
+ });
+ });
+
+ suite('_getHLJSUrl', () => {
+ suite('checking _getLibRoot', () => {
+ let libRootStub;
+ let root;
+
+ setup(() => {
+ libRootStub = sinon.stub(element, '_getLibRoot', () => root);
+ });
+
+ teardown(() => {
+ libRootStub.restore();
+ });
+
+ test('with no root', () => {
+ assert.isNull(element._getHLJSUrl());
+ });
+
+ test('with root', () => {
+ root = 'test-root.com/';
+ assert.equal(element._getHLJSUrl(),
+ 'test-root.com/bower_components/highlightjs/highlight.min.js');
+ });
+ });
+ });
});
</script>
