Commit should order by 'commit_order' as secondary key.
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 25 Feb 2017 08:58:18 +0000 (08:58 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 25 Feb 2017 08:58:18 +0000 (08:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=168866

Reviewed by Ryosuke Niwa.

Currently, commits are sorted by 'commit_time' only.
We should use 'commit_order' as secondary key when 'commit_time' is equal or null.

* public/include/commit-log-fetcher.php:
* public/include/db.php:
* server-tests/api-commits.js:
(return.addSlaveForReport.subversionCommits.then):
(then):

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/include/commit-log-fetcher.php
Websites/perf.webkit.org/public/include/db.php
Websites/perf.webkit.org/server-tests/api-commits.js

index 0075379f3b345a7176ce4f7bce21e8422f0ef069..be5239b68765028bde4a1bcd72bd1854aa3f55b4 100644 (file)
@@ -1,3 +1,19 @@
+2017-02-25  Dewei Zhu  <dewei_zhu@apple.com>
+
+        Commit should order by 'commit_order' as secondary key.
+        https://bugs.webkit.org/show_bug.cgi?id=168866
+
+        Reviewed by Ryosuke Niwa.
+
+        Currently, commits are sorted by 'commit_time' only.
+        We should use 'commit_order' as secondary key when 'commit_time' is equal or null.
+
+        * public/include/commit-log-fetcher.php:
+        * public/include/db.php:
+        * server-tests/api-commits.js:
+        (return.addSlaveForReport.subversionCommits.then):
+        (then):
+
 2017-02-24  Ryosuke Niwa  <rniwa@webkit.org>
 
         REGRESSION(r212853): Comparisons to baseline no longer shows up
index 0b76d2407de136accbf6ea56bc147f160a55d60f..7472819fa93ded7a85aa78b0115ac1635126dee3 100644 (file)
@@ -84,15 +84,15 @@ class CommitLogFetcher {
     }
 
     function fetch_oldest($repository_id) {
-        return $this->format_single_commit($this->db->select_first_row('commits', 'commit', array('repository' => $repository_id), 'time'));
+        return $this->format_single_commit($this->db->select_first_row('commits', 'commit', array('repository' => $repository_id), array('time', 'order')));
     }
 
     function fetch_latest($repository_id) {
-        return $this->format_single_commit($this->db->select_last_row('commits', 'commit', array('repository' => $repository_id), 'time'));
+        return $this->format_single_commit($this->db->select_last_row('commits', 'commit', array('repository' => $repository_id), array('time', 'order')));
     }
 
     function fetch_last_reported($repository_id) {
-        return $this->format_single_commit($this->db->select_last_row('commits', 'commit', array('repository' => $repository_id, 'reported' => true), 'time'));
+        return $this->format_single_commit($this->db->select_last_row('commits', 'commit', array('repository' => $repository_id, 'reported' => true), array('time', 'order')));
     }
 
     function fetch_revision($repository_id, $revision) {
index 1cc3278196126c7794dd201050d74e3660c9a760..d805d676223ccb845392244a3f7f60ddc4599824 100644 (file)
@@ -180,7 +180,7 @@ class Database
         }
         if (!$rows && $should_insert) {
             $rows = $this->query_and_fetch_all("INSERT INTO $table ($insert_column_names) SELECT $insert_placeholders
-                WHERE NOT EXISTS ($query) RETURNING $returning_column_name", $values);            
+                WHERE NOT EXISTS ($query) RETURNING $returning_column_name", $values);
         }
         if (!$should_update && !$rows)
             $rows = $this->query_and_fetch_all($query, $select_values);
@@ -212,10 +212,16 @@ class Database
             $column_names = $placeholders = '1';
         $query = "SELECT * FROM $table WHERE ($column_names) = ($placeholders)";
         if ($order_by) {
-            assert(ctype_alnum_underscore($order_by));
-            $query .= ' ORDER BY ' . $this->prefixed_name($order_by, $prefix);
-            if ($descending_order)
-                $query .= ' DESC';
+            if (!is_array($order_by))
+                $order_by = array($order_by);
+
+            $order_columns = array();
+            foreach ($order_by as $order_key) {
+                assert(ctype_alnum_underscore($order_key));
+                $order_column = $this->prefixed_name($order_key, $prefix) . ' ' . ($descending_order? 'DESC' : 'ASC');
+                array_push($order_columns, $order_column);
+            }
+            $query .= ' ORDER BY ' . join(', ', $order_columns);
         }
         if ($offset !== NULL)
             $query .= ' OFFSET ' . intval($offset);
index a42c60dc767b880c547508ec4cd85535d39a8b79..4c7c0066d5872e567f495aac98a592584c54443a 100644 (file)
@@ -39,6 +39,43 @@ describe("/api/commits/", () => {
         ]
     }
 
+    const systemVersionCommits = {
+        "slaveName": "someSlave",
+        "slavePassword": "somePassword",
+        "commits": [
+            {
+                "repository": "OSX",
+                "revision": "16D32",
+                "order": 6
+            },
+            {
+                "repository": "OSX",
+                "revision": "16C68",
+                "order": 5
+            },
+            {
+                "repository": "OSX",
+                "revision": "16C67",
+                "order": 4
+            },
+            {
+                "repository": "OSX",
+                "revision": "16B2657",
+                "order": 3
+            },
+            {
+                "repository": "OSX",
+                "revision": "16B2555",
+                "order": 2
+            },
+            {
+                "repository": "OSX",
+                "revision": "16A323",
+                "order": 1
+            }
+        ]
+    }
+
     const notYetReportedCommit = {
         revision: '210951',
         time: '2017-01-20T03:56:20.045Z'
@@ -96,6 +133,26 @@ describe("/api/commits/", () => {
                 assert.equal(commits[2]['previousCommit'], commits[1]['id']);
             });
         });
+
+        it("should return the list of ordered commits for a given repository", () => {
+            return addSlaveForReport(subversionCommits).then(() => {
+                return TestServer.remoteAPI().postJSON('/api/report-commits/', systemVersionCommits);
+            }).then(function (response) {
+                assert.equal(response['status'], 'OK');
+                return TestServer.remoteAPI().getJSON('/api/commits/OSX/');
+            }).then(function (result) {
+                assert.equal(result['status'], 'OK');
+                const commits = result['commits'];
+                const submittedCommits = systemVersionCommits['commits'];
+                assert.equal(commits.length, submittedCommits.length);
+                assert.equal(commits[0]['revision'], submittedCommits[5]['revision']);
+                assert.equal(commits[1]['revision'], submittedCommits[4]['revision']);
+                assert.equal(commits[2]['revision'], submittedCommits[3]['revision']);
+                assert.equal(commits[3]['revision'], submittedCommits[2]['revision']);
+                assert.equal(commits[4]['revision'], submittedCommits[1]['revision']);
+                assert.equal(commits[5]['revision'], submittedCommits[0]['revision']);
+            });
+        });
     });
 
     describe('/api/commits/<repository>/oldest', () => {
@@ -126,6 +183,19 @@ describe("/api/commits/", () => {
                 assertCommitIsSameAsOneSubmitted(result['commits'][0], subversionCommits['commits'][0]);
             });
         });
+
+        it("should return the oldest commit based on 'commit_order' when 'commit_time' is missing", () => {
+            const remote = TestServer.remoteAPI();
+            return addSlaveForReport(systemVersionCommits).then(() => {
+                return remote.postJSONWithStatus('/api/report-commits/', systemVersionCommits);
+            }).then(() => {
+                return remote.getJSON('/api/commits/OSX/oldest');
+            }).then(function (result) {
+                assert.equal(result['status'], 'OK');
+                assert.equal(result['commits'].length, 1);
+                assert.equal(result['commits'][0]['revision'], systemVersionCommits['commits'][5]['revision']);
+            });
+        });
     });
 
     describe('/api/commits/<repository>/latest', () => {
@@ -156,6 +226,19 @@ describe("/api/commits/", () => {
                 assertCommitIsSameAsOneSubmitted(result['commits'][0], subversionCommits['commits'].slice().pop());
             });
         });
+
+        it("should return the latest commit based on 'commit_order' when 'commit_time' is missing", () => {
+            const remote = TestServer.remoteAPI();
+            return addSlaveForReport(systemVersionCommits).then(() => {
+                return remote.postJSONWithStatus('/api/report-commits/', systemVersionCommits);
+            }).then(() => {
+                return remote.getJSON('/api/commits/OSX/latest');
+            }).then(function (result) {
+                assert.equal(result['status'], 'OK');
+                assert.equal(result['commits'].length, 1);
+                assert.equal(result['commits'][0]['revision'], systemVersionCommits['commits'][0]['revision']);
+            });
+        });
     });
 
     describe('/api/commits/<repository>/last-reported', () => {
@@ -204,6 +287,19 @@ describe("/api/commits/", () => {
                 assertCommitIsSameAsOneSubmitted(result['commits'][0], subversionCommits['commits'].slice().pop());
             });
         });
+
+        it("should return the last reported commit based on 'commit_order' when 'commit_time' is missing", () => {
+            const remote = TestServer.remoteAPI();
+            return addSlaveForReport(systemVersionCommits).then(() => {
+                return remote.postJSONWithStatus('/api/report-commits/', systemVersionCommits);
+            }).then(() => {
+                return remote.getJSON('/api/commits/OSX/last-reported');
+            }).then(function (result) {
+                assert.equal(result['status'], 'OK');
+                assert.equal(result['commits'].length, 1);
+                assert.equal(result['commits'][0]['revision'], systemVersionCommits['commits'][0]['revision']);
+            });
+        });
     });
 
     describe('/api/commits/<repository>/<commit>', () => {