From a496315881c0d2ea25258c796f99c1a62a8e1b25 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Thu, 26 Jul 2018 17:43:58 -0700 Subject: [PATCH] Make the Stage insertion API in CodePipeline more flexible. --- .../@aws-cdk/aws-codepipeline/lib/pipeline.ts | 84 +++++++++-- .../@aws-cdk/aws-codepipeline/lib/stage.ts | 50 ++++++- .../aws-codepipeline/test/test.stages.ts | 141 ++++++++++++++++++ packages/@aws-cdk/cdk/lib/core/construct.ts | 14 +- 4 files changed, 269 insertions(+), 20 deletions(-) create mode 100644 packages/@aws-cdk/aws-codepipeline/test/test.stages.ts diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts index fe226459edb8e..c82bf35651b23 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts @@ -4,7 +4,7 @@ import s3 = require('@aws-cdk/aws-s3'); import cdk = require('@aws-cdk/cdk'); import util = require('@aws-cdk/util'); import { cloudformation } from './codepipeline.generated'; -import { Stage } from './stage'; +import { Stage, StagePlacement, StageProps } from './stage'; import validation = require('./validation'); /** @@ -86,7 +86,7 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget { */ public readonly artifactBucket: s3.BucketRef; - private readonly stages = new Array(); + private readonly _stages = new Array(); private eventsRole?: iam.Role; constructor(parent: cdk.Construct, name: string, props?: PipelineProps) { @@ -131,6 +131,20 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget { })); } + /** + * Get a duplicate of this Pipeline's list of Stages. + */ + public get stages(): Stage[] { + return this._stages.slice(); + } + + /** + * Get the number of Stages in this Pipeline. + */ + public get stagesLength(): number { + return this._stages.length; + } + /** * Adds a statement to the pipeline role. */ @@ -216,23 +230,71 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget { * onChildAdded type hook. * @override */ - protected addChild(child: cdk.Construct, name: string) { - super.addChild(child, name); + protected addChild(child: cdk.Construct, name: string, props?: any) { + super.addChild(child, name, props); if (child instanceof Stage) { - this.appendStage(child); + this.appendStage(child, props ? (props as StageProps).placed : undefined); } } - private appendStage(stage: Stage) { - if (this.stages.find(x => x.name === stage.name)) { + private appendStage(stage: Stage, placement?: StagePlacement) { + if (this._stages.find(x => x.name === stage.name)) { throw new Error(`A stage with name '${stage.name}' already exists`); } - this.stages.push(stage); + const index = placement + ? this.calculateInsertIndexFromPlacement(placement) + : this.stagesLength; + + this._stages.splice(index, 0, stage); + } + + private calculateInsertIndexFromPlacement(placement: StagePlacement): number { + // check if at most one placement property was provided + const providedPlacementProps = ['rightBeforeStage', 'justAfterStage', 'atIndex'] + .filter((prop) => (placement as any)[prop] !== undefined); + if (providedPlacementProps.length > 1) { + throw new Error("Error adding Stage to the Pipeline: " + + `you can only provide at most one placement property, ${providedPlacementProps} were given`); + } + + if (placement.rightBeforeStage !== undefined) { + const targetIndex = this.findStageIndex(placement.rightBeforeStage); + if (targetIndex === -1) { + throw new Error("Error adding Stage to the Pipeline: " + + `the requested Stage to add it before, '${placement.rightBeforeStage.name}', was not found`); + } + return targetIndex; + } + + if (placement.justAfterStage !== undefined) { + const targetIndex = this.findStageIndex(placement.justAfterStage); + if (targetIndex === -1) { + throw new Error("Error adding Stage to the Pipeline: " + + `the requested Stage to add it after, '${placement.justAfterStage.name}', was not found`); + } + return targetIndex + 1; + } + + if (placement.atIndex !== undefined) { + const index = placement.atIndex; + if (index < 0 || index > this.stagesLength) { + throw new Error("Error adding Stage to the Pipeline: " + + `{ placed: atIndex } should be between 0 and the number of stages in the Pipeline (${this.stagesLength}), ` + + ` got: ${index}`); + } + return index; + } + + return this.stagesLength; + } + + private findStageIndex(targetStage: Stage) { + return this._stages.findIndex((stage: Stage) => stage === targetStage); } private validateSourceActionLocations(): string[] { - return util.flatMap(this.stages, (stage, i) => { + return util.flatMap(this._stages, (stage, i) => { const onlySourceActionsPermitted = i === 0; return util.flatMap(stage.actions, (action, _) => validation.validateSourceAction(onlySourceActionsPermitted, action.category, action.name, stage.name) @@ -241,7 +303,7 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget { } private validateHasStages(): string[] { - if (this.stages.length < 2) { + if (this._stages.length < 2) { return ['Pipeline must have at least two stages']; } return []; @@ -270,6 +332,6 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget { } private renderStages(): cloudformation.PipelineResource.StageDeclarationProperty[] { - return this.stages.map(stage => stage.render()); + return this._stages.map(stage => stage.render()); } } diff --git a/packages/@aws-cdk/aws-codepipeline/lib/stage.ts b/packages/@aws-cdk/aws-codepipeline/lib/stage.ts index 3f74cf9db9985..596095bdd4735 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/stage.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/stage.ts @@ -5,6 +5,52 @@ import { cloudformation } from './codepipeline.generated'; import { Pipeline } from './pipeline'; import validation = require('./validation'); +/** + * The construction properties for {@link Stage}. + */ +export interface StageProps { + /** + * Allows specifying where should the newly created {@link Stage} + * be placed in the Pipeline. + * + * @default the stage is added to the end of the Pipeline + */ + readonly placed?: StagePlacement; +} + +/** + * Allows you to control where to place a new Stage when it's added to the Pipeline. + * Note that you can provide only one of the below properties - + * specifying more than one will result in a validation error. + * + * @see #rightBeforeStage + * @see #justAfterStage + * @see #atIndex + */ +export interface StagePlacement { + /** + * Inserts the new Stage as a parent of the given Stage + * (changing its current parent Stage, if it had one). + */ + readonly rightBeforeStage?: Stage; + + /** + * Inserts the new Stage as a child of the given Stage + * (changing its current child Stage, if it had one). + */ + readonly justAfterStage?: Stage; + + /** + * Inserts the new Stage at the given index in the Pipeline, + * moving the Stage currently at that index, + * and any subsequent ones, one index down. + * Indexing starts at 0. + * The maximum allowed value is {@link Pipeline#stagesLength}, + * which will insert the new Stage at the end of the Pipeline. + */ + readonly atIndex?: number; +} + /** * A stage in a pipeline. Stages are added to a pipeline by constructing a Stage with * the pipeline as the first argument to the constructor. @@ -27,8 +73,8 @@ export class Stage extends cdk.Construct { * always be attached to a pipeline. It's illogical to construct a Stage * with any other parent. */ - constructor(parent: Pipeline, name: string) { - super(parent, name); + constructor(parent: Pipeline, name: string, props?: StageProps) { + super(parent, name, props); this.pipeline = parent; validation.validateName('Stage', name); } diff --git a/packages/@aws-cdk/aws-codepipeline/test/test.stages.ts b/packages/@aws-cdk/aws-codepipeline/test/test.stages.ts new file mode 100644 index 0000000000000..3f16c91eaec28 --- /dev/null +++ b/packages/@aws-cdk/aws-codepipeline/test/test.stages.ts @@ -0,0 +1,141 @@ +import cdk = require('@aws-cdk/cdk'); +import { Test } from 'nodeunit'; +import codepipeline = require('../lib'); + +// tslint:disable:object-literal-key-quotes + +export = { + 'Pipeline Stages': { + 'can be inserted at index 0'(test: Test) { + const pipeline = pipelineForTesting(); + + const secondStage = new codepipeline.Stage(pipeline, 'SecondStage'); + const firstStage = new codepipeline.Stage(pipeline, 'FirstStage', { + placed: { + atIndex: 0, + } + }); + + test.equal(pipeline.stages[0].name, firstStage.name); + test.equal(pipeline.stages[1].name, secondStage.name); + + test.done(); + }, + + 'can be inserted before another Stage'(test: Test) { + const pipeline = pipelineForTesting(); + + const secondStage = new codepipeline.Stage(pipeline, 'SecondStage'); + const firstStage = new codepipeline.Stage(pipeline, 'FirstStage', { + placed: { + rightBeforeStage: secondStage, + } + }); + + test.equal(pipeline.stages[0].name, firstStage.name); + test.equal(pipeline.stages[1].name, secondStage.name); + + test.done(); + }, + + 'can be inserted after another Stage'(test: Test) { + const pipeline = pipelineForTesting(); + + const firstStage = new codepipeline.Stage(pipeline, 'FirstStage'); + const thirdStage = new codepipeline.Stage(pipeline, 'ThirdStage'); + const secondStage = new codepipeline.Stage(pipeline, 'SecondStage', { + placed: { + justAfterStage: firstStage, + } + }); + + test.equal(pipeline.stages[0].name, firstStage.name); + test.equal(pipeline.stages[1].name, secondStage.name); + test.equal(pipeline.stages[2].name, thirdStage.name); + + test.done(); + }, + + 'attempting to insert a Stage at a negative index results in an error'(test: Test) { + const pipeline = pipelineForTesting(); + + test.throws(() => { + new codepipeline.Stage(pipeline, 'Stage', { + placed: { + atIndex: -1, + } + }); + }, /atIndex/); + + test.done(); + }, + + 'attempting to insert a Stage at an index larger than the current number of Stages results in an error'(test: Test) { + const pipeline = pipelineForTesting(); + + test.throws(() => { + new codepipeline.Stage(pipeline, 'Stage', { + placed: { + atIndex: 1, + } + }); + }, /atIndex/); + + test.done(); + }, + + "attempting to insert a Stage before a Stage that doesn't exist results in an error"(test: Test) { + const pipeline = pipelineForTesting(); + const stage = new codepipeline.Stage(pipeline, 'Stage'); + + const anotherPipeline = pipelineForTesting(); + test.throws(() => { + new codepipeline.Stage(anotherPipeline, 'Stage', { + placed: { + rightBeforeStage: stage, + } + }); + }, /before/i); + + test.done(); + }, + + "attempting to insert a Stage after a Stage that doesn't exist results in an error"(test: Test) { + const pipeline = pipelineForTesting(); + const stage = new codepipeline.Stage(pipeline, 'Stage'); + + const anotherPipeline = pipelineForTesting(); + test.throws(() => { + new codepipeline.Stage(anotherPipeline, 'Stage', { + placed: { + justAfterStage: stage, + } + }); + }, /after/i); + + test.done(); + }, + + "providing more than one placement value results in an error"(test: Test) { + const pipeline = pipelineForTesting(); + const stage = new codepipeline.Stage(pipeline, 'FirstStage'); + + test.throws(() => { + new codepipeline.Stage(pipeline, 'SecondStage', { + placed: { + rightBeforeStage: stage, + justAfterStage: stage, + } + }); + }); + + test.done(); + }, + }, +}; + +function pipelineForTesting(): codepipeline.Pipeline { + const stack = new cdk.Stack(); + const pipeline = new codepipeline.Pipeline(stack, 'Pipeline'); + return pipeline; +} diff --git a/packages/@aws-cdk/cdk/lib/core/construct.ts b/packages/@aws-cdk/cdk/lib/core/construct.ts index f07790d4c4c91..ff27135c986e7 100644 --- a/packages/@aws-cdk/cdk/lib/core/construct.ts +++ b/packages/@aws-cdk/cdk/lib/core/construct.ts @@ -35,9 +35,10 @@ export class Construct { * Creates a new construct node. * * @param parent The parent construct - * @param props Properties for this construct + * @param name The logical identfifier of the new Construct + * @param props Optional properties for this construct */ - constructor(parent: Construct, name: string) { + constructor(parent: Construct, name: string, props?: any) { this.name = name; this.parent = parent; @@ -49,7 +50,7 @@ export class Construct { } // Has side effect so must be very last thing in constructor - parent.addChild(this, this.name); + parent.addChild(this, this.name, props); } else { // This is a root construct. this.name = name; @@ -324,12 +325,11 @@ export class Construct { * Adds a child construct to this node. * * @param child The child construct - * @param name The type name of the child construct. - * @returns The resolved path part name of the child + * @param childName The logical ID of the child construct + * @param _props The optional properties of the child construct */ - protected addChild(child: Construct, childName: string) { + protected addChild(child: Construct, childName: string, _props?: any) { if (this.locked) { - // special error if root is locked if (!this.path) { throw new Error('Cannot add children during synthesis');