AutoPersistedQueries supported for mutations

[Toxu4]

No description provided.

[apollo-cla]

@Toxu4: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[martinbonnin]

Hi! Thanks for the contribution 👍 . My only concern is that it silently enables APQs (Auto Persisted Queries) on mutations for existing users upgrading from a previous version of apollo-android. While that should work in most cases, there's a chance that some servers do not handle that or that it causes various performance issues in apps, which might be hard to debug. The probability of this happening is certainly low but I don't think that's a risk we want to take.

If you want to enable APQs for mutation, you can use ApolloClient.Builder.setAutoPersistedOperationsInterceptorFactory:

builder.setAutoPersistedOperationsInterceptorFactory(ApolloAutoPersistedOperationInterceptor.Factory())

[Toxu4]

Hi! Thanks for the contribution 👍 . My only concern is that it silently enables APQs (Auto Persisted Queries) on mutations for existing users upgrading from a previous version of apollo-android. While that should work in most cases, there's a chance that some servers do not handle that or that it causes various performance issues in apps, which might be hard to debug. The probability of this happening is certainly low but I don't think that's a risk we want to take.

If you want to enable APQs for mutation, you can use ApolloClient.Builder.setAutoPersistedOperationsInterceptorFactory:

builder.setAutoPersistedOperationsInterceptorFactory(ApolloAutoPersistedOperationInterceptor.Factory())

Hi!
I simply implemented the same behaviour as implemented for iOS library. I think this is unwanted and unexpected behaviour - to not support APQ for mutations out of the box, without manually setting of ApolloAutoPersistedOperationInterceptor.Factory()

[DaniilSokolyuk]

I think this feature should work the same on js, ios and android clients, for js and ios it works now
apollographql/apollo-ios#1100

[martinbonnin]

If iOS users had no issue during the transition then we can certainly take that risk.

Another option would be to add:

    public Builder enableAutoPersistedOperations(boolean persistOperations) {
      return setAutoPersistedOperationsInterceptorFactory(
                       new ApolloAutoPersistedOperationInterceptor.Factory(false, persistOperations, persistOperations)
      )
    }

That would keep the existing behaviour untouched and also be more explicit about queries vs operations. @designatednerd any thoughts?

[Toxu4]

If iOS users had no issue during the transition then we can certainly take that risk.

Another option would be to add:

    public Builder enableAutoPersistedOperations(boolean persistOperations) {
      return setAutoPersistedOperationsInterceptorFactory(
                       new ApolloAutoPersistedOperationInterceptor.Factory(false, persistOperations, persistOperations)
      )
    }

That would keep the existing behaviour untouched and also be more explicit about queries vs operations. @designatednerd any thoughts?

I think this is a bug, so it should be fixed by changing behaviour, not by changing api.

In the long run, I would prefer the APQ to be renamed APO (AutoPersistedOperations) :)

[martinbonnin]

Looks like enabling mutations should be fine 🤞 . I'll merge this, make a minor 2.3.0 and put a warning in the release note to make sure the change is communicated.

[martinbonnin]

Thanks for bringing this up!