Skip to content

Commit

Permalink
Merge pull request #601 from forcedotcom/sm/file-moves-again
Browse files Browse the repository at this point in the history 
feat: handle file move then edit scenario
  • Loading branch information
@iowillhoit
iowillhoit authored Jun 7, 2024
2 parents 8754d62 + 78ac637 commit f8c3d86
Show file tree
Hide file tree
Showing 12 changed files with 466 additions and 770 deletions.
938 changes: 249 additions & 689 deletions CHANGELOG.md
View file

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"devDependencies": {
"@salesforce/cli-plugins-testkit": "^5.3.8",
"@salesforce/dev-scripts": "^10.1.0",
"@salesforce/schemas": "^1.9.0",
"@types/graceful-fs": "^4.1.9",
"eslint-plugin-sf-plugin": "^1.18.5",
"ts-node": "^10.9.2",
Expand Down
6 changes: 2 additions & 4 deletions src/shared/guards.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import {
SourceComponent,
MetadataMember,
FileResponse,
ComponentStatus,
Expand All @@ -15,9 +14,6 @@ import {
import { ChangeResult } from './types';
import { ChangeResultWithNameAndType } from './types';

export const sourceComponentGuard = (input: SourceComponent | undefined): input is SourceComponent =>
input instanceof SourceComponent;

export const metadataMemberGuard = (
input: MetadataMember | undefined | Partial<MetadataMember>
): input is MetadataMember =>
Expand All @@ -42,3 +38,5 @@ export const FileResponseHasPath = (

export const isChangeResultWithNameAndType = (cr?: ChangeResult): cr is ChangeResultWithNameAndType =>
typeof cr === 'object' && typeof cr.name === 'string' && typeof cr.type === 'string';

export const isDefined = <T>(x: T | undefined): x is T => x !== undefined;
20 changes: 9 additions & 11 deletions src/shared/local/localShadowRepo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import git from 'isomorphic-git';
import { Performance } from '@oclif/core/performance';
import { RegistryAccess } from '@salesforce/source-deploy-retrieve';
import { chunkArray, excludeLwcLocalOnlyTest, folderContainsPath } from '../functions';
import { filenameMatchesToMap, getMatches } from './moveDetection';
import { filenameMatchesToMap, getLogMessage, getMatches } from './moveDetection';
import { StatusRow } from './types';
import { isDeleted, isAdded, toFilenames } from './functions';

Expand Down Expand Up @@ -348,21 +348,19 @@ export class ShadowRepo {
const movedFilesMarker = Performance.mark('@salesforce/source-tracking', 'localShadowRepo.detectMovedFiles');
const matches = await filenameMatchesToMap(IS_WINDOWS)(this.registry)(this.projectPath)(this.gitDir)(matchingFiles);

if (matches.size === 0) return movedFilesMarker?.stop();
if (matches.deleteOnly.size === 0 && matches.fullMatches.size === 0) return movedFilesMarker?.stop();

this.logger.debug(
[
'Files have moved. Committing moved files:',
[...matches.entries()].map(([add, del]) => `- File ${del} was moved to ${add}`).join(os.EOL),
].join(os.EOL)
);
this.logger.debug(getLogMessage(matches));

movedFilesMarker?.addDetails({ filesMoved: matches.size });
movedFilesMarker?.addDetails({
filesMoved: matches.fullMatches.size,
filesMovedAndEdited: matches.deleteOnly.size,
});

// Commit the moved files and refresh the status
await this.commitChanges({
deletedFiles: [...matches.values()],
deployedFiles: [...matches.keys()],
deletedFiles: [...matches.fullMatches.values(), ...matches.deleteOnly.values()],
deployedFiles: [...matches.fullMatches.keys()],
message: 'Committing moved files',
});

Expand Down
149 changes: 96 additions & 53 deletions src/shared/local/moveDetection.ts
View file
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 path from 'node:path';
import { EOL } from 'node:os';
import { Logger, Lifecycle } from '@salesforce/core';
import {
MetadataResolver,
Expand All @@ -16,21 +17,28 @@ import {
import git from 'isomorphic-git';
import * as fs from 'graceful-fs';
import { Performance } from '@oclif/core/performance';
import { sourceComponentGuard } from '../guards';
import { isDefined } from '../guards';
import { isDeleted, isAdded, ensureWindows, toFilenames } from './functions';
import { AddAndDeleteMaps, FilenameBasenameHash, StatusRow, StringMap } from './types';

const JOIN_CHAR = '#__#'; // the __ makes it unlikely to be used in metadata names
type AddAndDeleteFileInfos = { addedInfo: FilenameBasenameHash[]; deletedInfo: FilenameBasenameHash[] };
type AddedAndDeletedFilenames = { added: Set<string>; deleted: Set<string> };
type StringMapsForMatches = {
/** these matches filename=>basename, metadata type/name, and git object hash */
fullMatches: StringMap;
/** these did not match the hash. They *probably* are matches where the "add" is also modified */
deleteOnly: StringMap;
};

/** composed functions to simplified use by the shadowRepo class */
export const filenameMatchesToMap =
(isWindows: boolean) =>
(registry: RegistryAccess) =>
(projectPath: string) =>
(gitDir: string) =>
async ({ added, deleted }: AddedAndDeletedFilenames): Promise<StringMap> =>
removeNonMatches(isWindows)(registry)(
async ({ added, deleted }: AddedAndDeletedFilenames): Promise<StringMapsForMatches> =>
excludeNonMatchingTypes(isWindows)(registry)(
compareHashes(
await buildMaps(
await toFileInfo({
Expand Down Expand Up @@ -73,7 +81,14 @@ export const getMatches = (status: StatusRow[]): AddedAndDeletedFilenames => {
return { added: addedFilenamesWithMatches, deleted: deletedFilenamesWithMatches };
};

/** build maps of the add/deletes with filenames, returning the matches Logs if non-matches */
export const getLogMessage = (matches: StringMapsForMatches): string =>
[
'Files have moved. Committing moved files:',
...[...matches.fullMatches.entries()].map(([add, del]) => `- File ${del} was moved to ${add}`),
...[...matches.deleteOnly.entries()].map(([add, del]) => `- File ${del} was moved to ${add} and modified`),
].join(EOL);

/** build maps of the add/deletes with filenames, returning the matches Logs if we can't make a match because buildMap puts them in the ignored bucket */
const buildMaps = async ({ addedInfo, deletedInfo }: AddAndDeleteFileInfos): Promise<AddAndDeleteMaps> => {
const [addedMap, addedIgnoredMap] = buildMap(addedInfo);
const [deletedMap, deletedIgnoredMap] = buildMap(deletedInfo);
Expand All @@ -96,51 +111,72 @@ const buildMaps = async ({ addedInfo, deletedInfo }: AddAndDeleteFileInfos): Pro
return { addedMap, deletedMap };
};

/** builds a map of the values from both maps */
const compareHashes = ({ addedMap, deletedMap }: AddAndDeleteMaps): StringMap => {
const matches: StringMap = new Map();
/**
* builds a map of the values from both maps
* side effect: mutates the passed-in maps!
*/
const compareHashes = ({ addedMap, deletedMap }: AddAndDeleteMaps): StringMapsForMatches => {
const matches = new Map<string, string>(
[...addedMap.entries()]
.map(([addedKey, addedValue]) => {
const deletedValue = deletedMap.get(addedKey);
if (deletedValue) {
// these are an exact basename and hash match
deletedMap.delete(addedKey);
addedMap.delete(addedKey);
return [addedValue, deletedValue] as const;
}
})
.filter(isDefined)
);

for (const [addedKey, addedValue] of addedMap) {
const deletedValue = deletedMap.get(addedKey);
if (deletedValue) {
matches.set(addedValue, deletedValue);
}
if (addedMap.size && deletedMap.size) {
// the remaining deletes didn't match the basename+hash of an add, and vice versa.
// They *might* match the basename of an add, in which case we *could* have the "move, then edit" case.
const addedBasenameMap = new Map([...addedMap.entries()].map(hashEntryToBasenameEntry));
const deletedBasenameMap = new Map([...deletedMap.entries()].map(hashEntryToBasenameEntry));
const deleteOnly = new Map<string, string>(
Array.from(deletedBasenameMap.entries())
.filter(([k]) => addedBasenameMap.has(k))
.map(([k, v]) => [addedBasenameMap.get(k) as string, v])
);
return { fullMatches: matches, deleteOnly };
}

return matches;
return { fullMatches: matches, deleteOnly: new Map<string, string>() };
};

/** given a StringMap, resolve the metadata types and return things that having matching type/parent */
const removeNonMatches =
const excludeNonMatchingTypes =
(isWindows: boolean) =>
(registry: RegistryAccess) =>
(matches: StringMap): StringMap => {
if (!matches.size) return matches;
const addedFiles = isWindows ? [...matches.keys()].map(ensureWindows) : [...matches.keys()];
const deletedFiles = isWindows ? [...matches.values()].map(ensureWindows) : [...matches.values()];
const resolverAdded = new MetadataResolver(registry, VirtualTreeContainer.fromFilePaths(addedFiles));
const resolverDeleted = new MetadataResolver(registry, VirtualTreeContainer.fromFilePaths(deletedFiles));

return new Map(
[...matches.entries()].filter(([addedFile, deletedFile]) => {
// we're only ever using the first element of the arrays
const [resolvedAdded] = resolveType(resolverAdded, isWindows ? [ensureWindows(addedFile)] : [addedFile]);
const [resolvedDeleted] = resolveType(
resolverDeleted,
isWindows ? [ensureWindows(deletedFile)] : [deletedFile]
);
return (
// they could match, or could both be undefined (because unresolved by SDR)
resolvedAdded?.type.name === resolvedDeleted?.type.name &&
// parent names match, if resolved and there are parents
resolvedAdded?.parent?.name === resolvedDeleted?.parent?.name &&
// parent types match, if resolved and there are parents
resolvedAdded?.parent?.type.name === resolvedDeleted?.parent?.type.name
);
})
);
({ fullMatches: matches, deleteOnly }: StringMapsForMatches): StringMapsForMatches => {
if (!matches.size && !deleteOnly.size) return { fullMatches: matches, deleteOnly };
const [resolvedAdded, resolvedDeleted] = [
[...matches.keys(), ...deleteOnly.keys()], // the keys/values are only used for the resolver, so we use 1 for both add and delete
[...matches.values(), ...deleteOnly.values()],
]
.map((filenames) => filenames.map(isWindows ? ensureWindows : stringNoOp))
.map((filenames) => new MetadataResolver(registry, VirtualTreeContainer.fromFilePaths(filenames)))
.map(resolveType);

return {
fullMatches: new Map([...matches.entries()].filter(typeFilter(isWindows)(resolvedAdded, resolvedDeleted))),
deleteOnly: new Map([...deleteOnly.entries()].filter(typeFilter(isWindows)(resolvedAdded, resolvedDeleted))),
};
};

const typeFilter =
(isWindows: boolean) =>
(resolveAdd: ReturnType<typeof resolveType>, resolveDelete: ReturnType<typeof resolveType>) =>
([added, deleted]: [string, string]): boolean => {
const [resolvedAdded] = resolveAdd(isWindows ? [ensureWindows(added)] : [added]);
const [resolvedDeleted] = resolveDelete(isWindows ? [ensureWindows(deleted)] : [deleted]);
return (
resolvedAdded?.type.name === resolvedDeleted?.type.name &&
resolvedAdded?.parent?.name === resolvedDeleted?.parent?.name &&
resolvedAdded?.parent?.type.name === resolvedDeleted?.parent?.type.name
);
};
/** enrich the filenames with basename and oid (hash) */
const toFileInfo = async ({
projectPath,
Expand Down Expand Up @@ -170,11 +206,12 @@ const toFileInfo = async ({
return { addedInfo, deletedInfo };
};

/** returns a map of <hash+basename, filepath>. If two items result in the same hash+basename, return that in the ignore bucket */
const buildMap = (info: FilenameBasenameHash[]): StringMap[] => {
const map: StringMap = new Map();
const ignore: StringMap = new Map();
info.map((i) => {
const key = `${i.hash}#${i.basename}`;
const key = `${i.hash}${JOIN_CHAR}${i.basename}`;
// If we find a duplicate key, we need to remove it and ignore it in the future.
// Finding duplicate hash#basename means that we cannot accurately determine where it was moved to or from
if (map.has(key) || ignore.has(key)) {
Expand All @@ -195,18 +232,20 @@ const getHashForAddedFile =
hash: (await git.hashBlob({ object: await fs.promises.readFile(path.join(projectPath, filepath)) })).oid,
});

const resolveType = (resolver: MetadataResolver, filenames: string[]): SourceComponent[] =>
filenames
.flatMap((filename) => {
try {
return resolver.getComponentsFromPath(filename);
} catch (e) {
const logger = Logger.childFromRoot('ShadowRepo.compareTypes');
logger.warn(`unable to resolve ${filename}`);
return undefined;
}
})
.filter(sourceComponentGuard);
const resolveType =
(resolver: MetadataResolver) =>
(filenames: string[]): SourceComponent[] =>
filenames
.flatMap((filename) => {
try {
return resolver.getComponentsFromPath(filename);
} catch (e) {
const logger = Logger.childFromRoot('ShadowRepo.compareTypes');
logger.warn(`unable to resolve ${filename}`);
return undefined;
}
})
.filter(isDefined);

/** where we don't have git objects to use, read the file contents to generate the hash */
const getHashFromActualFileContents =
Expand All @@ -218,3 +257,7 @@ const getHashFromActualFileContents =
basename: path.basename(filepath),
hash: (await git.readBlob({ fs, dir: projectPath, gitdir, filepath, oid })).oid,
});

const hashEntryToBasenameEntry = ([k, v]: [string, string]): [string, string] => [hashToBasename(k), v];
const hashToBasename = (hash: string): string => hash.split(JOIN_CHAR)[1];
const stringNoOp = (s: string): string => s;
8 changes: 4 additions & 4 deletions src/shared/localComponentSetArray.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
DestructiveChangesType,
RegistryAccess,
} from '@salesforce/source-deploy-retrieve';
import { sourceComponentGuard } from './guards';
import { isDefined } from './guards';
import { supportsPartialDelete, pathIsInFolder } from './functions';

type GroupedFileInput = {
Expand Down Expand Up @@ -83,7 +83,7 @@ export const getComponentSets = ({

grouping.deletes
.flatMap((filename) => resolverForDeletes.getComponentsFromPath(filename))
.filter(sourceComponentGuard)
.filter(isDefined)
.map((component) => {
// if the component supports partial delete AND there are files that are not deleted,
// set the component for deploy, not for delete.
Expand All @@ -92,7 +92,7 @@ export const getComponentSets = ({
try {
resolverForNonDeletes
.getComponentsFromPath(resolve(component.content))
.filter(sourceComponentGuard)
.filter(isDefined)
.map((nonDeletedComponent) => componentSet.add(nonDeletedComponent));
} catch (e) {
logger.warn(
Expand All @@ -113,7 +113,7 @@ export const getComponentSets = ({
return undefined;
}
})
.filter(sourceComponentGuard)
.filter(isDefined)
.map((component) => componentSet.add(component));
// there may have been ignored files, but componentSet.add doesn't automatically track them.
// We'll manually set the ignored paths from what the resolver has been tracking
Expand Down
4 changes: 2 additions & 2 deletions src/shared/populateTypesAndNames.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
RegistryAccess,
} from '@salesforce/source-deploy-retrieve';
import { ChangeResult } from './types';
import { isChangeResultWithNameAndType, sourceComponentGuard } from './guards';
import { isChangeResultWithNameAndType, isDefined } from './guards';
import {
ensureRelative,
excludeLwcLocalOnlyTest,
Expand Down Expand Up @@ -68,7 +68,7 @@ export const populateTypesAndNames =
return undefined;
}
})
.filter(sourceComponentGuard);
.filter(isDefined);

logger.debug(` matching SourceComponents have ${sourceComponents.length} items from local`);

Expand Down
4 changes: 2 additions & 2 deletions src/sourceTracking.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ import {
FileResponseIsDeleted,
FileResponseIsNotDeleted,
isChangeResultWithNameAndType,
isDefined,
isSdrSuccess,
sourceComponentGuard,
} from './shared/guards';
import { removeIgnored } from './shared/remoteChangeIgnoring';
import {
Expand Down Expand Up @@ -309,7 +309,7 @@ export class SourceTracking extends AsyncCreatable {
return undefined;
}
})
.filter(sourceComponentGuard);
.filter(isDefined);
}
}

Expand Down
Loading

0 comments on commit f8c3d86

Please sign in to comment.