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
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"@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,7 @@
"@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",
Expand Down Expand Up @@ -101,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 @@ -188,4 +191,4 @@
"output": []
}
}
}
}
6 changes: 3 additions & 3 deletions src/client/metadataTransfer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,13 @@ export abstract class MetadataTransfer<
}

protected async maybeSaveTempDirectory(target: SfdxFileFormat, cs?: ComponentSet): Promise<void> {
const mdapiTempDir = process.env.SFDX_MDAPI_TEMP_DIR;
const mdapiTempDir = process.env.SF_MDAPI_TEMP_DIR;
if (mdapiTempDir) {
await Lifecycle.getInstance().emitWarning(
'The SFDX_MDAPI_TEMP_DIR environment variable is set, which may degrade performance'
'The SF_MDAPI_TEMP_DIR environment variable is set, which may degrade performance'
);
this.logger.debug(
`Converting metadata to: ${mdapiTempDir} because the SFDX_MDAPI_TEMP_DIR environment variable is set`
`Converting metadata to: ${mdapiTempDir} because the SF_MDAPI_TEMP_DIR environment variable is set`
);
try {
const source = cs ?? this.components ?? new ComponentSet();
Expand Down
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
76 changes: 74 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,39 @@ export class MetadataResolver {
!adapter.allowMetadataWithContent();
return shouldResolve ? adapter.getComponent(fsPath, isResolvingSource) : undefined;
}

// Perform some additional checks to see if this is a package manifest
if (fsPath.endsWith('.xml') && !fsPath.endsWith(META_XML_SUFFIX)) {
// If it is named the default package.xml, assume it is a package manifest
if (fsPath.endsWith('package.xml')) return undefined;
try {
// If the file contains the string "<Package xmlns", it is a package manifest
if (this.tree.readFileSync(fsPath).toString().includes('<Package xmlns')) return undefined;
} catch (err) {
const error = err as Error;
if (error.message === 'Method not implemented') {
// Currently readFileSync is not implemented for zipTreeContainer
// Ignoring since this would have been ignored in the past
this.logger.warn(
`Type could not be inferred for ${fsPath}. It is likely this is a package manifest. Skipping...`
);
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 +230,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 +252,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
4 changes: 2 additions & 2 deletions test/client/metadataApiDeploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('MetadataApiDeploy', () => {

it('should save the temp directory if the environment variable is set', async () => {
try {
process.env.SFDX_MDAPI_TEMP_DIR = 'test';
process.env.SF_MDAPI_TEMP_DIR = 'test';
const components = new ComponentSet([matchingContentFile.COMPONENT]);
const { operation, convertStub, deployStub } = await stubMetadataDeploy($$, testOrg, {
components,
Expand All @@ -95,7 +95,7 @@ describe('MetadataApiDeploy', () => {
expect(deployStub.firstCall.args[0]).to.equal(zipBuffer);
expect(getString(convertStub.secondCall.args[2], 'outputDirectory', '')).to.equal('test');
} finally {
delete process.env.SFDX_MDAPI_TEMP_DIR;
delete process.env.SF_MDAPI_TEMP_DIR;
}
});

Expand Down
4 changes: 2 additions & 2 deletions test/client/metadataApiRetrieve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ describe('MetadataApiRetrieve', () => {

it('should save the temp directory if the environment variable is set', async () => {
try {
process.env.SFDX_MDAPI_TEMP_DIR = 'test';
process.env.SF_MDAPI_TEMP_DIR = 'test';
const toRetrieve = new ComponentSet([COMPONENT]);
const { operation, convertStub } = await stubMetadataRetrieve($$, testOrg, {
toRetrieve,
Expand All @@ -317,7 +317,7 @@ describe('MetadataApiRetrieve', () => {

expect(getString(convertStub.secondCall.args[2], 'outputDirectory', '')).to.equal('test');
} finally {
delete process.env.SFDX_MDAPI_TEMP_DIR;
delete process.env.SF_MDAPI_TEMP_DIR;
}
});

Expand Down
Loading