Skip to content

fix: adds support for nested InFolder metadata types #455

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

Merged
merged 6 commits into from
Sep 23, 2021
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
2 changes: 1 addition & 1 deletion src/convert/streams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { promisify } from 'util';
import { SourceComponent, MetadataResolver } from '../resolve';
import { SfdxFileFormat, WriteInfo, WriterFormat } from './types';
import { ensureFileExists } from '../utils/fileSystemHandler';
import { META_XML_SUFFIX, SourcePath, XML_DECL } from '../common';
import { SourcePath, XML_DECL } from '../common';
import { ConvertContext } from './convertContext';
import { MetadataTransformerFactory } from './transformers';
import { JsonMap } from '@salesforce/ts-types';
Expand Down
3 changes: 3 additions & 0 deletions src/registry/registry.json
Original file line number Diff line number Diff line change
Expand Up @@ -1429,20 +1429,23 @@
"name": "Territory2Model",
"suffix": "territory2Model",
"directoryName": "territory2Models",
"inFolder": false,
"folderType": "territory2model"
},
"territory2rule": {
"id": "territory2rule",
"name": "Territory2Rule",
"suffix": "territory2Rule",
"directoryName": "rules",
"inFolder": false,
"folderType": "territory2model"
},
"territory2": {
"id": "territory2",
"name": "Territory2",
"suffix": "territory2",
"directoryName": "territories",
"inFolder": false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding these back since they are returned by a describemetadata call, and that's what we should be using as the source of truth for type definitions.

Copy link
Contributor

Choose a reason for hiding this comment

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

source of untruth

"folderType": "territory2model"
},
"campaigninfluencemodel": {
Expand Down
31 changes: 28 additions & 3 deletions src/registry/registryAccess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import { MetadataRegistry, MetadataType } from './types';
export class RegistryAccess {
private registry: MetadataRegistry;

private strictFolderTypes: MetadataType[];
private folderContentTypes: MetadataType[];

constructor(registry: MetadataRegistry = defaultRegistry) {
this.registry = registry;
}
Expand Down Expand Up @@ -71,9 +74,31 @@ export class RegistryAccess {
* @returns An array of metadata type objects that require strict parent folder names
*/
public getStrictFolderTypes(): MetadataType[] {
return Object.values(this.registry.strictDirectoryNames).map(
(typeId) => this.registry.types[typeId]
);
if (!this.strictFolderTypes) {
this.strictFolderTypes = Object.values(this.registry.strictDirectoryNames).map(
(typeId) => this.registry.types[typeId]
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We only need to do this once. The registry data can't change once RegistryAccess is created.

}
return this.strictFolderTypes;
}

/**
* Query for the types that have the folderContentType property defined.
* E.g., reportFolder, dashboardFolder, documentFolder, emailFolder
* @see {@link MetadataType.folderContentType}
*
* @returns An array of metadata type objects that have folder content
*/
public getFolderContentTypes(): MetadataType[] {
if (!this.folderContentTypes) {
this.folderContentTypes = [];
for (const type of Object.values(this.registry.types)) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you think this is why the registry has so many other properties? To avoid the perf hit of iterating over every type just to find some that have a property defined? Would it make sense to add a folderContentTypes property to the registry and just access it directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By "so many other properties" are you referring to the indexes like suffixes, strictDirectoryNames, and childTypes? If so, yeah that's for better perf. There are only 4 folderContentTypes so I don't think it's worth indexing them. Building the array once makes sense to me though. I think RegistryAccess could use more memoization logic like this.

Copy link
Member

Choose a reason for hiding this comment

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

yeah exactly. I'd prefer not to iterate over the whole registry to find four entries, multiple times... but we could do this optimization later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's only done once, not multiple times.

if (type.folderContentType) {
this.folderContentTypes.push(type);
}
}
}
return this.folderContentTypes;
}

get apiVersion(): string {
Expand Down
36 changes: 25 additions & 11 deletions src/resolve/adapters/baseSourceAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { SourceAdapter, MetadataXml } from '../types';
import { parseMetadataXml } from '../../utils';
import { parseMetadataXml, parseNestedFullName } from '../../utils';
import { UnexpectedForceIgnore } from '../../errors';
import { parentName } from '../../utils/path';
import { ForceIgnore } from '../forceIgnore';
import { dirname, basename, sep } from 'path';
import { NodeFSTreeContainer, TreeContainer } from '../treeContainers';
Expand Down Expand Up @@ -123,14 +122,22 @@ export abstract class BaseSourceAdapter implements SourceAdapter {
* @param path File path of a metadata component
*/
private parseAsContentMetadataXml(path: SourcePath): MetadataXml {
// InFolder metadata can be nested more than 1 level beneath its
// associated directoryName.
if (this.type.inFolder) {
const fullName = parseNestedFullName(path, this.type.directoryName);
if (fullName) {
return { fullName, suffix: this.type.suffix, path };
}
}

const parentPath = dirname(path);
const parts = parentPath.split(sep);
const typeFolderIndex = parts.lastIndexOf(this.type.directoryName);
// nestedTypes (ex: territory2) have a folderType equal to their type but are themselves in a folder per metadata item, with child folders for rules/territories
// nestedTypes (ex: territory2) have a folderType equal to their type but are themselves
// in a folder per metadata item, with child folders for rules/territories
const allowedIndex =
this.type.inFolder || this.type.folderType === this.type.id
? parts.length - 2
: parts.length - 1;
this.type.folderType === this.type.id ? parts.length - 2 : parts.length - 1;

if (typeFolderIndex !== allowedIndex) {
return undefined;
Expand All @@ -150,14 +157,23 @@ export abstract class BaseSourceAdapter implements SourceAdapter {
}
}

// Given a MetadataXml, build a fullName from the path and type.
private calculateName(rootMetadata: MetadataXml): string {
const { directoryName, inFolder, folderType, folderContentType } = this.type;

// inFolder types (report, dashboard, emailTemplate, document) and their folder
// container types (reportFolder, dashboardFolder, emailFolder, documentFolder)
if (inFolder || folderContentType) {
return parseNestedFullName(rootMetadata.path, directoryName);
}

// not using folders? then name is fullname
if (!this.type.folderType) {
if (!folderType) {
return rootMetadata.fullName;
}
const grandparentType = this.registry.getTypeByName(this.type.folderType);
const grandparentType = this.registry.getTypeByName(folderType);

// type is in a nested inside another type (ex: Territory2Model). So the names are modelName.ruleName or modelName.territoryName
// type is nested inside another type (ex: Territory2Model). So the names are modelName.ruleName or modelName.territoryName
if (grandparentType.folderType && grandparentType.folderType !== this.type.id) {
const splits = rootMetadata.path.split(sep);
return `${splits[splits.indexOf(grandparentType.directoryName) + 1]}.${
Expand All @@ -168,8 +184,6 @@ export abstract class BaseSourceAdapter implements SourceAdapter {
if (grandparentType.folderType === this.type.id) {
return rootMetadata.fullName;
}
// other folderType scenarios (report, dashboard, emailTemplate, etc) where the parent is of a different type
return `${parentName(rootMetadata.path)}/${rootMetadata.fullName}`;
}

/**
Expand Down
40 changes: 35 additions & 5 deletions src/resolve/manifestResolver.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 { RegistryAccess } from '../registry';
import { MetadataType, RegistryAccess } from '../registry';
import { NodeFSTreeContainer, TreeContainer } from './treeContainers';
import { MetadataComponent } from './types';
import { parse as parseXml } from 'fast-xml-parser';
Expand Down Expand Up @@ -64,16 +64,46 @@ export class ManifestResolver {
const typeName = typeMembers.name;
const type = this.registry.getTypeByName(typeName);
const parentType = type.folderType ? this.registry.getTypeByName(type.folderType) : undefined;
for (const fullName of normalizeToArray(typeMembers.members)) {
const members = normalizeToArray(typeMembers.members);

for (const fullName of members) {
let mdType = type;
// if there is no / delimiter and it's a type in folders that aren't nestedType, infer folder component
if (type.folderType && !fullName.includes('/') && parentType.folderType !== parentType.id) {
mdType = this.registry.getTypeByName(type.folderType);
if (this.isNestedInFolder(fullName, type, parentType, members)) {
mdType = parentType;
}
components.push({ fullName, type: mdType });
}
}

return { components, apiVersion };
}

// Use the folderType instead of the type from the manifest when:
// 1. InFolder types: (report, dashboard, emailTemplate, document)
// 1a. type.inFolder === true (from registry.json) AND
// 1b. The fullName doesn't contain a forward slash character AND
// 1c. The fullName with a slash appended is contained in another member entry
// OR
// 2. Non-InFolder, folder types: (territory2, territory2Model, territory2Type, territory2Rule)
// 2a. type.inFolder !== true (from registry.json) AND
// 2b. type.folderType has a value (from registry.json) AND
// 2c. This type's parent type has a folderType that doesn't match its ID.
private isNestedInFolder(
fullName: string,
type: MetadataType,
parentType: MetadataType,
members: string[]
): boolean {
// Quick short-circuit for non-folderTypes
if (!type.folderType) {
return false;
}

const isInFolderType = type.inFolder;
const isNestedInFolder =
!fullName.includes('/') || members.some((m) => m.includes(`${fullName}/`));
Copy link
Contributor

Choose a reason for hiding this comment

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

can windows users put \ or \\ in their manifest?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that's valid. It's not a file path, which would be OS specific. It's the server-assigned API name.

const isNonMatchingFolder = parentType && parentType.folderType !== parentType.id;

return (isInFolderType && isNestedInFolder) || (!isInFolderType && isNonMatchingFolder);
}
}
25 changes: 24 additions & 1 deletion src/resolve/metadataResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export class MetadataResolver {
private sourceAdapterFactory: SourceAdapterFactory;
private tree: TreeContainer;
private registry: RegistryAccess;
private folderContentTypeDirNames: string[];

/**
* @param registry Custom registry data
Expand Down Expand Up @@ -234,19 +235,41 @@ export class MetadataResolver {
return !!this.registry.getTypeBySuffix(extName(fsPath));
}

// Get the array of directoryNames for types that have folderContentType
private getFolderContentTypeDirNames(): string[] {
if (!this.folderContentTypeDirNames) {
this.folderContentTypeDirNames = this.registry
.getFolderContentTypes()
.map((t) => t.directoryName);
}
return this.folderContentTypeDirNames;
}

/**
* Identify metadata xml for a folder component:
* .../email/TestFolder-meta.xml
* .../reports/foo/bar-meta.xml
*
* Do not match this pattern:
* .../tabs/TestFolder.tab-meta.xml
*/
private parseAsFolderMetadataXml(fsPath: string): string {
let folderName;
const match = basename(fsPath).match(/(.+)-meta\.xml/);
if (match && !match[1].includes('.')) {
const parts = fsPath.split(sep);
return parts.length > 1 ? parts[parts.length - 2] : undefined;
if (parts.length > 1) {
const folderContentTypesDirs = this.getFolderContentTypeDirNames();
// check if the path contains a folder content name as a directory
// e.g., `/reports/` and if it does return that folder name.
folderContentTypesDirs.some((dirName) => {
if (fsPath.includes(`/${dirName}/`)) {
folderName = dirName;
}
});
}
}
return folderName;
}

private isMetadata(fsPath: string): boolean {
Expand Down
19 changes: 9 additions & 10 deletions src/resolve/sourceComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* 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 { join, basename, sep } from 'path';
import { join, basename } from 'path';
import { parse } from 'fast-xml-parser';
import { ForceIgnore } from './forceIgnore';
import { NodeFSTreeContainer, TreeContainer, VirtualTreeContainer } from './treeContainers';
Expand Down Expand Up @@ -136,22 +136,21 @@ export class SourceComponent implements MetadataComponent {
}

private calculateRelativePath(fsPath: string): string {
const { directoryName, suffix, inFolder, folderType } = this.type;
const { directoryName, suffix, inFolder, folderType, folderContentType } = this.type;

// if there isn't a suffix, assume this is a mixed content component that must
// reside in the directoryName of its type. trimUntil maintains the folder structure
// the file resides in for the new destination.
if (!suffix) {
// the file resides in for the new destination. This also applies to inFolder types:
// (report, dashboard, emailTemplate, document) and their folder container types:
// (reportFolder, dashboardFolder, emailFolder, documentFolder)
if (!suffix || inFolder || folderContentType) {
return trimUntil(fsPath, directoryName);
}
// legacy version of folderType
if (inFolder) {
return join(directoryName, this.fullName.split('/')[0], basename(fsPath));
}

if (folderType) {
// types like Territory2Model have child types inside them. We have to preserve those folder structures
if (this.parentType?.folderType && this.parentType?.folderType !== this.type.id) {
const fsPathSplits = fsPath.split(sep);
return fsPathSplits.slice(fsPathSplits.indexOf(this.parentType.directoryName)).join(sep);
return trimUntil(fsPath, this.parentType.directoryName);
}
return join(directoryName, this.fullName.split('/')[0], basename(fsPath));
}
Expand Down
9 changes: 8 additions & 1 deletion src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,12 @@
*/
export { createFiles } from './fileSystemHandler';
export { generateMetaXML, generateMetaXMLPath, trimMetaXmlSuffix } from './metadata';
export { extName, baseName, parseMetadataXml, parentName, trimUntil } from './path';
export {
extName,
baseName,
parseMetadataXml,
parentName,
trimUntil,
parseNestedFullName,
} from './path';
export { normalizeToArray, deepFreeze } from './collections';
34 changes: 33 additions & 1 deletion src/utils/path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { basename, dirname, extname, sep } from 'path';
import { SourcePath } from '../common';
import { MetadataXml } from '../resolve';
import { Optional } from '@salesforce/ts-types';

/**
* Get the file or directory name at the end of a path. Different from `path.basename`
Expand Down Expand Up @@ -59,9 +60,40 @@ export function trimUntil(fsPath: SourcePath, part: string): string {
* @param fsPath - File path to parse
* @returns MetadataXml info or undefined
*/
export function parseMetadataXml(fsPath: string): MetadataXml | undefined {
export function parseMetadataXml(fsPath: string): Optional<MetadataXml> {
const match = basename(fsPath).match(/(.+)\.(.+)-meta\.xml/);
if (match) {
return { fullName: match[1], suffix: match[2], path: fsPath };
}
}

/**
* Returns the fullName for a nested metadata source file. This is for metadata
* types that can be nested more than 1 level such as report and reportFolder,
* dashboard and dashboardFolder, etc. It uses the directory name for the metadata type
* as the starting point (non-inclusively) to parse the fullName.
*
* Examples:
* (source format path)
* fsPath: force-app/main/default/reports/foo/bar/My_Report.report-meta.xml
* returns: foo/bar/My_Report
*
* (mdapi format path)
* fsPath: unpackaged/reports/foo/bar-meta.xml
* returns: foo/bar
*
* @param fsPath - File path to parse
* @param directoryName - name of directory to use as a parsing index
* @returns the FullName
*/
export function parseNestedFullName(fsPath: string, directoryName: string): Optional<string> {
const pathSplits = fsPath.split(sep);
// Exit if the directoryName is not included in the file path.
if (!pathSplits.includes(directoryName)) {
return;
}
const pathPrefix = pathSplits.slice(pathSplits.lastIndexOf(directoryName) + 1);
const fileName = pathSplits.pop().replace('-meta.xml', '').split('.')[0];
pathPrefix[pathPrefix.length - 1] = fileName;
return pathPrefix.join('/');
}
1 change: 1 addition & 0 deletions test/mock/registry/mockRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export const mockRegistryData = {
xmlinfolder: {
id: 'xmlinfolder',
directoryName: 'xmlinfolders',
inFolder: true,
name: 'XmlInFolder',
suffix: 'xif',
folderType: 'xmlinfolderfolder',
Expand Down
10 changes: 9 additions & 1 deletion test/registry/registryAccess.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { assert, expect } from 'chai';
import { RegistryError } from '../../src/errors';
import { nls } from '../../src/i18n';
import { MetadataRegistry, MetadataType } from '../../src/registry';
import { MetadataRegistry, MetadataType, RegistryAccess } from '../../src/registry';
import { mockRegistry, mockRegistryData } from '../mock/registry';

describe('RegistryAccess', () => {
Expand Down Expand Up @@ -80,4 +80,12 @@ describe('RegistryAccess', () => {
expect(mockRegistry.getStrictFolderTypes()).to.deep.equal(types);
});
});

describe('getFolderContentTypes', () => {
it('should return all the types with a folderContentType property defined', () => {
const type = mockRegistryData.types.xmlinfolderfolder;
const type2 = mockRegistryData.types.mciffolder;
expect(mockRegistry.getFolderContentTypes()).to.deep.equal([type, type2]);
});
});
});
Loading