Skip to content
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

@NonNull support for fields #77

Closed
Turbo87 opened this issue Sep 10, 2015 · 6 comments
Closed

@NonNull support for fields #77

Turbo87 opened this issue Sep 10, 2015 · 6 comments

Comments

@Turbo87
Copy link

Turbo87 commented Sep 10, 2015

It would be cool if Moshi would throw an exception if a field was annotated with @NonNull but fromJson() would assign null to it.

@JakeWharton
Copy link
Collaborator

You can do this yourself!

public final class NonNullJsonAdapterFactory implements JsonAdapter.Factory {
  @Override
  public JsonAdapter<?> create(Type type, Set<? extends Annotation> annotations, Moshi moshi) {
    for (Annotation annotation : annotations) {
      if ("NonNull".equals(annotation.getClass().getSimpleName())) { // Or NonNull.class.equals(..)
        JsonAdapter<Object> delegate = moshi.nextAdapter(this, type, annotations);
        return new NonNullJsonAdapter<>(delegate);
      }
    }
    return null;
  }

  static class NonNullJsonAdapter<T> extends JsonAdapter<T> {
    private final JsonAdapter<T> delagate;

    NonNullJsonAdapter(JsonAdapter<T> delagate) {
      this.delagate = delagate;
    }

    @Override public T fromJson(JsonReader reader) throws IOException {
      T value = delagate.fromJson(reader);
      if (value == null) {
        throw new IllegalStateException("value was null!");
      }
      return value;
    }

    @Override public void toJson(JsonWriter writer, T value) throws IOException {
      if (value == null) {
        throw new IllegalStateException("value was null!");
      }
      delagate.toJson(writer, value);
    }
  }
}

Putting validation inside Moshi is a dangerous precedent since it has no end to the types of validation people want.

@Turbo87
Copy link
Author

Turbo87 commented Sep 10, 2015 via email

@JakeWharton
Copy link
Collaborator

Yep! That'd make for a good one. I don't think you're the first to ask
about it.

On Thu, Sep 10, 2015 at 4:36 PM Tobias Bieniek notifications@github.com
wrote:

Ah cool, I didn't know that was possible. I guess that's one more reason
for a recipe/samples project or webpage.


Reply to this email directly or view it on GitHub
#77 (comment).

@Turbo87
Copy link
Author

Turbo87 commented Sep 11, 2015

@JakeWharton I just tried your code but it didn't seem to work as I had expected. What I tried was roughly the following:

public static class TestModel {
    @NonNull String a;
    String b;
}

Moshi moshi = new Moshi.Builder()
        .add(new NonNullJsonAdapterFactory())
        .build();

JsonAdapter<TestModel> adapter = moshi.adapter(TestModel.class);

// should work
assertThat(adapter.fromJson("{\"a\":\"test\"}").a).isEqualTo("test");

// should fail
adapter.fromJson("{}");

I was expecting that the second fromJson() call would throw the IllegalStateException, but it basically worked just like before. From my limited debugging it seems like the NonNullJsonAdapterFactory#create() method gets the annotations of the TestModel class itself (annotations parameter is empty), but not of its members, so annotating the members doesn't seem to have any effect.

Am I missing something?

@swankjesse
Copy link
Collaborator

I think it's the difference between rejecting null values and requiring non-null values. The example doesn't do anything about absent keys!

@Turbo87
Copy link
Author

Turbo87 commented Sep 11, 2015

adapter.fromJson("{\"a\":null}");

the above call is not throwing anything either...

but you are right, what I actually want to test is whether the API response included the field and that it is non-null.

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

3 participants