Config does not load optional settings from defaults

[JPSchellenberg]

If an option is passed via the defaults object, but not defined in the initial config, it is not loaded.
For example: contentFilesUrlPlayerOverride is not defined in the initial config. It is impossible to pass this setting in the defaults object passed in the constructor.

H5P-Nodejs-library/packages/h5p-server/src/implementation/H5PConfig.ts

Line 19 in 647e578

if (this[key] !== undefined) {

[JPSchellenberg]

The easiest solution would be to remove the if-statement in line 19. I don't think that this if-statement is really neccessary. @sr258 Would you agree to remove it?

[sr258]

The change would not make the class load the values from storage, but from the override defaults and saving would still have the same problem.

[JPSchellenberg]

Could you elaborate? I don't see how this would prevent loading values from storage. The if statement just prevents loading settings from the defaults-override that are not previously defined as far as I can see.
This problem occurs if you are not using a file, but in-memory-storage. Certain values are passed as environment-variables.

[sr258]

It doesn't prevent it, but also doesn't replace it. I would simply use a totally different class if you need some other behavior. The IH5PConfig interface is deliberately meant to be an interface, because you can simply plug in you own implementation.

[JPSchellenberg]

I don't need any other behavior. I think it is a little bit confusing to have an option in the configs listed (in this example contentFilesUrlPlayerOverride), but if you pass it via the defaults-object in the constructor it won't work. I just want to be able to provide the documented options via the default object.

[sr258]

The if statement looks like I put it in deliberately for some reason. You're right that the JavaDocs comment doesn't match the behavior, though. You can change it, but you should check if the samples all still work as they currently do.