fix: preProcessedFileSubstitution replacing to ""

[beardhatcode]

• Resolves: the issue in collabora-online: 24.04.6-1 -> 24.04.13-2 NixOS/nixpkgs#393618 (comment)
• Target version: master

Summary

preProcessedFileSubstitution did not work correctly if an empty map was given. In this case variables["ACCESS_TOKEN"] would be read as "" as that key was not present in the map.

Poco::replaceInPlace would in turn replace the %ACCESS_TOKEN% with "", while it should just keep the not specified parts as is.

I chose to add a function rather than ifs in the body to prevent future mistakes from happening.

Before this change I got an error when running the unittests from nix (see NixOS/nixpkgs#393618 (comment)
):

< orig (the one that is made with Poco::replaceInPlace)
> recon (the one that is made by .substitute(...))
10c10
< collabora-online>   <input type="hidden" id="init-buy-product-url" value="" />
---
> collabora-online>   <input type="hidden" id="init-buy-product-url" value="%BUYPRODUCT_URL%" />
12c12
< collabora-online>   <input type="hidden" id="init-css-vars" value="" />
---
> collabora-online>   <input type="hidden" id="init-css-vars" value="<!--%CSS_VARIABLES%-->" />
16c16
< collabora-online> <input type="hidden" id="init-branding-name" value="" />
---
> collabora-online> <input type="hidden" id="init-branding-name" value="%BRANDING_THEME%" />
145,148c145,148
< collabora-online>       data-access-token = ""
< collabora-online>       data-access-token-ttl = ""
< collabora-online>       data-access-header = ""
< collabora-online>       data-post-message-origin-ext = ""
---
> collabora-online>       data-access-token = "%ACCESS_TOKEN%"
> collabora-online>       data-access-token-ttl = "%ACCESS_TOKEN_TTL%"
> collabora-online>       data-access-header = "%ACCESS_HEADER%"
> collabora-online>       data-post-message-origin-ext = "%POSTMESSAGE_ORIGIN%"
167,168c167,168
< collabora-online>       data-ui-defaults = ""
< collabora-online>       data-check-file-info-override = ""
---
> collabora-online>       data-ui-defaults = "%UI_DEFAULTS%"
> collabora-online>       data-check-file-info-override = "%CHECK_FILE_INFO_OVERRIDE%"
181c181
< collabora-online>  <!-- logo onclick handler -->
---
> collabora-online> <!--%BRANDING_JS%--> <!-- logo onclick handler -->
182a183,184
> collabora-online> - orig != recon
> collabora-online> Failures !!!

Checklist

• I have run make prettier-write and formatted the code.
• All commits have Change-Id
• I have run tests with make check -> ran through nix
• I have issued make run and manually verified that everything looks okay -> even ran it through nextcloud
• Documentation (manuals or wiki) has been updated or is not required

[welcome]

Thanks for opening this pull request!

Things that will help get your PR across the finish line:

• Check out our contributing guidelines.
• Connect with us through one of our communication channels.
• Click on the details link next to the failing CI checks, if any, to see an explanation on how to fix it.
• Feel free to ping @pedropintosilva or the mentor of the easyHack you are solving if you need help.

[beardhatcode]

Hmmm, it seems that the CI here does exactly the opposite...

[meven]

Seems very sensible to me.

Placeholder key have no value in template output.

[beardhatcode]

Updated the top post

I figured it out, what the test did was

for(file in files){
   orig = file.subsitute(variables)
   recon = Poco::replaceInPlace(....,vaiables["SOME_KEY"])
   test_same(orig,recon)
}

if you unroll this loop you would get:

    // iter 0 all fine
   orig = file1.subsitute(vars);
   recon = Poco::replaceInPlace(read(file1),vars["SOME_KEY"]); // <- this sets vars["SOME_KEY"]=""
   test_same(orig,recon);

    // iter 1 -> problem,s
   orig = file2.subsitute(vars); // <- this vars is different, as it is no longer empty but {"SOME_KEY"→""}
   recon = Poco::replaceInPlace(read(file2),vars["SOME_KEY"]);
   test_same(orig,recon);

Because the order of the iterations (files reported by the file system) on my machine was different than those in CI

• CI the first file that was used was wasm.html (which does not contain and %VAR%), so in all future iterations the modified vars setting everything to the empty string was used incuding for cool.html
• on my machine the first file was cool.html (which does contain %VAR%), so there it did the substitution correctly (leaving all the %VAR%) since vars was empty, but that did not correspond to the Poco::replaceInPlace

My files may have been in a different order because I was using nix and/or because my poco version was different.

[beardhatcode]

force-pushed a typo fix and adding const to the variables arguments

[beardhatcode]

Rebased

[beardhatcode]

@Ashod , just quickly checking my PRs. If there's anything else I can
provide or if you'd like to discuss further in order to get this merged, please let me know!

[beardhatcode]

Rebased

[britter]

Any change to get this merged?

[beardhatcode]

Hi @Ashod and maintainers — I've rebased the branch onto master to keep it up to date.

I ran make check (via nix) and did a manual verification, (including a run with Nextcloud) — everything looked good locally. It would be nice if this could get merged.