Skip to content

Save and reuse WebClient response #328

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

Closed
ciscoo opened this issue Mar 14, 2022 · 7 comments
Closed

Save and reuse WebClient response #328

ciscoo opened this issue Mar 14, 2022 · 7 comments

Comments

@ciscoo
Copy link

ciscoo commented Mar 14, 2022

My company has various of services that provides customer data. I'm trying to build POC to showcase what is possible to maybe solve our own internal issues.

There is no single "view" of the data and instead we must retrieve the data we need from the various services. Using Reactor, I thought I would be able to save the response from a request, and reuse the response if a different field is queried from that same response.

For example, given a todo, let's say the description and completed come from the same API. I want to be able to save the response in the Reactor context and reuse if more fields are required.

@SchemaMapping(typeName = "Todo")
@Controller
public class TodoController {

    private static final ParameterizedTypeReference<Map<String, Object>> MAP_TYPE = new ParameterizedTypeReference<>() {};

    private static final String RESPONSE_ATTR = TodoController.class.getName() + ".todoResponse";

    private final WebClient todosWebClient;

    public TodoController(WebClient todosWebClient) {
        this.todosWebClient = todosWebClient;
    }

    @SchemaMapping(field = "title")
    public Mono<String> resolveTitle(Todo todo) {
        return Mono.deferContextual(contextView -> {
            Map<String, Object> cached = contextView.getOrDefault(RESPONSE_ATTR, null);
            if (cached != null) {
                return Mono.just(cached.get("title").toString());
            }
            return this.todosWebClient.get().uri("/todos/{id}", todo.getId())
                    .retrieve()
                    .bodyToMono(MAP_TYPE)
                    .flatMap(res -> Mono.just(res).contextWrite(context -> context.put(RESPONSE_ATTR, res)))
                    .flatMap(res -> Mono.just(res.get("title").toString()));
        });
    }

    @SchemaMapping(field = "completed")
    public Mono<Boolean> resolveCompleted(Todo todo) {
        return Mono.deferContextual(contextView -> {
            Map<String, Object> cached = contextView.getOrDefault(RESPONSE_ATTR, null);
            if (cached != null) {
                return Mono.just(Boolean.parseBoolean(cached.get("completed").toString()));
            }
            return this.todosWebClient.get().uri("/todos/{id}", todo.getId())
                    .retrieve()
                    .bodyToMono(MAP_TYPE)
                    .flatMap(res -> Mono.just(res).contextWrite(context -> context.put(RESPONSE_ATTR, res)))
                    .flatMap(res -> Mono.just(Boolean.parseBoolean(res.get("completed").toString())));
        });
    }

}

However, seems this approach does not work since the logs show (2) requests being sent:

2022-03-14 14:02:36.473 DEBUG 28960 --- [ttp@13e9f2e2-76] o.s.w.r.f.client.ExchangeFunctions       : [7f28f96f] HTTP GET https://jsonplaceholder.typicode.com/todos/2
2022-03-14 14:02:36.627 DEBUG 28960 --- [ttp@13e9f2e2-76] o.s.w.r.f.client.ExchangeFunctions       : [4301696d] HTTP GET https://jsonplaceholder.typicode.com/todos/2
2022-03-14 14:02:36.948 DEBUG 28960 --- [or-http-epoll-3] o.s.w.r.f.client.ExchangeFunctions       : [4301696d] [500c0d95-1] Response 200 OK
2022-03-14 14:02:36.948 DEBUG 28960 --- [or-http-epoll-2] o.s.w.r.f.client.ExchangeFunctions       : [7f28f96f] [d12daeed-1] Response 200 OK

I have uploaded a sample project of the above approach: https://github.com/ciscoo/cache-graphql


Side note, I'm trying to build a single view into our data. For example, the todoDetails comes from a service which I want to "unwrap" if that makes sense.

{
    "id": 1,
    "todoDetails": {
        "id": 1,
        "completed": false
    }
}

to

{
    "id": 1,
    "completed": false
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 14, 2022
@bclozel
Copy link
Member

bclozel commented Mar 15, 2022

Hello @ciscoo! I think there are several aspects to this issue.

First, I don't think the Reactor context is meant to be used as a cache shared between Publisher/Subscriber pairs. Each context is tied to a particular subscriber and context, can only happen at subscription time and moves up the subscription chain. In your case, I'm not sure if the nested usage is at fault, if things happen in different subscriber chains or if this is just a concurrency issue; in any case I don't think this is a valid use case. Even if we find a way to make it work somehow, this approach would probably have cache stampede problems.

I'm not 100% confident that I understand the use case here. BatchMapping would not work here because they don't apply to the same type? But in this sample, we're mapping fields one by one but we could fetch the entire ToDo entry from the remote REST API and map it to a type that fits the schema?

Maybe relying on a cache and conditional HTTP requests with your client would be a better fit?

@bclozel bclozel added the status: waiting-for-feedback We need additional information before we can continue label Mar 15, 2022
@ciscoo
Copy link
Author

ciscoo commented Mar 15, 2022

That makes sense for Reactor context and cache stampede problems.

Let me give a different example.

I can certainly do the following which avoids multiple requests to the same API:

input CustomerIdentifierInput {
    id: String!
}

type CustomerInformation {
    fristName: String!,
    lastName: String!,
    address: String!
}

type Customer {
    info: CustomerInformation!
}

type Query {
    customer(input: CustomerIdentifierInput): Customer!
}

CustomerInformation data comes from API X, so a single request would go to that API, and any additional fields selected from the response would not result in additional requests to that API.

However, what I'm trying to do is model as such:

input CustomerIdentifierInput {
    id: String!
}

type Customer {
    fristName: String!,
    lastName: String!,
    address: String!
}

type Query {
    customer(input: CustomerIdentifierInput): Customer!
}

The CustomerInformation type does not exist here and is "unwrapped" or mapped directly to Customer type. However, querying each field yields a request to API X which what I don't want to happen. That's where I thought I could cache the response and reuse as additional fields are queried.

My understanding of batch loading, you would be able to load or fetch a list of customers or a list of customer details/info independent of each other. But that does not work for our internal APIs since they are all structured to only provide data for the customer you request.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 16, 2022
@rstoyanchev
Copy link
Contributor

Wouldn't this be just one controller method for the "customer" query?

@Controller
public class CustomController {

    @QueryMapping
    public Customer customer(@Argument CustomerIdentifierInput input) {
        // call API X...
        return customer;
    }

For the Reactor context, the GraphQL Java engine which invokes each DataFetcher (or controller method in this case) isn't aware of Reactor. Currently we decorate each DataFetcher and propagate to it Reactor context from the web layer. Apart from that they are not in a single Reactor chain and Reactor context from one cannot be seen by others. This is something we might try to improve in the future, but for now if you want pass something across data fetchers, just use GraphQLContext.

@ciscoo
Copy link
Author

ciscoo commented Mar 18, 2022

I have updated my sample repo with more concrete examples on new customer branch:

Using the above updated sample:

  • The customer query sample is something I can certainly do, however, with this approach I need to add an extra object on customer. I want to "unwrap" that object and place its properties on the root/parent object if that makes sense.
  • altogetherCustomer query is what I think you are suggesting Rossen, but the problem with that approach is that I would be performing unnecessary HTTP requests every time the altogetherCustomer query is executed. I'd like to avoid that and only execute requests as they are needed or in other words as fields are queried.
  • idealCustomer query is what I'm after. If you compare it to the other queries, you'll see that the nested info type is not there and instead its properties are placed directly on IdealCustomer. But the problem with this approach is that I'm issuing duplicate requests which I tried to avoid by placing the response in Reactor context, but that did not work. Nor did using GraphQLContext work either.

Hopefully I have illustrated the problem more clear.

@ciscoo
Copy link
Author

ciscoo commented Apr 1, 2022

I was able to achieve what I'm after, albeit rather ugly IMO.

Using GraphQLContext as mentioned previously in conjunction with DataFetchingFieldSelectionSet, I can look ahead to see what fields are selected. If a certain field is selected, then invoke and cache that response before moving forward to other data fetchers:

    @QueryMapping
    public Mono<IdealCustomer> idealCustomer(@Argument CustomerIdentifierInput input, DataFetchingFieldSelectionSet selectionSet, GraphQLContext graphQLContext) {
        var customer = new IdealCustomer();
        customer.setId(input.getId());
        var mono = Mono.just(customer);
        if (selectionSet.contains("firstName")) {
            Mono<IdealCustomer> finalMono = mono;
            mono = this.customerInfoService.getCustomerInfo(input.getId()).flatMap(info -> {
                graphQLContext.put("customerInfo", info);
                return finalMono;
            });
        }
        return mono;
    }

then in other data fetchers:

    @SchemaMapping
    public Mono<String> firstName(IdealCustomer customer, GraphQLContext graphQLContext) {
        return getFromContextOrGet(customer, graphQLContext).map(CustomerInfo::getFirstName);
    }

    @SchemaMapping
    public Mono<String> lastName(IdealCustomer customer, GraphQLContext graphQLContext) {
        return getFromContextOrGet(customer, graphQLContext).map(CustomerInfo::getLastName);
    }

    private Mono<CustomerInfo> getFromContextOrGet(IdealCustomer customer, GraphQLContext context) {
        Object cached = context.get("customerInfo");
        if (cached != null) {
            return Mono.just((CustomerInfo) cached);
        }
        return this.customerInfoService.getCustomerInfo(customer.getId());
    }

This allows me to use the second schema I posted above. However, some of the downstream responses are huge (100+ fields), so I think the first schema would be a better approach in the end.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Apr 19, 2022

Thanks for the sample @ciscoo. Here is what I made of it.

Keep the separate CustomerInfo for fetching data from the remote HTTP service, but wrap it from IdealCustomer to match the flat structure in the schema:

public class IdealCustomer {

    private int id;

    private CustomerInfo info;

    public int getId() {
        return id;
    }

    public void setId(int id) {
        this.id = id;
    }

    public String getFirstName() {
        return info.getFirstName();
    }

    public String getLastName() {
        return info.getLastName();
    }

    public IdealCustomer initCustomerInfo(CustomerInfo info) {
        this.info = info;
        return this;
    }

    public boolean hasCustomerInfo() {
        return this.info != null;
    }

}

Then the QueryController:

@QueryMapping
public Mono<IdealCustomer> idealCustomer(@Argument CustomerIdentifierInput input) {
    var customer = new IdealCustomer();
    customer.setId(input.getId());
    return Mono.just(customer);
}

and IdealCustomerInfoController:

@Controller
public class IdealCustomerInfoController {

    private final CustomerInfoService customerInfoService;

    public IdealCustomerInfoController(CustomerInfoService customerInfoService) {
        this.customerInfoService = customerInfoService;
    }

    @SchemaMapping
    public Mono<String> firstName(IdealCustomer customer) {
        return initCustomerInfo(customer).map(IdealCustomer::getFirstName);
    }

    @SchemaMapping
    public Mono<String> lastName(IdealCustomer customer) {
        return initCustomerInfo(customer).map(IdealCustomer::getLastName);
    }

    private Mono<IdealCustomer> initCustomerInfo(IdealCustomer customer) {
        return (customer.hasCustomerInfo() ? Mono.just(customer) :
                this.customerInfoService.getCustomerInfo(customer.getId()).map(customer::initCustomerInfo));
    }

}

As an alternative, to improve on that, is to peek in the selection set, which means you don't need a separate IdealCustomerInfoController and encapsulate it all in on method in QueryController:

@QueryMapping
public Mono<IdealCustomer> idealCustomer(@Argument CustomerIdentifierInput input, DataFetchingFieldSelectionSet selectionSet) {
    var customer = new IdealCustomer();
    customer.setId(input.getId());
    return selectionSet.containsAnyOf("firstName", "lastName") ?
            this.customerInfoService.getCustomerInfo(customer.getId()).map(customer::initCustomerInfo) :
            Mono.just(customer);
}

@rstoyanchev rstoyanchev added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Apr 19, 2022
@ciscoo
Copy link
Author

ciscoo commented Apr 19, 2022

Yes, your last sample peeking in the selection set is what I my comment was. 😄

Seems I accidentally reopened this issue after closing which I did not notice I did. So, closing again since a solution was found.

@ciscoo ciscoo closed this as completed Apr 19, 2022
@rstoyanchev rstoyanchev removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants