SPQR Dataloder doesn't allow null values - Exception while fetching data : null

[BenNeighbour]

@kaqqao @BaldyLocks @vkachan @csueiras @kicktipp Hi all,

I have set up some batch DataLoaders for each entity (that have relationships with others) in order to solve the N+1 Problem with GraphQL/SPQR - and it works. HOWEVER, when I have a @OneToOne or @ManyToMany relation on some entity that is null, an Exception while fetching data (/getAllEmployees[6]/company) : null.

Here is the resolver for the company field of each employee:

@GraphQLQuery(name = "company")
public CompletableFuture<Company> getCompany(
        @GraphQLContext @Valid @GraphQLNonNull Employee employee, @GraphQLEnvironment ResolutionEnvironment env) {
  DataLoader<UUID, Company> loader = env.dataFetchingEnvironment.getDataLoader("company");
  return loader.load(employee.getCompany().getId());
}

And the DataFetcher,

@Component
public class DataLoaderFactory implements DataLoaderRegistryFactory {

  private static CompanyDao companyDao;
  private static final BatchLoader<UUID, Company> companyLoader = DataLoaderFactory::companies;
  /* ...... */

  @Autowired
  public DataLoaderFactory(CompanyDao companyDao, /* ...... */) {
    DataLoaderFactory.companyDao = companyDao;
    /* ...... */
  }

  public static CompletableFuture<List<Company>> companies(List<UUID> ids) {
    return CompletableFuture.completedFuture(companyDao.findAllById(ids));
  }

  /* ...... */

  @Override
  @Bean
  public DataLoaderRegistry createDataLoaderRegistry() {
    DataLoaderRegistry allLoaders = new DataLoaderRegistry();
    allLoaders.register("company", new DataLoader<>(companyLoader));
    /* ...... */

    return allLoaders;
  }
}

And for some context, here are the entities:

For the Employee

@Setter
@Getter
@Entity
@NoArgsConstructor
@AllArgsConstructor
public class Employee implements Serializable {

  private static final long serialVersionUID = 684733882540759135L;

  @Id
  @GeneratedValue
  @Column(columnDefinition = "uuid", updatable = false)
  @GraphQLQuery(name = "id", description = "A Person's Id")
  private UUID id;

  @NotNull(message = "There must be a Person's Name!")
  @GraphQLQuery(name = "fullName", description = "A Person's Name")
  private String fullName;

  @NotNull(message = "A Person must have an Age!")
  @GraphQLQuery(name = "age", description = "A Person's Age")
  private int age;

  @NotNull(message = "A Person must have a Vehicle!")
  @GraphQLQuery(name = "personalVehicle", description = "A Person's Mode of Transport")
  private Vehicle personalVehicle;

  @ManyToOne(targetEntity = Company.class, fetch = FetchType.LAZY, cascade = CascadeType.ALL)
  @GraphQLQuery(name = "company", description = "The Company a Person works for")
  private Company company;

  @GraphQLQuery(name = "roles", description = "The Role a Person plays in their Company")
  @OneToMany(
      targetEntity = Role.class,
      orphanRemoval = true,
      fetch = FetchType.LAZY,
      mappedBy = "employee",
      cascade = CascadeType.ALL)
  private List<Role> roles = new ArrayList<>();

  public enum Vehicle {
    CAR,
    BUS,
    VAN,
    BICYCLE,
    MOTORBIKE,
    SCOOTER
  }
}

For the Company

@Setter
@Getter
@Entity
@NoArgsConstructor
@AllArgsConstructor
public class Company implements Serializable {

  private static final long serialVersionUID = -6007975840330441233L;

  @Id
  @GeneratedValue
  @Column(columnDefinition = "uuid", updatable = false)
  @GraphQLQuery(name = "id", description = "A Company's Id")
  private UUID id;

  @NotNull(message = "There must be a Company Name!")
  @GraphQLQuery(name = "name", description = "A Company's Name")
  private String name;

  @Min(10000)
  @NotNull(message = "You gotta tell us how rich you are!")
  @GraphQLQuery(name = "balance", description = "A Company's Dollar")
  private BigDecimal balance;

  @NotNull(message = "There must be a Company Type!")
  @GraphQLQuery(name = "type", description = "The type of company")
  private CompanyType type;

  @GraphQLQuery(name = "offices", description = "The Company's offices")
  @OneToMany(
      targetEntity = Office.class,
      orphanRemoval = true,
      fetch = FetchType.LAZY,
      mappedBy = "company",
      cascade = CascadeType.ALL)
  private List<Office> offices;

  public enum CompanyType {
    PRIVATE_LIMITED,
    SOLE_TRADER,
    PUBLIC
  }
}

Any help, comments or clarification would be much, much appreciated - I'm probably missing out something stupid!!

[BenNeighbour]

Could I get some help please?

[BenNeighbour]

@kaqqao ?

[BenNeighbour]

Also my build.gradle dependency for this is: implementation "io.leangen.graphql:graphql-spqr-spring-boot-starter:0.0.6"

[kaqqao]

Pheeew, this is a rather involved scenario that I really can't answer it without investigation. There's nothing in SPQR that deals with DataLoader in any meaningful way. That is to say, you should be in the exact same situation if you were using DataLoader without SPQR. So whatever is happening likely isn't really SPQR specific.
That said, the Java impl of DataLoader has been a bit of mine field... so it may well be a bug in there. I think I even vaguely remember it freaking out on nulls. Try searching their issues.
Can you maybe try using their latest release? I believe there was a big release recently. If I'm wrong, please try building their master locally and using that. If it breaks compatibility, let me know and I'll have a look what changes are needed in the starter.

[BenNeighbour]

@kaqqao Yeah will do. I'm just on their official GitHub Docs trynna find out some useful info about null handing. I did end up fixing it about 2 days ago but I had to do extra iterations on the DB Query's result to bucket it into a list (that had the same number of elements) - because that's what the dataLoader.load(....) expects.

This would mean that any "nulls" in the releationships would be returned as just empty lists - which is okay but would prefer for it to just be null in some situations.

public static CompletableFuture<List<List<Role>>> loadRoles(List<UUID> ids) {
  /* Create a list (with multiple sub-lists) of the same size as the input */
  List<List<Role>> bucketedList =
      new ArrayList<>(Collections.nCopies(ids.size(), new ArrayList<>()));

  /* Batch load all the companies that contain the given ids */
  roleDao.findAllByEmployeeIdIn(ids).stream()
      /* Convert to some sort of Map */
      .flatMap(Collection::stream)
      /* Group the hashmap by the company ids */
      .collect(Collectors.groupingBy(role -> role.getEmployee().getId()))
      /* For each item that maps to an id, group it into the right "bucket" */
      .forEach(
          (uuid, roles) -> {
            bucketedList.set(ids.indexOf(uuid), roles);
          });

  /* Return the ordered list as a completed future */
  return CompletableFuture.completedFuture(bucketedList);
}

You'll have to refer to the context in the first comment (the Entities and their relations) for this specific DataFetcher to make sense. I feel like there's a better way of doing this - without having to create "n" copies (to make sure the size is the same as the input - as that's what the DataLoader expects).

The whole bucketing thing is hacky and I'm not sure how that - in combination with the forEach at the end - will affect the performance of the function. Thanks for your feedback @kaqqao I really like your framework - helps us do a lot with a little!

[BenNeighbour]

I mean, it's A solution - but I wouldn't want to settle on this for both Code Quality and performance

[BenNeighbour]

Because it's never "too bad" for @OneToOne relationships, it only becomes a problem when you have any @OneToMany or @ManyToOne's that you wanna eliminate N+1 for (which why wouldn't you), and you have to batch load a list of relations by a list of ids and the return list has to be the same size as the input list. Again this must be an internal thing within java-dataloader so I'm checking it out now

[BenNeighbour]

But won't using their latest release conflict with the peer dependency of your SPQR Starter?

[kaqqao]

I was hoping it would be compatible enough not to cause issues when you override the version. But after looking into the comments, issues and PRs on the DataLoader project, I'm not exactly hopeful any longer.
I personally find DataLoader to be too messy for its worth and tend to stay away from it, so I recommend you consider simpler approaches to batching e.g. looking ahead and prefetching using the LocalContext.