predeploy hook result doesn't include all files when there is API name collision

[filiprafalowicz]

Summary

Result type in predeploy hook for force:source:push command has following structure:

type PreDeployResult = {
  [aggregateName: string]: {
    mdapiFilePath: string;
    workspaceElements: {
      fullName: string;
      metadataName: string;
      sourcePath: string;
      state: string;
      deleteSupported: boolean;
    }[];
  };
}

(per https://developer.salesforce.com/docs/atlas.en-us.sfdx_cli_plugins.meta/sfdx_cli_plugins/cli_plugins_customize_hooks_list.htm)

Because aggregateName is essentially just an API name this is causing issues when two different metadata types share the same name. As an example, if you change Account.object and Account.workflow and run push command, this is how result will look like:

{
    "Account":
    {
        "workspaceElements":
        [
            {
                "fullName": "Account",
                "metadataName": "CustomObject",
                "sourcePath": "project/force-app/main/default/objects/Account/Account.object-meta.xml",
                "state": "c",
                "deleteSupported": false
            }
        ],
        "mdapiFilePath": "/var/folders/8n/hc4v11tj08qdc7dd3pcqfxq80000gp/T/sdx_mdpkg_1646314592101/objects/Account.object"
    }
}

There is no information about Account.workflow file that was also pushed at the same time. Additionally, if you do the same with force:source:deploy command, it will work as expected, as result does not have an object with API Name as a key, but it stores array of metadata components that are being deployed. I think you could either change aggregateName in the result to include metadata type like "CustomObject:Account" or align with the structure of the result from the deploy command.

Also, I saw similar issue, but for the different hook already reported: #743. Not sure if that was fixed or not.

Without that fixed, predeploy hook can be very often useless for larger organizations with a lot of metadata where API Name collision can occur a lot of times.

Steps To Reproduce:

1. Pull Account.object and Account.workflow files from the org
2. Modify both files by adding empty line in the XML
3. Call sfdx force:source:push

Expected result

Both Account.object and Account.workflow should be included in the predeploy hook result.

Actual result

Only Account.object is included in the predeploy hook result.

System Information

{
        "cliVersion": "sfdx-cli/7.139.0",
        "architecture": "darwin-x64",
        "nodeVersion": "node-v16.14.0",
        "pluginVersions": [
                "@oclif/plugin-autocomplete 0.3.0 (core)",
                "@oclif/plugin-commands 1.3.0 (core)",
                "@oclif/plugin-help 3.3.1 (core)",
                "@oclif/plugin-not-found 1.2.6 (core)",
                "@oclif/plugin-plugins 1.10.11 (core)",
                "@oclif/plugin-update 1.5.0 (core)",
                "@oclif/plugin-warn-if-update-available 1.7.3 (core)",
                "@oclif/plugin-which 1.0.4 (core)",
                "@salesforce/lwc-dev-server 2.11.0",
                "@salesforce/sfdx-plugin-lwc-test 0.1.7 (core)",
                "alias 1.2.1 (core)",
                "apex 0.9.0 (core)",
                "auth 1.8.1 (core)",
                "community 1.1.4 (core)",
                "config 1.3.19 (core)",
                "custom-metadata 1.0.12 (core)",
                "data 0.6.9 (core)",
                "generator 1.2.2 (core)",
                "info 1.2.1 (core)",
                "limits 1.3.0 (core)",
                "org 1.11.1 (core)",
                "plugin-metadata-hook-demo 1.0.0 (link) path",
                "salesforce-alm 53.10.2 (core)",
                "schema 1.1.0 (core)",
                "sfdx-cli 7.139.0 (core)",
                "source 1.8.12 (core)",
                "telemetry 1.4.0 (core)",
                "templates 53.6.0 (core)",
                "trust 1.1.0 (core)",
                "user 1.7.1 (core)"
        ],
        "osVersion": "Darwin 21.3.0"
}

[github-actions]

Thank you for filing this issue. We appreciate your feedback and will review the issue as soon as possible. Remember, however, that GitHub isn't a mechanism for receiving support under any agreement or SLA. If you require immediate assistance, contact Salesforce Customer Support.

[shetzel]

@filiprafalowicz - thanks for posting. The docs and the samples in that repo are currently wrong. I have a PR to fix the samples: salesforcecli/plugin-metadata-hook-demo#147
We will be fixing the documentation around it very soon as well. As we move commands from the salesforce-alm plugin to the plugin-source plugin the hooks were changed so we were in an intermediary state. However, with today's release of the CLI (yet to happen as I'm writing this) the hooks should all be in the new format specified in that PR. For the predeploy hook, the commands that fire it are: force:source:deploy, force:source:push, and force:source:delete. If this satisfies your issue please close at your convenience.

[filiprafalowicz]

@shetzel I just tested the changes in new sfdx version and they look promising, but there are couple of things that I have noticed. They may not be fully related to the initial issue I reported here as it seems to be fixed, but they are related to the predeploy hook:

1. When I run force:source:push to push Account.object-meta.xml and Account.workflow-meta.xml changes I can see both of them in the hook options result, but I can also see a lot of unrelated stuff like field translations for fields that exist on other objects. Example below:

{
    "markedForDelete": false,
    "name": "My_Custom_Field__c",
    "type":
    {
        "id": "customfieldtranslation",
        "name": "CustomFieldTranslation",
        "directoryName": "fields",
        "suffix": "fieldTranslation",
        "isAddressable": false,
        "unaddressableWithoutParent": true,
        "uniqueIdElement": "name"
    },
    "xml": "path/force-app/main/default/objectTranslations/My_CMT__mdt-ja/My_Custom_Field__c.fieldTranslation-meta.xml",
    "treeContainer":
    {},
    "forceIgnore": { ... }
}

When running force:source:deploy for the same two files I see only them in the options result which is expected.

2. In the old implementation there was mdapiFilePath property included for each file. Can this be added back? This is really useful when you want to do replace behind the scenes that you don't want to be reflected in local files. As an example you may want to keep placeholders in Named Credentials in the repository, but you want to replace them to the actual values before they are pushed / deployed.

3. Lastly small thing, but useful when you are debugging - when I call JSON.stringify(options.result) my JSON string is really large because forceIgnore node is added to every element of the returned array. I know this property and couple others (like treeContainer) are private, but JSON.stringify method is printing them anyway. I believe you could add toJSON method to the SourceComponent class to exclude them from the output JSON.

[mshanemc]

1. That seems...wrong. It's probably some source tracking error where those are being pushed when they don't need to be. Are they also showing up in the push results output, or just the hook? Can you open that up as a separate bug?
2. mdapiFilePath can't exist anymore. It's complicated enough that I wrote a blog for it: https://developer.salesforce.com/blogs/2022/02/we-broke-deploy-retrieve-hooks
3. That's a really great idea! that code is here if you want to submit a PR instead of waiting on us to prioritize it (will fix it in new but not old commands)

[filiprafalowicz]

Hey @mshanemc.

For the first one, I will try to reproduce it with a smaller codebase and open a separate bug if the issue persists. For the second one, I read your post and while it makes sense for the deploy command, I can't see how the workarounds listed there can work for the push.

While creating custom push command would give me more flexibility in doing what I need, it will also give me a lot of disadvantages as I couldn't leverage any of the new features you add to original push command and I would need to maintain my custom command.

I feel like with this change of removing mdapiFilePath, predeploy hook for push command is useless as it will always make local changes to the files I have in my project and they won't be rolled back. As an example, I want to replace secrets for Named Credentials and have XML in my project with placeholders like this:

<?xml version="1.0" encoding="UTF-8">
<NamedCredential xmlns="http://soap.sforce.com/2006/04/metadata">
    <allowMergeFieldsInBody>false</allowMergeFieldsInBody>
    <allowMergeFieldsInHeader>false</allowMergeFieldsInHeader>
    <endpoint>{endpoint}</endpoint>
    <generateAuthorizationHeader>false</generateAuthorizationHeader>
    <label>My API</label>
    <principalType>NamedUser</principalType>
    <protocol>password</protocol>
    <username>{username}</username>
    <password>{password}</password>
</NamedCredential>

So now every time this file is included in the push, I would like to make sure my predeploy hook can replace these placeholders with actual data secrets, but I don't want to have the files changed in my local project. These secrets can be stored on some secure data storage that requires MFA to retrieve them and I don't want to keep them as plain text on my machine.

I have similar scenario with custom labels - let's say I store all of the strings outside SF in external system where Content Team can manage and translate them. My Custom Labels file would store labels without values. Now every time Custom Labels file is included in the push, I want to be able to call the API to pull the labels from that system and make sure they are pushed to my scratch org correctly. If I do that logic in the current predeploy hook, the changes that were pulled from the service will remain in my CustomLabels file and if I want to add one more label, I will need to remember to revert local changes to this file first and then add a new label. This is the experience we would like to avoid.

In general I was hoping predeploy hook, or maybe some new hook, could be fired to make changes to the files on the fly, before they are streamed to that in-memory zip file. I don't know how much it can slow down the process, but let's keep in mind we won't be modifying every file that is included in the push. If you have 10 integrations and that's the only thing you want to dynamically inject into XML files then we need to modify only 10 files like that. Maybe if executing the hook for every file would be to slow, we can at least build some listeners and execute it only for certain files that you are subscribed to. Or maybe, worst case, we can still support old way of zipping the files based on the parameter to the push command.

[github-actions]

This issue has not received a response in 60 days. It will auto-close in 7 days unless a response is posted.

[filiprafalowicz]

@mshanemc following up on this one - I am unable to check how predeploy hook looks right now because of ongoing plugins issue (#1664) but I was wondering if you had a chance to review my last comment and if there is any way to accomplish the same with currently available options. In general what I described would work with the old way of how predeploy hook was designed and with API name collision issue fixed. Right now I am not sure what would be the use case for predeploy hook if it modifies your files locally.

[mshanemc]

We've got a design for string replacements during deploys that would also work for across CLIs and VSCode. But I don't know when we'll get to it.

[cromwellryan]

Relevant discussion

[mshanemc]

the final result on this one is to use the string replacements feature we shipped recently. We're not going to fix bugs like this one in the deprecated force:source: commands.