Slice driven encoding

[bernardnormier]

Currently, the Slice encoding of a request payload depends on:

• the parameters of the operation: if there is any Slice parameter, the encoding is 1.1. Otherwise, keep going
• the encoding of the proxy
  • if specified, the specified encoding is used
  • if not specified, the "default encoding" is the proxy's protocol encoding

And this works today since the proxy has always a protocol (ice1 or ice2).

This is a proposal to:

• stop using the proxy to select the encoding, and as a result remove the encoding property of proxies
• specify the Slice encoding to use when encoding a request payload in the Slice definitions, and nowhere else

With this proposal, it's also possible/easier to make the Ice protocol of a proxy truly optional.

Proposal

Encoding Attributes

Add [encoding11] and [encoding20] attributes to Slice operations and the containers of operations (interfaces, module openings).

This attribute applies only to request parameters and means: when encoding these parameters, the Slice engine uses the specified encoding. For example:

[encoding11]
interface Test
{
    op(str: string) -> string; // if I send an op request, the payload will be encoded with encoding11
}

This attribute is not used for response payloads: the server still uses the request's payload encoding to encode the response (like today).

Remove the "+" in encoding 1.1

Encoding 1.1 is about interop with ZeroC Ice. There is no point to support new Slice syntax such as optional types and streams with 1.1.

Enforce Encoding Compatibility During Slice Compilation

Unlike what we do today, the Slice compiler does not automatically infer the encoding to select from the parameters/return of your operation. If you specify [encoding11], all the parameters and return value of your operation must be 1.1-compatible (else, compilation failure). Likewise, if you specify [encoding20], all the parameters and return value of your operation must be 2.0-compatible (else, compilation failure).

If you don't specify anything, it's equivalent to specifying [encoding20] on the enclosing module opening*.

For example:

interface Foo
{
    op1(str: string) -> string; // ok, 2.0 encoding
    op2(c: MyClass);  // failure, can't encode MyClass with 2.0
    op3() -> MyClass; // failure, can't return MyClass from an encoding20 operation
    [encoding11] op4(str: string?) -> string?; // failure: can't send or return an optional string with 1.1
}

(*) module opening because the attribute applies to all the interfaces/operations in this particular module opening, not all opening of this module.

The net effect is most of the time the encoding of your request and even response is fully specified. This proposal uses attributes (not keywords) for encoding which allows for client / server Slice definition mismatches. For example:

For example:

interface Foo // client version
{
    [encoding11] op1(str: string) -> string;
}

interface Foo // server version
{
    op1(str: string) -> string;
}

Here the client will send a 1.1-encoded request and the server will return a 1.1-encoded response, even though the server Slice definitions make no mention of 1.1.

[pepone]

How would this work for testing, for example in the operation tests we have https://github.com/zeroc-ice/icerpc-csharp/blob/2a257ae36d94e0bbb3f43f80118a9c9f5fcda83d/tests/IceRpc.Tests.Slice/OperationsTests.cs#L10-L12

And the protocol encoding is used to select the request encoding, but once the encoding is defined in Slice can we change it without having duplicate Slice definitions?

[bentoi]

IMO, these tests are wrong in the first place by relying on the protocol encoding.

We could consider supporting an Invocation.SliceEncoding property to provide the Slice encoding to use if not explicitly set in the Slice (or perhaps it should be a feature to make the core APIs encoding independent).

[bernardnormier]

To clarify the effect of [encodingXx] attributes on the generated code:

• these attributes would affect only the generated proxy code: it determines which encoding is used to encode operation args, and a successful response is expected to return a payload encoded with the specified encoding
  For example:

    [encoding20] op(s: string) -> string;

The proxy generated code for C# uses a Slice20Encoder to encode the args and a Slice20Decoder to decode the return value. If we get back a 1.1-encoded return value, it's an error.

• on the server (skeleton) side, the generated code ignores the attributes. For example:

    [encoding20] op(s: string) -> string;

The skeleton generated code decodes the args based on the Slice encoding specified in the request, and encodes the return value with the same Slice encoding.

Naturally if any param / return implies a single encoding, the skeleton code can be encoding-specific:

    op(s: string?) -> string; // skeleton-code is 2.0-only since 1.1 doesn't support optional types

    [encoding11] op(s: string?) -> MyClass; // skeleton-code is 1.1-only since 2.0 doesn't support classes

[bernardnormier]

One interesting question is what is the encoding for the operations of IceRpc::Service:

module IceRpc
{
    /// All services implement this interface.
    interface Service
    {
        /// Gets the Ice type IDs of all the interfaces implemented by the target service.
        /// @return The Ice type IDs of all these interfaces, sorted alphabetically.
        sequence<string> ice_ids();

        /// Tests whether the target service implements the specified Slice interface.
        /// @param id The Ice type ID of the Slice interface to test against.
        /// @return True when the target service implements this interface; otherwise, false.
        bool ice_isA(string id);

        /// Pings the service.
        void ice_ping();
    }
}

I think it needs to be encoding11 for backwards compatibility with ZeroC Ice. And maybe we could new 2.0 encoded operations such as:

interface Service
{
    [encoding20] sequence<string> ice_rpc_ids();  // ice_rpc to map to IceRpc and not Icerpc.
    [encoding20] bool ice_rpc_isA(string id);
    [encoding20] void ice_rpc_ping();
    
    [encoding11] sequence<string> ice_ids(); 
    [encoding11] bool ice_isA(string id);
    [encoding11] void ice_ping();
}

Maybe overkill?

[bentoi]

Adding new methods sounds a bit overkill and aren't we going to run out of ideas for names when we add the 3.0 encoding :)?

[InsertCreativityHere]

I think this would be cleaner if it was a single attribute with an argument:

[encoding11]    ->    [encoding("1.1")]
[encoding20]    ->    [encoding("2.0")]

Maybe instead of strings, we could take enum arguments like 11 and 20 but that's a detail.
This is more flexible (don't need a new attribute for each new encoding), and IMO more natural feeling.

I think it would be nice to de-couple proxy protocol and request encoding, but I think this approach will really hurt code re-usability (like you already saw with Service). Most operations could be encoded with either 1.1 or 2.0, so tying it to a specified encoding at the Slice level seems pretty limiting.

Especially thinking in the long term. Right now this is reasonable because we only have 2 supported encodings, but eventually we'll likely have 2.1 and 3.0 etc. Like @bentoi says, this will get pretty untenable.

[bernardnormier]

For Service, I think we should have simply:

interface Service
{    
    [encoding11] sequence<string> ice_ids(); 
    [encoding11] bool ice_isA(string id);
    [encoding11] void ice_ping();
}

Please name a single downside of using the 1.1 encoding for these payloads.

but eventually we'll likely have 2.1 and 3.0 etc. Like @bentoi says, this will get pretty untenable.

I think it's actually unlikely; it's more likely we won't introduce another encoding version.

A common mistake is to think "new feature requires new encoding version". Look at the Unicode encodings: they keep adding new characters and introducing new versions of Unicode (latest is version version 14.0 - https://en.wikipedia.org/wiki/Unicode#Versions). And yet, when you encode/decode a UTF-8 string, you don't encode/decode the Unicode version first.

As for:

[encoding11]    ->    [encoding("1.1")]
[encoding20]    ->    [encoding("2.0")]

I find the replacement syntax more painful to type and more error-prone - not an improvement.

[bernardnormier]

Replaced by #87.