TEZ-3419. Tez UI: Applications page shows error, for users with only DAG level ACL permission (sree)
Project: http://git-wip-us.apache.org/repos/asf/tez/repo Commit: http://git-wip-us.apache.org/repos/asf/tez/commit/8033e3d5 Tree: http://git-wip-us.apache.org/repos/asf/tez/tree/8033e3d5 Diff: http://git-wip-us.apache.org/repos/asf/tez/diff/8033e3d5 Branch: refs/heads/TEZ-3334 Commit: 8033e3d58d967a06f4a7f5bf99f94854989f6db6 Parents: 67243a0 Author: Sreenath Somarajapuram <s...@apache.org> Authored: Wed Oct 19 15:22:08 2016 +0530 Committer: Sreenath Somarajapuram <s...@apache.org> Committed: Wed Oct 19 15:22:08 2016 +0530 ---------------------------------------------------------------------- CHANGES.txt | 1 + tez-ui/src/main/webapp/app/controllers/app.js | 6 +- tez-ui/src/main/webapp/app/routes/app.js | 15 +++- .../src/main/webapp/app/routes/app/configs.js | 5 +- tez-ui/src/main/webapp/app/routes/app/dags.js | 2 +- tez-ui/src/main/webapp/app/routes/app/index.js | 27 ++++++- .../src/main/webapp/app/templates/app/index.hbs | 47 +++++++++---- .../webapp/tests/unit/controllers/app-test.js | 6 +- .../main/webapp/tests/unit/routes/app-test.js | 74 ++++++++++++++++++++ .../tests/unit/routes/app/configs-test.js | 51 ++++++++++++++ .../webapp/tests/unit/routes/app/dags-test.js | 32 +++++++++ .../webapp/tests/unit/routes/app/index-test.js | 47 +++++++++++++ 12 files changed, 288 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tez/blob/8033e3d5/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index e8e328f..ca8898b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -121,6 +121,7 @@ ALL CHANGES: TEZ-3433. Tez UI: Searching using wrong ID causes error in all DAGs page TEZ-3428. Tez UI: First Tab not needed for few entries in DAG listings TEZ-3469. Tez UI: Bump Phantom JS version to 2.1.1 + TEZ-3419. Tez UI: Applications page shows error, for users with only DAG level ACL permission Release 0.8.5: Unreleased http://git-wip-us.apache.org/repos/asf/tez/blob/8033e3d5/tez-ui/src/main/webapp/app/controllers/app.js ---------------------------------------------------------------------- diff --git a/tez-ui/src/main/webapp/app/controllers/app.js b/tez-ui/src/main/webapp/app/controllers/app.js index 809625e..b189c98 100644 --- a/tez-ui/src/main/webapp/app/controllers/app.js +++ b/tez-ui/src/main/webapp/app/controllers/app.js @@ -21,13 +21,13 @@ import Ember from 'ember'; import ParentController from './parent'; export default ParentController.extend({ - breadcrumbs: Ember.computed("model.appID", "model.app.name", function () { - var name = this.get("model.app.name") || this.get("model.appID"); + breadcrumbs: Ember.computed("model.name", "model.entityID", function () { + var name = this.get("model.name") || this.get("model.entityID"); return [{ text: `Application [ ${name} ]`, routeName: "app.index", - model: this.get("model.appID") + model: this.get("model.entityID") }]; }), http://git-wip-us.apache.org/repos/asf/tez/blob/8033e3d5/tez-ui/src/main/webapp/app/routes/app.js ---------------------------------------------------------------------- diff --git a/tez-ui/src/main/webapp/app/routes/app.js b/tez-ui/src/main/webapp/app/routes/app.js index 395a9be..05242bc 100644 --- a/tez-ui/src/main/webapp/app/routes/app.js +++ b/tez-ui/src/main/webapp/app/routes/app.js @@ -17,6 +17,7 @@ */ import AbstractRoute from './abstract'; +import Ember from 'ember'; export default AbstractRoute.extend({ title: "Application", @@ -26,8 +27,18 @@ export default AbstractRoute.extend({ }, model: function (params) { - return this.get("loader").queryRecord('app', "tez_" + this.queryFromParams(params).id). - catch(this.onLoadFailure.bind(this)); + var loader = this.get("loader"), + appID = this.queryFromParams(params).id; + return loader.queryRecord('AhsApp', appID).catch(function () { + return loader.queryRecord('appRm', appID).catch(function () { + // Sending back a dummy object presuming app details might be behind ACL. + // Not throwing error bar at this level as we need to display DAG tab if + // DAG details are available. + return Ember.Object.create({ + entityID: appID + }); + }); + }); }, actions: { http://git-wip-us.apache.org/repos/asf/tez/blob/8033e3d5/tez-ui/src/main/webapp/app/routes/app/configs.js ---------------------------------------------------------------------- diff --git a/tez-ui/src/main/webapp/app/routes/app/configs.js b/tez-ui/src/main/webapp/app/routes/app/configs.js index c58f2f1..f024a1f 100644 --- a/tez-ui/src/main/webapp/app/routes/app/configs.js +++ b/tez-ui/src/main/webapp/app/routes/app/configs.js @@ -32,6 +32,9 @@ export default SingleAmPollsterRoute.extend({ }, load: function (value, query, options) { - return this.get("loader").queryRecord('app', this.modelFor("app").get("id"), options); + var ID = "tez_" + this.modelFor("app").get("entityID"); + return this.get("loader").queryRecord('app', ID, options).catch(function () { + return []; + }); }, }); http://git-wip-us.apache.org/repos/asf/tez/blob/8033e3d5/tez-ui/src/main/webapp/app/routes/app/dags.js ---------------------------------------------------------------------- diff --git a/tez-ui/src/main/webapp/app/routes/app/dags.js b/tez-ui/src/main/webapp/app/routes/app/dags.js index 8264299..91a0ac9 100644 --- a/tez-ui/src/main/webapp/app/routes/app/dags.js +++ b/tez-ui/src/main/webapp/app/routes/app/dags.js @@ -31,7 +31,7 @@ export default MultiAmPollsterRoute.extend({ load: function (value, query, options) { return this.get("loader").query('dag', { - appID: this.modelFor("app").get("appID") + appID: this.modelFor("app").get("entityID") }, options); } }); http://git-wip-us.apache.org/repos/asf/tez/blob/8033e3d5/tez-ui/src/main/webapp/app/routes/app/index.js ---------------------------------------------------------------------- diff --git a/tez-ui/src/main/webapp/app/routes/app/index.js b/tez-ui/src/main/webapp/app/routes/app/index.js index 7df42e5..a47cf90 100644 --- a/tez-ui/src/main/webapp/app/routes/app/index.js +++ b/tez-ui/src/main/webapp/app/routes/app/index.js @@ -19,6 +19,8 @@ import Ember from 'ember'; import SingleAmPollsterRoute from '../single-am-pollster'; +import DS from 'ember-data'; + export default SingleAmPollsterRoute.extend({ title: "Application Details", @@ -34,6 +36,29 @@ export default SingleAmPollsterRoute.extend({ }, load: function (value, query, options) { - return this.get("loader").queryRecord('app', this.modelFor("app").get("id"), options); + var appModel = this.modelFor("app"), + loader = this.get("loader"), + appID = appModel.get("entityID"); + + // If it's a dummy object then reset, we have already taken appID from it + if(!(appModel instanceof DS.Model)) { + appModel = null; + } + + return loader.queryRecord('app', "tez_" + appID, options).catch(function (appErr) { + return loader.query('dag', { + appID: appID, + limit: 1 + }, options).then(function (dags) { + // If DAG details or application history is available for the app, then don't throw error + if(dags.get("length") || appModel) { + return Ember.Object.create({ + app: appModel, + appID: appID + }); + } + throw(appErr); + }); + }); }, }); http://git-wip-us.apache.org/repos/asf/tez/blob/8033e3d5/tez-ui/src/main/webapp/app/templates/app/index.hbs ---------------------------------------------------------------------- diff --git a/tez-ui/src/main/webapp/app/templates/app/index.hbs b/tez-ui/src/main/webapp/app/templates/app/index.hbs index 2c04d2c..667e7fe 100644 --- a/tez-ui/src/main/webapp/app/templates/app/index.hbs +++ b/tez-ui/src/main/webapp/app/templates/app/index.hbs @@ -17,7 +17,7 @@ }} {{#if loaded}} - {{#if model.app}} + {{#if model.app.entityID}} <table class='detail-list'> <thead> <tr> @@ -79,13 +79,14 @@ </table> {{/if}} - <table class='detail-list'> - <thead> + {{#if model.entityID}} + <table class='detail-list'> + <thead> <tr> <th colspan=2>Tez Details</th> </tr> - </thead> - <tbody> + </thead> + <tbody> <tr> <td>Entity ID</td> <td>{{model.entityID}}</td> @@ -98,16 +99,16 @@ <td>User</td> <td>{{model.user}}</td> </tr> - </tbody> - </table> + </tbody> + </table> - <table class='detail-list'> - <thead> + <table class='detail-list'> + <thead> <tr> <th colspan=2>Version Details</th> </tr> - </thead> - <tbody> + </thead> + <tbody> <tr> <td>Build Version</td> <td>{{model.tezVersion}}</td> @@ -120,8 +121,28 @@ <td>Build Time</td> <td>{{model.buildTime}}</td> </tr> - </tbody> - </table> + </tbody> + </table> + {{/if}} + + {{#unless (and model.entityID model.app.entityID)}} + <h2>Some data is not available!</h2> + <h5>No data returned from URL: + <i> + {{#unless model.entityID}} + {{hosts.timeline}}/{{env.app.namespaces.webService.timeline}}/{{env.app.paths.timeline.app}}/tez_{{model.appID}} + {{/unless}} + {{#unless model.app.entityID}} + {{#unless model.entityID}} + & + {{/unless}} + {{hosts.timeline}}/{{env.app.namespaces.webService.appHistory}}/apps/{{model.appID}} + {{/unless}} + </i>. + The data may not be available in YARN Timeline or you may not have the necessary permissions to view this data. + </h5> + {{/unless}} + {{else}} {{partial "loading"}} {{/if}} http://git-wip-us.apache.org/repos/asf/tez/blob/8033e3d5/tez-ui/src/main/webapp/tests/unit/controllers/app-test.js ---------------------------------------------------------------------- diff --git a/tez-ui/src/main/webapp/tests/unit/controllers/app-test.js b/tez-ui/src/main/webapp/tests/unit/controllers/app-test.js index 318b665..991aa06 100644 --- a/tez-ui/src/main/webapp/tests/unit/controllers/app-test.js +++ b/tez-ui/src/main/webapp/tests/unit/controllers/app-test.js @@ -45,10 +45,8 @@ test('breadcrumbs test', function(assert) { send: Ember.K, initVisibleColumns: Ember.K, model: { - app: { - name: appName - }, - appID: appID + entityID: appID, + name: appName } }); http://git-wip-us.apache.org/repos/asf/tez/blob/8033e3d5/tez-ui/src/main/webapp/tests/unit/routes/app-test.js ---------------------------------------------------------------------- diff --git a/tez-ui/src/main/webapp/tests/unit/routes/app-test.js b/tez-ui/src/main/webapp/tests/unit/routes/app-test.js index 8e361ea..830198b 100644 --- a/tez-ui/src/main/webapp/tests/unit/routes/app-test.js +++ b/tez-ui/src/main/webapp/tests/unit/routes/app-test.js @@ -27,3 +27,77 @@ test('it exists', function(assert) { let route = this.subject(); assert.ok(route); }); + +test('Test model - Without app data', function(assert) { + let testID = "123", + route = this.subject({ + loader: { + queryRecord: function (type, id) { + assert.ok(type === 'AhsApp' || type === 'appRm'); + assert.equal(id, testID); + return { + catch: function (callBack) { + return callBack(); + } + }; + } + } + }), + data; + + assert.expect(2 + 2 + 1); + + data = route.model({ + "app_id": testID + }); + assert.equal(data.get("entityID"), testID); +}); + +test('Test model - With app data', function(assert) { + let testID1 = "123", + testData1 = {}, + testID2 = "456", + testData2 = {}, + route = this.subject({ + loader: { + queryRecord: function (type, id) { + if(id === "123"){ + assert.equal(type, 'AhsApp'); + return { + catch: function () { + return testData1; + } + }; + } + else if(id === "456") { + if(type === "AhsApp") { + return { + catch: function (callBack) { + return callBack(); + } + }; + } + assert.equal(type, 'appRm'); + return { + catch: function () { + return testData2; + } + }; + } + } + } + }), + data; + + assert.expect(2 + 2); + + data = route.model({ + "app_id": testID1 + }); + assert.equal(data, testData1); + + data = route.model({ + "app_id": testID2 + }); + assert.equal(data, testData2); +}); http://git-wip-us.apache.org/repos/asf/tez/blob/8033e3d5/tez-ui/src/main/webapp/tests/unit/routes/app/configs-test.js ---------------------------------------------------------------------- diff --git a/tez-ui/src/main/webapp/tests/unit/routes/app/configs-test.js b/tez-ui/src/main/webapp/tests/unit/routes/app/configs-test.js index c12189d..3df332b 100644 --- a/tez-ui/src/main/webapp/tests/unit/routes/app/configs-test.js +++ b/tez-ui/src/main/webapp/tests/unit/routes/app/configs-test.js @@ -18,6 +18,8 @@ import { moduleFor, test } from 'ember-qunit'; +import Ember from 'ember'; + moduleFor('route:app/configs', 'Unit | Route | app/configs', { // Specify the other units that are required for this test. // needs: ['controller:foo'] @@ -44,3 +46,52 @@ test('setupController test', function(assert) { route.setupController({}, {}); }); + +test('load test', function(assert) { + let entityID = "123", + testOptions = {}, + testData = {}, + route = this.subject({ + modelFor: function (type) { + assert.equal(type, "app"); + return Ember.Object.create({ + entityID: entityID + }); + } + }); + route.loader = { + queryRecord: function (type, id, options) { + assert.equal(type, "app"); + assert.equal(id, "tez_123"); + assert.equal(options, testOptions); + return Ember.RSVP.resolve(testData); + } + }; + + route.load(null, null, testOptions).then(function (data) { + assert.equal(data, testData); + }); + + assert.expect(1 + 3 + 1); +}); + +test('load failure test', function(assert) { + let route = this.subject({ + modelFor: function (type) { + assert.equal(type, "app"); + return Ember.Object.create(); + }, + }); + route.loader = { + queryRecord: function () { + return Ember.RSVP.reject(new Error()); + } + }; + + route.load(null, null, {}).then(function (data) { + assert.ok(Array.isArray(data)); + assert.equal(data.length, 0); + }); + + assert.expect(1 + 2); +}); \ No newline at end of file http://git-wip-us.apache.org/repos/asf/tez/blob/8033e3d5/tez-ui/src/main/webapp/tests/unit/routes/app/dags-test.js ---------------------------------------------------------------------- diff --git a/tez-ui/src/main/webapp/tests/unit/routes/app/dags-test.js b/tez-ui/src/main/webapp/tests/unit/routes/app/dags-test.js index 8c5f23d..82ee9d1 100644 --- a/tez-ui/src/main/webapp/tests/unit/routes/app/dags-test.js +++ b/tez-ui/src/main/webapp/tests/unit/routes/app/dags-test.js @@ -18,6 +18,8 @@ import { moduleFor, test } from 'ember-qunit'; +import Ember from 'ember'; + moduleFor('route:app/dags', 'Unit | Route | app/dags', { // Specify the other units that are required for this test. // needs: ['controller:foo'] @@ -44,3 +46,33 @@ test('setupController test', function(assert) { route.setupController({}, {}); }); + +test('Test load', function(assert) { + let testID = "123", + testOptions = {}, + testData = {}, + route = this.subject({ + modelFor: function (type) { + assert.equal(type, "app"); + return Ember.Object.create({ + entityID: testID + }); + }, + get: function () { + return { + query: function (type, query, options) { + assert.equal(type, "dag"); + assert.equal(query.appID, testID); + assert.equal(options, testOptions); + return testData; + } + }; + } + }), + data; + + assert.expect(1 + 3 + 1); + + data = route.load(null, null, testOptions); + assert.equal(data, testData); +}); \ No newline at end of file http://git-wip-us.apache.org/repos/asf/tez/blob/8033e3d5/tez-ui/src/main/webapp/tests/unit/routes/app/index-test.js ---------------------------------------------------------------------- diff --git a/tez-ui/src/main/webapp/tests/unit/routes/app/index-test.js b/tez-ui/src/main/webapp/tests/unit/routes/app/index-test.js index 191cc18..ef5917c 100644 --- a/tez-ui/src/main/webapp/tests/unit/routes/app/index-test.js +++ b/tez-ui/src/main/webapp/tests/unit/routes/app/index-test.js @@ -18,6 +18,8 @@ import { moduleFor, test } from 'ember-qunit'; +import Ember from 'ember'; + moduleFor('route:app/index', 'Unit | Route | app/index', { // Specify the other units that are required for this test. // needs: ['controller:foo'] @@ -44,3 +46,48 @@ test('setupController test', function(assert) { route.setupController({}, {}); }); + +test('Test load', function(assert) { + let testID = "123", + testOptions = {}, + testErr = {}, + route = this.subject({ + modelFor: function (type) { + assert.equal(type, "app"); + return Ember.Object.create({ + entityID: testID + }); + }, + get: function () { + return { // loader + queryRecord: function (type, id, options) { + assert.equal(type, "app"); + assert.equal(id, "tez_123"); + assert.equal(options, testOptions); + return { + catch: function (callback) { + return callback(testErr); + } + }; + }, + query: function (type, query, options) { + assert.equal(type, "dag"); + assert.equal(query.appID, testID); + assert.equal(query.limit, 1); + assert.equal(options, testOptions); + return { + then: function (callback) { + return callback([]); + } + }; + } + }; + } + }); + + assert.expect(1 + 3 + 4 + 1); + + assert.throws(function () { + route.load(null, null, testOptions); + }); +}); \ No newline at end of file