From 7c0c6e22f85fe58391835ab97b9c9287e7088ab8 Mon Sep 17 00:00:00 2001 From: Jim Unger Date: Thu, 7 Apr 2016 10:02:31 -0500 Subject: [PATCH 1/3] [add data] fixed pipeline output when no processors in error --- .../pipeline_setup/lib/__tests__/pipeline.js | 180 +++++++++--------- .../pipeline_setup/lib/pipeline.js | 7 +- .../pipeline_setup/lib/processor_types.js | 4 + 3 files changed, 99 insertions(+), 92 deletions(-) diff --git a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/__tests__/pipeline.js b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/__tests__/pipeline.js index aa03d76171b2c..44074f8cc43ee 100644 --- a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/__tests__/pipeline.js +++ b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/__tests__/pipeline.js @@ -1,18 +1,11 @@ import _ from 'lodash'; import expect from 'expect.js'; import sinon from 'sinon'; -import Pipeline from '../../lib/pipeline'; +import Pipeline from '../pipeline'; +import * as processorTypes from '../processor_types'; describe('processor pipeline', function () { - class TestProcessor { - constructor(processorId) { - this.processorId = processorId; - } - - setParent(newParent) { } - } - function getProcessorIds(pipeline) { return pipeline.processors.map(p => p.processorId); } @@ -30,8 +23,7 @@ describe('processor pipeline', function () { it('should access the model property of each processor', function () { const pipeline = new Pipeline(); pipeline.input = { foo: 'bar' }; - pipeline.add(TestProcessor); - pipeline.processors[0].model = { bar: 'baz' }; + pipeline.add(processorTypes.Set); const actual = pipeline.model; const expected = { @@ -48,15 +40,15 @@ describe('processor pipeline', function () { it('should remove existing processors from the pipeline', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); const oldProcessors = [ pipeline.processors[0], pipeline.processors[1], pipeline.processors[2] ]; const newPipeline = new Pipeline(); - newPipeline.add(TestProcessor); - newPipeline.add(TestProcessor); - newPipeline.add(TestProcessor); + newPipeline.add(processorTypes.Set); + newPipeline.add(processorTypes.Set); + newPipeline.add(processorTypes.Set); pipeline.load(newPipeline); @@ -70,9 +62,9 @@ describe('processor pipeline', function () { sinon.stub(pipeline, 'addExisting'); const newPipeline = new Pipeline(); - newPipeline.add(TestProcessor); - newPipeline.add(TestProcessor); - newPipeline.add(TestProcessor); + newPipeline.add(processorTypes.Set); + newPipeline.add(processorTypes.Set); + newPipeline.add(processorTypes.Set); pipeline.load(newPipeline); @@ -87,9 +79,9 @@ describe('processor pipeline', function () { it('remove the specified processor from the processors collection', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); const processorIds = getProcessorIds(pipeline); @@ -108,18 +100,18 @@ describe('processor pipeline', function () { expect(pipeline.processors.length).to.be(0); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); expect(pipeline.processors.length).to.be(3); }); it('should append assign each new processor a unique processorId', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); const ids = pipeline.processors.map((p) => { return p.processorId; }); expect(_.uniq(ids).length).to.be(3); @@ -127,13 +119,13 @@ describe('processor pipeline', function () { it('added processors should be an instance of the type supplied', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); - expect(pipeline.processors[0] instanceof TestProcessor).to.be(true); - expect(pipeline.processors[1] instanceof TestProcessor).to.be(true); - expect(pipeline.processors[2] instanceof TestProcessor).to.be(true); + expect(pipeline.processors[0] instanceof processorTypes.Set).to.be(true); + expect(pipeline.processors[1] instanceof processorTypes.Set).to.be(true); + expect(pipeline.processors[2] instanceof processorTypes.Set).to.be(true); }); }); @@ -145,36 +137,27 @@ describe('processor pipeline', function () { expect(pipeline.processors.length).to.be(0); - const testProcessor = new TestProcessor(); - testProcessor.processorId = 'foo'; - testProcessor.foo = 'bar'; - testProcessor.bar = 'baz'; + const testProcessor = new processorTypes.Set('foo'); pipeline.addExisting(testProcessor); expect(pipeline.processors.length).to.be(1); }); - it('should instanciate an object of the same class as the object passed in', function () { + it('should instantiate an object of the same class as the object passed in', function () { const pipeline = new Pipeline(); - const testProcessor = new TestProcessor(); - testProcessor.processorId = 'foo'; - testProcessor.foo = 'bar'; - testProcessor.bar = 'baz'; + const testProcessor = new processorTypes.Set('foo'); pipeline.addExisting(testProcessor); - expect(pipeline.processors[0] instanceof TestProcessor).to.be(true); + expect(pipeline.processors[0] instanceof processorTypes.Set).to.be(true); }); it('the object added should be a different instance than the object passed in', function () { const pipeline = new Pipeline(); - const testProcessor = new TestProcessor(); - testProcessor.processorId = 'foo'; - testProcessor.foo = 'bar'; - testProcessor.bar = 'baz'; + const testProcessor = new processorTypes.Set('foo'); pipeline.addExisting(testProcessor); @@ -184,8 +167,7 @@ describe('processor pipeline', function () { it('the object added should have the same property values as the object passed in (except id)', function () { const pipeline = new Pipeline(); - const testProcessor = new TestProcessor(); - testProcessor.processorId = 'foo'; + const testProcessor = new processorTypes.Set('foo'); testProcessor.foo = 'bar'; testProcessor.bar = 'baz'; @@ -202,9 +184,9 @@ describe('processor pipeline', function () { it('should be able to move an item up in the array', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); const processorIds = getProcessorIds(pipeline); const target = pipeline.processors[1]; @@ -217,9 +199,9 @@ describe('processor pipeline', function () { it('should be able to move the same item move than once', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); const processorIds = getProcessorIds(pipeline); const target = pipeline.processors[2]; @@ -233,9 +215,9 @@ describe('processor pipeline', function () { it('should not move the selected item past the top', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); const processorIds = getProcessorIds(pipeline); const target = pipeline.processors[2]; @@ -252,9 +234,9 @@ describe('processor pipeline', function () { it('should not allow the top item to be moved up', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); const processorIds = getProcessorIds(pipeline); const target = pipeline.processors[0]; @@ -271,9 +253,9 @@ describe('processor pipeline', function () { it('should be able to move an item down in the array', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); const processorIds = getProcessorIds(pipeline); const target = pipeline.processors[1]; @@ -286,9 +268,9 @@ describe('processor pipeline', function () { it('should be able to move the same item move than once', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); const processorIds = getProcessorIds(pipeline); const target = pipeline.processors[0]; @@ -302,9 +284,9 @@ describe('processor pipeline', function () { it('should not move the selected item past the bottom', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); const processorIds = getProcessorIds(pipeline); const target = pipeline.processors[0]; @@ -321,9 +303,9 @@ describe('processor pipeline', function () { it('should not allow the bottom item to be moved down', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); const processorIds = getProcessorIds(pipeline); const target = pipeline.processors[2]; @@ -342,8 +324,8 @@ describe('processor pipeline', function () { const pipeline = new Pipeline(); pipeline.input = { foo: 'bar' }; - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); pipeline.processors.forEach(p => sinon.stub(p, 'setParent')); @@ -356,10 +338,10 @@ describe('processor pipeline', function () { const pipeline = new Pipeline(); pipeline.input = { foo: 'bar' }; - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); pipeline.processors.forEach(p => sinon.stub(p, 'setParent')); @@ -383,9 +365,9 @@ describe('processor pipeline', function () { it('should return a processor when suppied its id', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); const processorIds = getProcessorIds(pipeline); const actual = pipeline.getProcessorById(processorIds[2]); @@ -404,9 +386,33 @@ describe('processor pipeline', function () { describe('updateOutput', function () { + it('should set the output to the last processor with valid output if a processor has an error', function () { + const pipeline = new Pipeline(); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + + pipeline.processors[0].outputObject = { field1: 'value1' }; + pipeline.processors[1].outputObject = { field1: 'value2' }; + pipeline.processors[2].outputObject = { field1: 'value3' }; + + pipeline.updateOutput(); + expect(pipeline.output).to.eql({ field1: 'value3' }); + + pipeline.processors[2].error = {}; //define an error + pipeline.updateOutput(); + expect(pipeline.output).to.eql({ field1: 'value2' }); + + pipeline.processors[1].error = {}; //define an error + pipeline.processors[2].error = undefined; //if processor[1] has an error, + pipeline.processors[2].locked = true; //subsequent processors will be locked. + pipeline.updateOutput(); + expect(pipeline.output).to.eql({ field1: 'value1' }); + }); + it('should set output to be last processors output if processors exist', function () { const pipeline = new Pipeline(); - pipeline.add(TestProcessor); + pipeline.add(processorTypes.Set); const expected = { foo: 'bar' }; pipeline.processors[0].outputObject = expected; @@ -415,11 +421,11 @@ describe('processor pipeline', function () { expect(pipeline.output).to.be(expected); }); - it('should set output to be undefined if no processors exist', function () { + it('should set output to be equal to input if no processors exist', function () { const pipeline = new Pipeline(); pipeline.updateOutput(); - expect(pipeline.output).to.be(undefined); + expect(pipeline.output).to.be(pipeline.input); }); it('should set pipeline.dirty', function () { diff --git a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/pipeline.js b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/pipeline.js index da9129414b036..c9e7af2b56201 100644 --- a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/pipeline.js +++ b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/pipeline.js @@ -156,11 +156,8 @@ export default class Pipeline { updateOutput() { const processors = this.processors; - - this.output = undefined; - if (processors.length > 0) { - this.output = processors[processors.length - 1].outputObject; - } + const index = _.findLastIndex(processors, processor => { return processor.hasValidOutput; }); + this.output = (index === -1) ? this.input : processors[index].outputObject; this.dirty = false; } diff --git a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/processor_types.js b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/processor_types.js index 06fb71514f23a..105801cd1c321 100644 --- a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/processor_types.js +++ b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/processor_types.js @@ -14,6 +14,10 @@ class Processor { this.error = undefined; } + get hasValidOutput() { + return !this.locked && !this.error; + } + setParent(newParent) { const oldParent = this.parent; this.parent = newParent; From be93f5c4bf24563336bad04052bf9081a3f12ceb Mon Sep 17 00:00:00 2001 From: Jim Unger Date: Thu, 7 Apr 2016 10:42:16 -0500 Subject: [PATCH 2/3] [add data] updated test case --- .../add_data_steps/pipeline_setup/lib/__tests__/pipeline.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/__tests__/pipeline.js b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/__tests__/pipeline.js index 44074f8cc43ee..d89cf15352324 100644 --- a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/__tests__/pipeline.js +++ b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/__tests__/pipeline.js @@ -412,6 +412,7 @@ describe('processor pipeline', function () { it('should set output to be last processors output if processors exist', function () { const pipeline = new Pipeline(); + pipeline.input = { bar: 'baz' }; pipeline.add(processorTypes.Set); const expected = { foo: 'bar' }; @@ -423,6 +424,7 @@ describe('processor pipeline', function () { it('should set output to be equal to input if no processors exist', function () { const pipeline = new Pipeline(); + pipeline.input = { bar: 'baz' }; pipeline.updateOutput(); expect(pipeline.output).to.be(pipeline.input); From 168a173bdf6b49b259c0e93342196f54215bd654 Mon Sep 17 00:00:00 2001 From: Jim Unger Date: Thu, 7 Apr 2016 13:36:29 -0500 Subject: [PATCH 3/3] [add data] removed hasValidOutput and reworked pipeline output logic --- .../pipeline_setup/lib/__tests__/pipeline.js | 35 ++++++++++++++++++- .../pipeline_setup/lib/pipeline.js | 7 ++-- .../pipeline_setup/lib/processor_types.js | 4 --- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/__tests__/pipeline.js b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/__tests__/pipeline.js index d89cf15352324..b6053b22a32ee 100644 --- a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/__tests__/pipeline.js +++ b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/__tests__/pipeline.js @@ -386,6 +386,39 @@ describe('processor pipeline', function () { describe('updateOutput', function () { + it('should set the output to input if first processor has error', function () { + const pipeline = new Pipeline(); + pipeline.input = { bar: 'baz' }; + pipeline.add(processorTypes.Set); + + pipeline.processors[0].outputObject = { field1: 'value1' }; + pipeline.processors[0].error = {}; //define an error + + pipeline.updateOutput(); + expect(pipeline.output).to.be(pipeline.input); + }); + + it('should set the output to the processor before the error on a compile error', function () { + const pipeline = new Pipeline(); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + pipeline.add(processorTypes.Set); + + pipeline.processors[0].outputObject = { field1: 'value1' }; + pipeline.processors[1].outputObject = { field1: 'value2' }; + pipeline.processors[2].outputObject = { field1: 'value3' }; + + pipeline.updateOutput(); + expect(pipeline.output).to.eql({ field1: 'value3' }); + + pipeline.processors[1].error = { compile: true }; //define a compile error + pipeline.processors[0].locked = true; //all other processors get locked. + pipeline.processors[2].locked = true; //all other processors get locked. + + pipeline.updateOutput(); + expect(pipeline.output).to.eql({ field1: 'value1' }); + }); + it('should set the output to the last processor with valid output if a processor has an error', function () { const pipeline = new Pipeline(); pipeline.add(processorTypes.Set); @@ -410,7 +443,7 @@ describe('processor pipeline', function () { expect(pipeline.output).to.eql({ field1: 'value1' }); }); - it('should set output to be last processors output if processors exist', function () { + it('should set output to be last processor output if processors exist', function () { const pipeline = new Pipeline(); pipeline.input = { bar: 'baz' }; pipeline.add(processorTypes.Set); diff --git a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/pipeline.js b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/pipeline.js index c9e7af2b56201..17f562e677ade 100644 --- a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/pipeline.js +++ b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/pipeline.js @@ -156,8 +156,11 @@ export default class Pipeline { updateOutput() { const processors = this.processors; - const index = _.findLastIndex(processors, processor => { return processor.hasValidOutput; }); - this.output = (index === -1) ? this.input : processors[index].outputObject; + + const errorIndex = _.findIndex(processors, 'error'); + const goodProcessor = errorIndex === -1 ? _.last(processors) : processors[errorIndex - 1]; + this.output = goodProcessor ? goodProcessor.outputObject : this.input; + this.dirty = false; } diff --git a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/processor_types.js b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/processor_types.js index 105801cd1c321..06fb71514f23a 100644 --- a/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/processor_types.js +++ b/src/plugins/kibana/public/settings/sections/indices/add_data_steps/pipeline_setup/lib/processor_types.js @@ -14,10 +14,6 @@ class Processor { this.error = undefined; } - get hasValidOutput() { - return !this.locked && !this.error; - } - setParent(newParent) { const oldParent = this.parent; this.parent = newParent;