[BUG] - malformed Composite External ID field causes Extraneous input error

[readeral]

Describe the bug

Error during execution of the command: line 1:932 extraneous input 'FROM' expecting {GROUP, ABOVE...}.
Execution of the command sfdmu:run has been completed. Exit code 4 (COMMAND_EXECUTION_ERROR).

When a Composite External ID field has a trailing semicolon (as often auto completed by IDE), sfdmu:run fails with the above (truncated) error. The issue is a potential misdirection as the problematic object's query utilises FROM, making it hard to realise the extraneous input is in the externalId field.

To Reproduce
Provide a malformed externalId with a trailing semicolon:

{
   "label": "contact",
   "operation": "Upsert",
   "externalId": "Name;Email;",
   "query": "SELECT Id, AccountId, LastName, FirstName, Email FROM Contact",
}

Expected behavior
Error reports a malformed externalID OR sfdmu:run strips trailing semi-colons from externalID fields

export.json
export.json

Log file
2024-09-11__10_51_02.log

[readeral]

(Don't mind my label key, that's just my solution to not having comments in JSON so I don't have to scroll to the end of the query to see what the object is - SFDMU happily ignores the unrecognised key)

[hknokh]

Hello, @readeral

Thank you for reporting a bug.
I will take a look at it as soon as possible and let you know of any updates.

Cheers

[github-actions]

This case has been marked as 'to-be-closed', since it has no activity for the 3 days.
It will be automatically closed in another 3 days of inactivity.

[readeral]

Bump

…

On Sat, 14 Sep 2024 at 11:58 AM, github-actions[bot] < ***@***.***> wrote: This case has been marked as 'to-be-closed', since it has no activity for the 3 days. It will be automatically closed in another 3 days of inactivity. — Reply to this email directly, view it on GitHub <#879 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADZK3CBWFKCTXHG6ML5BB6DZWOJ27AVCNFSM6AAAAABN73SW42VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJQG43TSNRRGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[readeral]

@hknokh we can move testing discussion here

[hknokh2]

Np. But not sure that I can add something to what I already said.

[readeral]

#881 (comment)

There is no specific standard for this project beyond the common standards that apply to all development. If you're not sure, it's best not to make a contribution.

@hknokh This isn't particularly satisfying. Do you want a contributor to test every function? That would be a common standard for many projects, because they have unit tests for every function. Many other open-source projects have no testing, you could call that a 'common standard' given how common it actually is.

Maybe a first step would be to commit your regression tests to the repo. We can match your testing methodology before committing. That would boost some confidence.

The contribution I want to make is to strip the trailing semicolon from the externalId field. It would be a minor/cosmetic contribution, just a minor help with usability, not introducing new functionality, but writing a suite of tests for all 105 references to externalId is a remarkable amount of work to do from scratch.

[hknokh2]

This project does not have unit tests because they are not necessary and won't effectively catch errors. Testing is done during real data migration jobs, which I run in debug mode, checking every line of code that could potentially cause issues. I have a couple of test jobs that cover common regression cases, and I am confident in this process.

I don't expect contributors to test every function. Instead, they should test the job that verifies their changes.

Regarding the semicolon, please create a PR and ensure it generates the correct export.json with the proper externalId, showing that you've fixed the error. However, please be aware that your PR might not be accepted if the change deviates from the overall code style of this application or isn't an efficient solution. This is what the code review process is for.

I'm also not certain that the semicolon issue is a minor change. You should properly address the autocomplete functionality and/or the part of code which creates the export.json file. But let's see your solution. Additionally, please note that I don't create a release for every change, so initially, your change will be available as source code in the main branch.

In general, a contributor should have a creative approach to their contributions and doesn't need to be overly rigid.

[readeral]

This project does not have unit tests because they are not needed and won't catch any errors. Testing should only be done in a real data migration job, which I run in debug mode and test every line of code which can cause issues. I have a couple of test jobs that cover common cases for regressions. And also I know what I do.

I don't expect contributors to test every function. Instead, they should test the job that can verify their changes.

Regarding the semicolon, please create a PR and ensure it generates the correct export.json with the proper externalId, indicating that you've fixed the error. However, please don't assume your PR will be accepted if the change differs from the overall code style of this application or isn't an efficient solution. This is the purpose of the code review process. And I am not sure that the semicolon is so small change. You should properly fix the autocomplete functionality. But let's see your solution. Also, please keep in mind that I don't create release in every change, so at the beginning your change will be available as a source code in the main branch.

I don't see it as a small change either, as I say, plenty of places for it to go wrong, but it fits within your definition of allowed contributions.

The autocompletion is a perfectly reasonable thing for my IDE to infer is needed, it would be wild for me to make a rule that is specific to export.json. What would be better is for your code to parse complex externalIds to ensure they are of an appropriate format to be consumed, and if not, to throw an error to identify the malformed externalId instead of letting the exception bubble up to the SF CLI error.

Contributing that much code would be problematic, so maybe I'll just leave that up to you. I don't want to risk deviating from your preferred code style.

[hknokh2]

Of course, I will fix it. But now I have no enough time. I will update you once it will done.

This is IDE issue and it should not be fixed in the sfdmu plugin during the parsing

[hknokh2]

@readeral
I updated the Code Contribution Policy and Support Policy to better highlight what we've previously discussed.

[hknokh]

@readeral,
I've resolved this issue. However, the fix will be included in the next app release, along with other improvements. Please keep an eye on the repository updates.
I'll close this issue for now, marking it as "waiting for release."
Regards.