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: profiles are searched only within package directories #397

Merged
merged 1 commit into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
62 changes: 26 additions & 36 deletions src/package/packageProfileApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ Messages.importMessagesDirectory(__dirname);
const profileApiMessages = Messages.loadMessages('@salesforce/packaging', 'profile_api');

/*
* This class provides functions used to re-write .profiles in the workspace when creating a package2 version.
* All profiles found in the workspaces are extracted out and then re-written to only include metadata in the profile
* that is relevant to the source in the package directory being packaged.
* This class provides functions used to re-write .profiles in the project package directories when creating a package2 version.
* All profiles found in the project package directories are extracted out and then re-written to only include metadata in the
* profile that is relevant to the source in the package directory being packaged.
*/
export class PackageProfileApi extends AsyncCreatable<ProfileApiOptions> {
public project: SfProject;
Expand All @@ -40,8 +40,8 @@ export class PackageProfileApi extends AsyncCreatable<ProfileApiOptions> {
public async init(): Promise<void> {}

/**
* For any profile present in the workspace, this function generates a subset of data that only contains references
* to items in the manifest.
* For any profile present in the project package directories, this function generates a subset of data that only
* contains references to items in the manifest.
*
* return a list of profile file locations that need to be removed from the package because they are empty
*
Expand All @@ -57,10 +57,7 @@ export class PackageProfileApi extends AsyncCreatable<ProfileApiOptions> {
const logger = Logger.childFromRoot('PackageProfileApi');

return (
getProfilesWithNamesAndPaths({
projectPath: this.project.getPath(),
excludedDirectories,
})
this.getProfilesWithNamesAndPaths(excludedDirectories)
.map(({ profilePath, name: profileName }) => {
const originalProfile = profileStringToProfile(fs.readFileSync(profilePath, 'utf-8'));
const adjustedProfile = profileRewriter(
Expand Down Expand Up @@ -97,7 +94,7 @@ export class PackageProfileApi extends AsyncCreatable<ProfileApiOptions> {
}

/**
* Filter out all profiles in the manifest and if any profiles exists in the workspace, add them to the manifest.
* Filter out all profiles in the manifest and if any profiles exist in the project package directories, add them to the manifest.
*
* @param typesArr array of objects { name[], members[] } that represent package types JSON.
* @param excludedDirectories Direcotires not to generate profiles for
Expand All @@ -106,28 +103,32 @@ export class PackageProfileApi extends AsyncCreatable<ProfileApiOptions> {
typesArr: PackageXml['types'],
excludedDirectories: string[] = []
): PackageXml['types'] {
const profilePathsWithNames = getProfilesWithNamesAndPaths({
projectPath: this.project.getPath(),
excludedDirectories,
});
const profilePathsWithNames = this.getProfilesWithNamesAndPaths(excludedDirectories);

// Filter all profiles, and add back the ones we found names for
return typesArr
.filter((kvp) => kvp.name !== 'Profile')
.concat([{ name: 'Profile', members: profilePathsWithNames.map((i) => i.name) }]);
}
}

const findAllProfiles = ({
projectPath,
excludedDirectories = [],
}: {
projectPath: string;
excludedDirectories?: string[];
}): string[] =>
glob.sync(path.join(projectPath, '**', '*.profile-meta.xml'), {
ignore: excludedDirectories.map((dir) => `**/${dir}/**`),
});
// Look for profiles in all package directories
private findAllProfiles(excludedDirectories: string[] = []): string[] {
const pkgDirs = this.project.getUniquePackageDirectories().map((pDir) => pDir.fullPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this would solve the problem completely. Imagine you have several packageDirectories A,B,C that contain profiles.

If you create a PV for A, this would be pulling in profiles from B,C, right?

Maybe this should still take a path to search in (the pkgDir corresponding to what's being packaged) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you look at the screenshot on forcedotcom/cli#2336 it's that scenario

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I understand, this is the desired behavior. To confirm, we'll need someone from the packaging team to review this.

return pkgDirs
.map((pDir) =>
glob.sync(path.join(pDir, '**', '*.profile-meta.xml'), {
ignore: excludedDirectories.map((dir) => `**/${dir}/**`),
})
)
.flat();
}

private getProfilesWithNamesAndPaths(excludedDirectories: string[]): Array<Required<ProfilePathWithName>> {
return this.findAllProfiles(excludedDirectories)
.map((profilePath) => ({ profilePath, name: profilePathToName(profilePath) }))
.filter(isProfilePathWithName);
}
}

type ProfilePathWithName = { profilePath: string; name?: string };

Expand All @@ -138,17 +139,6 @@ const isProfilePathWithName = (
const profilePathToName = (profilePath: string): string | undefined =>
profilePath.match(/([^/]+)\.profile-meta.xml/)?.[1];

const getProfilesWithNamesAndPaths = ({
projectPath,
excludedDirectories,
}: {
projectPath: string;
excludedDirectories: string[];
}): Array<Required<ProfilePathWithName>> =>
findAllProfiles({ projectPath, excludedDirectories })
.map((profilePath) => ({ profilePath, name: profilePathToName(profilePath) }))
.filter(isProfilePathWithName);

const getXmlFileLocation = (destPath: string, profilePath: string): string =>
path.join(destPath, path.basename(profilePath).replace(/(.*)(-meta.xml)/, '$1'));

Expand Down
22 changes: 21 additions & 1 deletion test/package/packageVersionCreate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ describe('Package Version Create', () => {

afterEach(async () => {
restoreContext($$);
await fs.promises.rmdir(path.join(project.getPath(), 'force-app'));
await fs.promises.rm(path.join(project.getPath(), 'force-app'), { recursive: true, force: true });
// @ts-ignore
project.packageDirectories = undefined;
});
Expand Down Expand Up @@ -774,6 +774,26 @@ describe('Package Version Create', () => {
expect(excludedDirsFilter).to.contain('unpackaged-force-app');
});

it('should not package profiles from outside of project package directories', async () => {
const pkgProfileApi = await PackageProfileApi.create({ project, includeUserLicenses: false });
const types = [
{ name: 'Layout', members: ['Test Layout'] },
{ name: 'Profile', members: ['Test Profile'] },
];
// write a Profile in the project but outside of the package dirs
const outsideDir = path.join(project.getPath(), 'outside-pkg-dirs');
const forceAppDir = path.join(project.getPath(), 'force-app');
await fs.promises.mkdir(outsideDir);
const fileContents = '<?xml version="1.0" encoding="UTF-8"?>';
await fs.promises.writeFile(path.join(outsideDir, 'Outside Profile.profile-meta.xml'), fileContents);
await fs.promises.writeFile(path.join(forceAppDir, 'Test Profile.profile-meta.xml'), fileContents);

const pkgTypeMembers = pkgProfileApi.filterAndGenerateProfilesForManifest(types);
expect(pkgTypeMembers.length).to.equal(2);
expect(pkgTypeMembers[0].members).to.deep.equal(['Test Layout']);
expect(pkgTypeMembers[1].members).to.deep.equal(['Test Profile']);
});

describe('validateAncestorId', () => {
let pvc: PackageVersionCreate;
beforeEach(() => {
Expand Down
Loading