Repository: square/retrofit
Issue: MoshiConverterFactory should have a lenient option

https://github.com/square/moshi/issues/132


Issue comments:
What are thoughts here?
Should all adapters be lenient or not lenient?
I suppose if you want only certain adapters to be lenient, you can easily make your own factory.

Definitely not be default

On Fri, Feb 12, 2016, 6:27 PM Eric Cochran notifications@github.com wrote:

> What are thoughts here?
> Should all adapters be lenient or not lenient?
> I suppose if you want only certain adapters to be lenient, you can easily
> make your own factory.
> 
> —
> Reply to this email directly or view it on GitHub
> https://github.com/square/retrofit/issues/1592#issuecomment-183526800.

I think I'd do an asLenient() that returned a new lenient factory from an
existing one.

On Fri, Feb 12, 2016, 6:39 PM Jake Wharton jakewharton@gmail.com wrote:

> Definitely not be default
> 
> On Fri, Feb 12, 2016, 6:27 PM Eric Cochran notifications@github.com
> wrote:
> 
> > What are thoughts here?
> > Should all adapters be lenient or not lenient?
> > I suppose if you want only certain adapters to be lenient, you can easily
> > make your own factory.
> > 
> > —
> > Reply to this email directly or view it on GitHub
> > https://github.com/square/retrofit/issues/1592#issuecomment-183526800.

Right, not by default, but it will be a factory that makes all adapters lenient. I'm not sure if that's truly useful, but perhaps.

fb94144db40f8d403ba57799b61e9547cb349b34


Pull request review comments:
This doesn't need to exist. Just call the constructor.

I would just delete this `if`. It'll never happen and even if it does it's not that big of a deal to create a new instance.


Patch that resolved the issue:
diff --git a/retrofit-converters/moshi/src/main/java/retrofit2/converter/moshi/MoshiConverterFactory.java b/retrofit-converters/moshi/src/main/java/retrofit2/converter/moshi/MoshiConverterFactory.java
index 1a98a78aa4..bf5a7bce69 100644
--- a/retrofit-converters/moshi/src/main/java/retrofit2/converter/moshi/MoshiConverterFactory.java
+++ b/retrofit-converters/moshi/src/main/java/retrofit2/converter/moshi/MoshiConverterFactory.java
@@ -40,20 +40,29 @@ public static MoshiConverterFactory create() {
 
   /** Create an instance using {@code moshi} for conversion. */
   public static MoshiConverterFactory create(Moshi moshi) {
-    return new MoshiConverterFactory(moshi);
+    return new MoshiConverterFactory(moshi, false);
   }
 
   private final Moshi moshi;
+  private final boolean lenient;
 
-  private MoshiConverterFactory(Moshi moshi) {
+  private MoshiConverterFactory(Moshi moshi, boolean lenient) {
     if (moshi == null) throw new NullPointerException("moshi == null");
     this.moshi = moshi;
+    this.lenient = lenient;
+  }
+
+  public MoshiConverterFactory asLenient() {
+    return new MoshiConverterFactory(moshi, true);
   }
 
   @Override
   public Converter<ResponseBody, ?> responseBodyConverter(Type type, Annotation[] annotations,
       Retrofit retrofit) {
     JsonAdapter<?> adapter = moshi.adapter(type);
+    if (lenient) {
+      adapter = adapter.lenient();
+    }
     return new MoshiResponseBodyConverter<>(adapter);
   }
 
@@ -61,6 +70,9 @@ private MoshiConverterFactory(Moshi moshi) {
   public Converter<?, RequestBody> requestBodyConverter(Type type,
       Annotation[] parameterAnnotations, Annotation[] methodAnnotations, Retrofit retrofit) {
     JsonAdapter<?> adapter = moshi.adapter(type);
+    if (lenient) {
+      adapter = adapter.lenient();
+    }
     return new MoshiRequestBodyConverter<>(adapter);
   }
 }


Test patch:
diff --git a/retrofit-converters/moshi/src/test/java/retrofit2/converter/moshi/MoshiConverterFactoryTest.java b/retrofit-converters/moshi/src/test/java/retrofit2/converter/moshi/MoshiConverterFactoryTest.java
index 2a85e58186..8583529fa4 100644
--- a/retrofit-converters/moshi/src/test/java/retrofit2/converter/moshi/MoshiConverterFactoryTest.java
+++ b/retrofit-converters/moshi/src/test/java/retrofit2/converter/moshi/MoshiConverterFactoryTest.java
@@ -30,11 +30,12 @@
 import retrofit2.Call;
 import retrofit2.Response;
 import retrofit2.Retrofit;
-import retrofit2.converter.moshi.MoshiConverterFactory;
 import retrofit2.http.Body;
 import retrofit2.http.POST;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
 public final class MoshiConverterFactoryTest {
   interface AnInterface {
@@ -85,16 +86,24 @@ interface Service {
   @Rule public final MockWebServer server = new MockWebServer();
 
   private Service service;
+  private Service serviceLenient;
 
   @Before public void setUp() {
     Moshi moshi = new Moshi.Builder()
         .add(new AnInterfaceAdapter())
         .build();
+    MoshiConverterFactory factory = MoshiConverterFactory.create(moshi);
+    MoshiConverterFactory factoryLenient = factory.asLenient();
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl(server.url("/"))
-        .addConverterFactory(MoshiConverterFactory.create(moshi))
+        .addConverterFactory(factory)
+        .build();
+    Retrofit retrofitLenient = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(factoryLenient)
         .build();
     service = retrofit.create(Service.class);
+    serviceLenient = retrofitLenient.create(Service.class);
   }
 
   @Test public void anInterface() throws IOException, InterruptedException {
@@ -122,4 +131,28 @@ interface Service {
     assertThat(request.getBody().readUtf8()).isEqualTo("{\"theName\":\"value\"}");
     assertThat(request.getHeader("Content-Type")).isEqualTo("application/json; charset=UTF-8");
   }
+
+  @Test public void asLenient() throws IOException, InterruptedException {
+    MockResponse malformedResponse = new MockResponse().setBody("{\"theName\":value}");
+
+    try {
+      server.enqueue(malformedResponse);
+      Call<AnImplementation> call = service.anImplementation(new AnImplementation("value"));
+      call.execute();
+      fail();
+    } catch (IOException e) {
+      assertEquals(e.getMessage(),
+          "Use JsonReader.setLenient(true) to accept malformed JSON at path $.theName");
+    }
+
+    server.enqueue(malformedResponse);
+    Call<AnImplementation> call = serviceLenient.anImplementation(new AnImplementation("value"));
+    Response<AnImplementation> response = call.execute();
+    AnImplementation body = response.body();
+    assertThat(body.theName).isEqualTo("value");
+
+    RecordedRequest request = server.takeRequest();
+    assertThat(request.getBody().readUtf8()).isEqualTo("{\"theName\":\"value\"}");
+    assertThat(request.getHeader("Content-Type")).isEqualTo("application/json; charset=UTF-8");
+  }
 }