Allow retrieval from languageHashes using environment in order to side-load i18n assets without rebuilding the application

[mahnkong]

References

• Fixes Allow retrieval from languageHashes using environment in order to side-load i18n assets without rebuilding the application #2702

Description

The code inside the angular application was changed to also consider the environment for obtaining the languageHashes instead of only considering the process.env.languageHash variable prepared by webpack.

The "languageHashes" environment key is populated by the buildAppConfig method of config.server.ts, but only if the angular server process has the environment variable DSPACE_DYNAMIC_LANGUAGE_HASHES set and it's value is set to be true

Instructions for Reviewers

Create a i18n asset folders to be mounted to the appropiate location inside the dspace-angular container:

example for server:

server/
server/en.4ed2840c555bc6830fceaf517b2bfbda.json
server/en.json5
server/de.b342992c6e376bfd7ede49aec77a2732.json
server/de.json5

example for browser/

browser/en.4ed2840c555bc6830fceaf517b2bfbda.json
browser/en.4ed2840c555bc6830fceaf517b2bfbda.json.br
browser/en.4ed2840c555bc6830fceaf517b2bfbda.json.gz
browser/en.json5
browser/de.b342992c6e376bfd7ede49aec77a2732.json
browser/de.b342992c6e376bfd7ede49aec77a2732.json.gz
browser/de.b342992c6e376bfd7ede49aec77a2732.json.br
browser/de.json5

Create volume mount:

• server -> /app/dist/server/assets/i18n
• browser -> /app/dist/browser/assets/i18n

Configure server application's environment:

DSPACE_DYNAMIC_LANGUAGE_HASHES=true

When loading the application, the side-loaded language files are fetched by the client.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

• My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
• My PR passes ESLint validation using yarn lint
• My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
• If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
• If my PR fixes an issue ticket, I've linked them together.

[github-actions]

Hi @mahnkong,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

[artlowel]

This seems to be a custom use case for your institution. I'm not convinced this should be part of the community codebase.

Hashes in the environment file that need to be updated by hand seem to be something that can cause unnecessary confusion: if users think they need to set them, and forget to update them after a rebuild it will break

And requiring a rebuild to have hashes automatically generated as they are now, doesn't seem like a big issue for most institutions.

What do you think @tdonohue?

[mahnkong]

This seems to be a custom use case for your institution. I'm not convinced this should be part of the community codebase.

Hashes in the environment file that need to be updated by hand seem to be something that can cause unnecessary confusion: if users think they need to set them, and forget to update them after a rebuild it will break

It depends, I think. Updating them by hand of course may lead to confusion. That's why we automate all these steps and hat do modify the dspace-angular application in order to support the use case. Having the opportunity to maintain the language files outside of the dspace-angular project and beeing able to generate the language files using userfriendly tooling nevertheless may be a use case which may be a valid one for other institutions, too.

[tdonohue]

@mahnkong : I agree with @artlowel at this point that I don't feel this change is the correct direction for DSpace at this time.

You are correct that it would be nice if sites could update language files without rebuilding the application. This is something many sites would likely agree with. However, the approach taken in this PR seems like it may not be the "best practice" to achieving this goal.

This implementation is simple, but could involve a lot of unnecessary confusion. Any sites that use it will encounter errors in the application if they forget to update the md5 hashes in their configuration after a simple rebuild. This makes this feature less useful because it can result in difficult to explain/understand errors when someone forgets to change a configuration.

Overall, there are pros/cons to both approaches:

• DSpace's current approach is to make it less likely that sites will make simple mistakes & ensure the build is always "stable". However, it means that any change to json5 files requires a rebuild (not ideal).
• Your approach in this PR is to ensure changes to json5 files do NOT require a rebuild. However, it means that sites may end up with a less stable "build" if they rebuild without update the hashes in configuration (not ideal).
  • NOTE: If there was a way to avoid having to manually update md5 values in configuration after every rebuild then that would be the ideal scenario.

Overall, I feel that I have a slight preference for DSpace's existing approach as it's more "stable" and less likely to result in human error. It's possible this could change over time if we find other sites need your solution or have found ways to better automate it to avoid human error.

Moving this off the 8.0 board for now. Additional discussion is still welcome & I'll keep this PR open for now. Waiting on feedback from others that this is something they need.

[mahnkong]

Thanks for the feedback @artlowel and @tdonohue - I will provide a code change to this PR, which will dynamically set the language hashes into the environment. This feature will be configurable to be active or not by providing an environment variable to the angular process.

[mahnkong]

I made a change to the buildAppConfig method of config.server.ts. Now the "languageHashes" environment key is populated during server startup, but only if the angular server process has the environment variable DSPACE_DYNAMIC_LANGUAGE_HASHES set and it's value is set to be true.

[artlowel]

@mahnkong thanks for the effort, but it doesn't seem to work for me

If I set the DYNAMIC_LANGUAGE_HASHES env var to true, start the server and then change the language files. I get errors such as

ERROR Error: Uncaught (in promise): Error: ENOENT: no such file or directory, open 'dist/server/assets/i18n/en.1bdafd1af91adc3f1d61bda29057afa9.json'

Where 1bdafd1af91adc3f1d61bda29057afa9 refers to the previous hash

Even restarting the server doesn't seem to fix that.

But ultimately, even you got this change to work, I'm not convinced it belongs in the dspace code base. We can't expect people to manually determine the hash, generate br and gz files, put them in both the browser and server folders etc.

All of that is something that would need to be scripted. As a matter of fact It is already scripted, by webpack. That's why it requires a rebuild. I'm not convinced it's worth maintaining an alternative to that.

[mahnkong]

Hi @artlowel, please see comments below:

@mahnkong thanks for the effort, but it doesn't seem to work for me

If I set the DYNAMIC_LANGUAGE_HASHES env var to true, start the server and then change the language files. I get errors such as

The env var should be DSPACE_DYNAMIC_LANGUAGE_HASHES.

ERROR Error: Uncaught (in promise): Error: ENOENT: no such file or directory, open 'dist/server/assets/i18n/en.1bdafd1af91adc3f1d61bda29057afa9.json'

Where 1bdafd1af91adc3f1d61bda29057afa9 refers to the previous hash

Even restarting the server doesn't seem to fix that.

But ultimately, even you got this change to work, I'm not convinced it belongs in the dspace code base. We can't expect people to manually determine the hash, generate br and gz files, put them in both the browser and server folders etc.

All of that is something that would need to be scripted. As a matter of fact It is already scripted, by webpack. That's why it requires a rebuild. I'm not convinced it's worth maintaining an alternative to that.

Exactly this generation of hash, br and gz files is scripted at our end, based on the contents of a CSV file maintained by non - IT personal. And this is the key benefit we have using this approach. But if there is no benefit for others I'm fine with that.

[github-actions]

Hi @mahnkong,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

[github-actions]

Hi @mahnkong,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

[mahnkong]

Hi @artlowel , is there anything you expect me to change? (the PR still has changes requested, but we could solve the issue you had in the conversation)

[artlowel]

No. Code wise it's fine. I'll bring it up in today's developer meeting to see if we want to include it in the codebase

[tdonohue]

@mahnkong : We discussed this PR in our Developers Meeting today. Overall, developers agree with the general idea that it'd be nice to be able to "side-load" (or update without a rebuild) these i18n files again. However, there's some concern about whether this PR is the right approach as it requires either external scripts or manual updates.

Therefore, I'm pressing "pause" on this PR as we feel it needs more feedback. The approach you've taken seems to work, but it also seems to require scripts that are specific to your institution in order to fully "automate it". Any other institution would need to have a similar process built or would need to do these updates more manually.

I've assigned some other developers to take a look at this when they get the chance. For now, this is low priority as we are not certain this will be a useful change to other institutions. If, however, we discover that other institutions can use this same approach then we will work to move this forward. So, this needs more feedback before we can move it forward.

[mahnkong]

@tdonohue thanks, the process you mentioned sounds good to me.

[github-actions]

Hi @mahnkong,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!