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.

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: offer suggestions for unresolved metadata types #948

Merged
merged 12 commits into from
May 8, 2023
14 changes: 14 additions & 0 deletions messages/sdr.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,17 @@ No uniqueIdElement found in registry for %s (reading %s at %s).
# uniqueIdElementNotInChild

The uniqueIdElement %s was not found the child (reading %s at %s).

# suggest_type_header

A search for the %s suffix found the following close matches:

# suggest_type_did_you_mean

-- Did you mean ".%s%s" instead for the "%s" metadata type?

# suggest_type_more_suggestions

Additional suggestions:
Confirm the file name, extension, and directory names are correct. Validate against the registry at:
https://github.com/forcedotcom/source-deploy-retrieve/blob/main/src/registry/metadataRegistry.json
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@
"node": ">=14.0.0"
},
"dependencies": {
"@salesforce/core": "^3.34.8",
"@salesforce/core": "^3.36.0",
"@salesforce/kit": "^1.9.2",
"@salesforce/ts-types": "^1.7.2",
"archiver": "^5.3.1",
"fast-levenshtein": "^3.0.0",
"fast-xml-parser": "^4.1.4",
"got": "^11.8.6",
"graceful-fs": "^4.2.11",
Expand All @@ -47,6 +48,8 @@
"@salesforce/ts-sinon": "^1.4.6",
"@types/archiver": "^5.3.1",
"@types/deep-equal-in-any-order": "^1.0.1",
"@types/fast-levenshtein": "^0.0.2",
"@types/graceful-fs": "^4.1.6",
"@types/mime": "2.0.3",
"@types/minimatch": "^5.1.2",
"@types/proxy-from-env": "^1.0.1",
Expand Down Expand Up @@ -100,6 +103,7 @@
"test": "wireit",
"test:nuts": "mocha \"test/nuts/local/**/*.nut.ts\" --timeout 500000",
"test:nuts:scale": "mocha \"test/nuts/scale/eda.nut.ts\" --timeout 500000; mocha \"test/nuts/scale/lotsOfClasses.nut.ts\" --timeout 500000; mocha \"test/nuts/scale/lotsOfClassesOneDir.nut.ts\" --timeout 500000",
"test:nuts:suggest": "mocha \"test/nuts/suggestType/suggestType.nut.ts\" --timeout 10000",
"test:only": "wireit",
"test:registry": "mocha ./test/registry/registryCompleteness.test.ts --timeout 50000",
"update-registry": "npx ts-node scripts/update-registry/update2.ts",
Expand Down Expand Up @@ -187,4 +191,4 @@
"output": []
}
}
}
}
35 changes: 34 additions & 1 deletion src/registry/registryAccess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { Messages, SfError } from '@salesforce/core';
import * as Levenshtein from 'fast-levenshtein';
import { registry as defaultRegistry } from './registry';
import { MetadataRegistry, MetadataType } from './types';

Expand Down Expand Up @@ -60,12 +61,44 @@ export class RegistryAccess {
* @returns The corresponding metadata type object
*/
public getTypeBySuffix(suffix: string): MetadataType | undefined {
if (this.registry.suffixes?.[suffix]) {
if (this.registry.suffixes[suffix]) {
const typeId = this.registry.suffixes[suffix];
return this.getTypeByName(typeId);
}
}

/**
* Find similar metadata type matches by its file suffix
*
* @param suffix - File suffix of the metadata type
* @returns An array of similar suffix and metadata type matches
*/
public guessTypeBySuffix(
iowillhoit marked this conversation as resolved.
Show resolved Hide resolved
suffix: string
): Array<{ suffixGuess: string; metadataTypeGuess: MetadataType }> | undefined {
const registryKeys = Object.keys(this.registry.suffixes);

const scores = registryKeys.map((registryKey) => ({
registryKey,
score: Levenshtein.get(suffix, registryKey, { useCollator: true }),
}));
const sortedScores = scores.sort((a, b) => a.score - b.score);
const lowestScore = sortedScores[0].score;
// Levenshtein uses positive integers for scores, find all scores that match the lowest score
const guesses = sortedScores.filter((score) => score.score === lowestScore);

if (guesses.length > 0) {
return guesses.map((guess) => {
const typeId = this.registry.suffixes[guess.registryKey];
const metadataType = this.getTypeByName(typeId);
return {
suffixGuess: guess.registryKey,
metadataTypeGuess: metadataType,
};
});
}
}

/**
* Searches for the first metadata type in the registry that returns `true`
* for the given predicate function.
Expand Down
2 changes: 1 addition & 1 deletion src/registry/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/
export interface MetadataRegistry {
types: TypeIndex;
suffixes?: SuffixIndex;
suffixes: SuffixIndex;
strictDirectoryNames: {
[directoryName: string]: string;
};
Expand Down
63 changes: 61 additions & 2 deletions src/resolve/metadataResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { basename, dirname, join, sep } from 'path';
import { Lifecycle, Messages, SfError } from '@salesforce/core';
import { Lifecycle, Messages, SfError, Logger } from '@salesforce/core';
import { extName, parentName, parseMetadataXml } from '../utils';
import { MetadataType, RegistryAccess } from '../registry';
import { ComponentSet } from '../collections';
Expand All @@ -25,6 +25,7 @@ const messages = Messages.loadMessages('@salesforce/source-deploy-retrieve', 'sd
*/
export class MetadataResolver {
public forceIgnoredPaths: Set<string>;
protected logger: Logger;
private forceIgnore?: ForceIgnore;
private sourceAdapterFactory: SourceAdapterFactory;
private folderContentTypeDirNames?: string[];
Expand All @@ -38,6 +39,7 @@ export class MetadataResolver {
private tree: TreeContainer = new NodeFSTreeContainer(),
private useFsForceIgnore = true
) {
this.logger = Logger.childFromRoot(this.constructor.name);
this.sourceAdapterFactory = new SourceAdapterFactory(this.registry, tree);
this.forceIgnoredPaths = new Set<string>();
}
Expand Down Expand Up @@ -139,13 +141,26 @@ export class MetadataResolver {
!adapter.allowMetadataWithContent();
return shouldResolve ? adapter.getComponent(fsPath, isResolvingSource) : undefined;
}

// If a file ends with .xml and is not a metadata type, it is likely a package manifest
iowillhoit marked this conversation as resolved.
Show resolved Hide resolved
// In the past, these were "resolved" as EmailServicesFunction. See note on "attempt 3" in resolveType() below.
if (fsPath.endsWith('.xml') && !fsPath.endsWith(META_XML_SUFFIX)) {
this.logger.debug(`Could not resolve type for ${fsPath}. It is likely a package manifest. Moving on.`);
return undefined;
}

void Lifecycle.getInstance().emitTelemetry({
eventName: 'metadata_resolver_type_inference_error',
library: 'SDR',
function: 'resolveComponent',
path: fsPath,
});
throw new SfError(messages.getMessage('error_could_not_infer_type', [fsPath]), 'TypeInferenceError');

// The metadata type could not be inferred
// Attempt to guess the type and throw an error with actions
const actions = this.getSuggestionsForUnresolvedTypes(fsPath);

throw new SfError(messages.getMessage('error_could_not_infer_type', [fsPath]), 'TypeInferenceError', actions);
}

private resolveTypeFromStrictFolder(fsPath: string): MetadataType | undefined {
Expand Down Expand Up @@ -202,6 +217,15 @@ export class MetadataResolver {
// attempt 3 - try treating the file extension name as a suffix
if (!resolvedType) {
resolvedType = this.registry.getTypeBySuffix(extName(fsPath));

// Metadata types with `strictDirectoryName` should have been caught in "attempt 1".
// If the metadata returned from this lookup has a `strictDirectoryName`, something is wrong.
// It is likely that the metadata file is misspelled or has the wrong suffix.
// A common occurrence is that a misspelled metadata file will fall back to
// `EmailServicesFunction` because that is the default for the `.xml` suffix
if (resolvedType?.strictDirectoryName === true) {
resolvedType = undefined;
}
}

// attempt 4 - try treating the content as metadata
Expand All @@ -215,6 +239,41 @@ export class MetadataResolver {
return resolvedType;
}

/**
* Attempt to find similar types for types that could not be inferred
* To be used after executing the resolveType() method
*
* @param fsPath
* @returns an array of suggestions
*/
private getSuggestionsForUnresolvedTypes(fsPath: string): string[] {
const parsedMetaXml = parseMetadataXml(fsPath);
const metaSuffix = parsedMetaXml?.suffix;

// Analogous to "attempt 2" and "attempt 3" above
const guesses = metaSuffix
? this.registry.guessTypeBySuffix(metaSuffix)
: this.registry.guessTypeBySuffix(extName(fsPath));

// If guesses were found, format an array of strings to be passed to SfError's actions
return guesses && guesses.length > 0
? [
messages.getMessage('suggest_type_header', [
metaSuffix ? `".${parsedMetaXml.suffix}-meta.xml" metadata` : `".${extName(fsPath)}" filename`,
]),
...guesses.map((guess) =>
messages.getMessage('suggest_type_did_you_mean', [
guess.suffixGuess,
metaSuffix ? '-meta.xml' : '',
guess.metadataTypeGuess.name,
])
),
'', // A blank line makes this much easier to read (it doesn't seem to be possible to start a markdown message entry with a newline)
messages.getMessage('suggest_type_more_suggestions'),
]
: [];
}

/**
* Whether or not a directory that represents a single component should be resolved as one,
* or if it should be walked for additional components.
Expand Down
126 changes: 126 additions & 0 deletions test/nuts/suggestType/suggestType.nut.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/*
* Copyright (c) 2020, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import * as path from 'path';
import { TestSession } from '@salesforce/cli-plugins-testkit';
import { expect } from 'chai';
import { SfError, Messages } from '@salesforce/core';
import { ComponentSetBuilder } from '../../../src';

Messages.importMessagesDirectory(__dirname);
const messages = Messages.loadMessages('@salesforce/source-deploy-retrieve', 'sdr');

describe('suggest types', () => {
let session: TestSession;

before(async () => {
session = await TestSession.create({
project: {
sourceDir: path.join('test', 'nuts', 'suggestType', 'testProj'),
},
devhubAuthStrategy: 'NONE',
});
});

after(async () => {
await session?.clean();
});

it('it offers a suggestions on an invalid metadata suffix', async () => {
try {
await ComponentSetBuilder.build({
sourcepath: [path.join(session.project.dir, 'force-app', 'main', 'default', 'objects')],
});
throw new Error('This test should have thrown');
} catch (err) {
const error = err as SfError;
expect(error.name).to.equal('TypeInferenceError');
expect(error.actions).to.include(
'A search for the ".objct-meta.xml" metadata suffix found the following close matches:'
);
expect(error.actions).to.include(
'-- Did you mean ".object-meta.xml" instead for the "CustomObject" metadata type?'
);
}
});

it('it offers a suggestions on an invalid filename suffix', async () => {
try {
await ComponentSetBuilder.build({
sourcepath: [path.join(session.project.dir, 'force-app', 'main', 'default', 'classes', 'DummyClass.clss')],
});
throw new Error('This test should have thrown');
} catch (err) {
const error = err as SfError;
expect(error.name).to.equal('TypeInferenceError');
expect(error.actions).to.include('A search for the ".clss" filename suffix found the following close matches:');
expect(error.actions).to.include('-- Did you mean ".cls" instead for the "ApexClass" metadata type?');
}
});

it('it offers a suggestions on a incorrect casing', async () => {
try {
await ComponentSetBuilder.build({
sourcepath: [path.join(session.project.dir, 'force-app', 'main', 'default', 'layouts')],
});
throw new Error('This test should have thrown');
} catch (err) {
const error = err as SfError;
expect(error.name).to.equal('TypeInferenceError');
expect(error.actions).to.include(
'A search for the ".Layout-meta.xml" metadata suffix found the following close matches:'
);
expect(error.actions).to.include('-- Did you mean ".layout-meta.xml" instead for the "Layout" metadata type?');
}
});

it('it offers multiple suggestions if Levenshtein distance is the same', async () => {
try {
await ComponentSetBuilder.build({
sourcepath: [path.join(session.project.dir, 'force-app', 'main', 'default', 'tabs')],
});
throw new Error('This test should have thrown');
} catch (err) {
const error = err as SfError;
expect(error.name).to.equal('TypeInferenceError');
expect(error.actions).to.include(
'A search for the ".tabsss-meta.xml" metadata suffix found the following close matches:'
);
expect(error.actions).to.include(
'-- Did you mean ".labels-meta.xml" instead for the "CustomLabels" metadata type?'
);
expect(error.actions).to.include('-- Did you mean ".tab-meta.xml" instead for the "CustomTab" metadata type?');
}
});

it('it offers additional suggestions to try', async () => {
try {
await ComponentSetBuilder.build({
sourcepath: [path.join(session.project.dir, 'force-app', 'main', 'default', 'tabs')],
});
throw new Error('This test should have thrown');
} catch (err) {
const error = err as SfError;
expect(error.name).to.equal('TypeInferenceError');
expect(error.actions).to.include(messages.getMessage('suggest_type_more_suggestions'));
}
});

// Since EmailServicesFunction uses the 'xml' suffix, we want to ensure it still resolves correctly
it('it still correctly resolves an EmailServicesFunction', async () => {
const cs = await ComponentSetBuilder.build({
sourcepath: [path.join(session.project.dir, 'force-app', 'main', 'default', 'emailservices')],
});
expect(cs['components'].size).to.equal(1);
expect((await cs.getObject()).Package.types[0].name).to.equal('EmailServicesFunction');
});

it('it ignores package manifest files', async () => {
const cs = await ComponentSetBuilder.build({ sourcepath: [path.join(session.project.dir, 'package-manifest')] });
expect(cs['components'].size).to.equal(0);
});
});
13 changes: 13 additions & 0 deletions test/nuts/suggestType/testProj/config/project-scratch-def.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"orgName": "ewillhoit company",
"edition": "Developer",
"features": ["EnableSetPasswordInApi"],
"settings": {
"lightningExperienceSettings": {
"enableS1DesktopEnabled": true
},
"mobileSettings": {
"enableS1EncryptedStoragePref2": false
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
global class DummyClass {
result.success = true;
return result;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
global class MyEmailHandler implements Messaging.InboundEmailHandler {
global Messaging.InboundEmailResult handleInboundEmail(Messaging.InboundEmail email, Messaging.InboundEnvelope envelope) {
Messaging.InboundEmailResult result = new Messaging.InboundEmailresult();

// logic here

result.success = true;
return result;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="UTF-8" ?>
<ApexClass xmlns="http://soap.sforce.com/2006/04/metadata">
<apiVersion>57.0</apiVersion>
<status>Active</status>
<fullName>MyEmailHandler.clz</fullName>
</ApexClass>
Loading