Rename 'commit_parent' in 'commits' table to 'commit_previous_commit'.
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Feb 2017 07:17:48 +0000 (07:17 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Feb 2017 07:17:48 +0000 (07:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=168816

Reviewed by Ryosuke Niwa.

Rename 'commit_parent' to avoid ambiguity in the coming feature.
For exisiting database, run
    "ALTER TABLE commits RENAME commit_parent TO commit_previous_commit;"
to update the database.

* init-database.sql:
* public/api/report-commits.php:
* public/include/commit-log-fetcher.php:
* server-tests/api-commits.js:
(then):
* server-tests/api-report-commits-tests.js:
(then):
* tools/sync-commits.py:
(main):
(Repository.fetch_commits_and_submit):
(GitRepository._revision_from_tokens):
* unit-tests/analysis-task-tests.js:
(sampleAnalysisTask):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@212947 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/init-database.sql
Websites/perf.webkit.org/public/api/report-commits.php
Websites/perf.webkit.org/public/include/commit-log-fetcher.php
Websites/perf.webkit.org/server-tests/api-commits.js
Websites/perf.webkit.org/server-tests/api-report-commits-tests.js
Websites/perf.webkit.org/tools/sync-commits.py
Websites/perf.webkit.org/unit-tests/analysis-task-tests.js

index 6967e13..204b86f 100644 (file)
@@ -1,3 +1,29 @@
+2017-02-23  Dewei Zhu  <dewei_zhu@apple.com>
+
+        Rename 'commit_parent' in 'commits' table to 'commit_previous_commit'.
+        https://bugs.webkit.org/show_bug.cgi?id=168816
+
+        Reviewed by Ryosuke Niwa.
+
+        Rename 'commit_parent' to avoid ambiguity in the coming feature.
+        For exisiting database, run
+            "ALTER TABLE commits RENAME commit_parent TO commit_previous_commit;"
+        to update the database.
+
+        * init-database.sql:
+        * public/api/report-commits.php:
+        * public/include/commit-log-fetcher.php:
+        * server-tests/api-commits.js:
+        (then):
+        * server-tests/api-report-commits-tests.js:
+        (then):
+        * tools/sync-commits.py:
+        (main):
+        (Repository.fetch_commits_and_submit):
+        (GitRepository._revision_from_tokens):
+        * unit-tests/analysis-task-tests.js:
+        (sampleAnalysisTask):
+
 2017-02-23  Ryosuke Niwa  <rniwa@webkit.org>
 
         New sampling algorithm shows very few points when zoomed out
index 2b56ce2..5a3dfcf 100644 (file)
@@ -88,7 +88,7 @@ CREATE TABLE commits (
     commit_id serial PRIMARY KEY,
     commit_repository integer NOT NULL REFERENCES repositories ON DELETE CASCADE,
     commit_revision varchar(64) NOT NULL,
-    commit_parent integer REFERENCES commits ON DELETE CASCADE,
+    commit_previous_commit integer REFERENCES commits ON DELETE CASCADE,
     commit_time timestamp,
     commit_order integer,
     commit_committer integer REFERENCES committers ON DELETE CASCADE,
@@ -261,5 +261,5 @@ CREATE TABLE build_requests (
     request_build integer REFERENCES builds,
     request_created_at timestamp NOT NULL DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC'),
     CONSTRAINT build_request_order_must_be_unique_in_group UNIQUE(request_group, request_order));
-CREATE INDEX build_request_triggerable ON build_requests(request_triggerable);    
+CREATE INDEX build_request_triggerable ON build_requests(request_triggerable);
 CREATE INDEX build_request_build ON build_requests(request_build);
index 0f2f9b7..fb2cab4 100644 (file)
@@ -47,21 +47,21 @@ function main($post_data) {
             }
         }
 
-        $parent_revision = array_get($commit_info, 'parent');
-        $parent_id = NULL;
-        if ($parent_revision) {
-            $parent_commit = $db->select_first_row('commits', 'commit', array('repository' => $repository_id, 'revision' => $parent_revision));
-            if (!$parent_commit) {
+        $previous_commit_revision = array_get($commit_info, 'previousCommit');
+        $previous_commit_id = NULL;
+        if ($previous_commit_revision) {
+            $previous_commit = $db->select_first_row('commits', 'commit', array('repository' => $repository_id, 'revision' => $previous_commit_revision));
+            if (!$previous_commit) {
                 $db->rollback_transaction();
-                exit_with_error('FailedToFindParentCommit', array('commit' => $commit_info));
+                exit_with_error('FailedToFindPreviousCommit', array('commit' => $commit_info));
             }
-            $parent_id = $parent_commit['commit_id'];
+            $previous_commit_id = $previous_commit['commit_id'];
         }
 
         $data = array(
             'repository' => $repository_id,
             'revision' => $commit_info['revision'],
-            'parent' => $parent_id,
+            'previous_commit' => $previous_commit_id,
             'order' => array_get($commit_info, 'order'),
             'time' => array_get($commit_info, 'time'),
             'committer' => $committer_id,
index 8e710b6..0b76d24 100644 (file)
@@ -36,7 +36,7 @@ class CommitLogFetcher {
     function fetch_between($repository_id, $first, $second, $keyword = NULL) {
         $statements = 'SELECT commit_id as "id",
             commit_revision as "revision",
-            commit_parent as "parent",
+            commit_previous_commit as "previousCommit",
             commit_time as "time",
             committer_name as "authorName",
             committer_account as "authorEmail",
@@ -121,7 +121,7 @@ class CommitLogFetcher {
         return array(
             'id' => $commit_row['commit_id'],
             'revision' => $commit_row['commit_revision'],
-            'parent' => $commit_row['commit_parent'],
+            'previousCommit' => $commit_row['commit_previous_commit'],
             'time' => Database::to_js_time($commit_row['commit_time']),
             'order' => $commit_row['commit_order'],
             'authorName' => $committer_row ? $committer_row['committer_name'] : null,
@@ -131,4 +131,4 @@ class CommitLogFetcher {
     }
 }
 
-?>
\ No newline at end of file
+?>
index a914653..a42c60d 100644 (file)
@@ -30,7 +30,7 @@ describe("/api/commits/", () => {
             },
             {
                 "repository": "WebKit",
-                "parent": "210949",
+                "previousCommit": "210949",
                 "revision": "210950",
                 "time": "2017-01-20T03:49:37.887Z",
                 "author": {"name": "Commit Queue", "account": "commit-queue@webkit.org"},
@@ -51,6 +51,11 @@ describe("/api/commits/", () => {
         assert.equal(commit['message'], submitted['message']);
         assert.equal(commit['authorName'], submitted['author']['name']);
         assert.equal(commit['authorEmail'], submitted['author']['account']);
+        if(submitted['previousCommit']) {
+            assert.ok(commit['previousCommit']);
+        } else {
+            assert.equal(commit['previousCommit'], null);
+        }
     }
 
     describe('/api/commits/<repository>/', () => {
@@ -88,8 +93,9 @@ describe("/api/commits/", () => {
                 assertCommitIsSameAsOneSubmitted(commits[0], submittedCommits[0]);
                 assertCommitIsSameAsOneSubmitted(commits[1], submittedCommits[1]);
                 assertCommitIsSameAsOneSubmitted(commits[2], submittedCommits[2]);
+                assert.equal(commits[2]['previousCommit'], commits[1]['id']);
             });
-        });        
+        });
     });
 
     describe('/api/commits/<repository>/oldest', () => {
@@ -230,7 +236,7 @@ describe("/api/commits/", () => {
                 assert.equal(result['status'], 'OK');
                 assert.equal(result['commits'].length, 1);
                 assertCommitIsSameAsOneSubmitted(result['commits'][0], {
-                    parent: null,
+                    previousCommit: null,
                     revision: '210950',
                     time: '2017-01-20T03:49:37.887Z',
                     author: {name: null, account: null},
@@ -299,14 +305,14 @@ describe("/api/commits/", () => {
                 assert.equal(result['status'], 'OK');
                 assert.deepEqual(result['commits'].length, 2);
                 assertCommitIsSameAsOneSubmitted(result['commits'][0], {
-                    parent: null,
+                    previousCommit: null,
                     revision: '210949',
                     time: '2017-01-20T03:23:50.645Z',
                     author: {name: null, account: null},
                     message: null,
                 });
                 assertCommitIsSameAsOneSubmitted(result['commits'][1], {
-                    parent: null,
+                    previousCommit: null,
                     revision: '210950',
                     time: '2017-01-20T03:49:37.887Z',
                     author: {name: null, account: null},
index bc07817..93d1ad9 100644 (file)
@@ -54,13 +54,28 @@ describe("/api/report-commits/", function () {
             },
             {
                 "repository": "WebKit",
-                "parent": "141977",
+                "previousCommit": "141977",
                 "revision": "141978",
                 "time": "2013-02-06T09:54:56.0Z",
                 "author": {"name": "Mikhail Pozdnyakov", "account": "mikhail.pozdnyakov@intel.com"},
                 "message": "another message",
             }
         ]
+    };
+
+    const subversionInvalidPreviousCommit = {
+        "slaveName": "someSlave",
+        "slavePassword": "somePassword",
+        "commits": [
+            {
+                "repository": "WebKit",
+                "previousCommit": "99999",
+                "revision": "12345",
+                "time": "2013-02-06T08:55:20.9Z",
+                "author": {"name": "Commit Queue", "account": "commit-queue@webkit.org"},
+                "message": "some message",
+            }
+        ]
     }
 
     it("should reject error when slave name is missing", function (done) {
@@ -158,6 +173,7 @@ describe("/api/report-commits/", function () {
             assert.equal(commits[0]['time'].toString(), new Date('2013-02-06 08:55:20.9').toString());
             assert.equal(commits[0]['message'], reportedData['message']);
             assert.equal(commits[0]['committer'], committers[0]['id']);
+            assert.equal(commits[0]['previous_commit'], null);
             assert.equal(committers[0]['name'], reportedData['author']['name']);
             assert.equal(committers[0]['account'], reportedData['author']['account']);
 
@@ -166,6 +182,7 @@ describe("/api/report-commits/", function () {
             assert.equal(commits[1]['time'].toString(), new Date('2013-02-06 09:54:56.0').toString());
             assert.equal(commits[1]['message'], reportedData['message']);
             assert.equal(commits[1]['committer'], committers[1]['id']);
+            assert.equal(commits[1]['previous_commit'], commits[0]['id']);
             assert.equal(committers[1]['name'], reportedData['author']['name']);
             assert.equal(committers[1]['account'], reportedData['author']['account']);
 
@@ -173,6 +190,19 @@ describe("/api/report-commits/", function () {
         }).catch(done);
     });
 
+    it("should fail if previous commit is invalid", function (done) {
+        const db = TestServer.database();
+        addSlaveForReport(subversionInvalidPreviousCommit).then(function () {
+            return TestServer.remoteAPI().postJSON('/api/report-commits/', subversionInvalidPreviousCommit);
+        }).then(function (response) {
+            assert.equal(response['status'], 'FailedToFindPreviousCommit');
+            return db.selectAll('commits');
+        }).then(function (result) {
+            assert.equal(result.length, 0);
+            done();
+        }).catch(done);
+    });
+
     it("should update an existing commit if there is one", function (done) {
         const db = TestServer.database();
         const reportedData = subversionCommit.commits[0];
@@ -227,7 +257,7 @@ describe("/api/report-commits/", function () {
             assert.equal(commits[0]['committer'], committers[0]['id']);
             assert.equal(committers[0]['name'], firstData['author']['name']);
             assert.equal(committers[0]['account'], firstData['author']['account']);
-            
+
             assert.equal(commits[1]['id'], 3);
             assert.equal(commits[1]['message'], null);
             assert.equal(commits[1]['committer'], null);
index 5b5ded0..a96ea23 100755 (executable)
@@ -23,7 +23,7 @@ def main(argv):
     parser.add_argument('--server-config-json', required=True, help='The path to a JSON file that specifies the perf dashboard')
     parser.add_argument('--seconds-to-sleep', type=float, default=900, help='The seconds to sleep between iterations')
     parser.add_argument('--max-fetch-count', type=int, default=10, help='The number of commits to fetch at once')
-    parser.add_argument('--max-ancestor-fetch-count', type=int, default=100, help='The number of commits to fetch at once if some commits are missing parents')
+    parser.add_argument('--max-ancestor-fetch-count', type=int, default=100, help='The number of commits to fetch at once if some commits are missing previous commits')
     args = parser.parse_args()
 
     with open(args.repository_config_json) as repository_config_json:
@@ -79,16 +79,16 @@ class Repository(object):
             print "Submitting revisions %s for %s to %s" % (revision_list, self._name, server_config['server']['url'])
 
             result = submit_commits(pending_commits, server_config['server']['url'],
-                server_config['slave']['name'], server_config['slave']['password'], ['OK', 'FailedToFindParentCommit'])
+                server_config['slave']['name'], server_config['slave']['password'], ['OK', 'FailedToFindPreviousCommit'])
 
             if result.get('status') == 'OK':
                 break
 
-            if result.get('status') == 'FailedToFindParentCommit':
-                parent_commit = self.fetch_commit(server_config, result['commit']['parent'])
-                if not parent_commit:
-                    raise Exception('Could not find the parent %s of %s' % (result['commit']['parent'], result['commit']['revision']))
-                pending_commits = [parent_commit] + pending_commits
+            if result.get('status') == 'FailedToFindPreviousCommit':
+                previous_commit = self.fetch_commit(server_config, result['commit']['previousCommit'])
+                if not previous_commit:
+                    raise Exception('Could not find the previous commit %s of %s' % (result['commit']['previousCommit'], result['commit']['revision']))
+                pending_commits = [previous_commit] + pending_commits
 
         if result.get('status') != 'OK':
             raise Exception(result)
@@ -223,7 +223,7 @@ class GitRepository(Repository):
         current_hash = tokens[0]
         commit_time = int(tokens[1])
         author_email = tokens[2]
-        parent_hash = tokens[3] if len(tokens) >= 4 else None
+        previous_hash = tokens[3] if len(tokens) >= 4 else None
 
         author_name = self._run_git_command(['log', current_hash, '-1', '--pretty=%cn'])
         message = self._run_git_command(['log', current_hash, '-1', '--pretty=%B'])
@@ -231,7 +231,7 @@ class GitRepository(Repository):
         return {
             'repository': self._name,
             'revision': current_hash,
-            'parent': parent_hash,
+            'previousCommit': previous_hash,
             'time': datetime.fromtimestamp(commit_time).strftime(r'%Y-%m-%dT%H:%M:%S.%f'),
             'author': {'account': author_email, 'name': author_name},
             'message': message,
index 3a39efb..5b91caa 100644 (file)
@@ -43,7 +43,7 @@ function sampleAnalysisTask()
                 'id': '105975',
                 'message': 'Commit message',
                 'order': null,
-                'parent': null,
+                'previousCommit': null,
                 'repository': '11',
                 'revision': '196051',
                 'time': 1454481246108