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

feat: beta tree exports/import #810

Merged
merged 41 commits into from
Feb 19, 2024
Merged

feat: beta tree exports/import #810

merged 41 commits into from
Feb 19, 2024

Conversation

mshanemc
Copy link
Contributor

@mshanemc mshanemc commented Feb 2, 2024

What does this PR do?

Phase 1: Beta

  1. data import beta tree and data export beta tree
  2. both use new helper functions
  3. existing commands encourage users to try the new ones

Breaking changes between existing and beta

  1. import beta removes its hidden, deprecated content-type. Only json files are supported. Usage: ~5 per year.
  2. import beta removes the --config-help flag. The schema stuff is in the command help. Usage: ~1 per week.

Other differences

  1. export --plan writes Object names as the file name. It used to append an s on the end. So the filename is now Account.json and Foo__c.json instead of Accounts.json and the awful Foo__cs.json
  2. export beta no longer writes empty child objects. Previously, you'd see properties with {records: []} that had no effect on imports.
  3. import beta with --plan does not care about resolveRefs and saveRefs
  4. import beta with --plan does not care about the order of files in your plan file. It'll defer unresolved references until they're resolved.
  5. export beta now handles more than 2 levels of child objects in a query using --plan (up to 5, that's the SOQL limit)
  6. both export beta and import beta handle objects that refer to objects of the same type (ex: Account with ParentId, User with Manager)
  7. import beta using --plan handles more than 200 records. It will batch them into groups of 200 per object. The new record limit is not documented--it most likely comes from your OS call stack depth or other org limits (data storage, api call limits)
  8. import supported plan files where the files property could contain objects. I'm really not sure how those files were generated, but I don't think the export command made them. For import beta only work with strings and will throw an error otherwise.
  9. import beta in --files mode (not --plan) will import the files in parallel (files can't reference each other without --plan)
  10. import provides deprecation warnings for both content-type and config-help flags

Export files created by export beta are compatible with import and import beta

Phase 2: GA the new commands, put the old under legacy. [July 10 2024]

  1. pin an issue when then change goes into RC so people with problems can quickly find legacy commands
  2. move the "old" commands to legacy and mark them hidden and deprecated with the Phase 3 date
  3. move the force: aliases to the new commands
  4. move the new commands to not be beta but have the beta alias. add deprecateAliases so people stop using the beta thing.
  5. change export with --json to warn that it will change json output (stop returning the saveRefs/resolveRefs) after Phase 3 date

Phase 3: retire the legacy commands and all their dependent code [Nov 10 2024]

  1. make export stop writing the unused saveRefs and resolveRefs properties on plan files, and stop returning them in json
  2. tighten schema to remove the object part of files, and remove saveRefs and resolveRefs
  3. check messages for unused messages
  4. remove the beta alias from import|export
  5. update the pinned issue to reflect these changes

changes

  • functions everywhere!

What issues does this PR fix or reference?

forcedotcom/cli#2663
@W-14876905@

@W-14980455@
forcedotcom/cli#248


QA notes

the query provided in the repro will get data and create files, but they aren't importable because of required fields on Events. This query will let you round trip it

/plugin-data/bin/run.js data export tree -o hub -q "SELECT Id, Name, (SELECT Id, Origin, (SELECT Id FROM Tasks), (SELECT Id, ActivityDateTime, DurationInMinutes FROM Events) FROM Cases) FROM Account" --output-dir data3 --plan

@mshanemc mshanemc changed the title test: ut for most of the functions feat: better exports Feb 2, 2024
@mshanemc mshanemc requested a review from a team as a code owner February 13, 2024 21:32
@mshanemc mshanemc changed the title feat: better exports feat: beta tree exports/import Feb 13, 2024
@iowillhoit
Copy link
Contributor

QA NOTES

See updates below

  • 🟢 Using the old command suggests using the new beta command
  • 🟢 Beta command shows beta warning
  • 🟢 Object refs and files no longer end in s.json
    • Account.json, not Accounts.json
  • 🟡 The --prefix flag works, but does not prefix the plan
  • 🟢 New beta command creates plan for greater than 2 nested records
    • Account
      • Case
        • Task
  • 🟢 Polymorphic records (WhatId) work on export
    • Query: "SELECT Id, Name, (SELECT Id, Name, StageName, CloseDate, (SELECT Id, Subject FROM Tasks) FROM Opportunities), (SELECT Id, Subject, Status, (SELECT Id, Subject FROM Tasks) FROM Cases) FROM Account"
    • Account
      • Case
        • Task ("WhatId": "@CaseRef1")
      • Opportunity
        • Task ("WhatId": "@OpportunityRef")
  • 🟢 Empty records are omitted
    • Used the same query ^ but did not created either Task in a new org
  • 🔴 Getting an error on polymorphic import:
"ERROR_HTTP_400": {
    "hasErrors": true,
    "results": [
        {
            "referenceId": "TaskRef2",
            "errors": [
                {
                    "statusCode": "MALFORMED_ID",
                    "message": "Related To ID: id value of incorrect type: @CaseRef1",
                    "fields": [
                        "WhatId"
                    ]
                }
            ]
        }
    ]
}
at SfError.wrap (/Users/ewillhoit/.nvm/versions/node/v20.10.0/lib/node_modules/@salesforce/cli/node_modules/@salesforce/core/lib/sfError.js:84:25)
    at ImportApi.importSObjectTreeFile (file:///Users/ewillhoit/.nvm/versions/node/v20.10.0/lib/node_modules/@salesforce/cli/node_modules/@salesforce/plugin-data/lib/api/data/tree/importApi.js:365:27)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async ImportApi.getPlanPromises (file:///Users/ewillhoit/.nvm/versions/node/v20.10.0/lib/node_modules/@salesforce/cli/node_modules/@salesforce/plugin-data/lib/api/data/tree/importApi.js:181:17)
    at async ImportApi.import (file:///Users/ewillhoit/.nvm/versions/node/v20.10.0/lib/node_modules/@salesforce/cli/node_modules/@salesforce/plugin-data/lib/api/data/tree/importApi.js:59:17)
    at async Import.run (file:///Users/ewillhoit/.nvm/versions/node/v20.10.0/lib/node_modules/@salesforce/cli/node_modules/@salesforce/plugin-data/lib/commands/data/import/tree.js:65:31)
    at async Import._run (/Users/ewillhoit/.nvm/versions/node/v20.10.0/lib/node_modules/@salesforce/cli/node_modules/@oclif/core/lib/command.js:304:22)
    at async Config.runCommand (/Users/ewillhoit/.nvm/versions/node/v20.10.0/lib/node_modules/@salesforce/cli/node_modules/@oclif/core/lib/config/config.js:417:25)
    at async run (/Users/ewillhoit/.nvm/versions/node/v20.10.0/lib/node_modules/@salesforce/cli/node_modules/@oclif/core/lib/main.js:85:16)",
  • 🟠 Oddly, this polymorphic import works if I only create a single Task (A Task tied to a Case but not one for an Opportunity)

QA POST-MERGE UPDATE (export and import)

  • 🟢 Polymorpic export/imports are now working

  • 🟢 The --prefix flag works on resource types and plans. Imports as expected

    • 🟡 If you use a / (--prefix "foo/bar") in the prefix it will failed due to a directory not existing. Since we have the --output-dir flag (-d), we should probably prevent slashes in the prefix.
  • 🟢 Woo! Can import more than 200 records. Imported 700+ on a single go.

  • 🟠 Something odd happens as you continue to increase records. It seems likely that it is an issue with export... With the following setup/query:

    • Create the following records:
      • Account
        • Case
          • Task
        • Opportunity
          • Task
    • Run this query over and over: "SELECT Id, Name, (SELECT Id, Name, StageName, CloseDate, (SELECT Id, Subject FROM Tasks) FROM Opportunities), (SELECT Id, Subject, Status, (SELECT Id, Subject FROM Tasks) FROM Cases) FROM Account"
    • The record count (THE_COMMAND --json | jq '.record | length') goes:
      • 6, 12, 24, 48, 96, 192, 384, 768, 1536, 7, 7, 7, ...
  • 🟢 Export/import works without a plan

  • 🟢 Export/import without a plan works with more than 2 levels

  • 🔴 Import without a plan fails at more than 200 records.

    • The request can’t contain more than 200 records total.

@iowillhoit
Copy link
Contributor

I still have a few things to test. If it is helpful, there is the bash script I am using:

  • Run from a Dreamhouse dir
  • Set the DATA_BETA_DIR to your local plugin-data branch
  • Make sure plugin-data is built
#!/bin/bash

# Create an Account
# - Create a Case related to Account
#   - Create a Task related to Case
# - Create an Opportunity related to Account
#   - Create a Task related to Opportunity

QUERIES=(
  # "SELECT Id, Name, (SELECT Name, Address__c FROM Properties__r), (SELECT FirstName) FROM Broker__c"
  # "SELECT Id, Name, (SELECT Id, CaseNumber, (SELECT Id, Subject FROM Tasks) FROM Cases) FROM Account"
  # "SELECT Id, Name, (SELECT Id, Name, StageName, CloseDate, (SELECT Id, Subject FROM Tasks) FROM Opportunities), (SELECT Id, CaseNumber, (SELECT Id, Subject FROM Tasks) FROM Cases) FROM Account"
  "SELECT Id, Name, (SELECT Id, Name, StageName, CloseDate, (SELECT Id, Subject FROM Tasks) FROM Opportunities), (SELECT Id, Subject, Status, (SELECT Id, Subject FROM Tasks) FROM Cases) FROM Account"
  # "SELECT Id, Name, (SELECT Id, Origin, (SELECT Id FROM Tasks), (SELECT Id, ActivityDateTime, DurationInMinutes FROM Events) FROM Cases) FROM Account"
  # "SELECT Id, (SELECT Id FROM Tasks), (SELECT Id FROM Events) FROM Case"
  # "SELECT Id, Name, Phone, Website, NumberOfEmployees, Industry, (SELECT Lastname, Title, Email FROM Contacts) FROM Account  WHERE Name LIKE 'SampleAccount%'"
  # "SELECT Id, Name, (SELECT Id, Origin, (SELECT Id FROM Tasks), (SELECT Id, ActivityDateTime, DurationInMinutes FROM Events) FROM Cases) FROM Account"
)

PACKAGE_NAME=$(cat package.json | jq -r '.name')

# if name is not dreamhouse-lwc, then echo warning and exit
if [ "$PACKAGE_NAME" != "dreamhouse-lwc" ]; then
  echo "This script should be ran from the root of a Dreamhouse project"
  exit 0
fi

DATA_BETA_DIR="/Users/ewillhoit/dev/plugin-data"
DREAMOUSE_DIR=$(pwd)

TIME=$(date +%s)

INDEX=0
for QUERY in "${QUERIES[@]}"
do
  TEST_DIR="$DREAMOUSE_DIR/data-export/$TIME/$INDEX"
  INDEX=$((INDEX+1))
  mkdir -p "$TEST_DIR"
  echo "$QUERY" > "$TEST_DIR/query.txt"

  echo "Running query: $QUERY"

  # echo "SF command"
  # sf data tree export -q "$QUERY" -d "$TEST_DIR" --plan --json 2>&1 | tee "$TEST_DIR/export-output.json"
  # git -C "$TEST_DIR" init
  # git -C "$TEST_DIR" add -A
  # git -C "$TEST_DIR" commit -m "sf command" --no-gpg-sign

  echo "Beta export command"
  # With --plan
  $DATA_BETA_DIR/bin/run.js data tree beta export -q "$QUERY" -d "$TEST_DIR" --plan --json 2>&1 | tee "$TEST_DIR/export-output.json"
  # Without --plan
  # $DATA_BETA_DIR/bin/run.js data tree beta export -q "$QUERY" -d "$TEST_DIR" --json 2>&1 | tee "$TEST_DIR/export-output.json"

  # git -C "$TEST_DIR" diff


  echo "Beta export command"
  # With --plan
  JSON=$($DATA_BETA_DIR/bin/run.js data import beta tree --plan "$(ls $TEST_DIR/*-plan.json)" --json)
  # Without --plan
  # JSON=$($DATA_BETA_DIR/bin/run.js data import beta tree --files "$(ls $TEST_DIR/Account*.json)" --json)
  echo $JSON | jq '.'
  echo $JSON > "$TEST_DIR/import-output.json"
  echo -n "Number or records: "
  echo $JSON | jq '.result | length'
done

@mshanemc
Copy link
Contributor Author

mshanemc commented Feb 14, 2024

Import without a plan fails at more than 200 records

That's the API's limit. The export command produces a warning about this when not using --plan and more than 200 records. It tells you to use --plan.

@iowillhoit
Copy link
Contributor

Ah, I see that now. That warning message was lost in all the json output

@iowillhoit
Copy link
Contributor

QA UPDATE

  • 🟢 The --prefix flag validation works
  • 🟢 Self referencing objects work with --plan
    • Created 3 levels of accounts, each referencing the "parent"
    • Modified my query to include the ParentId
    • Exported the plan
    • Moved one of the "child" Accounts to the top of the Accounts.json
    • Imported the plan
    • Validated that the account that I moved (which would not yet have its parent inserted) was skipped and imported at the end.
  • 🟡 (expected) Self referencing objects don't work with --files
    • Might be nice to try to detect this and suggest using --plan
  • 🟢 Import without a plan fails at more than 200 records* This is an API limit, it displays a warning to use --plan instead

@@ -30,7 +30,7 @@ Directory in which to generate the JSON files; default is current directory.

- Export records retrieved with the specified SOQL query into a single JSON file in the current directory; the command uses your default org:

<%= config.bin %> <%= command.id %> --query "SELECT Id, Name, (SELECT Name, Address__c FROM Properties__r) FROM Broker__c"
<%= config.bin %> <%= command.id %> --query "SELECT Id, Name, (SELECT Name, Address\_\_c FROM Properties\_\_r) FROM Broker\_\_c"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do a quick scan for incorrect formatting like these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

huh?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a formatter issue, it is escaping the underscores


# flags.files.summary

Comma-separated and in-order JSON files that contain the records, in sObject tree format, that you want to insert.
Copy link
Contributor

Choose a reason for hiding this comment

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

Haven't we been suggesting passing the flag multiple times with arrayWithDeprecation instead of separated values?

@iowillhoit
Copy link
Contributor

QA UPDATE

  • 🟢 Deprecation warnings on the --config-help and --content-type flags
  • 🟢 The --config-help and --content-type flags are gone from the new commands
  • 🟢 Can import multiple (non-referencing) files to the --files flag

@iowillhoit
Copy link
Contributor

QA UPDATE

  • 🟢 Nice! A warning is now shown if you try to import a non-plan (--files) file that contains references.
    • "The file Account.json includes references (ex: '@AccountRef1'). Those are only supported with --plan, not --files."

@iowillhoit
Copy link
Contributor

I am unable to reproduce large record error I was seeing a few days ago. I am now getting past 1536 records (to 3072) and on the next run a get a Maximum call stack exceeded which we expected at some point

@mshanemc mshanemc merged commit 9f6c02b into main Feb 19, 2024
10 checks passed
@mshanemc mshanemc deleted the sm/soql-5-levels branch February 19, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants