Add correct extension mode to plugin context

[eneufeld]

Fix #10201

Contributed on behalf of STMicroelectronics
Signed-off-by: Eugen Neufeld [email protected]

What it does

The plugin context was hardcoded to have the production extensionMode.
Now a flag is passed to mark a plugin as under development when
deploying it. This flag is stored in the plugin metadata and later set in
the plugin. When creating the context this flag is then converted to the
correct extensionMode.

How to test

Download this plugin project: theia-hello-world-plugin.zip .
It contains a built version of the plugin which you should add to your plugins folder. This add a new command which opens a message with Hello World! Mode: ${mode} where the mode is the extensionMode. For the deployed plugin the mode should be '1' (Production).
Also add the project to your workspace so that you can start the plugin in a hosted version, by calling Hosted mode: start instance. When you trigger the command now in the hosted mode, the message should be Hello World! Mode: 2 (Development).

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[vince-fugnitto]

@eneufeld thank you for the contribution, please be sure to properly fill out the pull-request template including steps on how to test for reviewers (ideally with an example plugin).

[eneufeld]

@vince-fugnitto sorry, I was to fast creating the PR. I updated the description now.

[colin-grant-work]

I think that this line suggests that the approach taken here is a little bit against the grain of how the plugin system should work: ideally the plugin reader system should set the metadata, but here isUnderDelevopment is grafted onto the metadata after the readers have had their say. Given that, I think the correct approach would be to ensure that whichever HostedPluginReader is responsible for setting the metadata of the plugin under development correctly sets this field, rather than passing it through the deployer system.

There's some terminological confusion here, unfortunately. HostedPluginReader is the name of the class implemented in hosted-plugin-reader.ts which passes the plugin under development into the plugin deployer system, but it's also the name of the class implemented in plugin-reader.ts. The latter is responsible for setting the metadata by delegating to MetadataScanners, and it's there that the plugin should be marked as 'under development'. It looks like the originID = 'Hosted Plugin' of the PluginDeployerEntry created in hosted-plugin-reader.ts should be a good marker for the metadata scanners to use to identify the plugin that is under development and set the isUnderDevelopment field by.

[eneufeld]

Thank you for the review and the hints to clean up the code.
Unfortunately the scanner never has access to the PluginDeployerEntry.
I now set a marker inside the PluginDeployerEntryImpl based on the originId and instead of passing the isUnderDelevopment through the deployment system, the value is read from the PluginDeployerEntry.
What do you think, is this a viable approach?

[colin-grant-work]

Unfortunately the scanner never has access to the PluginDeployerEntry.

Quite right.

I now set a marker inside the PluginDeployerEntryImpl based on the originId and instead of passing the isUnderDelevopment through the deployment system, the value is read from the PluginDeployerEntry.
What do you think, is this a viable approach?

I think that's fine. It still cuts a little against the grain, but the metadata system as it is currently implemented isn't worth working around that much. The alternative, which I don't really think is desirable in this case, since there will only ever be one plugin to which isUnderDevelopment applies, is to implement a separate service to track which plugin is underDevelopment and then access that service from the metadata reader. I've done something similar to that here for uninstalled plugins, but I don't think that much machinery is required for this use case.

[colin-grant-work]

I don't think this needs to be done in the PluginDeployerEntryImpl. The HostedPluginReader(packages/plugin-dev/src/node/hosted-plugin-reader.ts) can call storeValue on the plugin deployer entry that it creates.

[colin-grant-work]

This will need to be moved to the section for 1.26

[colin-grant-work]

The changes are sound and correctly set the plugin mode 👍