26174

Comment From: pivotal-issuemaster

@rudy2steiner Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

Comment From: pivotal-issuemaster

@rudy2steiner Thank you for signing the Contributor License Agreement!

Comment From: rudy2steiner

@rstoyanchev would you mind helping me review this PR!

Comment From: rudy2steiner

@rudy2steiner thanks for those changes but I'm afraid it's not going to work out in its present form. What I had in mind was a lot less extensive, simply passing MessageHeaders in PayloadMethodArgumentResolver as a hint under a well-known key so that a custom Decoder can take into account message headers, including extracted metadata.

I don't think RSocketRequester should be involved in any of these changes. Composite metadata already supports sending metadata of any mime type, and there is no reason for it to know what kind of metadata it is.

Yes, I see what you have in mind. Once we pass headers as hints to decoder, we could add any metadata with self-defined mime on client side, and the metadata will be available for server decoder. But how to keep encoder of client and decoder of server have same hints context is a problem.

I have to admit the current changes isn't the best solution for the issue .

Comment From: rudy2steiner

May we could introduce a new method on RequestSpec: data with hints for local encoders

RetrieveSpec data(Object data, Map<String,Object> hints );

and hints for remote decoder could implement by add a metadata use MetadataSpec#metadata

In this way, only a few changes and providing the ability to attach hints for encoder and decoder

Comment From: rstoyanchev

But how to keep encoder of client and decoder of server have same hints context is a problem.

I don't follow what you mean. Wouldn't the client just add some metadata which then the server can use for decoding?

Comment From: rudy2steiner

I don't follow what you mean. Wouldn't the client just add some metadata which then the server can use for decoding?

Do you think we should provide the ability for add hints for client encoding at the same time?

Client encoding always use a EMPTY_HINTS on DefaultRequestSpec#encodeData:

private <T> encodeData(T value, ResolvableType elementType, @Nullable Encoder<?> encoder) {
            if (encoder == null) {
                elementType = ResolvableType.forInstance(value);
                encoder = strategies.encoder(elementType, dataMimeType);
            }
            return ((Encoder<T>) encoder).encodeValue(
                    value, bufferFactory(), elementType, dataMimeType, EMPTY_HINTS);
}

Comment From: rstoyanchev

I think it would be useful if you could provide more details of what you are trying to achieve more concrete. That would help to think about possible solutions.

Comment From: rudy2steiner

I think it would be useful if you could provide more details of what you are trying to achieve more concrete. That would help to think about possible solutions.

In my case, the hints will be used to affect behavior of encoder and decoder, but found that hints are always empty. I need to rewrite a lot of code to implement the requirement by myself, but I think Spring Messaging should built-in support user to add hints for encoder and decoder for per message , the hints are possibly different for per message.

We could discuss how to implement this feature if you agree with this would benefit for user.

Comment From: rstoyanchev

I'm afraid that doesn't tell me much more. I gather that you want to pass hints but what's not clear is why you want to pass hints into a client Encoder that would then be serialized and passed to the server side as well. By "provide more details" I meant provide a concrete scenario to illustrate the use case.

Comment From: rudy2steiner

I have messages from different sources and be wrapped into instance of same class, but I want to serialize it using different algorithm for message from different source .

If I can pass a hint that indicate where the message from, then selecting corresponding algorithm to encode on client and decode it on server.

So I need a way to pass a hint for local encoder and remote decoder.

As i can seen, Hints as a metadata can be abstracted from headers and then passing it to decoder. at the same time, I need pass the same hint to local encoder.

Spring messaging has passed the hints into encoder and decoder, but it's alway empty and can't be customized by users.

Comment From: rstoyanchev

Sorry for the delay and thanks for the extra details. This is quite helpful although it's still a bit abstract. It would help to have a little more detail about the different encoding algorithms. Do they result in a different format on the wire for example, which could potentially be handled by using different data MIME types. Or perhaps something in the Object structure or type could be used to differentiate?

Comment From: rudy2steiner

Currently, Data Mime is connection not message level which will double connections if using different data MIME types. Object structure isn't a general solution for this problem.

On production environment, we use the first solution as a temporary solution, but we should have more elegant solution.

  • Supporting messaging level data MIME feature is a perfect solution in the future.
  • Supporting hints on client and server also benefits for solving my problem.

Anyway, Supporting hints on client and server is necessary.

I'll try to simplify my changes.

Comment From: rstoyanchev

Currently, Data Mime is connection not message level which will double connections if using different data MIME types.

Keep in mind there is this https://github.com/rsocket/rsocket/blob/master/Extensions/PerStreamDataMimeTypesDefinition.md which we need to add support for. Once that's implemented, doing the same via hints would amount to a workaround, so I think we should create a ticket to support per-stream/request MIME types.

Comment From: rudy2steiner

Is there any one has worked on Stream level data MIME, I'd like try if no ?

Comment From: rstoyanchev

I'd rather focus on support for per-request data MIME types first and close this. I've created the issue #26379. You can give it a try if you want. I don't have a good sense if it would work out well for a contribution.

Comment From: rudy2steiner

Ok, we could polish this feature in the future.