Port Java builder code from Apollo Android

[dwbutler]

Ports Java compiler builder code from Apollo Android.

Resolves #4312

[apollo-cla]

@dwbutler: 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/

[netlify]

👷 Deploy request for apollo-android-docs pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	3c3c0d6

[martinbonnin]

Add the present() helper for symmetry?

    @JvmStatic
    fun <T> present(value: T): Optional<T> = Present(value)

[dwbutler]

Might as well! java.util.Optional (the implementation I am most familiar with) defines of and ofNullable

[martinbonnin]

From the javadoc of of:

Returns an Optional with the specified present non-null value.

That's really not what we want because we want to allow present(null) so I'd stick with:

Optional.absent()
Optional.present(value: T)
Optional.presentIfNotNull(value:T)

[dwbutler]

Does Optional.present(null) make sense given that the parameter is defined to be not null? I think throwing a NullPointerException makes sense. (that's what java.util.Optional#of does)

[martinbonnin]

given that the parameter is defined to be not null?

The parameter is definitely nullable. If we wanted to enforce non-null, we'd put a Any upper bound:

// enforce non-null value (which we don't want)
fun <T: Any> present(value: T): Optional<T> = Present(value)

// allow null (which we want)
fun <T> present(value: T): Optional<T> = Present(value)

[dwbutler]

Ah, my misunderstanding of generics in Kotlin I guess. I thought T? would allow null, and T would not.

[martinbonnin]

I guess the rationale is that T in itself can be of any Kotlin type, including nullable ones. It allows to do stuff like this:

fun <T:Any> assertNotNull(value: T?): T

So T without the Any upper bound can be nullable but once you put the Any upper bound, you have to type T? to make it nullable

[dwbutler]

@martinbonnin How does the generateDataBuilders option differ from the ported generateModelBuilder option?

[martinbonnin]

generateDataBuilders generate data based on the schema and not the models. This is especially useful for fragments:

Assuming something like this:

query GetHero {
   hero {
      name
      ...droidFragment
   } 
}

fragment droidFragment on Droid {
   name
   primaryFunction
}

The code for each solution is going to look like below (not sure about the exact details but that should give the high level overview):

generateDataBuilders:

    BuilderFactory factory = BuilderFactory.DEFAULT
    GetHeroQuery.Data data = GetHeroQuery.buildData(
        factory.buildQuery()
            .hero(
                factory.buildDroid()
                    .name("R2-D2")
                    .primaryFunction("translation")
                    .build()
                )
            )
            .build()
      );

generateModelBuilders:

    GetHeroQuery.Data data = GetHeroQuery.builder()
       .hero(
          Hero.builder()
                .__typename("Droid")
                .name("R2-D2")
                .droidDetails(
                   DroidDetails.builder()
                      .__typename("Droid")
                      .name("R2-D2")
                      .primaryFunction("translation")
                      .build()
                 )
                .build()
       )
       .build()

generateDataBuilders has some initial overhead (the factory) but will be simpler as more fragments are involved

[dwbutler]

I see, it's a more fluent interface. Is this the right time to update documentation BTW? Or should that be a separate PR?

[martinbonnin]

Documentation should go in a separate MR so that we can hold the documentation updates until the version is actually released.

[martinbonnin]

Something else about data builders is that they work with FakeResolver to provide default values for model fields that haven't been set.

[martinbonnin]

Is there a specific reason you call unwrapOptionalValue here? Looks like it's only required for Optional<T> values which are not generated right now (see also #4337)

Even if Optional were generated, I think it's fine to have the builder field have the same type as the model? It's more consistent?

  public static class Animal {
    public Optional<String> name;
  }

  public static final class Builder {
    private Optional<String> name;
  }

[martinbonnin]

I guess it boils down to whether we want to be able to pass null to a previously non-null builder:

Data.Builder builder = new Data.Builder();

builder.name(Optional.of("Luke"));

// ... some code

// If we want to be able to do this ultimately, we'll need to keep the  `Optional` wrapper
builder.name(Optional.ofNullable(null))

[dwbutler]

TBH I just copied this code from Apollo Android without trying to understand it too much. 😅 I'll remove it and see if it has any effect.

[martinbonnin]

A few comments/nitpickings on the style. Given that we're aiming for a better way to address
that use case, I'll close this PR but feel free to continue the discussion here if anything is unclear

Nevermind, I commented in the wrong PR. This was meant for #4362

[martinbonnin]

Closing as it was merged with #4364. Thanks again!