fix(MessageProxy): channel conflict

[axel7083]

What does this PR do?

Every class that is registered to the MessageProxy system must have a static CHANNEL string property, which will be used to prefix every channel.

StudioAPI declared a function ping before the name of the method was used, which could lead to conflict with other registered method (InstructLabAPI), therefore we uses the CHANNEL as prefix, the channel become StudioAPI-ping.

Allowing scaling, and will avoid problem while working on instructlab api.

Screenshot / video of UI

N/A

What issues does this PR fix or reference?

Fixes https://github.com/containers/podman-desktop-extension-ai-lab/issues/1837

How to test this PR?

• [x] unit tests has been provided

Manually

You can ensure that nothing change by playing with this version of AI Lab

[axel7083]

Can't we use the class name as channel ?

@jeffmaury sadly no, the problem is the following

The frontend do not have access to the StudioApiImpl class (file on backend), it only has access to the StudioAPI abstract class (file on shared). Therefore on the frontend, we don't know the string StudioApiImpl.

The CHANNEL property is set on the abstract class, so it shared by both the frontend and the backend

[benoitf]

IMHO I think there should be some time where brainstorming should occur between between when issues are opened and PR being opened

Can't we use the class name as channel ? @jeffmaury sadly no, the problem is the following can we use the interface name instead ?

usually how it works on proxy is that you ask stuff for a given interface

client = myProxy = Proxy(Interface)

server = registerProxy(Interface, implementation)

so registering also two times the same interface name would conflict

I think the channel is an implementation details and should not be exposed

[axel7083]

IMHO I think there should be some time where brainstorming

Movind to draft

[axel7083]

I think the channel is an implementation details and should not be exposed

Fixed: when getting the proxy object, you do not pass the CHANNEL value, you pass the abstract class, which must contain a CHANNEL field.

[axel7083]

and signature would be something like getProxy(proxyClass: {name: string})

Fixed

[axel7083]

Moving to draft: abstract class do not seems to expose their abstract method through the .prototype property

[axel7083]

After reading a bit about abstract class in Typescript, I found There is no code generated for an abstract method^1

Therefore, in the MessageProxy we are unabled at runtime to list the methods and create the corresponding channels.

https://github.com/containers/podman-desktop-extension-ai-lab/blob/5d6a3428ed534da6ddc7d2949dc921240984df19/packages/shared/src/messages/MessageProxy.ts#L108-L111

We are getting an empty array when providing StudioAPI, only a non-abstract class would allow us to get a non-empty array. This is why we need StudioApiImpl

We cannot transform the StudioAPI to an interface for technical reasons, interfaces do not exists at runtime in JS

This is the reasons why abstract class was used when implementing the channels system.

Hardcoded CHANNEL property

I still believe my solution with the const CHANNEL is the right one.

[benoitf]

CHANNEL is in the abstract class, not per method and the .name property is on the abstract class as well

so I don't see the difference there as you need to provide the Abstract class both from client and server side

[axel7083]

Sorry if I was not clear in the previous message.

As I show above

We can clearly see at runtime, you cannot access to the list of methods from an abstract class compiled to javascript.

Now let's dig into how frontend and backends are communicating.

Backend

The backends, register a class object, and using reflection, read the prototype of the object provided to list all the methods available.

Today (before this PR) we iterate over all methods and the backend register channels. Example ping channel is created from the ping method declared in StudioApiImpl.

https://github.com/containers/podman-desktop-extension-ai-lab/blob/ea02986c994f9eb60e587bcf7a35a309cde6c315/packages/shared/src/messages/MessageProxy.ts#L113-L116

Then when we received an event from the webview, we can directly call the right method.

https://github.com/containers/podman-desktop-extension-ai-lab/blob/ea02986c994f9eb60e587bcf7a35a309cde6c315/packages/shared/src/messages/MessageProxy.ts#L79-L85

Frontend

The frontend, do not create channels, it create a proxy object, and when you call anything on the proxy, it will transform the method you are calling into string, and be used as channel.

If I do studioClient.ping() the proxy handler will received a property call, named ping. With this information the proxy handler will call the channel ping that the backend will receive and makes it corresponds.

https://github.com/containers/podman-desktop-extension-ai-lab/blob/ea02986c994f9eb60e587bcf7a35a309cde6c315/packages/shared/src/messages/MessageProxy.ts#L178-L181

The name problem

The issue with the name property is that StudioApiImpl.name === 'StudioApiImpl' and StudioApi.name === 'StudioApi'.

Yes they both have a name property, but not the same.

Let's look at our options

registerInstance<T extends Record<keyof T, UnaryRPC>>(classType: new (...args: any[]) => T, instance: T): void

The signature of registerInstance (backend) needs the following arguments

• classType is used to list the methods (need to be a real class not abstract, so we cannot use StudioApi).
• instance: kept to call when needed

We could simplified the first argument by not using it to generate the channel, and use the instance. By doing some hacky stuff we can get the prototype (methods name) from the instance Object.getOwnPropertyNames(Object.getPrototypeOf(instance)), and then we would provide the interface at first argument.

[axel7083]

New problem on the frontend side, we are minifiying the code, which mean the class.name property are not being consistent

The only possibility I see is: Disabling the minify.

I am against this solution, as it increase a lot size of the package.

[benoitf]

let's defer the discussion about how proxy should like (no abstract class but interfaces, etc) out of this PR and merge the current solution that fixes an issue (with constant) instead of trying to address multiple problems at once

[axel7083]

merge the current solution that fixes an issue (with constant)

I revert the code to use the CHANNEL property