Moshi: Best way to deal with broken remote API

[Tolriq]

Hi,

I'm facing the need to deal with a broken API and tried to look in the documentation for ways to deal with that without writing a low level JsonReader for the large object.

Basically the JSON should returns arrays for some fields, but on some case that I can't identify without data in that same object, the fields are returned as String.

I'd like to avoid manual parsing for that object that is very large and complex and it would require some kind of https://github.com/square/moshi/pull/287 for a proper handling.

It also seems JsonQualifier needs to know the type in advance to be used.

Is there a better way to deal with that kind of broken API, like an annotation to ignore field on error. Like @TransientOnError or other kind of high level workaround I can try?

Event a JsonQualifier that take a JsonReader would be cool.

Maybe one of those things is already, in or could be in as feature request?

[swankjesse]

Tough situation.

You might have success building a JsonAdapter that either skips or delegates, depending on what peek() returns. If you can get that to work for one type, you might be able to use a JsonAdapter.Factory to generalize it for many types.

Another option is to use readJsonValue() to strip out or patch the offending values. Then pass the patched object to your JsonAdapter.

[JakeWharton]

I've done a lot of this. Here's an example you can potentially adapt:

import com.squareup.moshi.JsonAdapter;
import com.squareup.moshi.JsonReader;
import com.squareup.moshi.JsonWriter;
import com.squareup.moshi.Moshi;
import com.squareup.moshi.Types;
import java.io.IOException;
import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.util.List;
import java.util.Set;

import static com.squareup.moshi.JsonReader.Token.BEGIN_ARRAY;

/** Converts empty JSON arrays to null for non-collection types. Use for poorly-implemented APIs. */
public final class EmptyListToNull implements JsonAdapter.Factory {
  @Override
  public JsonAdapter<?> create(Type type, Set<? extends Annotation> annotations, Moshi moshi) {
    Class<?> rawType = Types.getRawType(type);
    if (List.class.isAssignableFrom(rawType)
        || Set.class.isAssignableFrom(rawType)
        || rawType.isArray()) {
      return null; // We don't want to decorate actual collection types.
    }

    final JsonAdapter<Object> delegate = moshi.nextAdapter(this, type, annotations);
    return new JsonAdapter<Object>() {
      @Override public Object fromJson(JsonReader reader) throws IOException {
        if (reader.peek() != BEGIN_ARRAY) {
          return delegate.fromJson(reader);
        }

        reader.beginArray();
        reader.endArray();
        return null;
      }

      @Override public void toJson(JsonWriter writer, Object value) throws IOException {
        delegate.toJson(writer, value);
      }
    };
  }
}

[Tolriq]

Thanks both for quick answering.

For the moment I went with @JakeWharton factory solution.

    @Override
    public JsonAdapter<?> create(Type type, Set<? extends Annotation> annotations, Moshi moshi) {
        final Class<?> rawType = Types.getRawType(type);
        if (!List.class.isAssignableFrom(rawType)
                && !Set.class.isAssignableFrom(rawType)
                && !rawType.isArray()) {
            return null; // We only handle arrays
        }
        final JsonAdapter<Object> delegate = moshi.nextAdapter(this, type, annotations);
        return new JsonAdapter<Object>() {
            @Override
            public Object fromJson(JsonReader reader) throws IOException {
                JsonReader.Token peek = reader.peek();
                if (peek != JsonReader.Token.BEGIN_ARRAY) {
                    Logger.logError(TAG, "Skipping bad value at path: %s [%s for %s]", reader.getPath(), peek, JsonReader.Token.BEGIN_ARRAY);
                    reader.skipValue();
                    return null;
                }
                return delegate.fromJson(reader);
            }

            @Override
            public void toJson(JsonWriter writer, Object value) throws IOException {
                delegate.toJson(writer, value);
            }
        };
    }

It works well, but I'd like to control the fields that this hack is applied to, to keep control of the errors and not hide other potential real errors.

If I use standard annotations it seems they are not passed to the factory annotations and if I use @JsonQualifier then I'm back to the problem of defined type. The thing is that the problematic fields can be arrays of many different things depending on the field.

Ideally I'd like to annotate the field with some @IgnoreJsonArrayError and be able to get that annotation in the factory create.

Edit: And a small note for other users who read: Do no play with factories and Instant Run, most of the changes will not work correctly. Added to speed issues of #208.
Maybe this worth a note somewhere as it's easy to loose a lot of time for this.

[swankjesse]

Yup. Can you define @IgnoreJsonArrayError, annotate it as a qualifier annotation, and look for it in your factory?

[Tolriq]

@swankjesse I may be missing something but then nextAdapter errors saying

java.lang.IllegalArgumentException: No next JsonAdapter for java.util.List<xxx.api.model.Video$Cast> annotated [@xxxx.api.IgnoreStringForArrays$IgnoreErrors()]

Meaning I can't delegate after no? And since I'm dealing with List/Arrays I need the delegate to parse the actual objects inside the Arrays when API is correct.

Or I completely miss something.

[swankjesse]

Try calling Types.nextAnmotations() to strip the annotation before you delegate.

Also, since you have an annotation you can use adapter() and not nextAdapter() and Moshi won’t recurse infinitely.

[Tolriq]

Thanks Types.nextAnnotations() was the missing piece.

Calling adapter() did end up with infinite recursive call and stack overflow.

Thanks a lot for the quick help, discovering this remote API bug after release to production does not help to think properly and you end up trying too many things at the same time.

Final code for reference as it may be useful to others.

public final class IgnoreStringForArrays implements JsonAdapter.Factory {

    @Retention(RetentionPolicy.RUNTIME)
    @JsonQualifier
    public @interface IgnoreJsonArrayError {
    }

    @Override
    public JsonAdapter<?> create(Type type, Set<? extends Annotation> annotations, Moshi moshi) {
        if (annotations != null && annotations.size() > 0) {
            for (Annotation annotation : annotations) {
                if (annotation instanceof IgnoreJsonArrayError) {
                    final JsonAdapter<Object> delegate = moshi.nextAdapter(this, type, Types.nextAnnotations(annotations, IgnoreJsonArrayError.class));
                    return new JsonAdapter<Object>() {
                        @Override
                        public Object fromJson(JsonReader reader) throws IOException {
                            JsonReader.Token peek = reader.peek();
                            if (peek != JsonReader.Token.BEGIN_ARRAY) {
                                reader.skipValue();
                                return null;
                            }
                            return delegate.fromJson(reader);
                        }

                        @Override
                        public void toJson(JsonWriter writer, Object value) throws IOException {
                            delegate.toJson(writer, value);
                        }
                    };
                }
            }
        }
        return null;
    }
}

[shakil807g]

can i use same thing for objects instead of array ?

[NightlyNexus]

Yes. BEGIN_ARRAY => BEGIN_OBJECT.
I don't think there's anything for Moshi to do here, so closing.