Spring Update private methods to protected in AbstractJacskon2Decoder

Why do we need to change the private methods to protected in AbstractJackson2decoder?

• To have the ability to use all the default behavior but add some contextual information to the deserializer; Allowing getObjectReader, getContextClass, logValue, and processException to be protected allows the extended classes of AbstractJackson2Decoder to achieve adding additional contextual information

• Also, avoid duplicating the logic

Comment From: sarathkumar6

Including @jhoeller @rstoyanchev

Comment From: sarathkumar6

Including @jmessmer @jlaber @best2469 @jcrowell-work @EduardoKoster

Comment From: snicoll

@sarathkumar6 please don't mention individual team members like that, the team is triaging PRs and we've all received a notification when the PR was created (as well as individual comments here). Rather than opening up 4 methods with no context, can we please see the customizations that you'd like to build using those methods?

Comment From: rstoyanchev

We can make logValue protected but for the rest please provide more details. What contextual information you need to add and couldn't you configure that on the ObjectMapper? It's also not clear how that relates to processException.

Comment From: sarathkumar6

@snicoll Noted, I'll make sure to not include the members next time, since the process is automated. With regards to context on updating the private methods to protected, we're trying to add some additional details i.e., permissions to the Object Reader and read the value from the data buffer by extending Jackson2JsonDecoder which extends AbstractJackson2Decoder. The extends as follows

AddInfoJackson2JsonDecoder ---> Jackson2JsonDecoder ---> AbstractJackson2Decoder ---> Jackson2CodecSupport

To do the same, we're overriding the decodeToMono in AbstractJackson2Decoder, which uses private methods i.e., logValue, processExcception, and getObjectReader (which in turn invokes getContextClass).

Since we need to use most of the logic from the decode, we would be need to access the private methods that decode method uses otherwise we would be duplicating the logic.

Our usage looks something like this

@Override
public Mono<Object> decodeToMono(Publisher<DataBuffer> input, ResolvableType elementType,
                                     @Nullable MimeType mimeType, @Nullable Map<String, Object> hints) {

    return ReactiveSecurityContextHolder
        .getContext()
        .flatMap(ctx -> {
            AdditionalInfo additionalInfo = (AdditionalInfo)ctx.getAuthentication().getPrincipal();
                return DataBufferUtils.join(input, this.getMaxInMemorySize())
                    .flatMap(dataBuffer -> Mono.justOrEmpty(decode(dataBuffer, elementType, mimeType, hints, additionalInfo)));
                });
}

public Object decode(DataBuffer dataBuffer, ResolvableType targetType,
                         @Nullable MimeType mimeType, @Nullable Map<String, Object> hints, AdditionalDetails additionalDetails) throws DecodingException {

        try {
            ReactiveDeserializerPermissionProvider permissionProvider = new ReactiveDeserializerPermissionProvider(additionalDetails);
            ObjectReader objectReader = getObjectReader(targetType, hints).withAttribute("PERMISSION_PROVIDER", permissionProvider);
            Object value = objectReader.readValue(dataBuffer.asInputStream());
            logValue(value, hints);
            return value;
        }
        catch (IOException ex) {
            throw processException(ex);
        }
        finally {
            DataBufferUtils.release(dataBuffer);
        }
}

Updating the below methods to protected would avoid the duplication

getObjectReader
processException
logValue

Comment From: rstoyanchev

This seems to be related to authorization and it's a bit unusual to see authorization applied at level of a codec. If what you're trying to do is render different Jackson views, consider using Spring Data projections and manage the different views at the level of an @Service that fetches the appropriate view given the permissions of the user. Or perhaps use media types to serve different representations of the same resource through different controller methods.

Comment From: sarathkumar6

@rstoyanchev Thanks for those suggestions, but those don't really fit our needs for our application architecture.

We want the ability to provide contextual information to the deserializer in a Mono chain. Do you have a reason for not allowing this change versus disagreeing with our specific solution? There doesn't appear to be any other reactive hook points to allow consumers of the library to provide any contextual information to the deserializer (regardless of the specific use case).

From our analysis of the current available code, we feel this is the best spot for this, so we're just looking to have the library more extendable for consumers to be able to use current functionality but inject context information with minimal impact.

With denying the request, could you provide some other ideas for how one could inject contextual information to the deserializer in WebFlux, some reasoning why making these protected is a bad idea, or propose a design that we could implement with another PR to meet the requirement of adding contextual information?

We're not requesting to make this change to cause problems, but trying to open things up for the community to have a way to extend the functionality the library currently provides.