Skip to content
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
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@
"csv-parse": "^5.6.0",
"csv-stringify": "^6.6.0",
"form-data": "^4.0.5",
"terminal-link": "^3.0.0"
"terminal-link": "^3.0.0",
"zod": "^4.2.1"
},
"devDependencies": {
"@oclif/core": "^4.8.0",
Expand Down
72 changes: 0 additions & 72 deletions schema/dataImportPlanSchema.json

This file was deleted.

9 changes: 6 additions & 3 deletions src/api/data/tree/importFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
getResultsIfNoError,
transformRecordTypeEntries,
} from './importCommon.js';
import type { ImportResult, ResponseRefs, TreeResponse } from './importTypes.js';
import type { ImportResult, ImportStatus, ResponseRefs, TreeResponse } from './importTypes.js';
import { hasUnresolvedRefs } from './functions.js';

export type FileInfo = {
Expand All @@ -35,15 +35,18 @@ export type FileInfo = {
sobject: string;
};

export const importFromFiles = async (conn: Connection, dataFilePaths: string[]): Promise<ImportResult[]> => {
export const importFromFiles = async (conn: Connection, dataFilePaths: string[]): Promise<ImportStatus> => {
const logger = Logger.childFromRoot('data:import:tree:importSObjectTreeFile');
const fileInfos = (await Promise.all(dataFilePaths.map(parseFile))).map(logFileInfo(logger)).map(validateNoRefs);
await Promise.all(fileInfos.map(async (fi) => transformRecordTypeEntries(conn, fi.records)));
const refMap = createSObjectTypeMap(fileInfos.flatMap((fi) => fi.records));
const results = await Promise.allSettled(
fileInfos.map((fi) => sendSObjectTreeRequest(conn)(fi.sobject)(JSON.stringify({ records: fi.records })))
);
return results.map(getSuccessOrThrow).flatMap(getValueOrThrow(fileInfos)).map(addObjectTypes(refMap));
return {
results: results.map(getSuccessOrThrow).flatMap(getValueOrThrow(fileInfos)).map(addObjectTypes(refMap)),
warnings: [],
};
};

const getSuccessOrThrow = (result: PromiseSettledResult<TreeResponse>): PromiseFulfilledResult<TreeResponse> =>
Expand Down
95 changes: 38 additions & 57 deletions src/api/data/tree/importPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
*/
import path from 'node:path';
import { EOL } from 'node:os';
import { fileURLToPath } from 'node:url';
import fs from 'node:fs';
import { createHash } from 'node:crypto';

import { AnyJson, isString } from '@salesforce/ts-types';
import { Logger, SchemaValidator, SfError, Connection, Messages } from '@salesforce/core';
import type { GenericObject, SObjectTreeInput } from '../../../types.js';
import type { DataPlanPartFilesOnly, ImportResult } from './importTypes.js';
import { isString } from '@salesforce/ts-types';
import { Logger, Connection, Messages } from '@salesforce/core';
import type { DataPlanPart, GenericObject, SObjectTreeInput } from '../../../types.js';
import { DataImportPlanArraySchema, DataImportPlanArray } from '../../../schema/dataImportPlan.js';
import type { ImportResult, ImportStatus } from './importTypes.js';
import {
getResultsIfNoError,
parseDataFileContents,
Expand All @@ -37,7 +37,7 @@ Messages.importMessagesDirectoryFromMetaUrl(import.meta.url);
const messages = Messages.loadMessages('@salesforce/plugin-data', 'importApi');

// the "new" type for these. We're ignoring saveRefs/resolveRefs
export type EnrichedPlanPart = Omit<DataPlanPartFilesOnly, 'saveRefs' | 'resolveRefs'> & {
export type EnrichedPlanPart = Partial<DataPlanPart> & {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The properties coming from Omit<DataPlanPartFilesOnly, 'saveRefs' | 'resolveRefs'> are all also hard-defined in this type here, except for this Partial, which I included as well.
Now that type serves no purpose and could be deleted.

filePath: string;
sobject: string;
records: SObjectTreeInput[];
Expand All @@ -51,19 +51,17 @@ type ResultsSoFar = {
const TREE_API_LIMIT = 200;

const refRegex = (object: string): RegExp => new RegExp(`^@${object}Ref\\d+$`);
export const importFromPlan = async (conn: Connection, planFilePath: string): Promise<ImportResult[]> => {
export const importFromPlan = async (conn: Connection, planFilePath: string): Promise<ImportStatus> => {
const resolvedPlanPath = path.resolve(process.cwd(), planFilePath);
const logger = Logger.childFromRoot('data:import:tree:importFromPlan');

const warnings: string[] = [];
const planResultObj = validatePlanContents(resolvedPlanPath, JSON.parse(fs.readFileSync(resolvedPlanPath, 'utf-8')));
warnings.push(...planResultObj.warnings);
const planContents = await Promise.all(
(
await validatePlanContents(logger)(
resolvedPlanPath,
(await JSON.parse(fs.readFileSync(resolvedPlanPath, 'utf8'))) as DataPlanPartFilesOnly[]
planResultObj.parsedPlans
.flatMap((planPart) =>
planPart.files.map((f) => ({ ...planPart, filePath: path.resolve(path.dirname(resolvedPlanPath), f) }))
)
)
// there *shouldn't* be multiple files for the same sobject in a plan, but the legacy code allows that
.flatMap((dpp) => dpp.files.map((f) => ({ ...dpp, filePath: path.resolve(path.dirname(resolvedPlanPath), f) })))
.map(async (i) => ({
...i,
records: parseDataFileContents(i.filePath)(await fs.promises.readFile(i.filePath, 'utf-8')),
Expand All @@ -72,7 +70,7 @@ export const importFromPlan = async (conn: Connection, planFilePath: string): Pr
// using recursion to sequentially send the requests so we get refs back from each round
const { results } = await getResults(conn)(logger)({ results: [], fingerprints: new Set() })(planContents);

return results;
return { results, warnings };
};

/** recursively splits files (for size or unresolved refs) and makes API calls, storing results for subsequent calls */
Expand Down Expand Up @@ -189,54 +187,37 @@ const replaceRefWithId =
Object.entries(record).map(([k, v]) => [k, v === `@${ref.refId}` ? ref.id : v])
) as SObjectTreeInput;

export const validatePlanContents =
Copy link
Contributor

Choose a reason for hiding this comment

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

meta: don't break the rule and then add the comment to allow you to break the rule unless you've got a really compelling reason (and preferably explain your reason!)

here: you can export it for test only, it won't cause it to be publicly available unless you export it from index.js or some other package.json#exports file (I'm trying to guess why you'd want to do that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. I'm used to naming things that are exported solely for tests _blah instead of blah, because it makes it visually obvious to anyone reading the code that this method should be treated as private. The underscore signals 'oh, I shouldn't import this to whatever other code I'm writing'.
Part of my hesitance to futz with the code heavily the first time around was because so many things were exported. It wasn't immediatley obvious what was test-only and what wasn't.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, we can talk about why that distinction is important.

(logger: Logger) =>
async (planPath: string, planContents: unknown): Promise<DataPlanPartFilesOnly[]> => {
const childLogger = logger.child('validatePlanContents');
const planSchema = path.join(
path.dirname(fileURLToPath(import.meta.url)),
'..',
'..',
'..',
'..',
'schema',
'dataImportPlanSchema.json'
);

const val = new SchemaValidator(childLogger, planSchema);
try {
await val.validate(planContents as AnyJson);
const output = planContents as DataPlanPartFilesOnly[];
if (hasRefs(output)) {
childLogger.warn(
"The plan contains the 'saveRefs' and/or 'resolveRefs' properties. These properties will be ignored and can be removed."
);
}
if (!hasOnlySimpleFiles(output)) {
throw messages.createError('error.NonStringFiles');
}
return planContents as DataPlanPartFilesOnly[];
} catch (err) {
if (err instanceof Error && err.name === 'ValidationSchemaFieldError') {
throw messages.createError('error.InvalidDataImport', [planPath, err.message]);
} else if (err instanceof Error) {
throw SfError.wrap(err);
}
throw err;
export function validatePlanContents(
planPath: string,
planContents: unknown
): { parsedPlans: DataImportPlanArray; warnings: string[] } {
const warnings: string[] = [];
const parseResults = DataImportPlanArraySchema.safeParse(planContents);

if (parseResults.error) {
throw messages.createError('error.InvalidDataImport', [
planPath,
parseResults.error.issues.map((e) => e.message).join('\n'),
]);
}
const parsedPlans: DataImportPlanArray = parseResults.data;

for (const parsedPlan of parsedPlans) {
if (parsedPlan.saveRefs !== undefined || parsedPlan.resolveRefs !== undefined) {
warnings.push(
"The plan contains the 'saveRefs' and/or 'resolveRefs' properties. These properties will be ignored and can be removed."
);
break;
}
};
}
return { parsedPlans, warnings };
}

const matchesRefFilter =
(unresolvedRefRegex: RegExp) =>
(v: unknown): boolean =>
typeof v === 'string' && unresolvedRefRegex.test(v);

const hasOnlySimpleFiles = (planParts: DataPlanPartFilesOnly[]): boolean =>
planParts.every((p) => p.files.every((f) => typeof f === 'string'));

const hasRefs = (planParts: DataPlanPartFilesOnly[]): boolean =>
planParts.some((p) => p.saveRefs !== undefined || p.resolveRefs !== undefined);

// TODO: change this implementation to use Object.groupBy when it's on all supported node versions
const filterUnresolved = (
records: SObjectTreeInput[]
Expand Down
24 changes: 4 additions & 20 deletions src/api/data/tree/importTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import type { Dictionary } from '@salesforce/ts-types';
import type { DataPlanPart } from '../../../types.js';

/*
* Copyright (c) 2023, 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
*/
export type TreeResponse = TreeResponseSuccess | TreeResponseError;

type TreeResponseSuccess = {
Expand All @@ -48,21 +39,14 @@ export type ResponseRefs = {
referenceId: string;
id: string;
};
export type ImportResults = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This type was never being used anywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

cool. definitely get to know npx knip (we've included that in our cursor instructions within the extensions to avoid that kind of thing)

responseRefs?: ResponseRefs[];
sobjectTypes?: Dictionary;
errors?: string[];

export type ImportStatus = {
results: ImportResult[];
warnings: string[];
};

export type ImportResult = {
refId: string;
type: string;
id: string;
}; /** like the original DataPlanPart but without the non-string options inside files */

export type DataPlanPartFilesOnly = {
sobject: string;
files: string[];
saveRefs: boolean;
resolveRefs: boolean;
} & Partial<DataPlanPart>;
5 changes: 4 additions & 1 deletion src/commands/data/import/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,12 @@ export default class Import extends SfCommand<ImportResult[]> {
const conn = flags['target-org'].getConnection(flags['api-version']);

try {
const results = flags.plan
const { results, warnings } = flags.plan
? await importFromPlan(conn, flags.plan)
: await importFromFiles(conn, flags.files ?? []);
for (const warning of warnings) {
this.warn(warning);
}

this.table({
data: results,
Expand Down
47 changes: 47 additions & 0 deletions src/schema/dataImportPlan.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright 2025, Salesforce, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { z } from 'zod';

export const DataImportPlanArraySchema = z
.array(
z.object({
sobject: z.string().describe('Child file references must have SObject roots of this type'),
saveRefs: z
.boolean()
.optional()
.describe(
'Post-save, save references (Name/ID) to be used for reference replacement in subsequent saves. Applies to all data files for this SObject type.'
),
resolveRefs: z
.boolean()
.optional()
.describe(
'Pre-save, replace @<reference> with ID from previous save. Applies to all data files for this SObject type.'
),
files: z
.array(
z
.string('The `files` property of the plan objects must contain only strings')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of having zod accept an object here and then having us throw an error later if that object is actually used, decided to just have zod reject with the same message.

Copy link
Contributor

Choose a reason for hiding this comment

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

the goals of the change are

  1. schemaValidator => zod
  2. warning user in the UI instead of the logs
  3. use proper types instead of as

it's not meant to be a breaking change (imagine someone's script doing a data load, been working fine since 2020, etc)

If someone were using the old style did this break them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is that it would not. The only thing that has changed is where the error is being thrown. Even the text of the error remains the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I think you're right. I'm good with this functionally, then; everything else is "how to code" stuff.

.describe(
'Filepath string or object to point to a JSON or XML file having data defined in SObject Tree format.'
)
)
.describe('An array of files paths to load'),
})
)
.describe('Schema for data import plan JSON');

export type DataImportPlanArray = z.infer<typeof DataImportPlanArraySchema>;
Loading