Add sub-commit UI in commit log viewer.
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Apr 2017 23:03:19 +0000 (23:03 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Apr 2017 23:03:19 +0000 (23:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170379

Reviewed by Ryosuke Niwa.

Add an API to return sub-commits for a given commit.
Add sub-commit difference viewer into commit log viewer to show changed sub-commits between two commits.
Add 'ownsSubCommits' info in 'api/commits' return values.
Extend 'api/manifest' to include whether a repositories owns other repositories.
Only show this sub-commit difference viewer when a repository owns other repositories and both commits owns sub-commits.
Add unit tests for those new features.

* browser-tests/commit-log-viewer-tests.js: Updated test cases.
* public/api/commits.php: Added 'sub-commits' to provide sub-commit for a given commit.
* public/include/commit-log-fetcher.php: Added function to query sub-commit from database. Added 'repository' and 'ownsSubCommits' fields in returning commits.
* public/v3/components/expand-collapse-button.js: Added.
(ExpandCollapseButton):
(ExpandCollapseButton.prototype.didConstructShadowTree): Changes state on click and dispatches 'toggle' event.
(ExpandCollapseButton.sizeFactor):
(ExpandCollapseButton.buttonContent):
(ExpandCollapseButton.cssTemplate):
* public/v3/components/commit-log-viewer.js:
(CommitLogViewer.prototype._renderCommitList): Added sub-commit viewer if two adjacent commits both have sub-commits.
(CommitLogViewer.cssTemplate):
* public/v3/components/sub-commit-viewer.js: Added.
(SubCommitViewer): Added 'SubCommitViewer' class to represent the sub-commit differences between two given commits.`
(SubCommitViewer.prototype.didConstructShadowTree): Makes 'expand-collapse' button listen to 'toggle' event.
(SubCommitViewer.prototype._toggleVisibility): Updates UI once 'expand-collapse' button is clicked.
(SubCommitViewer.prototype.render): Render sub-commit view based on the state.
(SubCommitViewer.prototype._renderSubcommitTable): Generates sub-commits difference table entries.
(SubCommitViewer.htmlTemplate):
(SubCommitViewer.cssTemplate):
* public/v3/index.html: Added 'sub-commit-viewer.js' and 'expand-collapse-button.js'.
* public/v3/models/commit-log.js:
(CommitLog): Added '_subCommits'.
(CommitLog.prototype.updateSingleton): Updates 'rawData.ownsSubCommits' as well.
(CommitLog.prototype.ownsSubCommits):
(CommitLog.prototype.subCommits): Added. Returns sub-commits.
(CommitLog.prototype.fetchSubCommits): Added. Fetches sub-commits if haven't fetched them before.
(CommitLog.prototype._buildSubCommitMap): Added. Creates a map which maps repositories to commits.
(CommitLog.diffSubCommits): Added. Finds difference between two given commits.
(CommitLog.fetchBetweenRevisions): Updated due to '_constructFromRawData' change.
(CommitLog.fetchForSingleRevision): Updated due to '_constructFromRawData' change.
(CommitLog._constructFromRawData): Removed first argument 'repository' as it can be determined by calling 'Repository.findById'.
* public/v3/models/repository.js:
(Repository):
(Repository.prototype.owner): Returns the owner id.
(Repository.prototype.ownedRepositories): Returns a list of repositories owned by this repository.
* server-tests/api-commits-tests.js: Added tests for 'sub-commits' filter.
* server-tests/api-manifest-tests.js: Added a test.
* unit-tests/commit-log-tests.js: Added tests for 'fetchSubCommits' and 'diffSubCommits'.
* unit-tests/resources/mock-v3-models.js: Added 'ownerRepository' and 'ownedRepository'.

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

14 files changed:
Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/browser-tests/commit-log-viewer-tests.js
Websites/perf.webkit.org/public/api/commits.php
Websites/perf.webkit.org/public/include/commit-log-fetcher.php
Websites/perf.webkit.org/public/v3/components/commit-log-viewer.js
Websites/perf.webkit.org/public/v3/components/expand-collapse-button.js [new file with mode: 0644]
Websites/perf.webkit.org/public/v3/components/sub-commit-viewer.js [new file with mode: 0644]
Websites/perf.webkit.org/public/v3/index.html
Websites/perf.webkit.org/public/v3/models/commit-log.js
Websites/perf.webkit.org/public/v3/models/repository.js
Websites/perf.webkit.org/server-tests/api-commits-tests.js
Websites/perf.webkit.org/server-tests/api-manifest-tests.js
Websites/perf.webkit.org/unit-tests/commit-log-tests.js
Websites/perf.webkit.org/unit-tests/resources/mock-v3-models.js

index d3743c1..1926ecc 100644 (file)
@@ -1,3 +1,58 @@
+2017-04-14  Dewei Zhu  <dewei_zhu@apple.com>
+
+        Add sub-commit UI in commit log viewer.
+        https://bugs.webkit.org/show_bug.cgi?id=170379
+
+        Reviewed by Ryosuke Niwa.
+
+        Add an API to return sub-commits for a given commit.
+        Add sub-commit difference viewer into commit log viewer to show changed sub-commits between two commits.
+        Add 'ownsSubCommits' info in 'api/commits' return values.
+        Extend 'api/manifest' to include whether a repositories owns other repositories.
+        Only show this sub-commit difference viewer when a repository owns other repositories and both commits owns sub-commits.
+        Add unit tests for those new features.
+
+        * browser-tests/commit-log-viewer-tests.js: Updated test cases.
+        * public/api/commits.php: Added 'sub-commits' to provide sub-commit for a given commit.
+        * public/include/commit-log-fetcher.php: Added function to query sub-commit from database. Added 'repository' and 'ownsSubCommits' fields in returning commits.
+        * public/v3/components/expand-collapse-button.js: Added.
+        (ExpandCollapseButton):
+        (ExpandCollapseButton.prototype.didConstructShadowTree): Changes state on click and dispatches 'toggle' event.
+        (ExpandCollapseButton.sizeFactor):
+        (ExpandCollapseButton.buttonContent):
+        (ExpandCollapseButton.cssTemplate):
+        * public/v3/components/commit-log-viewer.js:
+        (CommitLogViewer.prototype._renderCommitList): Added sub-commit viewer if two adjacent commits both have sub-commits.
+        (CommitLogViewer.cssTemplate):
+        * public/v3/components/sub-commit-viewer.js: Added.
+        (SubCommitViewer): Added 'SubCommitViewer' class to represent the sub-commit differences between two given commits.`
+        (SubCommitViewer.prototype.didConstructShadowTree): Makes 'expand-collapse' button listen to 'toggle' event.
+        (SubCommitViewer.prototype._toggleVisibility): Updates UI once 'expand-collapse' button is clicked.
+        (SubCommitViewer.prototype.render): Render sub-commit view based on the state.
+        (SubCommitViewer.prototype._renderSubcommitTable): Generates sub-commits difference table entries.
+        (SubCommitViewer.htmlTemplate):
+        (SubCommitViewer.cssTemplate):
+        * public/v3/index.html: Added 'sub-commit-viewer.js' and 'expand-collapse-button.js'.
+        * public/v3/models/commit-log.js:
+        (CommitLog): Added '_subCommits'.
+        (CommitLog.prototype.updateSingleton): Updates 'rawData.ownsSubCommits' as well.
+        (CommitLog.prototype.ownsSubCommits):
+        (CommitLog.prototype.subCommits): Added. Returns sub-commits.
+        (CommitLog.prototype.fetchSubCommits): Added. Fetches sub-commits if haven't fetched them before.
+        (CommitLog.prototype._buildSubCommitMap): Added. Creates a map which maps repositories to commits.
+        (CommitLog.diffSubCommits): Added. Finds difference between two given commits.
+        (CommitLog.fetchBetweenRevisions): Updated due to '_constructFromRawData' change.
+        (CommitLog.fetchForSingleRevision): Updated due to '_constructFromRawData' change.
+        (CommitLog._constructFromRawData): Removed first argument 'repository' as it can be determined by calling 'Repository.findById'.
+        * public/v3/models/repository.js:
+        (Repository):
+        (Repository.prototype.owner): Returns the owner id.
+        (Repository.prototype.ownedRepositories): Returns a list of repositories owned by this repository.
+        * server-tests/api-commits-tests.js: Added tests for 'sub-commits' filter.
+        * server-tests/api-manifest-tests.js: Added a test.
+        * unit-tests/commit-log-tests.js: Added tests for 'fetchSubCommits' and 'diffSubCommits'.
+        * unit-tests/resources/mock-v3-models.js: Added 'ownerRepository' and 'ownedRepository'.
+
 2017-04-11  Ryosuke Niwa  <rniwa@webkit.org>
 
         Retrying an A/B testing does not set the repetition count in some cases
index 7f52dc1..265b6f7 100644 (file)
@@ -21,7 +21,9 @@ describe('CommitLogViewer', () => {
     const webkitCommit210949 = {
         "id": "185334",
         "revision": "210949",
+        "repository": 1,
         "previousCommit": null,
+        "ownsSubCommits": false,
         "time": +new Date("2017-01-20T03:23:50.645Z"),
         "authorName": "Chris Dumez",
         "authorEmail": "cdumez@apple.com",
@@ -31,7 +33,9 @@ describe('CommitLogViewer', () => {
     const webkitCommit210950 = {
         "id": "185338",
         "revision": "210950",
+        "repository": 1,
         "previousCommit": null,
+        "ownsSubCommits": false,
         "time": +new Date("2017-01-20T03:49:37.887Z"),
         "authorName": "Commit Queue",
         "authorEmail": "commit-queue@webkit.org",
index 2595162..bd8df22 100644 (file)
@@ -38,6 +38,9 @@ function main($paths) {
             $commits = $fetcher->fetch_latest_for_platform($repository_id, $platform_id);
         } else
             $commits = $fetcher->fetch_latest($repository_id);
+    } else if ($filter == 'sub-commits') {
+        $owner_revision = array_get($_GET, 'owner-revision');
+        $commits = $fetcher->fetch_subcommits_for_revision($repository_id, $owner_revision);
     } else if ($filter == 'last-reported') {
         $from = array_get($_GET, 'from');
         $to = array_get($_GET, 'to');
index d09d00b..11189c0 100644 (file)
@@ -17,7 +17,8 @@ class CommitLogFetcher {
         $commits = array();
         foreach ($commit_rows as &$commit_row) {
             $associated_task = &$task_by_id[$commit_row['taskcommit_task']];
-            $commit = $this->format_commit($commit_row, $commit_row);
+            # FIXME: The last parameter should be determined based on commit_ownerships.
+            $commit = $this->format_commit($commit_row, $commit_row, /* owns_sub_commits */ FALSE);
             $commit['repository'] = $commit_row['commit_repository'];
             array_push($commits, $commit);
             array_push($associated_task[Database::is_true($commit_row['taskcommit_is_fix']) ? 'fixes' : 'causes'], $commit_row['commit_id']);
@@ -41,7 +42,9 @@ class CommitLogFetcher {
             commit_time as "time",
             committer_name as "authorName",
             committer_account as "authorEmail",
-            commit_message as "message"
+            commit_repository as "repository",
+            commit_message as "message",
+            EXISTS(SELECT * FROM commit_ownerships WHERE commit_owner = commit_id) as "ownsSubCommits"
             FROM commits LEFT OUTER JOIN committers ON commit_committer = committer_id
             WHERE commit_repository = $1 AND commit_reported = true';
         $values = array($repository_id);
@@ -85,12 +88,24 @@ class CommitLogFetcher {
         if (!is_array($commits))
             return NULL;
 
-        foreach ($commits as &$commit)
+        foreach ($commits as &$commit) {
             $commit['time'] = Database::to_js_time($commit['time']);
+            $commit['ownsSubCommits'] = Database::is_true($commit['ownsSubCommits']);
+        }
 
         return $commits;
     }
 
+    function fetch_subcommits_for_revision($repository_id, $commit_revision) {
+        return $this->db->query_and_fetch_all('SELECT owned.commit_repository as "repository",
+            owned.commit_revision as "revision",
+            owned.commit_time as "time",
+            owned.commit_id as "id"
+            FROM commits AS owned, commit_ownerships, commits AS owner
+            WHERE owned.commit_id = commit_owned AND owner.commit_id = commit_owner AND owner.commit_revision = $1 AND owner.commit_repository = $2',
+            array($commit_revision, $repository_id));
+    }
+
     # FIXME: this is not DRY. Ideally, $db should provide the ability to search with criteria that specifies a range.
     function fetch_last_reported_between_orders($repository_id, $from, $to)
     {
@@ -163,19 +178,22 @@ class CommitLogFetcher {
         if (!$commit_row)
             return array();
         $committer = $this->db->select_first_row('committers', 'committer', array('id' => $commit_row['commit_committer']));
-        return array($this->format_commit($commit_row, $committer));
+        $owns_sub_commits = !!$this->db->select_first_row('commit_ownerships', 'commit', array('owner' => $commit_row['commit_id']));
+        return array($this->format_commit($commit_row, $committer, $owns_sub_commits));
     }
 
-    private function format_commit($commit_row, $committer_row) {
+    private function format_commit($commit_row, $committer_row, $owns_sub_commits) {
         return array(
             'id' => $commit_row['commit_id'],
             'revision' => $commit_row['commit_revision'],
+            'repository' => $commit_row['commit_repository'],
             '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,
             'authorEmail' => $committer_row ? $committer_row['committer_account'] : null,
-            'message' => $commit_row['commit_message']
+            'message' => $commit_row['commit_message'],
+            'ownsSubCommits' => $owns_sub_commits
         );
     }
 }
index 215072a..81a6e16 100644 (file)
@@ -65,13 +65,18 @@ class CommitLogViewer extends ComponentBase {
     {
         const element = ComponentBase.createElement;
         const link = ComponentBase.createLink;
+        let previousCommit = null;
 
         this.renderReplace(this.content('commits-list'), (commits || []).map((commit) => {
             const label = commit.label();
             const url = commit.url();
-            return element('tr', [
-                element('th', [element('h4', {class: 'revision'}, url ? link(label, commit.title(), url) : label), commit.author() || '']),
-                element('td', commit.message() ? commit.message().substring(0, 80) : '')]);
+            const ownsSubCommits = previousCommit && previousCommit.ownsSubCommits() && commit.ownsSubCommits();
+            const subCommitDifferenceRow = ownsSubCommits ? element('tr', element('td', {colspan: 2}, new SubCommitViewer(previousCommit, commit))) : [];
+            previousCommit = commit;
+            return [subCommitDifferenceRow,
+                element('tr', [
+                    element('th', [element('h4', {class: 'revision'}, url ? link(label, commit.title(), url) : label), commit.author() || '']),
+                    element('td', commit.message() ? commit.message().substring(0, 80) : '')])];
         }));
     }
 
@@ -96,7 +101,7 @@ class CommitLogViewer extends ComponentBase {
                 height: calc(100% - 2px);
                 overflow-y: scroll;
             }
-            
+
             #commits-viewer-table {
                 width: 100%;
                 border-collapse: collapse;
diff --git a/Websites/perf.webkit.org/public/v3/components/expand-collapse-button.js b/Websites/perf.webkit.org/public/v3/components/expand-collapse-button.js
new file mode 100644 (file)
index 0000000..46ba8a5
--- /dev/null
@@ -0,0 +1,40 @@
+
+class ExpandCollapseButton extends ButtonBase {
+    constructor()
+    {
+        super('expand-collapse-button');
+        this._expanded = false;
+    }
+
+    didConstructShadowTree()
+    {
+        super.didConstructShadowTree();
+        this.listenToAction('activate', () => {
+            this._expanded = !this._expanded;
+            this.content('button').className = this._expanded ? 'expanded' : null;
+            this.enqueueToRender();
+            this.dispatchAction('toggle', this._expanded);
+        });
+    }
+
+    static sizeFactor() { return 0.8; }
+
+    static buttonContent()
+    {
+        return `<g stroke="#333" fill="none" stroke-width="10">
+            <polyline points="0,25 50,50 100,25"/>
+            <polyline points="0,50 50,75 100,50"/>
+        </g>`;
+    }
+
+    static cssTemplate()
+    {
+        return super.cssTemplate() + `
+            a.expanded {
+                transform: rotate(180deg);
+            }
+        `;
+    }
+}
+
+ComponentBase.defineElement('expand-collapse-button', ExpandCollapseButton);
\ No newline at end of file
diff --git a/Websites/perf.webkit.org/public/v3/components/sub-commit-viewer.js b/Websites/perf.webkit.org/public/v3/components/sub-commit-viewer.js
new file mode 100644 (file)
index 0000000..79e1854
--- /dev/null
@@ -0,0 +1,98 @@
+class SubCommitViewer extends ComponentBase {
+
+    constructor(previousCommit, currentCommit)
+    {
+        super('sub-commit-viewer');
+        this._previousCommit = previousCommit;
+        this._currentCommit = currentCommit;
+        this._previousSubCommits = null;
+        this._currentSubCommits = null;
+        this._showingSubCommits = false;
+        this._renderSubCommitTableLazily = new LazilyEvaluatedFunction(this._renderSubcommitTable.bind(this));
+    }
+
+    didConstructShadowTree()
+    {
+        this.part('expand-collapse').listenToAction('toggle', (expanded) => this._toggleVisibility(expanded));
+    }
+
+    _toggleVisibility(expanded)
+    {
+        this._showingSubCommits = expanded;
+        this.enqueueToRender();
+
+        Promise.all([this._previousCommit.fetchSubCommits(), this._currentCommit.fetchSubCommits()]).then((subCommitsList) => {
+            this._previousSubCommits = subCommitsList[0];
+            this._currentSubCommits = subCommitsList[1];
+            this.enqueueToRender();
+        });
+    }
+
+    render()
+    {
+        const hideSpinner = (this._previousSubCommits && this._currentSubCommits) || !this._showingSubCommits;
+
+        this.content('difference-entries').style.display =  this._showingSubCommits ? null : 'none';
+        this.content('spinner-container').style.display = hideSpinner ? 'none' : null;
+        this.content('difference-table').style.display = this._showingSubCommits ? null : 'none';
+        this._renderSubCommitTableLazily.evaluate(this._previousSubCommits, this._currentSubCommits);
+    }
+
+    _renderSubcommitTable(previousSubCommits, currentSubCommits)
+    {
+        if (!previousSubCommits || !currentSubCommits)
+            return;
+
+        const difference = CommitLog.diffSubCommits(this._previousCommit, this._currentCommit);
+        const sortedRepositories = Repository.sortByName([...difference.keys()]);
+        const element = ComponentBase.createElement;
+
+        const tableEntries = sortedRepositories.map((repository) => {
+            const revisions = difference.get(repository);
+            return element('tr', [element('td', repository.name()),
+                element('td', revisions[0] ? revisions[0].revision() : ''),
+                element('td', revisions[1] ? revisions[1].revision() : '')]);
+        });
+        this.renderReplace(this.content('difference-entries'), tableEntries);
+    }
+
+    static htmlTemplate()
+    {
+        return `
+            <expand-collapse-button id="expand-collapse"></expand-collapse-button>
+            <table id="difference-table">
+                <tbody id="difference-entries"></tbody>
+            </table>
+            <div id="spinner-container"><spinner-icon id="spinner"></spinner-icon></div>`;
+    }
+
+    static cssTemplate() {
+        return `
+            :host {
+                display: block;
+                font-size: 0.8rem;
+                font-weight: normal;
+            }
+
+            expand-collapse-button {
+                margin-left: calc(50% - 0.8rem);
+                display: block;
+            }
+
+            td, th {
+                padding: 0.2rem;
+                margin: 0;
+                border-top: solid 1px #ccc;
+            }
+
+            #difference-table {
+                width: 100%;
+            }
+
+            #spinner-container {
+                text-align: center;
+            }`;
+    }
+}
+
+ComponentBase.defineElement('sub-commit-viewer', SubCommitViewer);
\ No newline at end of file
index ca84825..a37e345 100644 (file)
@@ -74,7 +74,9 @@ Run tools/bundle-v3-scripts to speed up the load time for production.`);
         <script src="components/warning-icon.js"></script>
         <script src="components/close-button.js"></script>
         <script src="components/commit-log-viewer.js"></script>
+        <script src="components/sub-commit-viewer.js"></script>
         <script src="components/editable-text.js"></script>
+        <script src="components/expand-collapse-button.js"></script>
         <script src="components/time-series-chart.js"></script>
         <script src="components/interactive-time-series-chart.js"></script>
         <script src="components/dashboard-chart-status-view.js"></script>
index bbcf7af..8a74a7c 100644 (file)
@@ -11,6 +11,7 @@ class CommitLog extends DataModelObject {
         this._remoteId = rawData.id;
         if (this._remoteId)
             this.ensureNamedStaticMap('remoteId')[this._remoteId] = this;
+        this._subCommits = null;
     }
 
     updateSingleton(rawData)
@@ -24,6 +25,8 @@ class CommitLog extends DataModelObject {
             this._rawData.authorName = rawData.authorName;
         if (rawData.message)
             this._rawData.message = rawData.message;
+        if (rawData.ownsSubCommits)
+            this._rawData.ownsSubCommits = rawData.ownsSubCommits;
     }
 
     repository() { return this._repository; }
@@ -32,6 +35,7 @@ class CommitLog extends DataModelObject {
     revision() { return this._rawData['revision']; }
     message() { return this._rawData['message']; }
     url() { return this._repository.urlForRevision(this._rawData['revision']); }
+    ownsSubCommits() { return this._rawData['ownsSubCommits']; }
 
     label()
     {
@@ -82,25 +86,72 @@ class CommitLog extends DataModelObject {
         });
     }
 
+    fetchSubCommits()
+    {
+        if (!this.repository().ownedRepositories())
+            return Promise.reject();
+
+        if (!this.ownsSubCommits())
+            return Promise.reject();
+
+        if (this._subCommits)
+            return Promise.resolve(this._subCommits);
+
+        return CommitLog.cachedFetch(`../api/commits/${this.repository().id()}/sub-commits?owner-revision=${escape(this.revision())}`).then((data) => {
+            this._subCommits = CommitLog._constructFromRawData(data);
+            return this._subCommits;
+        });
+    }
+
+    _buildSubCommitMap()
+    {
+        const subCommitMap = new Map;
+        for (const commit of this._subCommits)
+            subCommitMap.set(commit.repository(), commit);
+        return subCommitMap;
+    }
+
+    static diffSubCommits(previousCommit, currentCommit)
+    {
+        console.assert(previousCommit);
+        console.assert(currentCommit);
+        console.assert(previousCommit._subCommits);
+        console.assert(currentCommit._subCommits);
+
+        const previousSubCommitMap = previousCommit._buildSubCommitMap();
+        const currentSubCommitMap = currentCommit._buildSubCommitMap();
+        const subCommitRepositories = new Set([...currentSubCommitMap.keys(), ...previousSubCommitMap.keys()]);
+        const difference = new Map;
+
+        subCommitRepositories.forEach((subCommitRepository) => {
+            const currentRevision = currentSubCommitMap.get(subCommitRepository);
+            const previousRevision = previousSubCommitMap.get(subCommitRepository);
+            if (currentRevision != previousRevision)
+                difference.set(subCommitRepository, [previousRevision, currentRevision]);
+        });
+
+        return difference;
+    }
+
     static fetchBetweenRevisions(repository, precedingRevision, lastRevision)
     {
         // FIXME: The cache should be smarter about fetching a range within an already fetched range, etc...
         // FIXME: We should evict some entires from the cache in cachedFetch.
         return this.cachedFetch(`/api/commits/${repository.id()}/`, {precedingRevision, lastRevision})
-            .then((data) => this._constructFromRawData(repository, data));
+            .then((data) => this._constructFromRawData(data));
     }
 
     static fetchForSingleRevision(repository, revision)
     {
         return this.cachedFetch(`/api/commits/${repository.id()}/${revision}`).then((data) => {
-            return this._constructFromRawData(repository, data);
+            return this._constructFromRawData(data);
         });
     }
 
-    static _constructFromRawData(repository, data)
+    static _constructFromRawData(data)
     {
         return data['commits'].map((rawData) => {
-            rawData.repository = repository;
+            rawData.repository = Repository.findById(rawData.repository);
             return CommitLog.ensureSingleton(rawData.id, rawData);
         });
     }
index 7e2e6a3..cae9a4e 100644 (file)
@@ -7,10 +7,17 @@ class Repository extends LabeledObject {
         this._url = object.url;
         this._blameUrl = object.blameUrl;
         this._hasReportedCommits = object.hasReportedCommits;
-        this._owner = object.owner;
+        this._ownerId = object.owner;
 
-        if (!object.owner)
+        if (!this._ownerId)
             this.ensureNamedStaticMap('topLevelName')[this.name()] = this;
+        else {
+            const ownerships = this.ensureNamedStaticMap('repositoryOwnerships');
+            if (!(this._ownerId in ownerships))
+                ownerships[this._ownerId] = [this];
+            else
+                ownerships[this._ownerId].push(this);
+        }
     }
 
     static findTopLevelByName(name)
@@ -33,7 +40,13 @@ class Repository extends LabeledObject {
 
     owner()
     {
-        return this._owner;
+        return this._ownerId;
+    }
+
+    ownedRepositories()
+    {
+        const ownerships = this.namedStaticMap('repositoryOwnerships');
+        return ownerships ? ownerships[this.id()] : null;
     }
 
     static sortByNamePreferringOnesWithURL(repositories)
index 9b9c935..3d990d5 100644 (file)
@@ -394,6 +394,76 @@ describe("/api/commits/", function () {
             });
         });
 
+        it("should handle commit revision with space", () => {
+            const db = TestServer.database();
+            return Promise.all([
+                db.insert('repositories', {'id': 1, 'name': 'OS X'}),
+                db.insert('commits', {'repository': 1, 'revision': '10.11.10 Sierra16C67', 'order': 367, 'reported': true}),
+            ]).then(() => {
+                return TestServer.remoteAPI().getJSON('/api/commits/OS%20X/10.11.10%20Sierra16C67');
+            }).then((results) => {
+                assert.equal(results.status, 'OK');
+                assert.equal(results.commits.length, 1);
+
+                const commit = results.commits[0];
+                assert.equal(commit.id, 1);
+                assert.equal(commit.revision, '10.11.10 Sierra16C67');
+            });
+        });
+
+    });
+
+    describe('/api/commits/<repository>/sub-commits?owner-revision=<commit>', () => {
+        it("should return sub commits for a given commit", () => {
+            const db = TestServer.database();
+            return Promise.all([
+                db.insert('repositories', {'id': 1, 'name': 'macOS'}),
+                db.insert('repositories', {'id': 2, 'name': 'WebKit', 'owner': 1}),
+                db.insert('commits', {'id': 1, 'repository': 1, 'revision': '10.12 16A323', order: 1, 'reported': true}),
+                db.insert('commits', {'id': 2, 'repository': 2, 'revision': '210950', 'reported': true}),
+                db.insert('commit_ownerships', {'owner': 1, 'owned': 2})
+            ]).then(() => {
+                return TestServer.remoteAPI().getJSON('/api/commits/1/sub-commits?owner-revision=10.12%2016A323')
+            }).then((results) => {
+                assert.equal(results.status, 'OK');
+                assert.equal(results.commits.length, 1);
+
+                const subCommit = results.commits[0];
+                assert.equal(subCommit.repository, 2);
+                assert.equal(subCommit.revision, '210950');
+                assert.equal(subCommit.id, 2);
+            });
+        });
+
+        it("should return an empty list of commits if no sub-commits is associated with given commit", () => {
+            const db = TestServer.database();
+            return Promise.all([
+                db.insert('repositories', {'id': 1, 'name': 'macOS'}),
+                db.insert('repositories', {'id': 2, 'name': 'WebKit'}),
+                db.insert('commits', {'id': 1, 'repository': 1, 'revision': '10.12 16A323', order: 1, 'reported': true}),
+                db.insert('commits', {'id': 2, 'repository': 2, 'revision': '210950', 'reported': true})
+            ]).then(() => {
+                return TestServer.remoteAPI().getJSON('/api/commits/1/sub-commits?owner-revision=10.12%2016A323')
+            }).then((results) => {
+                assert.equal(results.status, 'OK');
+                assert.deepEqual(results.commits, []);
+            });
+        });
+
+        it("should return an empty list if commit revision is invalid", () => {
+            const db = TestServer.database();
+            return Promise.all([
+                db.insert('repositories', {'id': 1, 'name': 'macOS'}),
+                db.insert('repositories', {'id': 2, 'name': 'WebKit'}),
+                db.insert('commits', {'id': 1, 'repository': 1, 'revision': '10.12 16A323', order: 1, 'reported': true}),
+                db.insert('commits', {'id': 2, 'repository': 2, 'revision': '210950', 'reported': true})
+            ]).then(() => {
+                return TestServer.remoteAPI().getJSON('/api/commits/1/sub-commits?owner-revision=10.12%2016A324')
+            }).then((results) => {
+                assert.equal(results.status, 'OK');
+                assert.equal(results.commits.length, 0);
+            });
+        })
     });
 
     describe('/api/commits/<repository>/?precedingRevision=<commit-1>&lastRevision=<commit-2>', () => {
index f77123c..ffe47bc 100644 (file)
@@ -99,6 +99,36 @@ describe('/api/manifest', function () {
         });
     });
 
+    it("should generate manifest with repositories and each repository should know its owned repositories", () => {
+        const db = TestServer.database();
+        return Promise.all([
+            db.insert('repositories', {id: 11, name: 'WebKit', url: 'https://trac.webkit.org/$1'}),
+            db.insert('repositories', {id: 9, name: 'OS X'}),
+            db.insert('repositories', {id: 22, name: 'iOS'}),
+            db.insert('repositories', {id: 35, name: 'JavaScriptCore', owner: 9}),
+        ]).then(() => {
+            return TestServer.remoteAPI().getJSON('/api/manifest');
+        }).then((content) => {
+            let manifest = Manifest._didFetchManifest(content);
+
+            let webkit = Repository.findById(11);
+            assert(webkit);
+            assert.equal(webkit.name(), 'WebKit');
+            assert.equal(webkit.urlForRevision(123), 'https://trac.webkit.org/123');
+            assert.equal(webkit.ownedRepositories(), null);
+
+            let osx = Repository.findById(9);
+            assert(osx);
+            assert.equal(osx.name(), 'OS X');
+            assert.deepEqual(osx.ownedRepositories(), [Repository.findById(35)]);
+
+            let ios = Repository.findById(22);
+            assert(ios);
+            assert.equal(ios.name(), 'iOS');
+            assert.equal(ios.ownedRepositories(), null);
+        });
+    });
+
     it("should generate manifest with builders", () => {
         let db = TestServer.database();
         return Promise.all([
index 46210dd..1bd46e1 100644 (file)
@@ -4,6 +4,7 @@ const assert = require('assert');
 
 require('../tools/js/v3-models.js');
 const MockModels = require('./resources/mock-v3-models.js').MockModels;
+const MockRemoteAPI = require('../unit-tests/resources/mock-remote-api.js').MockRemoteAPI;
 
 function webkitCommit()
 {
@@ -59,6 +60,36 @@ function oldOSXCommit()
     });
 }
 
+function commitWithoutSubCommits()
+{
+    return new CommitLog(6, {
+        repository: MockModels.ownerRepository,
+        revision: '10.11.4 15E66',
+        ownsSubCommits: false,
+        time: null,
+    });
+}
+
+function ownerCommit()
+{
+    return new CommitLog(5, {
+        repository: MockModels.ownerRepository,
+        revision: '10.11.4 15E65',
+        ownsSubCommits: true,
+        time: null,
+    });
+}
+
+function otherOwnerCommit()
+{
+    return new CommitLog(5, {
+        repository: MockModels.ownerRepository,
+        revision: '10.11.4 15E66',
+        ownsSubCommits: true,
+        time: null,
+    });
+}
+
 describe('CommitLog', function () {
     MockModels.inject();
 
@@ -135,4 +166,132 @@ describe('CommitLog', function () {
             });
         });
     });
+
+    describe('fetchSubCommits', () => {
+        beforeEach(() => {
+            MockRemoteAPI.inject();
+        });
+
+        it('should reject if repository of the commit does not own other repositories', () => {
+            const commit = osxCommit();
+            return commit.fetchSubCommits().then(() => {
+               assert(false, 'Should not execute this line.');
+            }, (error) => {
+                assert.equal(error, undefined);
+            });
+        });
+
+        it('should reject if commit does not own other sub-commits', () => {
+            const commit = commitWithoutSubCommits();
+            return commit.fetchSubCommits().then(() => {
+                assert(false, 'Should not execute this line.');
+            }, (error) => {
+                assert.equal(error, undefined);
+            });
+        });
+
+        it('should return sub-commit for a valid commit revision', () => {
+            const fetchingPromise = ownerCommit().fetchSubCommits();
+            const requests = MockRemoteAPI.requests;
+            assert.equal(requests.length, 1);
+            assert.equal(requests[0].url, '../api/commits/111/sub-commits?owner-revision=10.11.4%2015E65');
+            assert.equal(requests[0].method, 'GET');
+
+            requests[0].resolve({commits: [{
+                id: 233,
+                repository: MockModels.ownedRepository.id(),
+                revision: '6f8b0dbbda95a440503b88db1dd03dad3a7b07fb',
+                time: +(new Date('2016-05-13T00:55:57.841344Z')),
+            }]});
+            return fetchingPromise.then((subCommits) => {
+                assert.equal(subCommits.length, 1);
+                assert.equal(subCommits[0].repository(), MockModels.ownedRepository);
+                assert.equal(subCommits[0].revision(), '6f8b0dbbda95a440503b88db1dd03dad3a7b07fb');
+                assert.equal(subCommits[0].id(), 233);
+            });
+        });
+
+        it('should only fetch sub-commits exactly once', () => {
+            const commit = ownerCommit();
+            const fetchingPromise = commit.fetchSubCommits();
+            const requests = MockRemoteAPI.requests;
+            let existingSubCommits = null;
+            assert.equal(requests.length, 1);
+            assert.equal(requests[0].url, '../api/commits/111/sub-commits?owner-revision=10.11.4%2015E65');
+            assert.equal(requests[0].method, 'GET');
+
+            MockRemoteAPI.requests[0].resolve({commits: [{
+                id: 233,
+                repository: MockModels.ownedRepository.id(),
+                revision: '6f8b0dbbda95a440503b88db1dd03dad3a7b07fb',
+                time: +(new Date('2016-05-13T00:55:57.841344Z')),
+            }]});
+
+            return fetchingPromise.then((subCommits) => {
+                existingSubCommits = subCommits;
+                assert.equal(subCommits.length, 1);
+                assert.equal(subCommits[0].repository(), MockModels.ownedRepository);
+                assert.equal(subCommits[0].revision(), '6f8b0dbbda95a440503b88db1dd03dad3a7b07fb');
+                assert.equal(subCommits[0].id(), 233);
+                return commit.fetchSubCommits();
+            }).then((subCommits) => {
+                assert.equal(requests.length, 1);
+                assert.equal(existingSubCommits, subCommits);
+            });
+        });
+    });
+
+    describe('diffSubCommits', () => {
+        beforeEach(() => {
+            MockRemoteAPI.reset();
+        });
+
+        it('should return difference between 2 sub-commits', () => {
+            const oneCommit = ownerCommit();
+            const otherCommit = otherOwnerCommit();
+            const fetchingPromise = oneCommit.fetchSubCommits();
+            const requests = MockRemoteAPI.requests;
+            assert.equal(requests.length, 1);
+            assert.equal(requests[0].url, '../api/commits/111/sub-commits?owner-revision=10.11.4%2015E65');
+            assert.equal(requests[0].method, 'GET');
+
+            requests[0].resolve({commits: [{
+                id: 233,
+                repository: MockModels.ownedRepository.id(),
+                revision: '6f8b0dbbda95a440503b88db1dd03dad3a7b07fb',
+                time: +(new Date('2016-05-13T00:55:57.841344Z')),
+            }, {
+                id: 299,
+                repository: MockModels.webkitGit.id(),
+                revision: '04a6c72038f0b771a19248ca2549e1258617b5fc',
+                time: +(new Date('2016-05-13T00:55:57.841344Z')),
+            }]});
+
+            return fetchingPromise.then(() => {
+                const otherFetchingPromise = otherCommit.fetchSubCommits();
+                assert.equal(requests.length, 2);
+                assert.equal(requests[1].url, '../api/commits/111/sub-commits?owner-revision=10.11.4%2015E66');
+                assert.equal(requests[1].method, 'GET');
+
+                requests[1].resolve({commits: [{
+                    id: 234,
+                    repository: MockModels.ownedRepository.id(),
+                    revision: 'd5099e03b482abdd77f6c4dcb875afd05bda5ab8',
+                    time: +(new Date('2016-05-13T00:55:57.841344Z')),
+                }, {
+                    id: 299,
+                    repository: MockModels.webkitGit.id(),
+                    revision: '04a6c72038f0b771a19248ca2549e1258617b5fc',
+                    time: +(new Date('2016-05-13T00:55:57.841344Z')),
+                }]});
+
+                return otherFetchingPromise;
+            }).then(() => {
+                const difference = CommitLog.diffSubCommits(oneCommit, otherCommit);
+                assert.equal(difference.size, 1);
+                assert.equal(difference.keys().next().value, MockModels.ownedRepository);
+            });
+
+        });
+    });
 });
index 657f07b..1053f78 100644 (file)
@@ -19,6 +19,8 @@ var MockModels = {
             MockModels.ios = Repository.ensureSingleton(22, {name: 'iOS'});
             MockModels.webkit = Repository.ensureSingleton(11, {name: 'WebKit', url: 'http://trac.webkit.org/changeset/$1'});
             MockModels.sharedRepository = Repository.ensureSingleton(16, {name: 'Shared'});
+            MockModels.ownerRepository = Repository.ensureSingleton(111, {name: 'Owner Repository'});
+            MockModels.ownedRepository = Repository.ensureSingleton(112, {name: 'Owned Repository', owner: 111});
             MockModels.webkitGit = Repository.ensureSingleton(17, {name: 'WebKit-Git'});
             MockModels.builder = new Builder(176, {name: 'WebKit Perf Builder', buildUrl: 'http://build.webkit.org/builders/$builderName/$buildNumber'});