From f969ef00f089eb2f10b0fb2d43892f764be8b565 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Tue, 23 Apr 2024 11:43:49 -0500 Subject: [PATCH 1/3] fix: handle components with empty string as their type --- package.json | 3 +- src/resolve/connectionResolver.ts | 126 +++++++++--------------- test/resolve/connectionResolver.test.ts | 47 +-------- yarn.lock | 31 +----- 4 files changed, 50 insertions(+), 157 deletions(-) diff --git a/package.json b/package.json index 889daf7e45..eab5057aa0 100644 --- a/package.json +++ b/package.json @@ -36,8 +36,7 @@ "jszip": "^3.10.1", "mime": "2.6.0", "minimatch": "^5.1.6", - "proxy-agent": "^6.4.0", - "ts-retry-promise": "^0.7.1" + "proxy-agent": "^6.4.0" }, "devDependencies": { "@jsforce/jsforce-node": "^3.1.0", diff --git a/src/resolve/connectionResolver.ts b/src/resolve/connectionResolver.ts index dcf15fc064..06bf0e31e1 100644 --- a/src/resolve/connectionResolver.ts +++ b/src/resolve/connectionResolver.ts @@ -5,9 +5,7 @@ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ -import { retry, NotRetryableError, RetryError } from 'ts-retry-promise'; -import { PollingClient, StatusResult, Connection, Logger, Messages, Lifecycle, SfError } from '@salesforce/core'; -import { Duration, ensureArray } from '@salesforce/kit'; +import { Connection, Logger, Messages, Lifecycle, SfError } from '@salesforce/core'; import { ensurePlainObject, ensureString, isPlainObject } from '@salesforce/ts-types'; import { RegistryAccess } from '../registry/registryAccess'; import { MetadataType } from '../registry/types'; @@ -57,21 +55,21 @@ export class ConnectionResolver { const lifecycle = Lifecycle.getInstance(); const componentFromDescribe = ( - await Promise.all(this.mdTypeNames.map((type) => this.listMembers({ type }))) + await Promise.all(this.mdTypeNames.map((type) => listMembers(this.registry)(this.connection)({ type }))) ).flat(); for (const component of componentFromDescribe) { let componentType: MetadataType; - if (typeof component.type === 'string' && component.type.length) { + if (isNonEmptyString(component.type)) { componentType = this.registry.getTypeByName(component.type); - } else if (typeof component.fileName === 'string' && component.fileName.length) { + } else if (isNonEmptyString(component.fileName)) { // fix { type: { "$": { "xsi:nil": "true" } } } componentType = ensurePlainObject( this.registry.getTypeBySuffix(extName(component.fileName)), `No type found for ${component.fileName} when matching by suffix. Check the file extension.` ); component.type = componentType.name; - } else if (component.type === undefined && component.fileName === undefined) { + } else if (!isNonEmptyString(component.type) && !isNonEmptyString(component.fileName)) { // has no type and has no filename! Warn and skip that component. // eslint-disable-next-line no-await-in-loop await Promise.all([ @@ -91,11 +89,10 @@ export class ConnectionResolver { Aggregator.push(component); componentTypes.add(componentType); - const folderContentType = componentType.folderContentType; - if (folderContentType) { + if (componentType.folderContentType) { childrenPromises.push( - this.listMembers({ - type: this.registry.getTypeByName(folderContentType).name, + listMembers(this.registry)(this.connection)({ + type: this.registry.getTypeByName(componentType.folderContentType).name, folder: component.fullName, }) ); @@ -106,7 +103,7 @@ export class ConnectionResolver { const childTypes = componentType.children?.types; if (childTypes) { Object.values(childTypes).map((childType) => { - childrenPromises.push(this.listMembers({ type: childType.name })); + childrenPromises.push(listMembers(this.registry)(this.connection)({ type: childType.name })); }); } } @@ -125,84 +122,35 @@ export class ConnectionResolver { apiVersion: this.connection.getApiVersion(), }; } +} - private async listMembers(query: ListMetadataQuery): Promise { - let members: RelevantFileProperties[] = []; - - const pollingOptions: PollingClient.Options = { - frequency: Duration.milliseconds(1000), - timeout: Duration.minutes(3), - poll: async (): Promise => { - const res = ensureArray(await this.connection.metadata.list(query)); - return { completed: true, payload: res }; - }, - }; - - const pollingClient = await PollingClient.create(pollingOptions); - - try { - members = await pollingClient.subscribe(); - } catch (error) { - // throw error if PollingClient timed out. - if (error instanceof NotRetryableError) { - throw NotRetryableError; - } - this.logger.debug((error as Error).message); - members = []; - } - - // if the Metadata Type doesn't return a correct fileName then help it out - for (const m of members) { - if (typeof m.fileName == 'object') { - const t = this.registry.getTypeByName(query.type); - m.fileName = `${t.directoryName}/${m.fullName}.${t.suffix}`; - } - } +const listMembers = + (registry: RegistryAccess) => + (connection: Connection) => + async (query: ListMetadataQuery): Promise => { + const mdType = registry.getTypeByName(query.type); // Workaround because metadata.list({ type: 'StandardValueSet' }) returns [] - if (query.type === this.registry.getRegistry().types.standardvalueset.name && members.length === 0) { + if (mdType.name === registry.getRegistry().types.standardvalueset.name) { + const members: RelevantFileProperties[] = []; + const standardValueSetPromises = standardValueSet.fullnames.map(async (standardValueSetFullName) => { try { - // The 'singleRecordQuery' method was having connection errors, using `retry` resolves this - // Note that this type of connection retry logic may someday be added to jsforce v2 - // Once that happens this logic could be reverted - const standardValueSetRecord: StdValueSetRecord = await retry(async () => { - try { - return await this.connection.singleRecordQuery( - `SELECT Id, MasterLabel, Metadata FROM StandardValueSet WHERE MasterLabel = '${standardValueSetFullName}'`, - { tooling: true } - ); - } catch (err) { - // We exit the retry loop with `NotRetryableError` if we get an (expected) unsupported metadata type error - const error = err as Error; - if (error.message.includes('either inaccessible or not supported in Metadata API')) { - this.logger.debug('Expected error:', error.message); - throw new NotRetryableError(error.message); - } - - // Otherwise throw the err so we can retry again - throw err; - } - }); + const standardValueSetRecord: StdValueSetRecord = await connection.singleRecordQuery( + `SELECT Id, MasterLabel, Metadata FROM StandardValueSet WHERE MasterLabel = '${standardValueSetFullName}'`, + { tooling: true } + ); return ( standardValueSetRecord.Metadata.standardValue.length && { fullName: standardValueSetRecord.MasterLabel, - fileName: `${this.registry.getRegistry().types.standardvalueset.directoryName}/${ - standardValueSetRecord.MasterLabel - }.${this.registry.getRegistry().types.standardvalueset.suffix}`, - type: this.registry.getRegistry().types.standardvalueset.name, + fileName: `${mdType.directoryName}/${standardValueSetRecord.MasterLabel}.${mdType.suffix}`, + type: mdType.name, } ); } catch (err) { - // error.message here will be overwritten by 'ts-retry-promise' - // Example error.message from the library: "All retries failed" or "Met not retryable error" - // 'ts-retry-promise' exposes the actual error on `error.lastError` - const error = err as RetryError; - - if (error.lastError?.message) { - this.logger.debug(error.lastError.message); - } + const logger = Logger.childFromRoot('ConnectionResolver.listMembers'); + logger.debug(err); } }); for await (const standardValueSetResult of standardValueSetPromises) { @@ -210,8 +158,24 @@ export class ConnectionResolver { members.push(standardValueSetResult); } } + return members; } - return members; - } -} + try { + return (await connection.metadata.list(query)).map(inferFilenamesFromType(mdType)); + } catch (error) { + const logger = Logger.childFromRoot('ConnectionResolver.listMembers'); + logger.debug((error as Error).message); + return []; + } + }; + +/* if the Metadata Type doesn't return a correct fileName then help it out */ +const inferFilenamesFromType = + (metadataType: MetadataType) => + (member: RelevantFileProperties): RelevantFileProperties => + typeof member.fileName === 'object' + ? { ...member, fileName: `${metadataType.directoryName}/${member.fullName}.${metadataType.suffix}` } + : member; + +const isNonEmptyString = (value: string | undefined): boolean => typeof value === 'string' && value.length > 0; diff --git a/test/resolve/connectionResolver.test.ts b/test/resolve/connectionResolver.test.ts index faf31ff34f..be1d115ec1 100644 --- a/test/resolve/connectionResolver.test.ts +++ b/test/resolve/connectionResolver.test.ts @@ -7,7 +7,7 @@ import { assert, expect, use } from 'chai'; import { MockTestOrgData, TestContext } from '@salesforce/core/testSetup'; -import { Connection, Logger } from '@salesforce/core'; +import { Connection } from '@salesforce/core'; import deepEqualInAnyOrder from 'deep-equal-in-any-order'; import { ManageableState } from '../../src/client/types'; import { ConnectionResolver } from '../../src/resolve'; @@ -291,51 +291,6 @@ describe('ConnectionResolver', () => { expect(result.components).to.deep.equal(expected); }); - it('should retry (ten times) if unexpected error occurs', async () => { - const loggerStub = $$.SANDBOX.stub(Logger.prototype, 'debug'); - - $$.SANDBOX.stub(connection.metadata, 'list'); - - const query = "SELECT Id, MasterLabel, Metadata FROM StandardValueSet WHERE MasterLabel = 'AccountOwnership'"; - - const mockToolingQuery = $$.SANDBOX.stub(connection, 'singleRecordQuery'); - mockToolingQuery.withArgs(query).rejects(new Error('Something happened. Oh no.')); - - const resolver = new ConnectionResolver(connection); - const result = await resolver.resolve(); - const expected: MetadataComponent[] = []; - - // filter over queries and find ones called with `query` - const retries = mockToolingQuery.args.filter((call) => call[0] === query); - - expect(retries.length).to.equal(11); // first call plus 10 retries - expect(loggerStub.calledOnce).to.be.true; - expect(loggerStub.args[0][0]).to.equal('Something happened. Oh no.'); - expect(result.components).to.deep.equal(expected); - }); - - it('should not retry query if expected unsupported metadata error is encountered', async () => { - const loggerStub = $$.SANDBOX.stub(Logger.prototype, 'debug'); - - $$.SANDBOX.stub(connection.metadata, 'list'); - - const errorMessage = 'WorkTypeGroupAddInfo is either inaccessible or not supported in Metadata API'; - - const mockToolingQuery = $$.SANDBOX.stub(connection, 'singleRecordQuery'); - mockToolingQuery - .withArgs("SELECT Id, MasterLabel, Metadata FROM StandardValueSet WHERE MasterLabel = 'WorkTypeGroupAddInfo'") - .rejects(new Error(errorMessage)); - - const resolver = new ConnectionResolver(connection); - const result = await resolver.resolve(); - const expected: MetadataComponent[] = []; - - expect(loggerStub.calledOnce).to.be.true; - expect(loggerStub.args[0][0]).to.equal('Expected error:'); - expect(loggerStub.args[0][1]).to.equal(errorMessage); - expect(result.components).to.deep.equal(expected); - }); - it('should resolve no managed components', async () => { const metadataQueryStub = $$.SANDBOX.stub(connection.metadata, 'list'); diff --git a/yarn.lock b/yarn.lock index 69a3c2e57c..f25755fa84 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5017,16 +5017,7 @@ srcset@^5.0.0: resolved "https://registry.yarnpkg.com/srcset/-/srcset-5.0.0.tgz#9df6c3961b5b44a02532ce6ae4544832609e2e3f" integrity sha512-SqEZaAEhe0A6ETEa9O1IhSPC7MdvehZtCnTR0AftXk3QhY2UNgb+NApFOUPZILXk/YTDfFxMTNJOBpzrJsEdIA== -"string-width-cjs@npm:string-width@^4.2.0": - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - -string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -5085,14 +5076,7 @@ string_decoder@~1.1.1: dependencies: safe-buffer "~5.1.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - -strip-ansi@6.0.1, strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@6.0.1, strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -5591,7 +5575,7 @@ workerpool@6.2.1: resolved "https://registry.yarnpkg.com/workerpool/-/workerpool-6.2.1.tgz#46fc150c17d826b86a008e5a4508656777e9c343" integrity sha512-ILEIE97kDZvF9Wb9f6h5aXK4swSlKGUcOEGiIYb2OOu/IrDU9iwj0fD//SsA6E5ibwJxpEvhullJY4Sl4GcpAw== -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -5609,15 +5593,6 @@ wrap-ansi@^6.2.0: string-width "^4.1.0" strip-ansi "^6.0.0" -wrap-ansi@^7.0.0: - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214" From 52b13e12aa59301ac3e00809b64ca8523b8fccf4 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Tue, 23 Apr 2024 11:51:22 -0500 Subject: [PATCH 2/3] chore: remove unused logger --- src/resolve/connectionResolver.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/resolve/connectionResolver.ts b/src/resolve/connectionResolver.ts index 06bf0e31e1..48163bf7e4 100644 --- a/src/resolve/connectionResolver.ts +++ b/src/resolve/connectionResolver.ts @@ -28,7 +28,6 @@ export type ResolveConnectionResult = { * Resolve MetadataComponents from an org connection */ export class ConnectionResolver { - protected logger: Logger; private connection: Connection; private registry: RegistryAccess; @@ -39,7 +38,6 @@ export class ConnectionResolver { public constructor(connection: Connection, registry = new RegistryAccess(), mdTypes?: string[]) { this.connection = connection; this.registry = registry; - this.logger = Logger.childFromRoot(this.constructor.name); this.mdTypeNames = mdTypes?.length ? // ensure the types passed in are valid per the registry mdTypes.filter((t) => this.registry.getTypeByName(t)) From c18d0d0a71e47553ae310c9436b96b120affde78 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Tue, 23 Apr 2024 14:13:25 -0500 Subject: [PATCH 3/3] test: more ut for new undefined/empty scenarios --- test/resolve/connectionResolver.test.ts | 100 ++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/test/resolve/connectionResolver.test.ts b/test/resolve/connectionResolver.test.ts index be1d115ec1..161adee2c0 100644 --- a/test/resolve/connectionResolver.test.ts +++ b/test/resolve/connectionResolver.test.ts @@ -185,6 +185,51 @@ describe('ConnectionResolver', () => { ]; expect(result.components).to.deep.equal(expected); }); + + it('should resolve components with undefined type returned by metadata api', async () => { + const metadataQueryStub = $$.SANDBOX.stub(connection.metadata, 'list'); + metadataQueryStub.withArgs({ type: 'CustomLabels' }).resolves([ + // @ts-expect-error missing type + { + ...StdFileProperty, + fileName: 'standardValueSetTranslations/CaseOrigin-de.standardValueSetTranslation', + fullName: 'CaseOrigin-de', + }, + ]); + + const resolver = new ConnectionResolver(connection); + const result = await resolver.resolve(); + const expected: MetadataComponent[] = [ + { + fullName: 'CaseOrigin-de', + type: registry.types.standardvaluesettranslation, + }, + ]; + expect(result.components).to.deep.equal(expected); + }); + + it('should resolve components with emptyString type returned by metadata api', async () => { + const metadataQueryStub = $$.SANDBOX.stub(connection.metadata, 'list'); + metadataQueryStub.withArgs({ type: 'CustomLabels' }).resolves([ + { + ...StdFileProperty, + fileName: 'standardValueSetTranslations/CaseOrigin-de.standardValueSetTranslation', + fullName: 'CaseOrigin-de', + type: '', + }, + ]); + + const resolver = new ConnectionResolver(connection); + const result = await resolver.resolve(); + const expected: MetadataComponent[] = [ + { + fullName: 'CaseOrigin-de', + type: registry.types.standardvaluesettranslation, + }, + ]; + expect(result.components).to.deep.equal(expected); + }); + it('should resolve components with invalid fileName returned by metadata api', async () => { const metadataQueryStub = $$.SANDBOX.stub(connection.metadata, 'list'); metadataQueryStub.withArgs({ type: 'SynonymDictionary' }).resolves([ @@ -358,4 +403,59 @@ describe('ConnectionResolver', () => { expect(result.components).to.deep.equalInAnyOrder(expected); }); }); + + describe('missing filenane and type', () => { + it('should skip if component has undefined type and filename', async () => { + const metadataQueryStub = $$.SANDBOX.stub(connection.metadata, 'list'); + + metadataQueryStub.withArgs({ type: 'CustomObject' }).resolves([ + // @ts-expect-error - testing invalid data that the API returns sometimes + { + ...StdFileProperty, + fullName: 'Account', + }, + ]); + + const resolver = new ConnectionResolver(connection); + const result = await resolver.resolve(); + + expect(result.components).to.deep.equal([]); + }); + + it('should skip if component has empty string type and filename', async () => { + const metadataQueryStub = $$.SANDBOX.stub(connection.metadata, 'list'); + + metadataQueryStub.withArgs({ type: 'CustomObject' }).resolves([ + { + ...StdFileProperty, + fullName: 'Account', + type: '', + fileName: '', + }, + ]); + + const resolver = new ConnectionResolver(connection); + const result = await resolver.resolve(); + + expect(result.components).to.deep.equal([]); + }); + + it('should skip if component has empty string type and undefined filename', async () => { + const metadataQueryStub = $$.SANDBOX.stub(connection.metadata, 'list'); + + metadataQueryStub.withArgs({ type: 'CustomObject' }).resolves([ + // @ts-expect-error - testing invalid data that the API returns sometimes + { + ...StdFileProperty, + fullName: 'Account', + type: '', + }, + ]); + + const resolver = new ConnectionResolver(connection); + const result = await resolver.resolve(); + + expect(result.components).to.deep.equal([]); + }); + }); });