-
Notifications
You must be signed in to change notification settings - Fork 15
chore: removing dependency on deprecated schemaValidator @W-20495414@ #1362
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
Changes from all commits
aea89d1
b5985ca
b88d798
c458367
981cd11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
|
|
@@ -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> & { | ||
| filePath: string; | ||
| sobject: string; | ||
| records: SObjectTreeInput[]; | ||
|
|
@@ -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')), | ||
|
|
@@ -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 */ | ||
|
|
@@ -189,54 +187,37 @@ const replaceRefWithId = | |
| Object.entries(record).map(([k, v]) => [k, v === `@${ref.refId}` ? ref.id : v]) | ||
| ) as SObjectTreeInput; | ||
|
|
||
| export const validatePlanContents = | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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[] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 = { | ||
|
|
@@ -48,21 +39,14 @@ export type ResponseRefs = { | |
| referenceId: string; | ||
| id: string; | ||
| }; | ||
| export type ImportResults = { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This type was never being used anywhere.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cool. definitely get to know |
||
| 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>; | ||
| 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') | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of having
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the goals of the change are
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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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>; | ||
There was a problem hiding this comment.
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 thisPartial, which I included as well.Now that type serves no purpose and could be deleted.