Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: from-org refactor #1296

Merged
merged 3 commits into from
Apr 23, 2024
Merged

fix: from-org refactor #1296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f969ef0
fix: handle components with empty string as their type
mshanemc Apr 23, 2024
52b13e1
chore: remove unused logger
mshanemc Apr 23, 2024
c18d0d0
test: more ut for new undefined/empty scenarios
mshanemc Apr 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
128 changes: 45 additions & 83 deletions src/resolve/connectionResolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -30,7 +28,6 @@ export type ResolveConnectionResult = {
* Resolve MetadataComponents from an org connection
*/
export class ConnectionResolver {
protected logger: Logger;
private connection: Connection;
private registry: RegistryAccess;
Comment on lines 31 to 32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be readonly if we want to


Expand All @@ -41,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))
Expand All @@ -57,21 +53,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([
Expand All @@ -91,11 +87,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,
})
);
Expand All @@ -106,7 +101,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 }));
});
}
}
Expand All @@ -125,93 +120,60 @@ export class ConnectionResolver {
apiVersion: this.connection.getApiVersion(),
};
}
}

private async listMembers(query: ListMetadataQuery): Promise<RelevantFileProperties[]> {
let members: RelevantFileProperties[] = [];

const pollingOptions: PollingClient.Options = {
frequency: Duration.milliseconds(1000),
timeout: Duration.minutes(3),
poll: async (): Promise<StatusResult> => {
const res = ensureArray(await this.connection.metadata.list(query));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have our connections just become more resilient to remove this polling? I don't understand why we're doing this to begin with

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, part of the jsforce stuff that Cristian did jsforce/jsforce#1407.

I think the retry logic was using pollingClient to work around those network issues.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WillieRuemmele Shane is correct, I added the pollingClient logic here:
#941

jsforce retries on the same errors that pollingClient and more.

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<RelevantFileProperties[]> => {
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
Comment on lines -166 to -168
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

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) {
if (standardValueSetResult) {
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;
147 changes: 101 additions & 46 deletions test/resolve/connectionResolver.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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([
Expand Down Expand Up @@ -291,51 +336,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');

Expand Down Expand Up @@ -403,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([]);
});
});
});
Loading
Loading