[Management] Relationships API test coverage #19737

cuff-links · 2018-06-07T17:20:04Z

Added some tests around the relationships endpoints for management to validate responses and check error handling.

…eturn 404.

elasticmachine · 2018-06-07T18:32:51Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

epixa · 2018-06-07T18:48:32Z

Just to clarify: these are management APIs, not our saved object APIs. The saved object apis are documented here: https://www.elastic.co/guide/en/kibana/6.3/saved-objects-api.html

cuff-links · 2018-06-07T18:53:59Z

@epixa Thanks for the reminder. Keep getting confused because of the directory structure. Noted. Does this title and description work better?

…tionships API for when no result is found. Return 404.

cuff-links · 2018-06-07T21:35:19Z

Fixes #19713

elasticmachine · 2018-06-07T22:18:14Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cuff-links · 2018-06-07T22:35:50Z

jenkins test this

elasticmachine · 2018-06-07T23:45:39Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

chrisronline · 2018-06-12T19:41:29Z

Looks great @silne30!

One thing we might want to do, per our conversation on zoom, is to leave the lib file alone and capture all 404 errors at the route level, then return an explicit 404.

Something like this diff:

diff --git a/src/core_plugins/kibana/server/lib/management/saved_objects/relationships.js b/src/core_plugins/kibana/server/lib/management/saved_objects/relationships.js
index 61e204b8bf..e98bebe4c4 100644
--- a/src/core_plugins/kibana/server/lib/management/saved_objects/relationships.js
+++ b/src/core_plugins/kibana/server/lib/management/saved_objects/relationships.js
@@ -45,11 +45,11 @@ async function findDashboardRelationships(id, size, savedObjectsClient) {
     );
   }
 
-  visualizations.length > 0  ? { visualizations } : 404;
   return { visualizations };
 }
 
 async function findVisualizationRelationships(id, size, savedObjectsClient) {
+  await savedObjectsClient.get('visualization', id);
   const allDashboardsResponse = await savedObjectsClient.find({
     type: 'dashboard',
     fields: ['title', 'panelsJSON'],
@@ -76,7 +76,7 @@ async function findVisualizationRelationships(id, size, savedObjectsClient) {
       break;
     }
   }
-  return dashboards.length > 0 ? { dashboards } : 404;
+  return { dashboards };
 }
 
 async function findSavedSearchRelationships(id, size, savedObjectsClient) {
@@ -109,10 +109,11 @@ async function findSavedSearchRelationships(id, size, savedObjectsClient) {
     return accum;
   }, []);
 
-  return visualizations.length > 0 && indexPatterns.length > 0 ? { visualizations, indexPatterns } : 404;
+  return { visualizations, indexPatterns };
 }
 
 async function findIndexPatternRelationships(id, size, savedObjectsClient) {
+  await savedObjectsClient.get('index-pattern', id);
   const [allVisualizationsResponse, savedSearchResponse] = await Promise.all([
     savedObjectsClient.find({
       type: 'visualization',
@@ -163,7 +164,7 @@ async function findIndexPatternRelationships(id, size, savedObjectsClient) {
       break;
     }
   }
-  return visualizations.length > 0 && searches.length > 0 ? { visualizations, searches } : 404;
+  return { visualizations, searches };
 }
 
 export async function findRelationships(type, id, size, savedObjectsClient) {
diff --git a/src/core_plugins/kibana/server/routes/api/management/saved_objects/relationships.js b/src/core_plugins/kibana/server/routes/api/management/saved_objects/relationships.js
index 015606725d..34eca71cc2 100644
--- a/src/core_plugins/kibana/server/routes/api/management/saved_objects/relationships.js
+++ b/src/core_plugins/kibana/server/routes/api/management/saved_objects/relationships.js
@@ -20,6 +20,7 @@
 import Boom from 'boom';
 import Joi from 'joi';
 import { findRelationships } from '../../../../lib/management/saved_objects/relationships';
+import { isNotFoundError } from '../../../../../../../server/saved_objects/service/lib/errors';
 
 export function registerRelationships(server) {
   server.route({
@@ -44,13 +45,12 @@ export function registerRelationships(server) {
 
       try {
         const response = await findRelationships(type, id, size, req.getSavedObjectsClient());
-
-        if (typeof response === 'number') {
-          reply(Boom.boomify(new Error('Resource not found'), { statusCode: response }));
-        }
-
         reply(response);
       } catch (err) {
+        if (isNotFoundError(err)) {
+          reply(Boom.boomify(new Error('Resource not found'), { statusCode: 404 }));
+          return;
+        }
         reply(Boom.boomify(err, { statusCode: 500 }));
       }
     },

elasticmachine · 2018-06-13T19:53:47Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cuff-links · 2018-06-13T20:02:15Z

jenkins test this

elasticmachine · 2018-06-13T21:13:27Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cuff-links · 2018-06-14T16:02:13Z

jenkins test this again

elasticmachine · 2018-06-14T17:23:24Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

chrisronline · 2018-06-15T13:38:00Z

Hey @silne30

Sorry for leaving you hanging for a few days about these tests.

This patch should fix the build errors:

--- a/src/core_plugins/kibana/server/lib/__tests__/relationships.js
+++ b/src/core_plugins/kibana/server/lib/__tests__/relationships.js
@@ -77,6 +77,7 @@ describe('findRelationships', () => {
     const size = 10;
 
     const savedObjectsClient = {
+      get: () => {},
       find: () => ({
         saved_objects: [
           {
@@ -202,6 +203,7 @@ describe('findRelationships', () => {
     const size = 10;
 
     const savedObjectsClient = {
+      get: () => {},
       find: options => {
         if (options.type === 'visualization') {
           return {

elasticmachine · 2018-06-15T21:02:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-06-15T22:16:26Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

cuff-links · 2018-06-16T03:56:19Z

@chrisronline @jen-huang Whichever one of you wanna merge this bad boy, feel free.

chrisronline · 2018-06-18T13:41:27Z

@silne30 We currently have https://github.com/hapijs/joi for schema validation. Is it possible to use that instead of ajv?

cuff-links · 2018-06-19T16:03:29Z

@chrisronline Yeah. I just throw code on the wall for you guys to tell me all the things I need to change. Working on that now.

elasticmachine · 2018-06-19T21:37:35Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

chrisronline

Looks great! One request

chrisronline · 2018-06-20T16:36:01Z

package.json

@@ -1,15 +1,8 @@
 {
  "name": "kibana",
-  "description": "Kibana is an open source (Apache Licensed), browser based analytics and search dashboard for Elasticsearch. Kibana is a snap to setup and start using. Kibana strives to be easy to get started with, while also being flexible and powerful, just like Elasticsearch.",


Can revert these changes? Doesn't seem necessary

Absolutely. Not even sure when that happened.

I'm still seeing changes here. Try just copying the file from master (https://raw.githubusercontent.com/elastic/kibana/master/package.json) into your copy and commit/push that up

chrisronline

LGTM!

elasticmachine · 2018-06-20T19:56:41Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* Added coverage around search and dashboard tests. * Added tests to check for whether the resource is available. If not, return 404. * Skipped two tests due to elastic#19713. Added error handling for relationships API for when no result is found. Return 404. * Applied patch file per PR. * Applied Chris patch and tested locally. No failures. * Removed ajv and utilised joi for schema validation. * Fixed package.json. * Copied package.json description from master. * Reverted package.json and made proper edit.

* Added coverage around search and dashboard tests. * Added tests to check for whether the resource is available. If not, return 404. * Skipped two tests due to #19713. Added error handling for relationships API for when no result is found. Return 404. * Applied patch file per PR. * Applied Chris patch and tested locally. No failures. * Removed ajv and utilised joi for schema validation. * Fixed package.json. * Copied package.json description from master. * Reverted package.json and made proper edit.

John Dorlus added 2 commits June 6, 2018 18:42

Added coverage around search and dashboard tests.

0ba0763

Added tests to check for whether the resource is available. If not, r…

bf35171

…eturn 404.

cuff-links requested review from timroes and chrisronline June 7, 2018 17:20

cuff-links changed the title ~~Saved objects integration test coverage~~ [Management] Relationships API test coverage Jun 7, 2018

Skipped two tests due to elastic#19713. Added error handling for rela…

3f7a682

…tionships API for when no result is found. Return 404.

jen-huang requested review from jen-huang and removed request for timroes June 7, 2018 21:31

jen-huang added :Management v7.0.0 v6.4.0 labels Jun 7, 2018

Applied patch file per PR.

59d6863

Applied Chris patch and tested locally. No failures.

f05958e

cjcenizal self-requested a review June 18, 2018 17:51

Removed ajv and utilised joi for schema validation.

b9ce932

chrisronline suggested changes Jun 20, 2018

View reviewed changes

John Dorlus added 3 commits June 20, 2018 14:01

Fixed package.json.

851ddb9

Copied package.json description from master.

fff44b2

Reverted package.json and made proper edit.

d871324

chrisronline approved these changes Jun 20, 2018

View reviewed changes

cuff-links merged commit fea9c42 into elastic:master Jun 20, 2018

cuff-links deleted the Saved_Objects_Integration_Test_Coverage branch June 20, 2018 20:07

cuff-links mentioned this pull request Jul 13, 2018

[6.x] [Management] Relationships API test coverage (#19737) #20785

Closed

cuff-links mentioned this pull request Jul 13, 2018

[6.x] [Management] Relationships API test coverage (#19737) #20788

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Management] Relationships API test coverage #19737

[Management] Relationships API test coverage #19737

cuff-links commented Jun 7, 2018 •

edited

Loading

elasticmachine commented Jun 7, 2018

epixa commented Jun 7, 2018

cuff-links commented Jun 7, 2018

cuff-links commented Jun 7, 2018

elasticmachine commented Jun 7, 2018

cuff-links commented Jun 7, 2018

elasticmachine commented Jun 7, 2018

chrisronline commented Jun 12, 2018

elasticmachine commented Jun 13, 2018

cuff-links commented Jun 13, 2018

elasticmachine commented Jun 13, 2018

cuff-links commented Jun 14, 2018

elasticmachine commented Jun 14, 2018

chrisronline commented Jun 15, 2018

elasticmachine commented Jun 15, 2018

elasticmachine commented Jun 15, 2018

cuff-links commented Jun 16, 2018

chrisronline commented Jun 18, 2018

cuff-links commented Jun 19, 2018

elasticmachine commented Jun 19, 2018

chrisronline left a comment

chrisronline Jun 20, 2018

cuff-links Jun 20, 2018

chrisronline Jun 20, 2018

chrisronline left a comment

elasticmachine commented Jun 20, 2018

[Management] Relationships API test coverage #19737

[Management] Relationships API test coverage #19737

Conversation

cuff-links commented Jun 7, 2018 • edited Loading

elasticmachine commented Jun 7, 2018

💔 Build Failed

epixa commented Jun 7, 2018

cuff-links commented Jun 7, 2018

cuff-links commented Jun 7, 2018

elasticmachine commented Jun 7, 2018

💔 Build Failed

cuff-links commented Jun 7, 2018

elasticmachine commented Jun 7, 2018

💚 Build Succeeded

chrisronline commented Jun 12, 2018

elasticmachine commented Jun 13, 2018

💔 Build Failed

cuff-links commented Jun 13, 2018

elasticmachine commented Jun 13, 2018

💔 Build Failed

cuff-links commented Jun 14, 2018

elasticmachine commented Jun 14, 2018

💔 Build Failed

chrisronline commented Jun 15, 2018

elasticmachine commented Jun 15, 2018

💚 Build Succeeded

elasticmachine commented Jun 15, 2018

💚 Build Succeeded

cuff-links commented Jun 16, 2018

chrisronline commented Jun 18, 2018

cuff-links commented Jun 19, 2018

elasticmachine commented Jun 19, 2018

💚 Build Succeeded

chrisronline left a comment

Choose a reason for hiding this comment

chrisronline Jun 20, 2018

Choose a reason for hiding this comment

cuff-links Jun 20, 2018

Choose a reason for hiding this comment

chrisronline Jun 20, 2018

Choose a reason for hiding this comment

chrisronline left a comment

Choose a reason for hiding this comment

elasticmachine commented Jun 20, 2018

💚 Build Succeeded

cuff-links commented Jun 7, 2018 •

edited

Loading