Merge changes I92fa8ce1,I08822edb
* changes:
Validate checker properties in CheckerCreation and CheckerUpdate
Make name of checker mandatory
diff --git a/gr-checks/gr-checkers-list.html b/gr-checks/gr-checkers-list.html
new file mode 100644
index 0000000..36e8f85
--- /dev/null
+++ b/gr-checks/gr-checkers-list.html
@@ -0,0 +1,163 @@
+<link rel="import" href="gr-create-checkers-dialog.html">
+
+<!-- Expects core to import these functionalities
+ gr-list-view/gr-list-view.html
+ styles/gr-form-styles.html
+ /gr-icons/gr-icons.html
+ /iron-input/iron-input.html -->
+
+<dom-module id="gr-checkers-list">
+ <template>
+ <style include="shared-styles"></style>
+ <style include="gr-table-styles"></style>
+ <style>
+ iron-icon {
+ cursor: pointer;
+ }
+ #filter {
+ font-size: var(--font-size-normal);
+ max-width: 25em;
+ }
+ #filter:focus {
+ outline: none;
+ }
+ #topContainer {
+ align-items: center;
+ display: flex;
+ height: 3rem;
+ justify-content: space-between;
+ margin: 0 1em;
+ }
+ #createNewContainer:not(.show) {
+ display: none;
+ }
+ a {
+ color: var(--primary-text-color);
+ text-decoration: none;
+ }
+ nav {
+ align-items: center;
+ display: flex;
+ height: 3rem;
+ justify-content: flex-end;
+ margin-right: 20px;
+ }
+ nav,
+ iron-icon {
+ color: var(--deemphasized-text-color);
+ }
+ .nav-iron-icon {
+ height: 1.85rem;
+ margin-left: 16px;
+ width: 1.85rem;
+ }
+ .nav-buttons:hover {
+ text-decoration: underline;
+ cursor: pointer;
+ }
+ </style>
+
+
+ <div id="topContainer">
+ <div>
+ <label>Filter:</label>
+ <iron-input
+ type="text"
+ bind-value="{{_filter}}">
+ <input
+ is="iron-input"
+ type="text"
+ id="filter"
+ bind-value="{{_filter}}">
+ </iron-input>
+ </div>
+ <div id="createNewContainer"
+ class$="[[_computeCreateClass(_createNewCapability)]]">
+ <gr-button primary link id="createNew" on-tap="_handleCreateClicked">
+ Create New
+ </gr-button>
+ </div>
+ </div>
+
+ <table id="list" class="genericList">
+ <tr class="headerRow">
+ <th class="name topHeader">Checker Name</th>
+ <th class="name topHeader">Repository</th>
+ <th class="name topHeader">Status</th>
+ <th class="name topHeader">Required</th>
+ <th class="topHeader description">Checker Description</th>
+ <th class="name topHeader"> Edit </th>
+ </tr>
+ <tbody class$="[[computeLoadingClass(_loading)]]">
+ <template is="dom-repeat" items="[[_visibleCheckers]]">
+ <tr class="table">
+ <td class="name">
+ <a>[[item.name]]</a>
+ </td>
+ <td class="name">[[item.repository]]</td>
+ <td class="name">[[item.status]]</td>
+ <td class="name">[[_computeBlocking(item)]]</td>
+ <td class="description">[[item.description]]</td>
+ <td on-tap="_handleEditIconClicked">
+ <iron-icon icon="gr-icons:edit"></iron-icon>
+ </td>
+ </tr>
+ </template>
+ </tbody>
+ </table>
+
+ <nav>
+ <template is="dom-if" if="[[_showPrevButton]]">
+ <a class="nav-buttons" id="prevArrow"
+ on-tap="_handlePrevClicked">
+ <iron-icon class="nav-iron-icon" icon="gr-icons:chevron-left"></iron-icon>
+ </a>
+ </template>
+ <template is="dom-if" if="[[_showNextButton]]">
+ <a class="nav-buttons" id="nextArrow"
+ on-tap="_handleNextClicked">
+ <iron-icon icon="gr-icons:chevron-right"></iron-icon>
+ </a>
+ </template>
+ </nav>
+
+ <gr-overlay id="createOverlay" with-backdrop>
+ <gr-dialog
+ id="createDialog"
+ confirm-label="Create"
+ on-confirm="_handleCreateConfirm"
+ on-cancel="_handleCreateCancel">
+ <div class="header" slot="header">
+ Create Checkers
+ </div>
+ <div slot="main">
+ <gr-create-checkers-dialog
+ id="createNewModal"
+ plugin-rest-api="[[pluginRestApi]]">
+ </gr-create-checkers-dialog>
+ </div>
+ </gr-dialog>
+ </gr-overlay>
+ <gr-overlay id="editOverlay" with-backdrop>
+ <gr-dialog
+ id="editDialog"
+ confirm-label="Save"
+ on-confirm="_handleEditConfirm"
+ on-cancel="_handleEditCancel">
+ <div class="header" slot="header">
+ Edit Checker
+ </div>
+ <div slot="main">
+ <gr-create-checkers-dialog
+ checker="[[checker]]"
+ plugin-rest-api="[[pluginRestApi]]"
+ on-cancel="_handleEditCancel"
+ id="editModal">
+ </gr-create-checkers-dialog>
+ </div>
+ </gr-dialog>
+ </gr-overlay>
+ <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
+ </template>
+ <script src="gr-checkers-list.js"></script>
+</dom-module>
diff --git a/gr-checks/gr-checkers-list.js b/gr-checks/gr-checkers-list.js
new file mode 100644
index 0000000..0967c03
--- /dev/null
+++ b/gr-checks/gr-checkers-list.js
@@ -0,0 +1,174 @@
+(function() {
+ 'use strict';
+ const CHECKERS_PER_PAGE = 15;
+ const GET_CHECKERS_URL = "/a/plugins/checks/checkers/";
+
+ /**
+ * Show a list of all checkers along with creating/editing them
+ */
+ Polymer({
+ is: 'gr-checkers-list',
+ properties: {
+ pluginRestApi: Object,
+ // Checker that will be passed to the editOverlay modal
+ checker: Object,
+ _checkers: Array,
+ // List of checkers that contain the filtered text
+ _filteredCheckers: Array,
+ _loading: {
+ type: Boolean,
+ value: true,
+ },
+ _filter: {
+ type: String,
+ value: '',
+ },
+ _visibleCheckers: {
+ type: Array,
+ computed: '_computeVisibleCheckers(_startingIndex, _filteredCheckers)',
+ observer: '_visibleCheckersChanged'
+ },
+ _createNewCapability: {
+ type: Boolean,
+ value: true,
+ },
+ _startingIndex: {
+ type: Number,
+ value: 0
+ },
+ _showNextButton: {
+ type: Boolean,
+ value: true,
+ computed: '_computeShowNextButton(_startingIndex, _filteredCheckers)'
+ },
+ _showPrevButton: {
+ type: Boolean,
+ value: true,
+ computed: '_computeShowPrevButton(_startingIndex, _filteredCheckers)'
+ },
+ },
+ observers: [
+ '_showCheckers(_checkers, _filter)',
+ ],
+
+ attached() {
+ this._getCheckers();
+ },
+
+ _contains(target, keyword) {
+ return target.toLowerCase().includes(keyword.toLowerCase().trim());
+ },
+
+ _visibleCheckersChanged(currentVisibleCheckers, previousVisibleCheckers) {
+ if (!currentVisibleCheckers || !previousVisibleCheckers) {
+ return;
+ }
+ if (currentVisibleCheckers.length !== previousVisibleCheckers.length) {
+ this.fire('resize', {bubbles: false});
+ }
+ },
+
+ _showCheckers(_checkers, _filter) {
+ if (!_checkers) return;
+ if (!_filter) _filter = '';
+ // TODO(dhruvsri): highlight matching part
+ this._filteredCheckers = this._checkers.filter(checker =>
+ this._contains(checker.name, this._filter) ||
+ this._contains(checker.repository, this._filter))
+ this._startingIndex = 0;
+ },
+
+ computeLoadingClass(loading) {
+ return loading ? 'loading' : '';
+ },
+
+ _computeVisibleCheckers(_startingIndex, _filteredCheckers) {
+ if (!_filteredCheckers) {
+ return [];
+ }
+ return this._filteredCheckers.slice(this._startingIndex,
+ this._startingIndex + CHECKERS_PER_PAGE);
+ },
+
+ _computeShowNextButton(_startingIndex, _filteredCheckers) {
+ if (!_filteredCheckers) {
+ return false;
+ }
+ return _startingIndex + CHECKERS_PER_PAGE < _filteredCheckers.length;
+ },
+
+ _computeShowPrevButton(_startingIndex, _filteredCheckers) {
+ if (!_filteredCheckers) {
+ return false;
+ }
+ return _startingIndex >= CHECKERS_PER_PAGE;
+ },
+
+ _handleNextClicked() {
+ if (this._startingIndex + CHECKERS_PER_PAGE <
+ this._filteredCheckers.length) {
+ this._startingIndex += CHECKERS_PER_PAGE;
+ }
+ },
+
+ _handlePrevClicked() {
+ if (this._startingIndex >= CHECKERS_PER_PAGE) {
+ this._startingIndex -= CHECKERS_PER_PAGE;
+ }
+ },
+
+ _getCheckers() {
+ this.pluginRestApi.fetchJSON({
+ method: 'GET',
+ url: GET_CHECKERS_URL,
+ }).then(checkers => {
+ if (!checkers) { return; }
+ this._checkers = checkers;
+ this._startingIndex = 0;
+ this._loading = false;
+ });
+ },
+
+ _handleEditConfirm() {
+ this.$.editModal.handleEditChecker();
+ },
+
+ _handleEditIconClicked(e) {
+ let checker = e.model.item;
+ this.checker = checker;
+ this.$.editOverlay.open();
+ },
+
+ _handleEditCancel(e) {
+ if (e.detail.reload) {
+ this._getCheckers();
+ }
+ this.$.editOverlay.close();
+ },
+
+ _computeCreateClass(createNew) {
+ return createNew ? 'show' : '';
+ },
+
+ _computeBlocking(checker) {
+ return (checker && checker.blocking && checker.blocking.length > 0)
+ ? "YES": "NO";
+ },
+
+ _handleCreateConfirm() {
+ this.$.createNewModal.handleCreateChecker();
+ },
+
+ _handleCreateClicked() {
+ this.$.createOverlay.open();
+ },
+
+ _handleCreateCancel(e) {
+ if (e.detail.reload) {
+ this._getCheckers();
+ }
+ this.$.createOverlay.close();
+ },
+
+ })
+})();
\ No newline at end of file
diff --git a/gr-checks/gr-checks-all-statuses.js b/gr-checks/gr-checks-all-statuses.js
index 17a7bee..c4be5bc 100644
--- a/gr-checks/gr-checks-all-statuses.js
+++ b/gr-checks/gr-checks-all-statuses.js
@@ -17,6 +17,8 @@
SUCCESSFUL: 'SUCCESSFUL',
FAILED: 'FAILED',
NOT_RELEVANT: 'NOT_RELEVANT',
+
+ STATUS_UNKNOWN: 'UNKNOWN'
};
function isStatus(status, includedStatuses) {
@@ -27,6 +29,14 @@
return isStatus(status, [Statuses.NOT_STARTED, Statuses.NOT_RELEVANT]);
}
+ function isScheduled(status) {
+ return isStatus(status, [Statuses.SCHEDULED]);
+ }
+
+ function isRunning(status) {
+ return isStatus(status, [Statuses.RUNNING]);
+ }
+
function isInProgress(status) {
return isStatus(status, [Statuses.SCHEDULED, Statuses.RUNNING]);
}
@@ -57,6 +67,8 @@
window.Gerrit.Checks.Statuses = Statuses;
window.Gerrit.Checks.isUnevaluated = isUnevaluated;
+ window.Gerrit.Checks.isScheduled = isScheduled;
+ window.Gerrit.Checks.isRunning = isRunning;
window.Gerrit.Checks.isInProgress = isInProgress;
window.Gerrit.Checks.isSuccessful = isSuccessful;
window.Gerrit.Checks.isFailed = isFailed;
diff --git a/gr-checks/gr-checks-chip-view.js b/gr-checks/gr-checks-chip-view.js
index cd07acb..a2933ef 100644
--- a/gr-checks/gr-checks-chip-view.js
+++ b/gr-checks/gr-checks-chip-view.js
@@ -53,13 +53,14 @@
return check.state == Statuses.FAILED;
}
)
- if (!hasFailedCheck) return false;
+ if (!hasFailedCheck) return '';
const hasRequiredFailedCheck = checks.some(
(check) => {
- return check.state == Statuses.FAILED && check.blocking && check.blocking.length > 0;
+ return check.state == Statuses.FAILED &&
+ check.blocking && check.blocking.length > 0;
}
)
- return !hasRequiredFailedCheck;
+ return hasRequiredFailedCheck ? '' : 'set';
}
@@ -74,15 +75,18 @@
getChecks: Function,
_checkStatuses: Object,
_hasChecks: Boolean,
+ _failedRequiredChecksCount: Number,
_status: {type: String, computed: '_computeStatus(_checkStatuses)'},
_statusString: {
type: String,
- computed: '_computeStatusString(_status, _checkStatuses)',
+ computed: '_computeStatusString(_status, _checkStatuses, _failedRequiredChecksCount)',
},
_chipClasses: {type: String, computed: '_computeChipClass(_status)'},
+ // Type is set as string so that it reflects on changes
+ // Polymer does not support reflecting changes in Boolean property
_downgradeFailureToWarning: {
- type: Boolean,
- value: false
+ type: String,
+ value: ''
},
pollChecksInterval: Object,
visibilityChangeListenerAdded: {
@@ -127,8 +131,11 @@
getChecks(change._number, revision._number).then(checks => {
this.set('_hasChecks', checks.length > 0);
if (checks.length > 0) {
- this.set('_checkStatuses', computeCheckStatuses(checks));
- this.set('_downgradeFailureToWarning', downgradeFailureToWarning(checks));
+ this._downgradeFailureToWarning =
+ downgradeFailureToWarning(checks);
+ this._failedRequiredChecksCount =
+ this.computeFailedRequiredChecksCount(checks);
+ this._checkStatuses = computeCheckStatuses(checks);
}
});
},
@@ -164,15 +171,30 @@
Statuses.STATUS_UNKNOWN;
},
+ computeFailedRequiredChecksCount(checks) {
+ const failedRequiredChecks = checks.filter(
+ check => {
+ return check.state == Statuses.FAILED &&
+ check.blocking && check.blocking.length > 0;
+ }
+ );
+ return failedRequiredChecks.length;
+ },
+
/**
* @param {string} status The overall status of the checks.
* @param {!Object} checkStatuses The number of checks in each status.
* @return {string}
*/
- _computeStatusString(status, checkStatuses) {
+ _computeStatusString(status, checkStatuses, failedRequiredChecksCount) {
+ if (!checkStatuses) return;
if (checkStatuses.total === 0) return 'No checks';
- return `${checkStatuses[status]} of ${
+ let statusString = `${checkStatuses[status]} of ${
checkStatuses.total} checks ${HumanizedStatuses[status]}`;
+ if (status === Statuses.FAILED && failedRequiredChecksCount > 0) {
+ statusString += ` (${failedRequiredChecksCount} required)`;
+ }
+ return statusString;
},
/**
diff --git a/gr-checks/gr-checks-item.html b/gr-checks/gr-checks-item.html
index 64a061b..602cffb 100644
--- a/gr-checks/gr-checks-item.html
+++ b/gr-checks/gr-checks-item.html
@@ -18,8 +18,17 @@
margin-right: 16px;
display: inline-block;
}
+ .nav-icon {
+ cursor: pointer;
+ }
</style>
-
+ <td>
+ <template is="dom-if" if="[[check.message]]">
+ <iron-icon class="nav-icon" on-tap="_toggleMessageShown"
+ icon="[[_computeExpandIcon(showCheckMessage)]]">
+ </iron-icon>
+ </template>
+ </td>
<td>[[check.checker_name]]</td>
<td>[[_requiredForMerge]]</td>
<td>
@@ -42,6 +51,7 @@
<!-- Re-run-->
<!-- </gr-button>-->
</td>
+ <td>[[check.checker_description]]</td>
</template>
<script src="gr-checks-item.js"></script>
</dom-module>
diff --git a/gr-checks/gr-checks-item.js b/gr-checks/gr-checks-item.js
index 199fc92..2c0d98b 100644
--- a/gr-checks/gr-checks-item.js
+++ b/gr-checks/gr-checks-item.js
@@ -41,6 +41,10 @@
_requiredForMerge: {
type: String,
computed: '_computeRequiredForMerge(check)'
+ },
+ showCheckMessage: {
+ type: Boolean,
+ value: false,
}
},
@@ -53,6 +57,15 @@
return moment(check.started).format('LTS');
},
+ _toggleMessageShown() {
+ this.showCheckMessage = !this.showCheckMessage;
+ this.fire('toggle-check-message', {uuid: this.check.checker_uuid})
+ },
+
+ _computeExpandIcon(showCheckMessage) {
+ return showCheckMessage ? "gr-icons:expand-less": "gr-icons:expand-more";
+ },
+
/**
* @param {!Defs.Check} check
* @return {string}
diff --git a/gr-checks/gr-checks-status.html b/gr-checks/gr-checks-status.html
index 591d228..0341a3f 100644
--- a/gr-checks/gr-checks-status.html
+++ b/gr-checks/gr-checks-status.html
@@ -33,7 +33,7 @@
</span>
</template>
</template>
- <template is="dom-if" if="[[_isInProgress(status)]]">
+ <template is="dom-if" if="[[_isScheduled(status)]]">
<svg width="18" height="18" xmlns="http://www.w3.org/2000/svg">
<g fill="none" fill-rule="evenodd">
<path d="M12.184 9.293c-.86 0-1.641-.39-2.15-.976L9.8 9.489l.82.78V13.2h-.78v-2.344l-.821-.781-.39 1.719-2.736-.547.157-.782 1.914.391.625-3.164-.703.273v1.328h-.782V7.457l2.032-.86c.117 0 .196-.039.313-.039.273 0 .508.157.664.391l.39.625c.313.547.939.938 1.68.938v.781zM10.034 4.8c.43 0 .782.352.782.782 0 .43-.352.781-.781.781a.783.783 0 0 1-.782-.781c0-.43.352-.782.782-.782zM9 2a7 7 0 1 0 .002 14.002A7 7 0 0 0 9 2z" fill="#9C27B0"/>
@@ -42,7 +42,20 @@
</svg>
<template is="dom-if" if="[[showText]]">
<span>
- In progress
+ Scheduled
+ </span>
+ </template>
+ </template>
+ <template is="dom-if" if="[[_isRunning(status)]]">
+ <svg width="18" height="18" xmlns="http://www.w3.org/2000/svg">
+ <g fill="none" fill-rule="evenodd">
+ <path d="M12.184 9.293c-.86 0-1.641-.39-2.15-.976L9.8 9.489l.82.78V13.2h-.78v-2.344l-.821-.781-.39 1.719-2.736-.547.157-.782 1.914.391.625-3.164-.703.273v1.328h-.782V7.457l2.032-.86c.117 0 .196-.039.313-.039.273 0 .508.157.664.391l.39.625c.313.547.939.938 1.68.938v.781zM10.034 4.8c.43 0 .782.352.782.782 0 .43-.352.781-.781.781a.783.783 0 0 1-.782-.781c0-.43.352-.782.782-.782zM9 2a7 7 0 1 0 .002 14.002A7 7 0 0 0 9 2z" fill="#9C27B0"/>
+ <path d="M0 0h18v18H0z"/>
+ </g>
+ </svg>
+ <template is="dom-if" if="[[showText]]">
+ <span>
+ Running
</span>
</template>
</template>
diff --git a/gr-checks/gr-checks-status.js b/gr-checks/gr-checks-status.js
index 0f18339..67ba4e4 100644
--- a/gr-checks/gr-checks-status.js
+++ b/gr-checks/gr-checks-status.js
@@ -12,7 +12,7 @@
reflectToAttribute: true,
},
status: String,
- downgradeFailureToWarning: Boolean
+ downgradeFailureToWarning: String
},
_isUnevaluated(status) {
@@ -23,6 +23,14 @@
return window.Gerrit.Checks.isInProgress(status);
},
+ _isRunning(status) {
+ return window.Gerrit.Checks.isRunning(status);
+ },
+
+ _isScheduled(status) {
+ return window.Gerrit.Checks.isScheduled(status);
+ },
+
_isSuccessful(status) {
return window.Gerrit.Checks.isSuccessful(status);
},
diff --git a/gr-checks/gr-checks-view.html b/gr-checks/gr-checks-view.html
index 67129ee..ee22cbb 100644
--- a/gr-checks/gr-checks-view.html
+++ b/gr-checks/gr-checks-view.html
@@ -1,4 +1,6 @@
<link rel="import" href="gr-checks-status.html">
+<link rel="import" href="gr-checkers-list.html">
+
<dom-module id="gr-checks-view">
<template>
<style>
@@ -61,8 +63,43 @@
padding: 24px 0;
text-align: center;
}
+
+ /* Add max-width 1px to check-message make sure content doesn't
+ expand beyond the table width */
+ .check-message {
+ padding-left: 1rem;
+ background: var(--table-subheader-background-color);
+ max-width: 1px;
+ }
+
+ .message {
+ white-space: pre-wrap;
+ word-wrap: break-word;
+ }
+
+ .message-heading {
+ font-weight: bold;
+ margin-top: 1em;
+ }
+
+ .check-message-heading {
+ padding-left: 1em;
+ }
+
+ .configure-button {
+ float:right;
+ margin-top: 10px;
+ }
+
+ #listOverlay {
+ width: 75%;
+ }
</style>
+ <template is="dom-if" if="[[_createCheckerCapability]]">
+ <gr-button class="configure-button" on-tap="_handleConfigureClicked"> Configure </gr-button>
+ </template>
+
<template is="dom-if" if="[[_isLoading(_status)]]">
<div class="no-content">
<p>Loading...</p>
@@ -91,6 +128,7 @@
<table>
<thead>
<tr class="headerRow">
+ <th class="topHeader"></th>
<th class="topHeader">Name</th>
<th class="topHeader">For submit</th>
<th class="topHeader">Status</th>
@@ -98,16 +136,39 @@
<th class="topHeader">Started</th>
<th class="topHeader">Duration</th>
<th class="topHeader"><!-- actions --></th>
+ <th class="topHeader">Description</th>
</tr>
</thead>
<tbody>
<template is="dom-repeat" items="[[_checks]]" as="check">
- <gr-checks-item check="[[check]]" retry-check="[[retryCheck]]"></gr-checks-item>
+ <tr>
+ <gr-checks-item on-toggle-check-message="_toggleCheckMessage"
+ check="[[check]]" retry-check="[[retryCheck]]">
+ </gr-checks-item>
+ </tr>
+ <template is="dom-if" if="[[check.showCheckMessage]]">
+ <tr>
+ <td colspan="9" class="check-message-heading">
+ <span class="message-heading">
+ Message
+ </span>
+ </td>
+ </tr>
+ <tr>
+ <td colspan="9" class="check-message">
+ <span class="message"> [[check.message]] </span>
+ </td>
+ </tr>
+ </template>
</template>
</tbody>
</table>
</template>
- </template>
+ <gr-overlay id="listOverlay" with-backdrop>
+ <gr-checkers-list on-resize="_handleCheckersListResize" plugin-rest-api="[[pluginRestApi]]"></gr-checkers-list>
+ </gr-overlay>
+
+ </template>
<script src="gr-checks-view.js"></script>
</dom-module>
diff --git a/gr-checks/gr-checks-view.js b/gr-checks/gr-checks-view.js
index f5d083f..c312163 100644
--- a/gr-checks/gr-checks-view.js
+++ b/gr-checks/gr-checks-view.js
@@ -48,27 +48,60 @@
isConfigured: Function,
/** @type {function(string, string): !Promise<!Object>} */
retryCheck: Function,
+ pluginRestApi: Object,
_checks: Object,
_status: {
type: Object,
value: LoadingStatus.LOADING,
},
- pollChecksInterval: Object,
+ pollChecksInterval: Number,
visibilityChangeListenerAdded: {
type: Boolean,
value: false
- }
+ },
+ _createCheckerCapability: {
+ type: Boolean,
+ value: false
+ },
},
observers: [
'_pollChecksRegularly(change, revision, getChecks)',
],
+ attached() {
+ this.pluginRestApi = this.plugin.restApi();
+ this._initCreateCheckerCapability();
+ },
+
detached() {
clearInterval(this.pollChecksInterval);
this.unlisten(document, 'visibilitychange', '_onVisibililityChange');
},
+ _handleCheckersListResize() {
+ // Force polymer to recalculate position of overlay when length of
+ // checkers changes
+ this.$.listOverlay.refit();
+ },
+
+ _initCreateCheckerCapability() {
+ return this.pluginRestApi.getAccount().then(account => {
+ if (!account) { return; }
+ return this.pluginRestApi
+ .getAccountCapabilities(['checks-administrateCheckers'])
+ .then(capabilities => {
+ if (capabilities['checks-administrateCheckers']) {
+ this._createCheckerCapability = true;
+ }
+ });
+ });
+ },
+
+ _handleConfigureClicked() {
+ this.$.listOverlay.open();
+ },
+
_orderChecks(a, b) {
if (a.state != b.state) {
let indexA = StatusPriorityOrder.indexOf(a.state);
@@ -95,7 +128,20 @@
getChecks(change._number, revision._number).then(checks => {
if (checks && checks.length) {
checks.sort(this._orderChecks);
- this.set('_checks', checks);
+ if (!this._checks) {
+ this._checks = checks;
+ } else {
+ // Merge checks & this_checks to keep showCheckMessage property
+ this._checks = checks.map(
+ (check) => {
+ const prevCheck = this._checks.find(
+ (c) => { return c.checker_uuid === check.checker_uuid }
+ )
+ if (!prevCheck) return check;
+ return Object.assign({}, check, prevCheck)
+ }
+ )
+ }
this.set('_status', LoadingStatus.RESULTS);
} else {
this._checkConfigured();
@@ -111,6 +157,22 @@
this._pollChecksRegularly(this.change, this.revision, this.getChecks);
},
+ _toggleCheckMessage(e) {
+ const uuid = e.detail.uuid;
+ if (!uuid) {
+ console.warn("uuid not found");
+ return;
+ }
+ const idx = this._checks.findIndex(check => check.checker_uuid === uuid);
+ if (idx == -1) {
+ console.warn("check not found");
+ return;
+ }
+ // Update subproperty of _checks[idx] so that it reflects to polymer
+ this.set(`_checks.${idx}.showCheckMessage`,
+ !this._checks[idx].showCheckMessage)
+ },
+
_pollChecksRegularly(change, revision, getChecks) {
if (this.pollChecksInterval) {
clearInterval(this.pollChecksInterval);
diff --git a/gr-checks/gr-checks.html b/gr-checks/gr-checks.html
index cc4f6ca..2108121 100644
--- a/gr-checks/gr-checks.html
+++ b/gr-checks/gr-checks.html
@@ -12,7 +12,7 @@
const getChecks = (change, revision) => {
return plugin.restApi().get(
- '/changes/' + change + '/revisions/' + revision + '/checks?o=CHECKER');
+ '/changes/' + change + '/revisions/' + revision + '/checks?o=CHECKER');
};
// TODO(brohlfs): Enable this dashboard column when search queries start
@@ -24,23 +24,25 @@
// 'change-list-item-cell',
// 'gr-checks-change-list-item-cell-view');
plugin.registerCustomComponent(
- 'commit-container',
- 'gr-checks-chip-view').onAttached(
- view => {
- view['getChecks'] = getChecks;
- });
+ 'commit-container',
+ 'gr-checks-chip-view').onAttached(
+ view => {
+ view['getChecks'] = getChecks;
+ }
+ );
plugin.registerDynamicCustomComponent(
- 'change-view-tab-header',
- 'gr-checks-change-view-tab-header-view');
+ 'change-view-tab-header',
+ 'gr-checks-change-view-tab-header-view'
+ );
plugin.registerDynamicCustomComponent(
- 'change-view-tab-content',
- 'gr-checks-view').onAttached(
- view => {
- view['isConfigured'] = (repository) => Promise.resolve(true);
- // TODO(brohlfs): Implement retry.
- view['retryCheck'] = (buildId) => undefined;
- view['getChecks'] = getChecks;
- });
- });
+ 'change-view-tab-content',
+ 'gr-checks-view').onAttached(
+ view => {
+ view['isConfigured'] = (repository) => Promise.resolve(true);
+ // TODO(brohlfs): Implement retry.
+ view['retryCheck'] = (buildId) => undefined;
+ view['getChecks'] = getChecks;
+ });
+ });
</script>
</dom-module>
diff --git a/gr-checks/gr-create-checkers-dialog.html b/gr-checks/gr-create-checkers-dialog.html
new file mode 100644
index 0000000..e9aad47
--- /dev/null
+++ b/gr-checks/gr-create-checkers-dialog.html
@@ -0,0 +1,157 @@
+<link rel="import" href="gr-repo-chip.html">
+<dom-module id="gr-create-checkers-dialog">
+ <template>
+ <style include="gr-form-styles">
+ :host {
+ display: inline-block;
+ }
+ input {
+ width: 20em;
+ }
+ gr-autocomplete {
+ border: none;
+ --gr-autocomplete: {
+ border: 1px solid var(--border-color);
+ border-radius: 2px;
+ font-size: var(--font-size-normal);
+ height: 2em;
+ padding: 0 .15em;
+ width: 20em;
+ }
+ }
+ .error {
+ color: red;
+ }
+ #checkerSchemaInput[disabled] {
+ background-color: var(--table-subheader-background-color);
+ }
+ #checkerIdInput[disabled] {
+ background-color: var(--table-subheader-background-color);
+ }
+ </style>
+
+ <div class="gr-form-styles">
+ <div id="form">
+ <section hidden$="[[_errorMsg.length > 0]]">
+ <span class="error"> {{_errorMsg}} </span>
+ </section>
+ <section>
+ <span class="title">Name*</span>
+ <iron-input autocomplete="on"
+ bind-value="{{_name}}">
+ <input is="iron-input"
+ id="checkerNameInput"
+ autocomplete="on"
+ bind-value="{{_name}}">
+ </iron-input>
+ </section>
+ <section>
+ <span class="title">Description</span>
+ <iron-input autocomplete="on"
+ bind-value="{{_description}}">
+ <input is="iron-input"
+ id="checkerDescriptionInput"
+ autocomplete="on"
+ bind-value="{{_description}}">
+ </iron-input>
+ </section>
+ <section>
+ <span class="title">Repository*</span>
+ <div class="list">
+ <template id="chips" is="dom-repeat" items="[[_repos]]" as="repo">
+ <gr-repo-chip
+ repo="[[repo]]"
+ on-keydown="_handleChipKeydown"
+ on-remove="_handleOnRemove"
+ tabindex="-1">
+ </gr-repo-chip>
+ </template>
+ </div>
+ <div hidden$="[[_repositorySelected]]">
+ <gr-autocomplete
+ id="input"
+ threshold="[[suggestFrom]]"
+ query="[[_getRepoSuggestions]]"
+ on-commit="_handleRepositorySelected"
+ clear-on-commit
+ warn-uncommitted
+ text="{{_inputText}}">
+ </gr-autocomplete>
+ </div>
+ </section>
+
+ <section>
+ <span class="title">Scheme*</span>
+ <iron-input autocomplete="on"
+ bind-value="{{_scheme}}">
+ <input is="iron-input"
+ id="checkerSchemaInput"
+ disabled$="[[_edit]]"
+ autocomplete="on"
+ bind-value="{{_scheme}}">
+ </iron-input>
+ </section>
+
+ <section>
+ <span class="title">ID*</span>
+ <iron-input autocomplete="on"
+ bind-value="{{_id}}">
+ <input is="iron-input"
+ id="checkerIdInput"
+ disabled$="[[_edit]]"
+ autocomplete="on"
+ bind-value="{{_id}}">
+ </iron-input>
+ </section>
+
+ <section>
+ <span class="title">Url</span>
+ <iron-input autocomplete="on"
+ bind-value="{{_url}}">
+ <input is="iron-input"
+ id="checkerUrlInput"
+ autocomplete="on"
+ bind-value="{{_url}}">
+ </iron-input>
+ </section>
+
+ <section>
+ <span class="title"> UUID {{_uuid}}</span>
+ </section>
+
+ <section>
+ <span class="title">Status</span>
+ <gr-dropdown-list
+ items="[[_statuses]]"
+ on-value-change="_handleStatusChange"
+ text="Status"
+ value="[[_status]]">
+ </gr-dropdown-list>
+ </section>
+
+ <section>
+ <span class="title">Required</span>
+ <input
+ on-click = "_handleRequiredCheckBoxClicked"
+ type="checkbox"
+ id="privateChangeCheckBox"
+ checked$="[[_required]]">
+ </section>
+
+ <section>
+ <span class="title">Query</span>
+ <iron-input autocomplete="on"
+ bind-value="{{_query}}">
+ <input is="iron-input"
+ id="checkerQueryInput"
+ autocomplete="on"
+ bind-value="{{_query}}">
+ </iron-input>
+ </section>
+
+ </div>
+ </div>
+ <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
+ </template>
+ <script src="gr-create-checkers-dialog.js"></script>
+</dom-module>
diff --git a/gr-checks/gr-create-checkers-dialog.js b/gr-checks/gr-create-checkers-dialog.js
new file mode 100644
index 0000000..cdb4127
--- /dev/null
+++ b/gr-checks/gr-create-checkers-dialog.js
@@ -0,0 +1,258 @@
+(function() {
+ 'use strict';
+
+ const REPOS_PER_PAGE = 6;
+ const CREATE_CHECKER_URL = '/plugins/checks/checkers/';
+ const SCHEME_PATTERN = /^[\w-_.]*$/;
+
+ Polymer({
+ is: 'gr-create-checkers-dialog',
+ _legacyUndefinedCheck: true,
+
+ properties: {
+ checker: {
+ type: Object,
+ observer: '_checkerChanged'
+ },
+ _name: String,
+ _scheme: String,
+ _id: String,
+ _uuid: {
+ type: String,
+ value: ""
+ },
+ pluginRestApi: Object,
+ _url: String,
+ _description: String,
+ _getRepoSuggestions: {
+ type: Function,
+ value() {
+ return this._repoSuggestions.bind(this)
+ }
+ },
+ // The backend might support multiple repos in the future
+ // which is why I decided to keep it as an array.
+ _repos: {
+ type: Array,
+ value: [],
+ notify: true,
+ },
+ _repositorySelected: {
+ type: Boolean,
+ value: false
+ },
+ _handleOnRemove: Function,
+ _errorMsg: {
+ type: String,
+ value: ''
+ },
+ _statuses: {
+ type: Array,
+ value: [
+ {
+ "text": "ENABLED",
+ "value": "ENABLED"
+ },
+ {
+ "text": "DISABLED",
+ "value": "DISABLED"
+ }
+ ],
+ readOnly: true
+ },
+ _required: {
+ type: Boolean,
+ value: false
+ },
+ _status: String,
+ _edit: {
+ type: Boolean,
+ value: false
+ },
+ _query: String
+ },
+
+ behaviours: [
+ Gerrit.FireBehavior,
+ ],
+ /**
+ * Fired when the cancel button is pressed.
+ *
+ * @event cancel
+ */
+
+
+ observers: [
+ '_updateUUID(_scheme, _id)',
+ ],
+
+ _checkerChanged() {
+ if (!this.checker) {
+ console.warn("checker not set");
+ return;
+ }
+ this._edit = true;
+ this._scheme = this.checker.uuid.split(':')[0];
+ this._id = this.checker.uuid.split(':')[1];
+ this._name = this.checker.name;
+ this._description = this.checker.description || '';
+ this._url = this.checker.url || '';
+ this._query = this.checker.query || '';
+ this._required = this.checker.blocking &&
+ this.checker.blocking.length > 0;
+ if (this.checker.repository) {
+ this._repositorySelected = true;
+ this.set('_repos', [{name: this.checker.repository}]);
+ }
+ this._status = this.checker.status;
+ },
+
+ _updateUUID(_scheme, _id) {
+ this._uuid = _scheme + ":" + _id;
+ },
+
+ _handleStatusChange(e) {
+ this._status = e.detail.value;
+ },
+
+ _validateRequest() {
+ if (!this._name) {
+ this._errorMsg = 'Name cannot be empty';
+ return false;
+ }
+ if (this._description && this._description.length > 1000) {
+ this._errorMsg = 'Description should be less than 1000 characters';
+ return false;
+ }
+ if (!this._repositorySelected) {
+ this._errorMsg = 'Select a repository';
+ return false;
+ }
+ if (!this._scheme) {
+ this._errorMsg = 'Scheme cannot be empty.';
+ return false;
+ }
+ if (this._scheme.match(SCHEME_PATTERN) == null) {
+ this._errorMsg =
+ 'Scheme must contain [A-Z], [a-z], [0-9] or {"-" , "_" , "."}';
+ return false;
+ }
+ if (this._scheme.length > 100) {
+ this._errorMsg = 'Scheme must be shorter than 100 characters';
+ return false;
+ }
+ if (!this._id) {
+ this._errorMsg = 'ID cannot be empty.';
+ return false;
+ }
+ if (this._id.match(SCHEME_PATTERN) == null) {
+ this._errorMsg =
+ 'ID must contain [A-Z], [a-z], [0-9] or {"-" , "_" , "."}';
+ return false;
+ }
+ return true;
+ },
+
+ // TODO(dhruvsri): make sure dialog is scrollable.
+
+ _createChecker(checker) {
+ return this.pluginRestApi.send(
+ 'POST',
+ CREATE_CHECKER_URL,
+ checker,
+ )
+ },
+
+ _editChecker(checker) {
+ const url = CREATE_CHECKER_URL + checker.uuid;
+ return this.pluginRestApi.send(
+ 'POST',
+ url,
+ checker
+ )
+ },
+
+ handleEditChecker() {
+ if (!this._validateRequest()) return;
+ this._editChecker(this._getCheckerRequestObject()).then(
+ res => {
+ if (res) {
+ this._errorMsg = '';
+ this.fire('cancel', {reload: true}, {bubbles: true});
+ }
+ },
+ error => {
+ this._errorMsg = error;
+ }
+ )
+ },
+
+ _getCheckerRequestObject() {
+ return {
+ "name" : this._name,
+ "description" : this._description || '',
+ "uuid" : this._uuid,
+ "repository": this._repos[0].name,
+ "url" : this._url,
+ "status": this._status,
+ "blocking": this._required ? ["STATE_NOT_PASSING"] : [],
+ "query": this._query
+ }
+ },
+
+ handleCreateChecker() {
+ if (!this._validateRequest()) return;
+ // Currently after creating checker there is no reload happening (as
+ // this would result in the user exiting the screen).
+ this._createChecker(this._getCheckerRequestObject()).then(
+ res => {
+ if (res) this._cleanUp();
+ },
+ error => {
+ this._errorMsg = error;
+ }
+ )
+ },
+
+ _cleanUp() {
+ this._name = '';
+ this._scheme = '';
+ this._id = '';
+ this._uuid = '';
+ this._description = '';
+ this._repos = [];
+ this._repositorySelected = false;
+ this._errorMsg = '';
+ this._required = false;
+ this._query = '';
+ this._status = '';
+ this.fire('cancel', {reload: true}, {bubbles: true});
+ },
+
+ _repoSuggestions(filter) {
+ const _makeSuggestion = repo => {return {name: repo.name, value: repo}};
+ return this.pluginRestApi.getRepos(filter, REPOS_PER_PAGE).then(
+ repos => repos.map(repo => _makeSuggestion(repo))
+ )
+ },
+
+ _handleRepositorySelected(e) {
+ this.push('_repos', e.detail.value);
+ this._repositorySelected = true;
+ },
+
+ _handleRequiredCheckBoxClicked() {
+ this._required = !this._required;
+ },
+
+ _handleOnRemove(e) {
+ let idx = this._repos.indexOf(e.detail.repo);
+ if (idx == -1) return;
+ this.splice('_repos', idx, 1);
+ if (this._repos.length == 0) {
+ this._repositorySelected = false;
+ }
+ },
+
+ });
+})();
diff --git a/gr-checks/gr-repo-chip.html b/gr-checks/gr-repo-chip.html
new file mode 100644
index 0000000..b695fa9
--- /dev/null
+++ b/gr-checks/gr-repo-chip.html
@@ -0,0 +1,25 @@
+<dom-module id="gr-repo-chip">
+ <template>
+ <style>
+ iron-icon {
+ height: 1.2rem;
+ width: 1.2rem;
+ }
+ :host {
+ display: inline-block;
+ }
+ </style>
+ <span> {{repo.name}} </span>
+ <gr-button
+ id="remove"
+ link
+ hidden$="[[!removable]]"
+ tabindex="-1"
+ aria-label="Remove"
+ class="remove"
+ on-tap="_handleRemoveTap">
+ <iron-icon icon="gr-icons:close"></iron-icon>
+ </gr-button>
+ </template>
+ <script src="gr-repo-chip.js"></script>
+</dom-module>
\ No newline at end of file
diff --git a/gr-checks/gr-repo-chip.js b/gr-checks/gr-repo-chip.js
new file mode 100644
index 0000000..d63a476
--- /dev/null
+++ b/gr-checks/gr-repo-chip.js
@@ -0,0 +1,24 @@
+(function() {
+ 'use strict';
+
+ /**
+ * autocomplete chip for getting repository suggestions
+ */
+ Polymer({
+ is: 'gr-repo-chip',
+ _legacyUndefinedCheck: true,
+ properties: {
+ // repo type is ProjectInfo
+ repo: Object,
+ removable: {
+ type: Boolean,
+ value: true,
+ },
+ },
+ _handleRemoveTap(e) {
+ e.preventDefault();
+ this.fire('remove', {repo: this.repo});
+ },
+ })
+
+})();
\ No newline at end of file
diff --git a/java/com/google/gerrit/plugins/checks/CheckJson.java b/java/com/google/gerrit/plugins/checks/CheckJson.java
index f36c79f..535fbc4 100644
--- a/java/com/google/gerrit/plugins/checks/CheckJson.java
+++ b/java/com/google/gerrit/plugins/checks/CheckJson.java
@@ -89,6 +89,7 @@
info.checkerName = checker.getName();
info.checkerStatus = checker.getStatus();
info.blocking = checker.getBlockingConditions();
+ info.checkerDescription = checker.getDescription().orElse(null);
});
} catch (ConfigInvalidException e) {
logger.atWarning().withCause(e).log("skipping invalid checker %s", checkerUuid);
diff --git a/java/com/google/gerrit/plugins/checks/CheckUpdate.java b/java/com/google/gerrit/plugins/checks/CheckUpdate.java
index 315160d..0b86796 100644
--- a/java/com/google/gerrit/plugins/checks/CheckUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/CheckUpdate.java
@@ -16,6 +16,7 @@
import com.google.auto.value.AutoValue;
import com.google.gerrit.plugins.checks.api.CheckState;
+import com.google.gerrit.server.util.time.TimeUtil;
import java.sql.Timestamp;
import java.util.Optional;
@@ -49,6 +50,14 @@
public abstract Builder setFinished(Timestamp finished);
+ public Builder unsetStarted() {
+ return setStarted(TimeUtil.never());
+ }
+
+ public Builder unsetFinished() {
+ return setFinished(TimeUtil.never());
+ }
+
public abstract CheckUpdate build();
}
}
diff --git a/java/com/google/gerrit/plugins/checks/Checks.java b/java/com/google/gerrit/plugins/checks/Checks.java
index c4527e5..fa15c0e 100644
--- a/java/com/google/gerrit/plugins/checks/Checks.java
+++ b/java/com/google/gerrit/plugins/checks/Checks.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.plugins.checks.api.CombinedCheckState;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import java.io.IOException;
@@ -90,6 +91,16 @@
boolean areAllRequiredCheckersPassing(Project.NameKey projectName, PatchSet.Id patchSetId)
throws IOException, StorageException;
+ /**
+ * Computes an ETag for the checks of the given change.
+ *
+ * @param projectName the name of the project that contains the change
+ * @param changeId ID of the change for which the ETag should be computed
+ * @return ETag for the checks of the given change
+ * @throws IOException if failed to access the checks data
+ */
+ String getETag(Project.NameKey projectName, Change.Id changeId) throws IOException;
+
@AutoValue
abstract class GetCheckOptions {
public static GetCheckOptions defaults() {
diff --git a/java/com/google/gerrit/plugins/checks/ChecksETagComputation.java b/java/com/google/gerrit/plugins/checks/ChecksETagComputation.java
new file mode 100644
index 0000000..ff66eff
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/ChecksETagComputation.java
@@ -0,0 +1,54 @@
+// Copyright (C) 2019 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.plugins.checks;
+
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.change.ChangeETagComputation;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+
+/**
+ * Ensures that the ETag of a change changes when there was any check update for the change.
+ *
+ * <p>This prevents callers using ETags from potentially seeing outdated submittability and combined
+ * check state information in {@link com.google.gerrit.extensions.common.ChangeInfo} when a check
+ * for the change was created or updated.
+ */
+@Singleton
+public class ChecksETagComputation implements ChangeETagComputation {
+ private final Checks checks;
+
+ @Inject
+ ChecksETagComputation(Checks checks) {
+ this.checks = checks;
+ }
+
+ @Override
+ public String getETag(Project.NameKey projectName, Change.Id changeId) {
+ try {
+ return checks.getETag(projectName, changeId);
+ } catch (IOException e) {
+ // The change ETag will be invalidated if the checks can be accessed the next time.
+ throw new StorageException(
+ String.format(
+ "Failed to compute ETag for checks of change %s in project %s",
+ changeId, projectName),
+ e);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/plugins/checks/Module.java b/java/com/google/gerrit/plugins/checks/Module.java
index 8440501..140497b 100644
--- a/java/com/google/gerrit/plugins/checks/Module.java
+++ b/java/com/google/gerrit/plugins/checks/Module.java
@@ -28,6 +28,7 @@
import com.google.gerrit.plugins.checks.rules.ChecksSubmitRule;
import com.google.gerrit.server.DynamicOptions;
import com.google.gerrit.server.change.ChangeAttributeFactory;
+import com.google.gerrit.server.change.ChangeETagComputation;
import com.google.gerrit.server.git.validators.CommitValidationListener;
import com.google.gerrit.server.git.validators.MergeValidationListener;
import com.google.gerrit.server.git.validators.RefOperationValidationListener;
@@ -56,6 +57,10 @@
.to(CheckerRefOperationValidator.class)
.in(SINGLETON);
+ DynamicSet.bind(binder(), ChangeETagComputation.class)
+ .to(ChecksETagComputation.class)
+ .in(SINGLETON);
+
DynamicSet.bind(binder(), ChangeAttributeFactory.class).to(ChangeCheckAttributeFactory.class);
bind(DynamicOptions.DynamicBean.class)
.annotatedWith(Exports.named(GetChange.class))
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/AbstractCheckersTest.java b/java/com/google/gerrit/plugins/checks/acceptance/AbstractCheckersTest.java
index e476537..51fcc41 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/AbstractCheckersTest.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/AbstractCheckersTest.java
@@ -38,7 +38,7 @@
sysModule = "com.google.gerrit.plugins.checks.acceptance.TestModule",
httpModule = "com.google.gerrit.plugins.checks.HttpModule")
public class AbstractCheckersTest extends LightweightPluginDaemonTest {
- @Inject protected ProjectOperations projectOperations;
+ @Inject private ProjectOperations projectOperations;
protected CheckerOperations checkerOperations;
protected CheckOperations checkOperations;
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/BUILD b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/BUILD
index 62a5299..577bfd4 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/BUILD
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/BUILD
@@ -8,6 +8,7 @@
srcs = glob(["*.java"]),
deps = [
"//java/com/google/gerrit/acceptance:lib",
+ "//java/com/google/gerrit/server/util/time",
"//plugins/checks:checks__plugin",
],
)
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckUpdate.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckUpdate.java
index 90864a6..16b4316 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckUpdate.java
@@ -18,6 +18,7 @@
import com.google.gerrit.acceptance.testsuite.ThrowingConsumer;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.api.CheckState;
+import com.google.gerrit.server.util.time.TimeUtil;
import java.sql.Timestamp;
import java.util.Optional;
@@ -65,6 +66,14 @@
public abstract Builder finished(Timestamp finished);
+ public Builder clearStarted() {
+ return started(TimeUtil.never());
+ }
+
+ public Builder clearFinished() {
+ return finished(TimeUtil.never());
+ }
+
abstract Builder checkUpdater(ThrowingConsumer<TestCheckUpdate> checkUpdate);
abstract TestCheckUpdate autoBuild();
diff --git a/java/com/google/gerrit/plugins/checks/api/ApiModule.java b/java/com/google/gerrit/plugins/checks/api/ApiModule.java
index ad1e7e3..88fb5cf 100644
--- a/java/com/google/gerrit/plugins/checks/api/ApiModule.java
+++ b/java/com/google/gerrit/plugins/checks/api/ApiModule.java
@@ -50,7 +50,7 @@
postOnCollection(CHECK_KIND).to(PostCheck.class);
get(CHECK_KIND).to(GetCheck.class);
post(CHECK_KIND).to(UpdateCheck.class);
-
+ post(CHECK_KIND, "rerun").to(RerunCheck.class);
DynamicMap.mapOf(binder(), PENDING_CHECK_KIND);
}
});
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckApi.java b/java/com/google/gerrit/plugins/checks/api/CheckApi.java
index d137645..14acb41 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckApi.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckApi.java
@@ -26,6 +26,8 @@
/** Updates a check and returns the {@link CheckInfo} for the updated resource. */
CheckInfo update(CheckInput input) throws RestApiException;
+ /** Reruns the check and returns the {@link CheckInfo} for the updated check. */
+ CheckInfo rerun() throws RestApiException;
/**
* A default implementation which allows source compatibility when adding new methods to the
* interface.
@@ -40,5 +42,10 @@
public CheckInfo update(CheckInput input) throws RestApiException {
throw new NotImplementedException();
}
+
+ @Override
+ public CheckInfo rerun() throws RestApiException {
+ throw new NotImplementedException();
+ }
}
}
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckApiImpl.java b/java/com/google/gerrit/plugins/checks/api/CheckApiImpl.java
index 378ce64..3be0a74 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckApiImpl.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckApiImpl.java
@@ -30,19 +30,25 @@
private final GetCheck getCheck;
private final UpdateCheck updateCheck;
private final CheckResource checkResource;
+ private final RerunCheck rerunCheck;
@Inject
- CheckApiImpl(GetCheck getCheck, UpdateCheck updateCheck, @Assisted CheckResource checkResource) {
+ CheckApiImpl(
+ GetCheck getCheck,
+ UpdateCheck updateCheck,
+ @Assisted CheckResource checkResource,
+ RerunCheck rerunCheck) {
this.getCheck = getCheck;
this.updateCheck = updateCheck;
this.checkResource = checkResource;
+ this.rerunCheck = rerunCheck;
}
@Override
public CheckInfo get(ListChecksOption... options) throws RestApiException {
try {
Arrays.stream(options).forEach(getCheck::addOption);
- return getCheck.apply(checkResource);
+ return getCheck.apply(checkResource).value();
} catch (Exception e) {
throw asRestApiException("Cannot retrieve check", e);
}
@@ -51,9 +57,18 @@
@Override
public CheckInfo update(CheckInput input) throws RestApiException {
try {
- return updateCheck.apply(checkResource, input);
+ return updateCheck.apply(checkResource, input).value();
} catch (Exception e) {
throw asRestApiException("Cannot update check", e);
}
}
+
+ @Override
+ public CheckInfo rerun() throws RestApiException {
+ try {
+ return rerunCheck.apply(checkResource, null).value();
+ } catch (Exception e) {
+ throw asRestApiException("Cannot rerun check", e);
+ }
+ }
}
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckInfo.java b/java/com/google/gerrit/plugins/checks/api/CheckInfo.java
index 77cbdef..d0d5c38 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckInfo.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckInfo.java
@@ -56,6 +56,9 @@
/** Blocking conditions that apply to this check. */
public Set<BlockingCondition> blocking;
+ /** Description of the checker that produced this check */
+ public String checkerDescription;
+
@Override
public boolean equals(Object o) {
if (!(o instanceof CheckInfo)) {
@@ -75,7 +78,8 @@
&& Objects.equals(other.updated, updated)
&& Objects.equals(other.checkerName, checkerName)
&& Objects.equals(other.checkerStatus, checkerStatus)
- && Objects.equals(other.blocking, blocking);
+ && Objects.equals(other.blocking, blocking)
+ && Objects.equals(other.checkerDescription, checkerDescription);
}
@Override
@@ -94,7 +98,8 @@
updated,
checkerName,
checkerStatus,
- blocking);
+ blocking,
+ checkerDescription);
}
@Override
@@ -114,6 +119,7 @@
.add("checkerName", checkerName)
.add("checkerStatus", checkerStatus)
.add("blocking", blocking)
+ .add("checkerDescription", checkerDescription)
.toString();
}
}
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckerApiImpl.java b/java/com/google/gerrit/plugins/checks/api/CheckerApiImpl.java
index db74c00..1a63eac 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckerApiImpl.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckerApiImpl.java
@@ -41,7 +41,7 @@
@Override
public CheckerInfo get() throws RestApiException {
try {
- return getChecker.apply(rsrc);
+ return getChecker.apply(rsrc).value();
} catch (Exception e) {
throw asRestApiException("Cannot retrieve checker", e);
}
@@ -50,7 +50,7 @@
@Override
public CheckerInfo update(CheckerInput input) throws RestApiException {
try {
- return updateChecker.apply(rsrc, input);
+ return updateChecker.apply(rsrc, input).value();
} catch (Exception e) {
throw asRestApiException("Cannot update checker", e);
}
diff --git a/java/com/google/gerrit/plugins/checks/api/CheckersImpl.java b/java/com/google/gerrit/plugins/checks/api/CheckersImpl.java
index 73e0f5d..a5d101b 100644
--- a/java/com/google/gerrit/plugins/checks/api/CheckersImpl.java
+++ b/java/com/google/gerrit/plugins/checks/api/CheckersImpl.java
@@ -64,7 +64,7 @@
@Override
public List<CheckerInfo> all() throws RestApiException {
try {
- return listCheckers.apply(TopLevelResource.INSTANCE);
+ return listCheckers.apply(TopLevelResource.INSTANCE).value();
} catch (Exception e) {
throw asRestApiException("Cannot list all checkers ", e);
}
diff --git a/java/com/google/gerrit/plugins/checks/api/ChecksImpl.java b/java/com/google/gerrit/plugins/checks/api/ChecksImpl.java
index 206e7b0..36d1550 100644
--- a/java/com/google/gerrit/plugins/checks/api/ChecksImpl.java
+++ b/java/com/google/gerrit/plugins/checks/api/ChecksImpl.java
@@ -65,7 +65,7 @@
@Override
public CheckApi create(CheckInput input) throws RestApiException {
try {
- CheckInfo checkInfo = postCheck.apply(revisionResource, input);
+ CheckInfo checkInfo = postCheck.apply(revisionResource, input).value();
return id(CheckerUuid.parse(checkInfo.checkerUuid));
} catch (Exception e) {
throw asRestApiException("Cannot create check", e);
@@ -76,7 +76,7 @@
public ImmutableList<CheckInfo> list(ListChecksOption... options) throws RestApiException {
try {
Arrays.stream(options).forEach(listChecks::addOption);
- return listChecks.apply(revisionResource);
+ return listChecks.apply(revisionResource).value();
} catch (Exception e) {
throw asRestApiException("Cannot list checks", e);
}
diff --git a/java/com/google/gerrit/plugins/checks/api/GetCheck.java b/java/com/google/gerrit/plugins/checks/api/GetCheck.java
index ab2abde..2c35307 100644
--- a/java/com/google/gerrit/plugins/checks/api/GetCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/GetCheck.java
@@ -18,6 +18,7 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.plugins.checks.CheckJson;
import com.google.gerrit.plugins.checks.ListChecksOption;
@@ -47,8 +48,8 @@
}
@Override
- public CheckInfo apply(CheckResource resource)
+ public Response<CheckInfo> apply(CheckResource resource)
throws AuthException, BadRequestException, ResourceConflictException, IOException {
- return checkJsonFactory.create(options).format(resource.getCheck());
+ return Response.ok(checkJsonFactory.create(options).format(resource.getCheck()));
}
}
diff --git a/java/com/google/gerrit/plugins/checks/api/GetChecker.java b/java/com/google/gerrit/plugins/checks/api/GetChecker.java
index 9a84a91..28d95fa 100644
--- a/java/com/google/gerrit/plugins/checks/api/GetChecker.java
+++ b/java/com/google/gerrit/plugins/checks/api/GetChecker.java
@@ -14,6 +14,7 @@
package com.google.gerrit.plugins.checks.api;
+import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.plugins.checks.CheckerJson;
import com.google.inject.Inject;
@@ -29,7 +30,7 @@
}
@Override
- public CheckerInfo apply(CheckerResource resource) {
- return checkerJson.format(resource.getChecker());
+ public Response<CheckerInfo> apply(CheckerResource resource) {
+ return Response.ok(checkerJson.format(resource.getChecker()));
}
}
diff --git a/java/com/google/gerrit/plugins/checks/api/ListCheckers.java b/java/com/google/gerrit/plugins/checks/api/ListCheckers.java
index 0328a99..ae36fb9 100644
--- a/java/com/google/gerrit/plugins/checks/api/ListCheckers.java
+++ b/java/com/google/gerrit/plugins/checks/api/ListCheckers.java
@@ -17,6 +17,7 @@
import static java.util.stream.Collectors.toList;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
@@ -55,13 +56,13 @@
}
@Override
- public List<CheckerInfo> apply(TopLevelResource resource)
+ public Response<List<CheckerInfo>> apply(TopLevelResource resource)
throws RestApiException, PermissionBackendException, IOException {
if (!self.get().isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
permissionBackend.currentUser().check(permission);
- return checkers.listCheckers().stream().map(checkerJson::format).collect(toList());
+ return Response.ok(checkers.listCheckers().stream().map(checkerJson::format).collect(toList()));
}
}
diff --git a/java/com/google/gerrit/plugins/checks/api/ListChecks.java b/java/com/google/gerrit/plugins/checks/api/ListChecks.java
index 39346e4..4b5e0ae 100644
--- a/java/com/google/gerrit/plugins/checks/api/ListChecks.java
+++ b/java/com/google/gerrit/plugins/checks/api/ListChecks.java
@@ -20,6 +20,7 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.plugins.checks.Check;
import com.google.gerrit.plugins.checks.CheckJson;
@@ -55,7 +56,7 @@
}
@Override
- public ImmutableList<CheckInfo> apply(RevisionResource resource)
+ public Response<ImmutableList<CheckInfo>> apply(RevisionResource resource)
throws AuthException, BadRequestException, ResourceConflictException, StorageException,
IOException {
if (resource.getEdit().isPresent()) {
@@ -72,6 +73,6 @@
for (Check check : allChecks) {
result.add(checkJson.format(check));
}
- return result.build();
+ return Response.ok(result.build());
}
}
diff --git a/java/com/google/gerrit/plugins/checks/api/PostCheck.java b/java/com/google/gerrit/plugins/checks/api/PostCheck.java
index 18572e9..5531d9e 100644
--- a/java/com/google/gerrit/plugins/checks/api/PostCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/PostCheck.java
@@ -18,6 +18,7 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollectionModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -74,7 +75,7 @@
}
@Override
- public CheckInfo apply(RevisionResource rsrc, CheckInput input)
+ public Response<CheckInfo> apply(RevisionResource rsrc, CheckInput input)
throws StorageException, IOException, RestApiException, PermissionBackendException,
ConfigInvalidException {
if (!self.get().isIdentifiedUser()) {
@@ -112,7 +113,7 @@
} else {
updatedCheck = checksUpdate.get().updateCheck(key, toCheckUpdate(input));
}
- return checkJsonFactory.noOptions().format(updatedCheck);
+ return Response.ok(checkJsonFactory.noOptions().format(updatedCheck));
}
private static CheckUpdate toCheckUpdate(CheckInput input) throws BadRequestException {
diff --git a/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java b/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java
index 281417c..870cd61 100644
--- a/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java
+++ b/java/com/google/gerrit/plugins/checks/api/QueryPendingChecks.java
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
@@ -81,11 +82,11 @@
public List<PendingChecksInfo> apply()
throws RestApiException, IOException, ConfigInvalidException, StorageException {
- return apply(TopLevelResource.INSTANCE);
+ return apply(TopLevelResource.INSTANCE).value();
}
@Override
- public List<PendingChecksInfo> apply(TopLevelResource resource)
+ public Response<List<PendingChecksInfo>> apply(TopLevelResource resource)
throws RestApiException, IOException, ConfigInvalidException, StorageException {
if (queryString == null) {
throw new BadRequestException("query is required");
@@ -98,7 +99,7 @@
Optional<Checker> checker = checkers.getChecker(getCheckerUuidFromQuery(query));
if (!checker.isPresent() || checker.get().isDisabled()) {
- return ImmutableList.of();
+ return Response.ok(ImmutableList.of());
}
// The query system can only match against the current patch set; ignore non-current patch sets
@@ -123,7 +124,7 @@
pendingChecks.add(createPendingChecksInfo(cd.project(), patchSet, checkerUuid, check));
}
}
- return pendingChecks;
+ return Response.ok(pendingChecks);
}
private Predicate<Check> parseQuery(String query) throws BadRequestException {
diff --git a/java/com/google/gerrit/plugins/checks/api/RerunCheck.java b/java/com/google/gerrit/plugins/checks/api/RerunCheck.java
new file mode 100644
index 0000000..4c6b0a4
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/api/RerunCheck.java
@@ -0,0 +1,119 @@
+// Copyright (C) 2019 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.plugins.checks.api;
+
+import com.google.gerrit.extensions.common.Input;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.plugins.checks.AdministrateCheckersPermission;
+import com.google.gerrit.plugins.checks.Check;
+import com.google.gerrit.plugins.checks.CheckJson;
+import com.google.gerrit.plugins.checks.CheckKey;
+import com.google.gerrit.plugins.checks.CheckUpdate;
+import com.google.gerrit.plugins.checks.Checker;
+import com.google.gerrit.plugins.checks.CheckerUuid;
+import com.google.gerrit.plugins.checks.Checkers;
+import com.google.gerrit.plugins.checks.Checks;
+import com.google.gerrit.plugins.checks.Checks.GetCheckOptions;
+import com.google.gerrit.plugins.checks.ChecksUpdate;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.UserInitiated;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.Optional;
+import javax.inject.Provider;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+
+@Singleton
+public class RerunCheck implements RestModifyView<CheckResource, Input> {
+ private final Provider<CurrentUser> self;
+ private final PermissionBackend permissionBackend;
+ private final AdministrateCheckersPermission permission;
+ private final Checks checks;
+ private final Provider<ChecksUpdate> checksUpdate;
+ private final CheckJson.Factory checkJsonFactory;
+ private final Checkers checkers;
+
+ @Inject
+ RerunCheck(
+ Provider<CurrentUser> self,
+ PermissionBackend permissionBackend,
+ AdministrateCheckersPermission permission,
+ Checks checks,
+ @UserInitiated Provider<ChecksUpdate> checksUpdate,
+ CheckJson.Factory checkJsonFactory,
+ Checkers checkers) {
+ this.self = self;
+ this.permissionBackend = permissionBackend;
+ this.permission = permission;
+ this.checks = checks;
+ this.checksUpdate = checksUpdate;
+ this.checkJsonFactory = checkJsonFactory;
+ this.checkers = checkers;
+ }
+
+ @Override
+ public Response<CheckInfo> apply(CheckResource checkResource, Input input)
+ throws RestApiException, IOException, PermissionBackendException, ConfigInvalidException {
+ if (!self.get().isIdentifiedUser()) {
+ throw new AuthException("Authentication required");
+ }
+ permissionBackend.currentUser().check(permission);
+ if (checkResource.getRevisionResource().getEdit().isPresent()) {
+ throw new ResourceConflictException("checks are not supported on a change edit");
+ }
+ CheckKey key =
+ CheckKey.create(
+ checkResource.getRevisionResource().getProject(),
+ checkResource.getRevisionResource().getPatchSet().id(),
+ checkResource.getCheckerUuid());
+ Optional<Check> check = checks.getCheck(key, GetCheckOptions.defaults());
+ CheckerUuid checkerUuid = checkResource.getCheckerUuid();
+ Check updatedCheck;
+ if (!check.isPresent()) {
+ Checker checker =
+ checkers
+ .getChecker(checkerUuid)
+ .orElseThrow(
+ () ->
+ new ResourceNotFoundException(
+ String.format("checker %s not found", checkerUuid)));
+ // This error should not be thrown since this case is filtered before reaching this code.
+ // Also return a backfilled check for checkers that do not apply to the change.
+ updatedCheck =
+ Check.newBackfilledCheck(
+ checkResource.getRevisionResource().getProject(),
+ checkResource.getRevisionResource().getPatchSet(),
+ checker);
+ } else {
+ CheckUpdate.Builder builder = CheckUpdate.builder();
+ builder
+ .setState(CheckState.NOT_STARTED)
+ .unsetFinished()
+ .unsetStarted()
+ .setMessage("")
+ .setUrl("");
+ updatedCheck = checksUpdate.get().updateCheck(key, builder.build());
+ }
+ return Response.ok(checkJsonFactory.noOptions().format(updatedCheck));
+ }
+}
diff --git a/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java b/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
index ef7539f..1a57142 100644
--- a/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
+++ b/java/com/google/gerrit/plugins/checks/api/UpdateCheck.java
@@ -16,6 +16,7 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -34,7 +35,7 @@
}
@Override
- public CheckInfo apply(CheckResource checkResource, CheckInput input)
+ public Response<CheckInfo> apply(CheckResource checkResource, CheckInput input)
throws RestApiException, IOException, StorageException, PermissionBackendException,
ConfigInvalidException {
if (input == null) {
diff --git a/java/com/google/gerrit/plugins/checks/api/UpdateChecker.java b/java/com/google/gerrit/plugins/checks/api/UpdateChecker.java
index 6512b69..1f81284 100644
--- a/java/com/google/gerrit/plugins/checks/api/UpdateChecker.java
+++ b/java/com/google/gerrit/plugins/checks/api/UpdateChecker.java
@@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableSortedSet;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
@@ -68,7 +69,7 @@
}
@Override
- public CheckerInfo apply(CheckerResource resource, CheckerInput input)
+ public Response<CheckerInfo> apply(CheckerResource resource, CheckerInput input)
throws RestApiException, PermissionBackendException, NoSuchCheckerException, IOException,
ConfigInvalidException, StorageException {
permissionBackend.currentUser().check(permission);
@@ -120,7 +121,7 @@
Checker updatedChecker =
checkersUpdate.get().updateChecker(checkerUuid, checkerUpdateBuilder.build());
- return checkerJson.format(updatedChecker);
+ return Response.ok(checkerJson.format(updatedChecker));
}
private Project.NameKey resolveRepository(String repository)
diff --git a/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java b/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java
index 57fda1e..2594a96 100644
--- a/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java
+++ b/java/com/google/gerrit/plugins/checks/db/CheckerConfig.java
@@ -28,7 +28,6 @@
import com.google.gerrit.plugins.checks.Checkers;
import com.google.gerrit.plugins.checks.CheckersUpdate;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.git.meta.VersionedMetaData;
import com.google.gerrit.server.util.time.TimeUtil;
@@ -152,7 +151,8 @@
* @throws IOException if the repository can't be accessed for some reason
* @throws ConfigInvalidException if the checker exists but can't be read due to an invalid format
*/
- public static CheckerConfig loadForChecker(NameKey projectName, Repository repository, Ref ref)
+ public static CheckerConfig loadForChecker(
+ Project.NameKey projectName, Repository repository, Ref ref)
throws IOException, ConfigInvalidException {
CheckerConfig checkerConfig = new CheckerConfig(ref.getName());
checkerConfig.load(projectName, repository);
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java b/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
index cb60d2c..043028a 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbCheck.java
@@ -9,6 +9,7 @@
import com.google.gerrit.plugins.checks.api.CheckState;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.util.time.TimeUtil;
import java.sql.Timestamp;
/** Representation of {@link Check} that can be serialized with GSON. */
@@ -74,11 +75,19 @@
modified = true;
}
if (update.started().isPresent() && !update.started().get().equals(started)) {
- started = update.started().get();
+ if (update.started().get().equals(TimeUtil.never())) {
+ started = null;
+ } else {
+ started = update.started().get();
+ }
modified = true;
}
if (update.finished().isPresent() && !update.finished().get().equals(finished)) {
- finished = update.finished().get();
+ if (update.finished().get().equals(TimeUtil.never())) {
+ finished = null;
+ } else {
+ finished = update.finished().get();
+ }
modified = true;
}
return modified;
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
index 02847e8..1e245fd 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
@@ -26,6 +26,7 @@
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.Checker;
import com.google.gerrit.plugins.checks.CheckerQuery;
+import com.google.gerrit.plugins.checks.CheckerRef;
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.Checkers;
import com.google.gerrit.plugins.checks.Checks;
@@ -33,10 +34,10 @@
import com.google.gerrit.plugins.checks.api.CheckerStatus;
import com.google.gerrit.plugins.checks.api.CombinedCheckState;
import com.google.gerrit.plugins.checks.api.CombinedCheckState.CheckStateCount;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSet.Id;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.Project.NameKey;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -46,6 +47,10 @@
import java.util.Map;
import java.util.Optional;
import java.util.stream.Stream;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
/** Class to read checks from NoteDb. */
@Singleton
@@ -55,6 +60,7 @@
private final Checkers checkers;
private final CheckBackfiller checkBackfiller;
private final Provider<CheckerQuery> checkerQueryProvider;
+ private final GitRepositoryManager repoManager;
@Inject
NoteDbChecks(
@@ -62,12 +68,14 @@
CheckNotes.Factory checkNotesFactory,
Checkers checkers,
CheckBackfiller checkBackfiller,
- Provider<CheckerQuery> checkerQueryProvider) {
+ Provider<CheckerQuery> checkerQueryProvider,
+ GitRepositoryManager repoManager) {
this.changeDataFactory = changeDataFactory;
this.checkNotesFactory = checkNotesFactory;
this.checkers = checkers;
this.checkBackfiller = checkBackfiller;
this.checkerQueryProvider = checkerQueryProvider;
+ this.repoManager = repoManager;
}
@Override
@@ -131,15 +139,15 @@
}
@Override
- public CombinedCheckState getCombinedCheckState(NameKey projectName, Id patchSetId)
- throws IOException, StorageException {
+ public CombinedCheckState getCombinedCheckState(
+ Project.NameKey projectName, PatchSet.Id patchSetId) throws IOException, StorageException {
ImmutableListMultimap<CheckState, Boolean> statesAndRequired =
getStatesAndRequiredMap(projectName, patchSetId);
return CombinedCheckState.combine(statesAndRequired);
}
@Override
- public boolean areAllRequiredCheckersPassing(NameKey projectName, Id patchSetId)
+ public boolean areAllRequiredCheckersPassing(Project.NameKey projectName, PatchSet.Id patchSetId)
throws IOException, StorageException {
ImmutableListMultimap<CheckState, Boolean> statesAndRequired =
getStatesAndRequiredMap(projectName, patchSetId);
@@ -148,8 +156,17 @@
&& checkStateCount.inProgressRequiredCount() == 0;
}
+ @Override
+ public String getETag(Project.NameKey projectName, Change.Id changeId) throws IOException {
+ try (Repository repo = repoManager.openRepository(projectName);
+ RevWalk rw = new RevWalk(repo)) {
+ Ref checkRef = repo.getRefDatabase().exactRef(CheckerRef.checksRef(changeId));
+ return checkRef != null ? checkRef.getObjectId().getName() : ObjectId.zeroId().getName();
+ }
+ }
+
private ImmutableListMultimap<CheckState, Boolean> getStatesAndRequiredMap(
- NameKey projectName, Id patchSetId) throws IOException, StorageException {
+ Project.NameKey projectName, PatchSet.Id patchSetId) throws IOException, StorageException {
ChangeData changeData = changeDataFactory.create(projectName, patchSetId.changeId());
ImmutableMap<String, Checker> allCheckersOfProject =
checkers.checkersOf(projectName).stream()
diff --git a/java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java b/java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java
index bf4fb78..a1df4f5 100644
--- a/java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java
+++ b/java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java
@@ -15,7 +15,6 @@
package com.google.gerrit.plugins.checks.rules;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitRecord.Status;
@@ -26,13 +25,12 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.rules.SubmitRule;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.util.Collection;
+import java.util.Optional;
@Singleton
public class ChecksSubmitRule implements SubmitRule {
@@ -61,7 +59,7 @@
}
@Override
- public Collection<SubmitRecord> evaluate(ChangeData changeData, SubmitRuleOptions options) {
+ public Optional<SubmitRecord> evaluate(ChangeData changeData) {
Project.NameKey project = changeData.project();
Change.Id changeId = changeData.getId();
@@ -72,7 +70,7 @@
String errorMessage =
String.format("failed to load the current patch set of change %s", changeId);
logger.atSevere().withCause(e).log(errorMessage);
- return singletonRecordForRuleError(errorMessage);
+ return recordForRuleError(errorMessage);
}
boolean areAllRequiredCheckersPassing;
@@ -83,24 +81,24 @@
String errorMessage =
String.format("failed to evaluate check states for change %s", changeId);
logger.atSevere().withCause(e).log(errorMessage);
- return singletonRecordForRuleError(errorMessage);
+ return recordForRuleError(errorMessage);
}
SubmitRecord submitRecord = new SubmitRecord();
if (areAllRequiredCheckersPassing) {
submitRecord.status = Status.OK;
- return ImmutableList.of(submitRecord);
+ return Optional.of(submitRecord);
}
submitRecord.status = Status.NOT_READY;
submitRecord.requirements = ImmutableList.of(DEFAULT_SUBMIT_REQUIREMENT_FOR_CHECKS);
- return ImmutableSet.of(submitRecord);
+ return Optional.of(submitRecord);
}
- private static Collection<SubmitRecord> singletonRecordForRuleError(String reason) {
+ private static Optional<SubmitRecord> recordForRuleError(String reason) {
SubmitRecord submitRecord = new SubmitRecord();
submitRecord.errorMessage = reason;
submitRecord.status = SubmitRecord.Status.RULE_ERROR;
- return ImmutableList.of(submitRecord);
+ return Optional.of(submitRecord);
}
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
index ee4b51f..71d468d 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckerRefsIT.java
@@ -24,6 +24,7 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.SkipProjectClone;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.common.ChangeInput;
@@ -51,6 +52,7 @@
@SkipProjectClone
public class CheckerRefsIT extends AbstractCheckersTest {
+ @Inject private ProjectOperations projectOperations;
@Inject private Sequences seq;
@Inject private ChangeInserter.Factory changeInserterFactory;
@Inject private BatchUpdate.Factory updateFactory;
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/ChecksRestApiBindingsIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/ChecksRestApiBindingsIT.java
index 557aab0..03a9e7d 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/ChecksRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/ChecksRestApiBindingsIT.java
@@ -47,7 +47,8 @@
private static final ImmutableList<RestCall> SCOPED_CHECK_ENDPOINTS =
ImmutableList.of(
RestCall.get("/changes/%s/revisions/%s/checks~checks/%s"),
- RestCall.post("/changes/%s/revisions/%s/checks~checks/%s"));
+ RestCall.post("/changes/%s/revisions/%s/checks~checks/%s"),
+ RestCall.post("/changes/%s/revisions/%s/checks~checks/%s/rerun"));
@Test
public void rootEndpoints() throws Exception {
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
index a8a8d19..d16df11 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
@@ -372,6 +372,36 @@
assertThat(response.getEntityContent()).isEqualTo("checks are not supported on a change edit");
}
+ @Test
+ public void creationOfCheckChangesETagOfChange() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ String oldETag = parseChangeResource(patchSetId.changeId().toString()).getETag();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+ input.state = CheckState.RUNNING;
+ checksApiFactory.revision(patchSetId).create(input).get();
+
+ String newETag = parseChangeResource(patchSetId.changeId().toString()).getETag();
+ assertThat(newETag).isNotEqualTo(oldETag);
+ }
+
+ @Test
+ public void creationOfCheckChangesETagOfRevisionActions() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ String oldETag = gApi.changes().id(patchSetId.changeId().toString()).current().etag();
+
+ CheckInput input = new CheckInput();
+ input.checkerUuid = checkerUuid.get();
+ input.state = CheckState.RUNNING;
+ checksApiFactory.revision(patchSetId).create(input).get();
+
+ String newETag = gApi.changes().id(patchSetId.changeId().toString()).current().etag();
+ assertThat(newETag).isNotEqualTo(oldETag);
+ }
+
// TODO(gerrit-team) More tests, especially for multiple checkers and PS and how commits behave
private Check getCheck(Project.NameKey project, PatchSet.Id patchSetId, CheckerUuid checkerUuid)
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
index 9e2a066..ecd2596 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
@@ -89,7 +89,12 @@
@Test
public void getCheckWithOptions() throws Exception {
CheckerUuid checkerUuid =
- checkerOperations.newChecker().repository(project).name("My Checker").create();
+ checkerOperations
+ .newChecker()
+ .repository(project)
+ .name("My Checker")
+ .description("Description")
+ .create();
CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(checkKey).state(CheckState.RUNNING).upsert();
@@ -99,7 +104,7 @@
expectedCheckInfo.checkerName = "My Checker";
expectedCheckInfo.checkerStatus = CheckerStatus.ENABLED;
expectedCheckInfo.blocking = ImmutableSortedSet.of();
-
+ expectedCheckInfo.checkerDescription = "Description";
assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER))
.isEqualTo(expectedCheckInfo);
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
index a19dee2..3f1f132 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
@@ -320,7 +320,7 @@
@Test
public void administrateCheckersCapabilityIsAdvertised() throws Exception {
- Map<String, CapabilityInfo> capabilities = listCapabilities.apply(new ConfigResource());
+ Map<String, CapabilityInfo> capabilities = listCapabilities.apply(new ConfigResource()).value();
String capability = "checks-administrateCheckers";
assertThat(capabilities).containsKey(capability);
CapabilityInfo info = capabilities.get(capability);
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
index 5bfbee0..2727203 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
@@ -23,6 +23,7 @@
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -50,6 +51,7 @@
import org.junit.Test;
public class QueryPendingChecksIT extends AbstractCheckersTest {
+ @Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
private PatchSet.Id patchSetId;
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/RerunCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/RerunCheckIT.java
new file mode 100644
index 0000000..9cc539f
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/RerunCheckIT.java
@@ -0,0 +1,150 @@
+// Copyright (C) 2019 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.plugins.checks.acceptance.api;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.plugins.checks.CheckKey;
+import com.google.gerrit.plugins.checks.CheckerUuid;
+import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
+import com.google.gerrit.plugins.checks.api.CheckInfo;
+import com.google.gerrit.plugins.checks.api.CheckState;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.util.time.TimeUtil;
+import com.google.gerrit.testing.TestTimeUtil;
+import com.google.inject.Inject;
+import java.sql.Timestamp;
+import java.time.Instant;
+import java.util.concurrent.TimeUnit;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class RerunCheckIT extends AbstractCheckersTest {
+ @Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private ProjectOperations projectOperations;
+
+ private PatchSet.Id patchSetId;
+ private CheckKey checkKey;
+
+ @Before
+ public void setUp() throws Exception {
+ TestTimeUtil.resetWithClockStep(1, TimeUnit.SECONDS);
+ TestTimeUtil.setClock(Timestamp.from(Instant.EPOCH));
+
+ patchSetId = createChange().getPatchSetId();
+
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ checkKey = CheckKey.create(project, patchSetId, checkerUuid);
+ }
+
+ @After
+ public void resetTime() {
+ TestTimeUtil.useSystemTime();
+ }
+
+ @Test
+ public void rerunResetsCheckInfo() throws Exception {
+ checkOperations
+ .newCheck(checkKey)
+ .state(CheckState.FAILED)
+ .started(TimeUtil.nowTs())
+ .finished(TimeUtil.nowTs())
+ .message("message")
+ .url("url.com")
+ .upsert();
+ Timestamp created = checkOperations.check(checkKey).get().created();
+ Timestamp updated = checkOperations.check(checkKey).get().updated();
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).rerun();
+ assertThat(info.state).isEqualTo(CheckState.NOT_STARTED);
+ assertThat(info.message).isEqualTo(null);
+ assertThat(info.url).isEqualTo(null);
+ assertThat(info.started).isEqualTo(null);
+ assertThat(info.finished).isEqualTo(null);
+ assertThat(info.created).isEqualTo(created);
+ assertThat(info.updated).isGreaterThan(updated);
+ }
+
+ @Test
+ public void rerunNotStartedCheck() throws Exception {
+ checkOperations.newCheck(checkKey).state(CheckState.NOT_STARTED).upsert();
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).rerun();
+ assertThat(info.state).isEqualTo(CheckState.NOT_STARTED);
+ }
+
+ @Test
+ public void rerunFinishedCheck() throws Exception {
+ checkOperations.newCheck(checkKey).state(CheckState.SUCCESSFUL).upsert();
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).rerun();
+ assertThat(info.state).isEqualTo(CheckState.NOT_STARTED);
+ assertThat(info.updated).isGreaterThan(info.created);
+ }
+
+ @Test
+ public void rerunCheckNotExistingButBackfilled() throws Exception {
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).rerun();
+ assertThat(info.state).isEqualTo(CheckState.NOT_STARTED);
+ assertThat(checkOperations.check(checkKey).exists()).isFalse();
+ }
+
+ @Test
+ public void rerunExistingCheckWithCheckerNotAppliedToChange() throws Exception {
+ Project.NameKey otherProject = projectOperations.newProject().create();
+ checkerOperations.checker(checkKey.checkerUuid()).forUpdate().repository(otherProject).update();
+ checkOperations.newCheck(checkKey).upsert();
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).rerun();
+ assertThat(info.state).isEqualTo(CheckState.NOT_STARTED);
+ }
+
+ @Test
+ public void rerunNonExistingCheckWithCheckerNotAppliedToChange() throws Exception {
+ Project.NameKey otherProject = projectOperations.newProject().create();
+ checkerOperations.checker(checkKey.checkerUuid()).forUpdate().repository(otherProject).update();
+ assertThrows(
+ ResourceNotFoundException.class,
+ () -> checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).rerun());
+ assertThat(checkOperations.check(checkKey).exists()).isFalse();
+ }
+
+ @Test
+ public void cannotUpdateCheckWithoutAdministrateCheckers() throws Exception {
+ requestScopeOperations.setApiUser(user.id());
+ checkOperations.newCheck(checkKey).state(CheckState.SUCCESSFUL).upsert();
+
+ AuthException thrown =
+ assertThrows(
+ AuthException.class,
+ () -> checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).rerun());
+ assertThat(thrown).hasMessageThat().contains("not permitted");
+ }
+
+ @Test
+ public void cannotUpdateCheckAnonymously() throws Exception {
+ requestScopeOperations.setApiUserAnonymous();
+ checkOperations.newCheck(checkKey).state(CheckState.SUCCESSFUL).upsert();
+
+ AuthException thrown =
+ assertThrows(
+ AuthException.class,
+ () -> checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).rerun());
+ assertThat(thrown).hasMessageThat().contains("Authentication required");
+ }
+}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
index 6d15caa..7cc2ce8 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
@@ -17,9 +17,11 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.plugins.checks.CheckKey;
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
@@ -42,6 +44,7 @@
public class UpdateCheckIT extends AbstractCheckersTest {
@Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private ProjectOperations projectOperations;
private PatchSet.Id patchSetId;
private CheckKey checkKey;
@@ -156,6 +159,16 @@
}
@Test
+ public void unsetStarted() throws Exception {
+ checkOperations.check(checkKey).forUpdate().started(TimeUtil.nowTs()).upsert();
+
+ CheckInput input = new CheckInput();
+ input.started = TimeUtil.never();
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ assertThat(info.started).isNull();
+ }
+
+ @Test
public void updateFinished() throws Exception {
CheckInput input = new CheckInput();
input.finished = TimeUtil.nowTs();
@@ -165,6 +178,16 @@
}
@Test
+ public void unsetFinished() throws Exception {
+ checkOperations.check(checkKey).forUpdate().finished(TimeUtil.nowTs()).upsert();
+
+ CheckInput input = new CheckInput();
+ input.finished = TimeUtil.never();
+ CheckInfo info = checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+ assertThat(info.finished).isNull();
+ }
+
+ @Test
public void updateWithEmptyInput() throws Exception {
assertThat(
checksApiFactory
@@ -207,8 +230,9 @@
}
@Test
- public void canUpdateCheckForCheckerThatDoesNotApplyToTheProject() throws Exception {
- Project.NameKey otherProject = createProjectOverAPI("other", null, true, null);
+ public void canUpdateCheckForCheckerThatDoesNotApplyToTheProjectAndCheckExists()
+ throws Exception {
+ Project.NameKey otherProject = projectOperations.newProject().create();
checkerOperations.checker(checkKey.checkerUuid()).forUpdate().repository(otherProject).update();
CheckInput input = new CheckInput();
@@ -219,6 +243,22 @@
}
@Test
+ public void
+ throwExceptionForUpdateCheckForCheckerThatDoesNotApplyToTheProjectAndCheckDoesNotExist()
+ throws Exception {
+ Project.NameKey otherProject = projectOperations.newProject().create();
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(otherProject).create();
+ CheckKey checkKey = CheckKey.create(otherProject, patchSetId, checkerUuid);
+ assertThrows(
+ ResourceNotFoundException.class,
+ () ->
+ checksApiFactory
+ .revision(patchSetId)
+ .id(checkKey.checkerUuid())
+ .update(new CheckInput()));
+ }
+
+ @Test
public void canUpdateCheckForCheckerWithUnsupportedOperatorInQuery() throws Exception {
checkerOperations
.checker(checkKey.checkerUuid())
@@ -360,4 +400,56 @@
assertThat(info.message).isEqualTo("some message");
assertThat(info.url).isEqualTo("https://www.example.com");
}
+
+ @Test
+ public void updateOfCheckChangesETagOfChange() throws Exception {
+ String oldETag = parseChangeResource(patchSetId.changeId().toString()).getETag();
+
+ CheckInput input = new CheckInput();
+ input.state = CheckState.FAILED;
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+
+ String newETag = parseChangeResource(patchSetId.changeId().toString()).getETag();
+ assertThat(newETag).isNotEqualTo(oldETag);
+ }
+
+ @Test
+ public void noOpUpdateOfCheckDoesNotChangeETagOfChange() throws Exception {
+ CheckInput input = new CheckInput();
+ input.state = CheckState.FAILED;
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+
+ String oldETag = parseChangeResource(patchSetId.changeId().toString()).getETag();
+
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+
+ String newETag = parseChangeResource(patchSetId.changeId().toString()).getETag();
+ assertThat(newETag).isEqualTo(oldETag);
+ }
+
+ @Test
+ public void updateOfCheckChangesETagOfRevisionActions() throws Exception {
+ String oldETag = gApi.changes().id(patchSetId.changeId().toString()).current().etag();
+
+ CheckInput input = new CheckInput();
+ input.state = CheckState.FAILED;
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+
+ String newETag = gApi.changes().id(patchSetId.changeId().toString()).current().etag();
+ assertThat(newETag).isNotEqualTo(oldETag);
+ }
+
+ @Test
+ public void noOpUpdateOfCheckDoesNotChangeETagOfRevisionActions() throws Exception {
+ CheckInput input = new CheckInput();
+ input.state = CheckState.FAILED;
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+
+ String oldETag = gApi.changes().id(patchSetId.changeId().toString()).current().etag();
+
+ checksApiFactory.revision(patchSetId).id(checkKey.checkerUuid()).update(input);
+
+ String newETag = gApi.changes().id(patchSetId.changeId().toString()).current().etag();
+ assertThat(newETag).isEqualTo(oldETag);
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
index 4e63bde..6458099 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckOperationsImplTest.java
@@ -476,6 +476,18 @@
}
@Test
+ public void startedCanBeCleared() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ CheckKey checkKey = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
+ checkOperations.newCheck(checkKey).started(TimeUtil.nowTs()).upsert();
+
+ checkOperations.check(checkKey).forUpdate().clearStarted().upsert();
+
+ Optional<Timestamp> currentStarted = checkOperations.check(checkKey).get().started();
+ assertThat(currentStarted).isEmpty();
+ }
+
+ @Test
public void finishedCanBeUpdated() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
CheckKey checkKey = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
@@ -489,6 +501,18 @@
}
@Test
+ public void finishedCanBeCleared() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+ CheckKey checkKey = CheckKey.create(project, createChange().getPatchSetId(), checkerUuid);
+ checkOperations.newCheck(checkKey).finished(TimeUtil.nowTs()).upsert();
+
+ checkOperations.check(checkKey).forUpdate().clearFinished().upsert();
+
+ Optional<Timestamp> currentFinished = checkOperations.check(checkKey).get().finished();
+ assertThat(currentFinished).isEmpty();
+ }
+
+ @Test
public void getNotesAsText() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
PatchSet.Id patchSetId = createChange().getPatchSetId();
diff --git a/javatests/com/google/gerrit/plugins/checks/rules/BUILD b/javatests/com/google/gerrit/plugins/checks/rules/BUILD
index 17fdb2a..2e76709 100644
--- a/javatests/com/google/gerrit/plugins/checks/rules/BUILD
+++ b/javatests/com/google/gerrit/plugins/checks/rules/BUILD
@@ -14,6 +14,7 @@
"//lib:guava",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/truth",
+ "//lib/truth:truth-java8-extension",
"//plugins/checks:checks__plugin",
],
)
diff --git a/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java b/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java
index afdda35..2d75e6b 100644
--- a/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java
@@ -15,22 +15,21 @@
package com.google.gerrit.plugins.checks.rules;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import static org.easymock.EasyMock.anyObject;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
-import com.google.common.collect.Iterables;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.plugins.checks.Checks;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.util.time.TimeUtil;
import java.io.IOException;
-import java.util.Collection;
+import java.util.Optional;
import org.easymock.EasyMock;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
@@ -47,8 +46,7 @@
expect(cd.currentPatchSet()).andThrow(new IllegalStateException("Fail for test"));
replay(cd);
- Collection<SubmitRecord> submitRecords =
- checksSubmitRule.evaluate(cd, SubmitRuleOptions.defaults());
+ Optional<SubmitRecord> submitRecords = checksSubmitRule.evaluate(cd);
assertErrorRecord(submitRecords, "failed to load the current patch set of change 1");
}
@@ -75,17 +73,15 @@
.build());
replay(cd);
- Collection<SubmitRecord> submitRecords =
- checksSubmitRule.evaluate(cd, SubmitRuleOptions.defaults());
+ Optional<SubmitRecord> submitRecords = checksSubmitRule.evaluate(cd);
assertErrorRecord(submitRecords, "failed to evaluate check states for change 1");
}
private static void assertErrorRecord(
- Collection<SubmitRecord> submitRecords, String expectedErrorMessage) {
- assertThat(submitRecords).hasSize(1);
+ Optional<SubmitRecord> submitRecord, String expectedErrorMessage) {
+ assertThat(submitRecord).isPresent();
- SubmitRecord submitRecord = Iterables.getOnlyElement(submitRecords);
- assertThat(submitRecord.status).isEqualTo(SubmitRecord.Status.RULE_ERROR);
- assertThat(submitRecord.errorMessage).isEqualTo(expectedErrorMessage);
+ assertThat(submitRecord.get().status).isEqualTo(SubmitRecord.Status.RULE_ERROR);
+ assertThat(submitRecord.get().errorMessage).isEqualTo(expectedErrorMessage);
}
}
diff --git a/resources/Documentation/rest-api-checks.md b/resources/Documentation/rest-api-checks.md
index b34ed31..4395644 100644
--- a/resources/Documentation/rest-api-checks.md
+++ b/resources/Documentation/rest-api-checks.md
@@ -1,4 +1,4 @@
-# /changes/<id>/revisions/<id>/checks/ REST API
+# /changes/`id`/revisions/`id`/checks/ REST API
This page describes the check-related REST endpoints that are added by the
@PLUGIN@ plugin.
@@ -143,6 +143,7 @@
### <a id="update-check"> Update Check
_'POST /changes/1/revisions/1/checks/'_
+
_'POST /changes/1/revisions/1/checks/test:my-checker'_
Updates a check. The semantics are the same as for [CreateCheck](#create-check).
@@ -156,27 +157,39 @@
the URL, it must either match the value provided in the request body via
[CheckInput](#check-input) or the value in the request body is omitted.
+### <a id="rerun-check"> Rerun Check
+
+_'POST /changes/1/revisions/1/checks/test:my-checker/rerun'_
+
+Reruns a check. As response the [CheckInfo](#check-info) entity is returned that
+describes the created check.
+
+
+This REST endpoint supports rerunning a check. It also resets all relevant check
+fields such as `message`, `url`, `started` and `finished`.
+
## <a id="json-entities"> JSON Entities
### <a id="check-info"> CheckInfo
The `CheckInfo` entity describes a check.
-| Field Name | | Description |
-| ----------------- | -------- | ----------- |
-| `repository` | | The repository name that this check applies to.
-| `change_number` | | The change number that this check applies to.
-| `patch_set_id` | | The patch set that this check applies to.
-| `checker_uuid` | | The [UUID](./rest-api-checkers.md#checker-id) of the checker that reported this check.
-| `state` | | The state as string-serialized form of [CheckState](#check-state)
-| `message` | optional | Short message explaining the check state.
-| `url` | optional | A fully-qualified URL pointing to the result of the check on the checker's infrastructure.
-| `started` | optional | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check started processing.
-| `finished` | optional | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check finished processing.
-| `created` | | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check was created.
-| `updated` | | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check was last updated.
-| `checker_name` | optional | The name of the checker that produced this check.<br />Only set if [checker details](#option-checker) are requested.
-| `checker_status` | optional | The [status](rest-api-checkers.md#checker-info) of the checker that produced this check.<br />Only set if [checker details](#option-checker) are requested.
-| `blocking` | optional | Set of [blocking conditions](rest-api-checkers.md#blocking-conditions) that apply to this checker.<br />Only set if [checker details](#option-checker) are requested.
+| Field Name | | Description |
+| --------------------- | -------- | ----------- |
+| `repository` | | The repository name that this check applies to.
+| `change_number` | | The change number that this check applies to.
+| `patch_set_id` | | The patch set that this check applies to.
+| `checker_uuid` | | The [UUID](./rest-api-checkers.md#checker-id) of the checker that reported this check.
+| `state` | | The state as string-serialized form of [CheckState](#check-state)
+| `message` | optional | Short message explaining the check state.
+| `url` | optional | A fully-qualified URL pointing to the result of the check on the checker's infrastructure.
+| `started` | optional | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check started processing.
+| `finished` | optional | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check finished processing.
+| `created` | | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check was created.
+| `updated` | | The [timestamp](../../../Documentation/rest-api.html#timestamp) of when the check was last updated.
+| `checker_name` | optional | The name of the checker that produced this check.<br />Only set if [checker details](#option-checker) are requested.
+| `checker_status` | optional | The [status](rest-api-checkers.md#checker-info) of the checker that produced this check.<br />Only set if [checker details](#option-checker) are requested.
+| `blocking` | optional | Set of [blocking conditions](rest-api-checkers.md#blocking-conditions) that apply to this checker.<br />Only set if [checker details](#option-checker) are requested.
+| `checker_description` | optional | The description of the checker that reported this check.
### <a id="check-input"> CheckInput
The `CheckInput` entity contains information for creating or updating a check.
