From a2d4389587b5bf0893b820cb18d3f89434993349 Mon Sep 17 00:00:00 2001 From: Colton Padden Date: Sun, 31 Oct 2021 11:15:47 -0400 Subject: [PATCH 1/3] Use Parameter Objects for Publisher Constructors Refactored Techdocs publishers to use a single parameter object as the constructor argument to ease extendability as more options are introduced. Updated local publisher to use `.fromConfig` for instantiation so that it follows the same design pattern as the other publishers. Signed-off-by: Colton Padden --- .changeset/metal-impalas-allow.md | 12 ++++++ .../src/stages/publish/awsS3.ts | 40 ++++++++++-------- .../src/stages/publish/azureBlobStorage.ts | 41 +++++++++++-------- .../src/stages/publish/googleStorage.ts | 38 +++++++++-------- .../src/stages/publish/local.test.ts | 20 +++++++-- .../src/stages/publish/local.ts | 41 ++++++++++++------- .../src/stages/publish/openStackSwift.ts | 26 +++++++----- .../src/stages/publish/publish.ts | 4 +- 8 files changed, 140 insertions(+), 82 deletions(-) create mode 100644 .changeset/metal-impalas-allow.md diff --git a/.changeset/metal-impalas-allow.md b/.changeset/metal-impalas-allow.md new file mode 100644 index 0000000000..6f18969f3d --- /dev/null +++ b/.changeset/metal-impalas-allow.md @@ -0,0 +1,12 @@ +--- +'@backstage/techdocs-common': patch +--- + +1. Techdocs publishers constructors now use parameter objects when being instantiated + +2. The `LocalPublish` publisher can now be created using `fromConfig`: + +``` +--- const publisher = new LocalPublish(config, logger, discovery); ++++ const publisher = LocalPublish.fromConfig(config, logger, discovery); +``` diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 29eecb8d03..cd6914460f 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -57,6 +57,26 @@ const streamToBuffer = (stream: Readable): Promise => { }; export class AwsS3Publish implements PublisherBase { + private readonly storageClient: aws.S3; + private readonly bucketName: string; + private readonly legacyPathCasing: boolean; + private readonly logger: Logger; + private readonly bucketRootPath: string; + + constructor(options: { + storageClient: aws.S3; + bucketName: string; + legacyPathCasing: boolean; + logger: Logger; + bucketRootPath: string; + }) { + this.storageClient = options.storageClient; + this.bucketName = options.bucketName; + this.legacyPathCasing = options.legacyPathCasing; + this.logger = options.logger; + this.bucketRootPath = options.bucketRootPath; + } + static fromConfig(config: Config, logger: Logger): PublisherBase { let bucketName = ''; try { @@ -112,13 +132,13 @@ export class AwsS3Publish implements PublisherBase { 'techdocs.legacyUseCaseSensitiveTripletPaths', ) || false; - return new AwsS3Publish( + return new AwsS3Publish({ storageClient, bucketName, + bucketRootPath, legacyPathCasing, logger, - bucketRootPath, - ); + }); } private static buildCredentials( @@ -152,20 +172,6 @@ export class AwsS3Publish implements PublisherBase { return explicitCredentials; } - constructor( - private readonly storageClient: aws.S3, - private readonly bucketName: string, - private readonly legacyPathCasing: boolean, - private readonly logger: Logger, - private readonly bucketRootPath: string, - ) { - this.storageClient = storageClient; - this.bucketName = bucketName; - this.legacyPathCasing = legacyPathCasing; - this.logger = logger; - this.bucketRootPath = bucketRootPath; - } - /** * Check if the defined bucket exists. Being able to connect means the configuration is good * and the storage client will work. diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index f1d60f709a..7a58d92095 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -47,6 +47,23 @@ import { const BATCH_CONCURRENCY = 3; export class AzureBlobStoragePublish implements PublisherBase { + private readonly storageClient: BlobServiceClient; + private readonly containerName: string; + private readonly legacyPathCasing: boolean; + private readonly logger: Logger; + + constructor(options: { + storageClient: BlobServiceClient; + containerName: string; + legacyPathCasing: boolean; + logger: Logger; + }) { + this.storageClient = options.storageClient; + this.containerName = options.containerName; + this.legacyPathCasing = options.legacyPathCasing; + this.logger = options.logger; + } + static fromConfig(config: Config, logger: Logger): PublisherBase { let containerName = ''; try { @@ -95,24 +112,12 @@ export class AzureBlobStoragePublish implements PublisherBase { 'techdocs.legacyUseCaseSensitiveTripletPaths', ) || false; - return new AzureBlobStoragePublish( - storageClient, - containerName, - legacyPathCasing, - logger, - ); - } - - constructor( - private readonly storageClient: BlobServiceClient, - private readonly containerName: string, - private readonly legacyPathCasing: boolean, - private readonly logger: Logger, - ) { - this.storageClient = storageClient; - this.containerName = containerName; - this.legacyPathCasing = legacyPathCasing; - this.logger = logger; + return new AzureBlobStoragePublish({ + storageClient: storageClient, + containerName: containerName, + legacyPathCasing: legacyPathCasing, + logger: logger, + }); } async getReadiness(): Promise { diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index 5a142e800a..8d91b31dbb 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -41,6 +41,26 @@ import { } from './types'; export class GoogleGCSPublish implements PublisherBase { + private readonly storageClient: Storage; + private readonly bucketName: string; + private readonly legacyPathCasing: boolean; + private readonly logger: Logger; + private readonly bucketRootPath: string; + + constructor(options: { + storageClient: Storage; + bucketName: string; + legacyPathCasing: boolean; + logger: Logger; + bucketRootPath: string; + }) { + this.storageClient = options.storageClient; + this.bucketName = options.bucketName; + this.legacyPathCasing = options.legacyPathCasing; + this.logger = options.logger; + this.bucketRootPath = options.bucketRootPath; + } + static fromConfig(config: Config, logger: Logger): PublisherBase { let bucketName = ''; try { @@ -84,27 +104,13 @@ export class GoogleGCSPublish implements PublisherBase { 'techdocs.legacyUseCaseSensitiveTripletPaths', ) || false; - return new GoogleGCSPublish( + return new GoogleGCSPublish({ storageClient, bucketName, legacyPathCasing, logger, bucketRootPath, - ); - } - - constructor( - private readonly storageClient: Storage, - private readonly bucketName: string, - private readonly legacyPathCasing: boolean, - private readonly logger: Logger, - private readonly bucketRootPath: string, - ) { - this.storageClient = storageClient; - this.bucketName = bucketName; - this.legacyPathCasing = legacyPathCasing; - this.logger = logger; - this.bucketRootPath = bucketRootPath; + }); } /** diff --git a/packages/techdocs-common/src/stages/publish/local.test.ts b/packages/techdocs-common/src/stages/publish/local.test.ts index 38ead236bd..de56580e02 100644 --- a/packages/techdocs-common/src/stages/publish/local.test.ts +++ b/packages/techdocs-common/src/stages/publish/local.test.ts @@ -63,7 +63,11 @@ describe('local publisher', () => { const mockConfig = new ConfigReader({}); - const publisher = new LocalPublish(mockConfig, logger, testDiscovery); + const publisher = LocalPublish.fromConfig( + mockConfig, + logger, + testDiscovery, + ); const mockEntity = createMockEntity(); const lowerMockEntity = createMockEntity(undefined, true); @@ -90,7 +94,11 @@ describe('local publisher', () => { }, }); - const publisher = new LocalPublish(mockConfig, logger, testDiscovery); + const publisher = LocalPublish.fromConfig( + mockConfig, + logger, + testDiscovery, + ); const mockEntity = createMockEntity(); const lowerMockEntity = createMockEntity(undefined, true); @@ -106,7 +114,11 @@ describe('local publisher', () => { describe('docsRouter', () => { const mockConfig = new ConfigReader({}); - const publisher = new LocalPublish(mockConfig, logger, testDiscovery); + const publisher = LocalPublish.fromConfig( + mockConfig, + logger, + testDiscovery, + ); let app: express.Express; beforeEach(() => { @@ -166,7 +178,7 @@ describe('local publisher', () => { legacyUseCaseSensitiveTripletPaths: true, }, }); - const legacyPublisher = new LocalPublish( + const legacyPublisher = LocalPublish.fromConfig( legacyConfig, logger, testDiscovery, diff --git a/packages/techdocs-common/src/stages/publish/local.ts b/packages/techdocs-common/src/stages/publish/local.ts index de0072de7a..70b4eb3ff2 100644 --- a/packages/techdocs-common/src/stages/publish/local.ts +++ b/packages/techdocs-common/src/stages/publish/local.ts @@ -59,24 +59,37 @@ try { * called "static" at the root of techdocs-backend plugin. */ export class LocalPublish implements PublisherBase { - private legacyPathCasing: boolean; + private readonly legacyPathCasing: boolean; + private readonly logger: Logger; + private readonly discovery: PluginEndpointDiscovery; - // TODO: Use a static fromConfig method to create a LocalPublish instance, similar to aws/gcs publishers. - // Move the logic of setting staticDocsDir based on config over to fromConfig, - // and set the value as a class parameter. - constructor( - // @ts-ignore - private readonly config: Config, - private readonly logger: Logger, - private readonly discovery: PluginEndpointDiscovery, - ) { - this.config = config; - this.logger = logger; - this.discovery = discovery; - this.legacyPathCasing = + // TODO: Move the logic of setting staticDocsDir based on config over to + // fromConfig, and set the value as a class parameter. + constructor(options: { + logger: Logger; + discovery: PluginEndpointDiscovery; + legacyPathCasing: boolean; + }) { + this.logger = options.logger; + this.discovery = options.discovery; + this.legacyPathCasing = options.legacyPathCasing; + } + + static fromConfig( + config: Config, + logger: Logger, + discovery: PluginEndpointDiscovery, + ): PublisherBase { + const legacyPathCasing = config.getOptionalBoolean( 'techdocs.legacyUseCaseSensitiveTripletPaths', ) || false; + + return new LocalPublish({ + logger, + discovery, + legacyPathCasing, + }); } async getReadiness(): Promise { diff --git a/packages/techdocs-common/src/stages/publish/openStackSwift.ts b/packages/techdocs-common/src/stages/publish/openStackSwift.ts index 51e7b76406..a463157552 100644 --- a/packages/techdocs-common/src/stages/publish/openStackSwift.ts +++ b/packages/techdocs-common/src/stages/publish/openStackSwift.ts @@ -54,6 +54,20 @@ const bufferToStream = (buffer: Buffer): Readable => { }; export class OpenStackSwiftPublish implements PublisherBase { + private readonly storageClient: SwiftClient; + private readonly containerName: string; + private readonly logger: Logger; + + constructor(options: { + storageClient: SwiftClient; + containerName: string; + logger: Logger; + }) { + this.storageClient = options.storageClient; + this.containerName = options.containerName; + this.logger = options.logger; + } + static fromConfig(config: Config, logger: Logger): PublisherBase { let containerName = ''; try { @@ -78,17 +92,7 @@ export class OpenStackSwiftPublish implements PublisherBase { secret: openStackSwiftConfig.getString('credentials.secret'), }); - return new OpenStackSwiftPublish(storageClient, containerName, logger); - } - - constructor( - private readonly storageClient: SwiftClient, - private readonly containerName: string, - private readonly logger: Logger, - ) { - this.storageClient = storageClient; - this.containerName = containerName; - this.logger = logger; + return new OpenStackSwiftPublish({ storageClient, containerName, logger }); } /* diff --git a/packages/techdocs-common/src/stages/publish/publish.ts b/packages/techdocs-common/src/stages/publish/publish.ts index c7c6154114..eb9e3104bf 100644 --- a/packages/techdocs-common/src/stages/publish/publish.ts +++ b/packages/techdocs-common/src/stages/publish/publish.ts @@ -61,10 +61,10 @@ export class Publisher { return OpenStackSwiftPublish.fromConfig(config, logger); case 'local': logger.info('Creating Local publisher for TechDocs'); - return new LocalPublish(config, logger, discovery); + return LocalPublish.fromConfig(config, logger, discovery); default: logger.info('Creating Local publisher for TechDocs'); - return new LocalPublish(config, logger, discovery); + return LocalPublish.fromConfig(config, logger, discovery); } } } From 97eed9f67f12359429e260e74cbe32adc35e6815 Mon Sep 17 00:00:00 2001 From: Colton Padden Date: Mon, 8 Nov 2021 10:16:55 -0500 Subject: [PATCH 2/3] use diff codeblock in changeset markdown Signed-off-by: Colton Padden --- .changeset/metal-impalas-allow.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.changeset/metal-impalas-allow.md b/.changeset/metal-impalas-allow.md index 6f18969f3d..d3ca3b6e49 100644 --- a/.changeset/metal-impalas-allow.md +++ b/.changeset/metal-impalas-allow.md @@ -6,7 +6,7 @@ 2. The `LocalPublish` publisher can now be created using `fromConfig`: -``` ---- const publisher = new LocalPublish(config, logger, discovery); -+++ const publisher = LocalPublish.fromConfig(config, logger, discovery); +```diff +- const publisher = new LocalPublish(config, logger, discovery); ++ const publisher = LocalPublish.fromConfig(config, logger, discovery); ``` From 6d4f06699b977a7a91bbe95916177b70459a78cd Mon Sep 17 00:00:00 2001 From: Colton Padden Date: Tue, 9 Nov 2021 13:45:26 -0500 Subject: [PATCH 3/3] note internal refactor of LocalPublish changes and impact on usage Signed-off-by: Colton Padden --- .changeset/metal-impalas-allow.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.changeset/metal-impalas-allow.md b/.changeset/metal-impalas-allow.md index d3ca3b6e49..8ad727f759 100644 --- a/.changeset/metal-impalas-allow.md +++ b/.changeset/metal-impalas-allow.md @@ -2,9 +2,12 @@ '@backstage/techdocs-common': patch --- -1. Techdocs publishers constructors now use parameter objects when being instantiated +1. Techdocs publisher constructors now use parameter objects when being + instantiated -2. The `LocalPublish` publisher can now be created using `fromConfig`: +2. Internal refactor of `LocalPublish` publisher to use `fromConfig` for + creation to be aligned with other publishers; this does not impact + `LocalPublish` usage. ```diff - const publisher = new LocalPublish(config, logger, discovery);