Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor proxy-as-data-type #673

Closed
bernardnormier opened this issue Nov 19, 2023 · 6 comments
Closed

Refactor proxy-as-data-type #673

bernardnormier opened this issue Nov 19, 2023 · 6 comments
Assignees
Labels
Milestone

Comments

@bernardnormier
Copy link
Member

Currently, when you define a Slice interface X, you get a data type that you can encode/decode - and for example use as a struct field or operation parameter. This data type is a "proxy" to a service that implements X.

This is problematic for a number of reasons:
a) Unlike other user-defined types, the encoded representation of this type is not intuitive. You see an enum, you have some idea of what an enum parameter looks like "over the wire". The same isn't true for a proxy.

b) The mapping for interfaces is RPC-specific, but the encoding/decoding of data types is not, so we can't have the clean-separation:

  • data types => RPC-independent
  • interfaces => RPC dependent

since defining an interface automatically defines a proxy data type.

c) For many applications, this Slice-level proxy data type is never used. It's not that common to pass a proxy as a parameter or return value.

d) Passing proxies around in Slice operations may not be a good design choice. And arguably this feature (any Slice interface gives you automatically a proxy data type) encourages this design.

Proposal

a) Defining an interface no longer defines a Slice-level encodable proxy data type.

For example, the following definition would no longer compile:

interface Widget {} // no longer a data type

interface WidgetFactory {
    create() -> Widget // error
}

b) Allow the user to "opt-in" the old behavior with a custom type. For example:

module FooBar

interface Widget {}

// Encodes / decodes WidgetProxy as a service address; its C# API is FooBar.WidgetProxy
[cs::type("FooBar.WidgetProxy")]
custom WidgetProxy

interface WidgetFactory {
    create() -> WidgetProxy // returns a regular custom data type
}

It's just a regular custom data type that maps in C# to WidgetProxy.

The only special feature is as long as the user calls this custom type InterfaceNameProxy, he doesn't need to provide the Encode and Decode methods. slicec generates them automatically alongside the proxy struct for this interface.

This keeps the convenience of using proxy as data types, but makes this convenience opt-in. It also provides a clean separation between Slice data type (fully RPC-independent) and interfaces (RPC-dependent).

Note that it's a breaking change can since you need to:
i) add this custom type to your Slice file (the opt-int part), and
ii) call this custom type NameProxy - not Name

@InsertCreativityHere
Copy link
Member

Sounds fine to me. This shouldn't cost any functionality, and will finally finish separating protocol constructs out of Slice.
The only downsides I see are slight:

  • Passing proxies is a little more advanced now (which is fine)
  • We add a slightly special case to how custom types work.

We briefly discussed splitting off a 0.2 branch for slicec to put this feature on, but honestly, I think the easiest path is we update slicec-cs to use the latest slicec. If it encounters an enum with associated values, we emit a warning/error saying "not supported".

This way we keep slicec to a single active branch.

@pepone
Copy link
Member

pepone commented Nov 21, 2023

The special use of cs::custom, relying on a naming convention, doesn't look great. I think we should consider not providing a special mechanism. You can return a ServiceAddress and let the user construct the proxy object manually.

@bernardnormier
Copy link
Member Author

bernardnormier commented Nov 21, 2023

The proposed custom is not special:

  • the "base" Slice code generator handles it like any other custom
  • the "icerpc" code generator doesn't generate any code for this custom

You can return a ServiceAddress and let the user construct the proxy object manually.

That's true, and this proposal is consistent with this logic. Say you have:

interface WidgetFactory {
    create() -> ServiceAddress // returns the service address of the new Widget
}

but you wish this API would be a bit more typed and convenient. In C#, you wish you'd get an instance of WidgetProxy, with this service address and the invoker and encode options of your choice. The solution is for you to create a custom that gives this more convenient API.

That's a little bit of work because creating your own custom requires you provide encode and decode methods. My proposal is to make your life a little easier when you name this custom NameProxy.

@pepone
Copy link
Member

pepone commented Nov 22, 2023

That's a little bit of work because creating your own custom requires you provide encode and decode methods. My proposal is to make your life a little easier when you name this custom NameProxy.

I think is fine if the users has to do this extra work, as we are saying here using proxies as data types should not be that common.

@bernardnormier
Copy link
Member Author

I think it would be impractical to require this full custom impl, especially in an Ice - IceRPC interop or migration scenario, where the Ice Slice files take full advantage of proxies as data type.

We don't want to generate c# code from ice2slice.

@InsertCreativityHere
Copy link
Member

The parser side of this (disallowing them as types) was implemented in #675,
and the C# side was implemented by icerpc/icerpc-csharp#3803.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants