From 483025400fc03e10dc58f09eb7cbe25a79ca6e1f Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Thu, 26 Jul 2018 17:43:58 -0700 Subject: [PATCH] feat(aws-codepipeline): Make the Stage insertion API in CodePipeline more flexible. This commit allows clients of CodePipeline to create new Stages placed at an arbitrary index in the Pipeline, or before/after a given Stage (instead of only appending new Stages at the end). --- .../aws-codepipeline-api/lib/action.ts | 4 +- packages/@aws-cdk/aws-codepipeline/README.md | 14 ++ .../@aws-cdk/aws-codepipeline/lib/pipeline.ts | 73 ++++++++- .../@aws-cdk/aws-codepipeline/lib/stage.ts | 56 ++++++- .../aws-codepipeline/test/test.stages.ts | 145 +++++++++++++++++- 5 files changed, 277 insertions(+), 15 deletions(-) diff --git a/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts b/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts index ef00f15384163..e5fa0f06b8801 100644 --- a/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts +++ b/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts @@ -70,7 +70,7 @@ export interface IStage { * * @param action the Action to add to this Stage */ - _addAction(action: Action): void; + _attachAction(action: Action): void; } /** @@ -151,7 +151,7 @@ export abstract class Action extends cdk.Construct { this.runOrder = 1; this.stage = props.stage; - this.stage._addAction(this); + this.stage._attachAction(this); } public validate(): string[] { diff --git a/packages/@aws-cdk/aws-codepipeline/README.md b/packages/@aws-cdk/aws-codepipeline/README.md index c47348e969990..5318dd1135ce8 100644 --- a/packages/@aws-cdk/aws-codepipeline/README.md +++ b/packages/@aws-cdk/aws-codepipeline/README.md @@ -17,6 +17,20 @@ const sourceStage = pipeline.addStage('Source'); You can also instantiate the `Stage` Construct directly, which will add it to the Pipeline provided in its construction properties. +You can insert the new Stage at an arbitrary point in the Pipeline: + +```ts +const sourceStage = pipeline.addStage('Source', { + placement: { + // note: you can only specify one of the below properties + rightBefore: anotherStage, + justAfter: anotherStage, + atIndex: 3, // indexing starts at 0 + // pipeline.stageCount returns the number of Stages currently in the Pipeline + } +}); +``` + Add an Action to a Stage: ```ts diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts index cfdbbeccb1b13..43f0543a124ce 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts @@ -5,7 +5,7 @@ import s3 = require('@aws-cdk/aws-s3'); import cdk = require('@aws-cdk/cdk'); import util = require('@aws-cdk/util'); import { cloudformation, PipelineName, PipelineVersion } from './codepipeline.generated'; -import { Stage } from './stage'; +import { CommonStageProps, Stage, StagePlacement } from './stage'; /** * The ARN of a pipeline @@ -126,11 +126,13 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget { * and adding it to this Pipeline. * * @param name the name of the newly created Stage + * @param props the optional construction properties of the new Stage * @returns the newly created Stage */ - public addStage(name: string): Stage { + public addStage(name: string, props?: CommonStageProps): Stage { return new Stage(this, name, { pipeline: this, + ...props, }); } @@ -213,6 +215,13 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget { ]); } + /** + * Get the number of Stages in this Pipeline. + */ + public get stageCount(): number { + return this.stages.length; + } + /** * Adds a Stage to this Pipeline. * This is an internal operation - @@ -221,9 +230,12 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget { * so there is never a need to call this method explicitly. * * @param stage the newly created Stage to add to this Pipeline + * @param placement an optional specification of where to place the newly added Stage in the Pipeline */ - public _addStage(stage: Stage): void { - // _addStage should be idempotent, in case a customer ever calls it directly + // ignore unused private method (it's actually used in Stage) + // @ts-ignore + private _attachStage(stage: Stage, placement?: StagePlacement): void { + // _attachStage should be idempotent, in case a customer ever calls it directly if (this.stages.includes(stage)) { return; } @@ -232,7 +244,56 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget { throw new Error(`A stage with name '${stage.name}' already exists`); } - this.stages.push(stage); + const index = placement + ? this.calculateInsertIndexFromPlacement(placement) + : this.stageCount; + + this.stages.splice(index, 0, stage); + } + + private calculateInsertIndexFromPlacement(placement: StagePlacement): number { + // check if at most one placement property was provided + const providedPlacementProps = ['rightBefore', 'justAfter', '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, but ' + + `'${providedPlacementProps.join(', ')}' were given`); + } + + if (placement.rightBefore !== undefined) { + const targetIndex = this.findStageIndex(placement.rightBefore); + if (targetIndex === -1) { + throw new Error("Error adding Stage to the Pipeline: " + + `the requested Stage to add it before, '${placement.rightBefore.name}', was not found`); + } + return targetIndex; + } + + if (placement.justAfter !== undefined) { + const targetIndex = this.findStageIndex(placement.justAfter); + if (targetIndex === -1) { + throw new Error("Error adding Stage to the Pipeline: " + + `the requested Stage to add it after, '${placement.justAfter.name}', was not found`); + } + return targetIndex + 1; + } + + if (placement.atIndex !== undefined) { + const index = placement.atIndex; + if (index < 0 || index > this.stageCount) { + throw new Error("Error adding Stage to the Pipeline: " + + `{ placed: atIndex } should be between 0 and the number of stages in the Pipeline (${this.stageCount}), ` + + ` got: ${index}`); + } + return index; + } + + return this.stageCount; + } + + private findStageIndex(targetStage: Stage) { + return this.stages.findIndex((stage: Stage) => stage === targetStage); } private validateSourceActionLocations(): string[] { @@ -245,7 +306,7 @@ export class Pipeline extends cdk.Construct implements events.IEventRuleTarget { } private validateHasStages(): string[] { - if (this.stages.length < 2) { + if (this.stageCount < 2) { return ['Pipeline must have at least two stages']; } return []; diff --git a/packages/@aws-cdk/aws-codepipeline/lib/stage.ts b/packages/@aws-cdk/aws-codepipeline/lib/stage.ts index b2094f0ff3127..da16972c21b95 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/stage.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/stage.ts @@ -5,10 +5,56 @@ import cdk = require('@aws-cdk/cdk'); import { cloudformation } from './codepipeline.generated'; import { Pipeline } from './pipeline'; +/** + * 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 #rightBefore + * @see #justAfter + * @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 rightBefore?: Stage; + + /** + * Inserts the new Stage as a child of the given Stage + * (changing its current child Stage, if it had one). + */ + readonly justAfter?: 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#stageCount}, + * which will insert the new Stage at the end of the Pipeline. + */ + readonly atIndex?: number; +} + +/** + * The properties for the {@link Pipeline#addStage} method. + */ +export interface CommonStageProps { + /** + * Allows specifying where should the newly created {@link Stage} + * be placed in the Pipeline. + * + * @default the stage is added at the end of the Pipeline + */ + placement?: StagePlacement; +} + /** * The construction properties for {@link Stage}. */ -export interface StageProps { +export interface StageProps extends CommonStageProps { /** * The Pipeline to add the newly created Stage to. */ @@ -44,7 +90,7 @@ export class Stage extends cdk.Construct implements actions.IStage { this.pipeline = props.pipeline; actions.validateName('Stage', name); - this.pipeline._addStage(this); + (this.pipeline as any)._attachStage(this, props.placement); } /** @@ -91,8 +137,10 @@ export class Stage extends cdk.Construct implements actions.IStage { return this.pipeline.role; } - public _addAction(action: actions.Action): void { - // _addAction should be idempotent in case a customer ever calls it directly + // can't make this method private like Pipeline#_attachStage, + // as it comes from the IStage interface + public _attachAction(action: actions.Action): void { + // _attachAction should be idempotent in case a customer ever calls it directly if (!this._actions.includes(action)) { this._actions.push(action); } diff --git a/packages/@aws-cdk/aws-codepipeline/test/test.stages.ts b/packages/@aws-cdk/aws-codepipeline/test/test.stages.ts index 754008f63bfa2..290034f2fea4c 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/test.stages.ts +++ b/packages/@aws-cdk/aws-codepipeline/test/test.stages.ts @@ -7,18 +7,157 @@ import codepipeline = require('../lib'); export = { 'Pipeline Stages': { - 'can also be created by using the Pipeline#addStage method'(test: Test) { + 'can be inserted at index 0'(test: Test) { const stack = new cdk.Stack(); const pipeline = new codepipeline.Pipeline(stack, 'Pipeline'); - pipeline.addStage('Stage'); + + new codepipeline.Stage(stack, 'SecondStage', { pipeline }); + new codepipeline.Stage(stack, 'FirstStage', { + pipeline, + placement: { + atIndex: 0, + }, + }); + + expect(stack, true).to(haveResource('AWS::CodePipeline::Pipeline', { + "Stages": [ + { "Name": "FirstStage" }, + { "Name": "SecondStage" }, + ], + })); + + test.done(); + }, + + 'can be inserted before another Stage'(test: Test) { + const stack = new cdk.Stack(); + const pipeline = new codepipeline.Pipeline(stack, 'Pipeline'); + + const secondStage = pipeline.addStage('SecondStage'); + pipeline.addStage('FirstStage', { + placement: { + rightBefore: secondStage, + }, + }); + + expect(stack, true).to(haveResource('AWS::CodePipeline::Pipeline', { + "Stages": [ + { "Name": "FirstStage" }, + { "Name": "SecondStage" }, + ], + })); + + test.done(); + }, + + 'can be inserted after another Stage'(test: Test) { + const stack = new cdk.Stack(); + const pipeline = new codepipeline.Pipeline(stack, 'Pipeline'); + + const firstStage = pipeline.addStage('FirstStage'); + pipeline.addStage('ThirdStage'); + pipeline.addStage('SecondStage', { + placement: { + justAfter: firstStage, + }, + }); expect(stack, true).to(haveResource('AWS::CodePipeline::Pipeline', { "Stages": [ - { "Name": "Stage" }, + { "Name": "FirstStage" }, + { "Name": "SecondStage" }, + { "Name": "ThirdStage" }, ], })); test.done(); }, + + 'attempting to insert a Stage at a negative index results in an error'(test: Test) { + const stack = new cdk.Stack(); + const pipeline = new codepipeline.Pipeline(stack, 'Pipeline'); + + test.throws(() => { + new codepipeline.Stage(stack, 'Stage', { + pipeline, + placement: { + 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 stack = new cdk.Stack(); + const pipeline = new codepipeline.Pipeline(stack, 'Pipeline'); + + test.throws(() => { + pipeline.addStage('Stage', { + placement: { + atIndex: 1, + }, + }); + }, /atIndex/); + + test.done(); + }, + + "attempting to insert a Stage before a Stage that doesn't exist results in an error"(test: Test) { + const stack = new cdk.Stack(); + const pipeline = new codepipeline.Pipeline(stack, 'Pipeline'); + const stage = pipeline.addStage('Stage'); + + const anotherPipeline = new codepipeline.Pipeline(stack, 'AnotherPipeline'); + test.throws(() => { + anotherPipeline.addStage('AnotherStage', { + placement: { + rightBefore: stage, + }, + }); + }, /before/i); + + test.done(); + }, + + "attempting to insert a Stage after a Stage that doesn't exist results in an error"(test: Test) { + const stack = new cdk.Stack(); + const pipeline = new codepipeline.Pipeline(stack, 'Pipeline'); + const stage = pipeline.addStage('Stage'); + + const anotherPipeline = new codepipeline.Pipeline(stack, 'AnotherPipeline'); + test.throws(() => { + anotherPipeline.addStage('AnotherStage', { + placement: { + justAfter: stage, + }, + }); + }, /after/i); + + test.done(); + }, + + "providing more than one placement value results in an error"(test: Test) { + const stack = new cdk.Stack(); + const pipeline = new codepipeline.Pipeline(stack, 'Pipeline'); + const stage = pipeline.addStage('FirstStage'); + + test.throws(() => { + pipeline.addStage('SecondStage', { + placement: { + rightBefore: stage, + justAfter: stage, + }, + }); + // incredibly, an arrow function below causes nodeunit to crap out with: + // "TypeError: Function has non-object prototype 'undefined' in instanceof check" + // tslint:disable-next-line:only-arrow-functions + }, function(e: any) { + return /rightBefore/.test(e) && /justAfter/.test(e); + }); + + test.done(); + }, }, };