[JavaToolInstallerV0 ] Changed logic of extraction, fix inactivated Clean Destination Directory issue

[AlexandrKravchuk]

Task name: JavaToolInstaller

Description:

1. Naming of extraction directory (each archive extracts into a directory named as 'JAVA_HOME_<archive_name>_<archive_extension>'
2. No extraction if it's in cache
3. Refactoring logic, dividing code into methods

Documentation changes required: (Y/N) Yes, #88

Added unit tests: (Y/N) No, will be done in #81

Attached related issue: (Y/N) https://github.com/microsoft/build-task-team/issues/10

Checklist:

• Task version was bumped - please check instruction how to do it
• Checked that applied changes work as expected

Test Build Result: Build

[alexander-smolyakov]

Please take a look at the comments, other changes are LGTM, thanks!

[alexander-smolyakov]

I guess a better way is to extract the file extension firstly and then check if it supported.

Also maybe better move array with the supported file extension as global constant?

const fileEnding = file.substring(file.lastIndexOf('.'));

const supportedFileExtension: Array<string> = ['.tar', '.tar.gz', '.zip', '.7z'];

if (!supportedFileEndings.some((extension) => extension === fileEnding)) {
    throw new Error(taskLib.loc('UnsupportedFileExtension'));
}

return fileEnding;

[anatolybolshakov]

@alexander-smolyakov the problem we saw here is that there's a validation for file ending - it differs from file extension... For example, for filename.tar.gz is considered to have file ending '.tar.gz', but '.gz' as extension. There's also could be the case when there's a dot in filename (like 'file.name.tar.gz'), so we can't simply extract substring after first dot...

[alexander-smolyakov]

@anatolybolshakov Oh, I see now. I think it will be useful to add information about these cases in the function description.

[AlexandrKravchuk]

That function was just moved here from javatoolinstaller.ts to avoid the circular dependency

[anatolybolshakov]

Yes, I think it also makes sense to describe difference between file ending/file extension here - I asked @ekaterina-tatanova to update comments since she reworked this method also and added some js doc for it in another on-code-review PR - #13203 (comment)

[alexander-smolyakov]

nit: Add EOL

[anatolybolshakov]

LGTM, thanks!

[AlexandrKravchuk]

After Testing by Canary Sceneries from Canary Build repo, I found some issues with Azure Blob JDK Installation.
These issues were fixed and tested again: here's the results:

1. Build for Azure Blob
2. Build for local storage

[damccorm]

Will this change of behavior break existing customers? Do we need to hide it behind an input flag?

[AlexandrKravchuk]

Good question. In order to keep the task working for existing customers, it's better to get rid of this line at all.

[AlexandrKravchuk]

I mean if someone wants to use archives from cache they can just inactivate Clean Directory checkbox.