Feature 216/move converter #218

sav007 · 2017-02-19T23:13:57Z

Closes #216

First version of ApolloClient.

Api:
<T extends Operation> ApolloCall newCall(@Nonnull T operation)

public interface ApolloCall {

  void cancel();

  <T extends Operation.Data> Response<T> execute() throws IOException;

  <T extends Operation.Data> ApolloCall enqueue(@Nullable Callback<T> callback);

  ApolloCall clone();

  public interface Callback<T extends Operation.Data> {
    void onResponse(Response<T> response);

    void onFailure(Exception e);
  }
}

Usage (will be scheduled on okhttp3.Dispatcher#executorService):

apolloClient.newCall(new AllPlanets()).enqueue(new ApolloCall.Callback<AllPlanets.Data>() {
      @Override public void onResponse(Response<AllPlanets.Data> response) {
      ...
      }

      @Override public void onFailure(Exception e) {
      ...
      }
    });

Or (sync call)

Response<AllPlanets.Data> body = apolloClient.newCall(new AllPlanets()).execute();

sav007 · 2017-02-19T23:18:00Z

apollo-runtime/src/main/java/com/apollographql/android/ApolloClient.java

+      return this;
+    }
+
+    public <T> Builder withResponseFieldMapper(@Nonnull Class type,


Will be removed in next PR, as we drop support of interfaces

sav007 · 2017-02-19T23:18:07Z

apollo-runtime/src/main/java/com/apollographql/android/ApolloClient.java

+      return this;
+    }
+
+    public Builder withResponseFieldMappers(@Nonnull Map<Type, ResponseFieldMapper> mappers) {


Will be removed in next PR, as we drop support of interfaces

sav007 · 2017-02-19T23:19:19Z

apollo-runtime/src/main/java/com/apollographql/android/HttpOperationRequest.java

+
+    Call httpCall = callFactory.newCall(request);
+    if (callRef.compareAndSet(null, httpCall)) {
+      httpCall.enqueue(new okhttp3.Callback() {


So it will be scheduled to be executed on okhttp3.Dispatcher that has own ExecutorService

sav007 · 2017-02-19T23:20:45Z

apollo-runtime/src/test/java/com/apollographql/android/converter/IntegrationTest.java

-            .withCustomTypeAdapter(CustomType.DATETIME, dateCustomTypeAdapter)
-            .build())
+        .okHttpClient(new OkHttpClient.Builder().build())
+        .withResponseFieldMapper(AllPlanets.class, new AllPlanets.Data.Mapper(AllPlanets.Data.FACTORY))


with next PR these mapper registrations won't be needed at all.

digitalbuddha · 2017-02-20T00:15:28Z

apollo-runtime/src/main/java/com/apollographql/android/ApolloClient.java

+import static com.apollographql.android.Utils.checkNotNull;
+
+/** TODO */
+public final class ApolloClient {


Should implement something like

interface Factory { ApolloCall newCall(Operation operation); }

This would keep client stateless. Instead ApolloCall is an interface similar to Okhttp3.Call which can actually do exection

Do we need to make it implement Factory, now?

Don't need but would be nice to have interface that defines that ApolloClient is a factory that creates ApolloCall. In case we ever want to migrate to another Client that can also produce ApolooCall

digitalbuddha · 2017-02-20T00:17:07Z

apollo-runtime/src/main/java/com/apollographql/android/HttpOperationRequest.java

+import okhttp3.RequestBody;
+import okio.Buffer;
+
+final class HttpOperationRequest<T extends Operation.Data> implements OperationRequest<T> {


Let's move all this to a RealApolloCall which implements ApolloCall, you can model that after okhttp3.RealCall which imlpements okhttp3.Call below

public interface Call extends Cloneable { /** Returns the original request that initiated this call. */ Request request(); /** * Invokes the request immediately, and blocks until the response can be processed or is in * error. * * <p>The caller may read the response body with the response's {@link Response#body} method. To * avoid leaking resources callers must {@linkplain ResponseBody close the response body}. * * <p>Note that transport-layer success (receiving a HTTP response code, headers and body) does * not necessarily indicate application-layer success: {@code response} may still indicate an * unhappy HTTP response code like 404 or 500. * * @throws IOException if the request could not be executed due to cancellation, a connectivity * problem or timeout. Because networks can fail during an exchange, it is possible that the * remote server accepted the request before the failure. * @throws IllegalStateException when the call has already been executed. */ Response execute() throws IOException; /** * Schedules the request to be executed at some point in the future. * * <p>The {@link OkHttpClient#dispatcher dispatcher} defines when the request will run: usually * immediately unless there are several other requests currently being executed. * * <p>This client will later call back {@code responseCallback} with either an HTTP response or a * failure exception. * * @throws IllegalStateException when the call has already been executed. */ void enqueue(Callback responseCallback); /** Cancels the request, if possible. Requests that are already complete cannot be canceled. */ void cancel(); /** * Returns true if this call has been either {@linkplain #execute() executed} or {@linkplain * #enqueue(Callback) enqueued}. It is an error to execute a call more than once. */ boolean isExecuted(); boolean isCanceled(); /** * Create a new, identical call to this one which can be enqueued or executed even if this call * has already been. */ Call clone(); interface Factory { Call newCall(Request request); } }

digitalbuddha · 2017-02-20T00:19:19Z

apollo-runtime/src/main/java/com/apollographql/android/OperationRequest.java

+
+import javax.annotation.Nullable;
+
+public interface OperationRequest<T extends Operation.Data> {


ApolloCall

brianPlummer · 2017-02-20T00:57:34Z

apollo-runtime/src/main/java/com/apollographql/android/HttpOperationRequest.java

+
+  private Response<T> parse(okhttp3.Response response) throws IOException {
+    int code = response.code();
+    if (code < 200 || code >= 300) {


you probably could use: https://square.github.io/okhttp/3.x/okhttp/okhttp3/Response.html#isSuccessful--

purposefully excluding 300 and up...my bad...

digitalbuddha

I like it :-)

digitalbuddha · 2017-02-20T01:10:55Z

apollo-api/src/main/java/com/apollographql/android/api/graphql/Response.java

@@ -18,6 +21,10 @@ public boolean isSuccessful() {
    return errors == null || errors.isEmpty();
  }

+  @Nonnull public Operation getOperation() {


nitpick - if there is no setOperation I don't think the get prefix is necessary

digitalbuddha · 2017-02-20T01:17:03Z

apollo-runtime/src/main/java/com/apollographql/android/RealApolloCall.java

+        .build();
+
+    Call call = callFactory.newCall(request);
+    if (callRef.compareAndSet(null, call)) {


is this necessary with implementation of call.execute being

@Override public Response execute() throws IOException { synchronized (this) { if (executed) throw new IllegalStateException("Already Executed"); executed = true; } captureCallStackTrace(); try { client.dispatcher().executed(this); Response result = getResponseWithInterceptorChain(); if (result == null) throw new IOException("Canceled"); return result; } finally { client.dispatcher().finished(this); } }

yes we need this as RealApolloCall keeps ref on okhttp3.Call in order to cancel request.

sav007 · 2017-02-20T02:48:39Z

apollo-gradle-plugin/src/main/groovy/com/apollographql/android/gradle/ApolloPlugin.groovy

@@ -42,7 +42,7 @@ class ApolloPlugin implements Plugin<Project> {
    project.getGradle().addListener(new DependencyResolutionListener() {
      @Override
      void beforeResolve(ResolvableDependencies resolvableDependencies) {
-        compileDepSet.add(project.dependencies.create("com.apollographql.android:api:${VersionKt.VERSION}"))
+        compileDepSet.add(project.dependencies.create("com.apollographql.android:api:0.2.1"))


Just to make plugin test happy, as now they are failing due to missing new snapshot verion

ok but can't merge this way or it will make snapshot like this

digitalbuddha · 2017-02-20T02:58:56Z

apollo-runtime/build.gradle

 }

 apply from: rootProject.file('gradle/gradle-mvn-push.gradle')
+
+tasks.withType(Checkstyle) {
+  exclude '**/BufferedSourceJsonReader.java'


why excluded?

Because checkstyle will fail and BufferedSourceJsonReader was copied from retrofit as is without modification (we need it for ResponseReader)

digitalbuddha · 2017-02-20T02:59:56Z

apollo-runtime/src/main/java/com/apollographql/android/ApolloCall.java

+
+  void cancel();
+
+  <T extends Operation.Data> Response<T> execute() throws IOException;


Nullability annotation on return value

digitalbuddha · 2017-02-20T03:00:09Z

apollo-runtime/src/main/java/com/apollographql/android/ApolloCall.java

+
+  <T extends Operation.Data> ApolloCall enqueue(@Nullable Callback<T> callback);
+
+  ApolloCall clone();


digitalbuddha · 2017-02-20T03:00:23Z

apollo-runtime/src/main/java/com/apollographql/android/ApolloCall.java

+  ApolloCall clone();
+
+  public interface Callback<T extends Operation.Data> {
+    void onResponse(Response<T> response);


nullability on both methods

digitalbuddha · 2017-02-20T03:00:36Z

apollo-runtime/src/main/java/com/apollographql/android/ApolloCallFactory.java

+import javax.annotation.Nonnull;
+
+public interface ApolloCallFactory {
+  <T extends Operation> ApolloCall newCall(@Nonnull T operation);


nullability

digitalbuddha · 2017-02-20T03:03:06Z

apollo-runtime/src/main/java/com/apollographql/android/RealApolloCall.java

+    }
+  }
+
+  @Override public <T extends Operation.Data> Response<T> execute() throws IOException {


nullability (I'm really calling out only public api methods)

digitalbuddha · 2017-02-20T03:03:15Z

apollo-runtime/src/main/java/com/apollographql/android/RealApolloCall.java

+    return parseHttpResponse(httpCall.execute());
+  }
+
+  @Override public <T extends Operation.Data> ApolloCall enqueue(final Callback<T> callback) {


digitalbuddha · 2017-02-20T03:04:52Z

👍 @brianPlummer mind taking a 1 over again?

brianPlummer · 2017-02-20T03:51:46Z

build.gradle

@@ -31,7 +29,8 @@ ext {
      spock: 'org.spockframework:spock-core:0.7-groovy-2.0',
      truth: 'com.google.truth:truth:0.30',
      mockWebServer: 'com.squareup.okhttp3:mockwebserver:3.5.0',
-      okhttpLoggingInterceptor: 'com.squareup.okhttp3:logging-interceptor:3.5.0'
+      okhttpLoggingInterceptor: 'com.squareup.okhttp3:logging-interceptor:3.5.0',


this should be 3.6.0 as well....feel free to leave it if you want...i can make this an okhttp version var

brianPlummer · 2017-02-20T03:53:00Z

+1 looks great! good work! :)

sav007 added 3 commits February 17, 2017 22:10

Drop support interface generation

e676879

Merge converter into runtime, drop retrofit

7c6077b

Merge branch 'master' into feature-216/move-converter

28bd472

sav007 requested review from martijnwalraven, felipecsl, brianPlummer, BenSchwab, digitalbuddha and marwanad February 19, 2017 23:15

sav007 added Status: Review Needed 🚗 Runtime labels Feb 19, 2017

sav007 commented Feb 19, 2017

View reviewed changes

sav007 added 2 commits February 19, 2017 18:22

Remove retrofit dependency

306b654

Add test for async call

f107439

digitalbuddha reviewed Feb 20, 2017

View reviewed changes

digitalbuddha approved these changes Feb 20, 2017

View reviewed changes

brianPlummer reviewed Feb 20, 2017

View reviewed changes

brianPlummer approved these changes Feb 20, 2017

View reviewed changes

sav007 added 6 commits February 19, 2017 20:08

Refactor

1380ee6

Refactor

91f0ff8

Refactor

5976625

Refactor

ee80b7c

Refactor

8304b94

Refactor

757db57

digitalbuddha approved these changes Feb 20, 2017

View reviewed changes

sav007 added 7 commits February 19, 2017 20:39

Refactor

748e9e1

Refactor

09fc696

Fix build

cc99ea2

Fix build

50f3142

Refactor and fix build

bde861f

Renaming

45e0e57

Checkstyle

f5a5937

sav007 commented Feb 20, 2017

View reviewed changes

digitalbuddha reviewed Feb 20, 2017

View reviewed changes

Add annotations

61a2c32

digitalbuddha reviewed Feb 20, 2017

View reviewed changes

Add annotations

1a49b4c

brianPlummer reviewed Feb 20, 2017

View reviewed changes

brianPlummer approved these changes Feb 20, 2017

View reviewed changes

sav007 added 2 commits February 19, 2017 23:20

Update

0275a88

Disable plugin test

10aabb3

sav007 merged commit 9b53ac8 into apollographql:master Feb 20, 2017

sav007 deleted the feature-216/move-converter branch February 20, 2017 06:10

marwanad removed the Status: Review Needed label Feb 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature 216/move converter #218

Feature 216/move converter #218

sav007 commented Feb 19, 2017 •

edited

Loading

sav007 Feb 19, 2017

sav007 Feb 19, 2017

sav007 Feb 19, 2017

sav007 Feb 19, 2017

digitalbuddha Feb 20, 2017

sav007 Feb 20, 2017

digitalbuddha Feb 20, 2017

digitalbuddha Feb 20, 2017 •

edited

Loading

digitalbuddha Feb 20, 2017

brianPlummer Feb 20, 2017

brianPlummer Feb 20, 2017

digitalbuddha left a comment

digitalbuddha Feb 20, 2017

digitalbuddha Feb 20, 2017

sav007 Feb 20, 2017

sav007 Feb 20, 2017 •

edited

Loading

digitalbuddha Feb 20, 2017

digitalbuddha Feb 20, 2017

sav007 Feb 20, 2017

digitalbuddha Feb 20, 2017

digitalbuddha Feb 20, 2017

digitalbuddha Feb 20, 2017

digitalbuddha Feb 20, 2017

digitalbuddha Feb 20, 2017

digitalbuddha Feb 20, 2017

digitalbuddha commented Feb 20, 2017

brianPlummer Feb 20, 2017

brianPlummer commented Feb 20, 2017


		import javax.annotation.Nullable;

		public interface OperationRequest<T extends Operation.Data> {


		void cancel();

		<T extends Operation.Data> Response<T> execute() throws IOException;


		<T extends Operation.Data> ApolloCall enqueue(@Nullable Callback<T> callback);

		ApolloCall clone();

Feature 216/move converter #218

Feature 216/move converter #218

Conversation

sav007 commented Feb 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

digitalbuddha Feb 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

digitalbuddha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sav007 Feb 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

digitalbuddha commented Feb 20, 2017

Choose a reason for hiding this comment

brianPlummer commented Feb 20, 2017

sav007 commented Feb 19, 2017 •

edited

Loading

digitalbuddha Feb 20, 2017 •

edited

Loading

sav007 Feb 20, 2017 •

edited

Loading