leangen / graphql-spqr

Build a GraphQL service in seconds
Apache License 2.0
1.09k stars 179 forks source link

Schema Generation Regression in 0.12.4 #486

Open austinarbor opened 5 months ago

austinarbor commented 5 months ago

Hello, I think a regression has been added with the 0.12.4 release

Using the below models, on 0.12.3 I get the following schema:

type AnotherModel {
  id: Long
}

"Mutation root"
type Mutation {
  createMappingGroup(arg0: SomeModelInputInput): SomeModel
}

"Query root"
type Query {
  mappingGroup: SomeModel
}

type SomeModel {
  anotherModels: [AnotherModel]
  id: Int
}

"A 64-bit signed integer"
scalar Long

input AnotherModelInputInput {
  id: Long
}

input SomeModelInputInput {
  anotherModels: [AnotherModelInputInput]
  id: Int
}

However, using the same models on 0.12.4, I get an error during schema generation:

Exception in thread "main" java.lang.IllegalArgumentException: Conflicting setter definitions for property "anotherModels": dev.aga.model.SomeModelInput#setAnotherModels(java.util.Set) vs dev.aga.model.SomeModelInput#setAnotherModelInputs(java.util.List)

Is the @GraphQLIgnore no longer being applied, or is there something else going on?

@Data
@Accessors(chain = true)
@ToString(callSuper = true, onlyExplicitlyIncluded = true)
public class SomeModel implements Serializable {

  @Serial
  private static final long serialVersionUID = 1L;

  @ToString.Include
  @EqualsAndHashCode.Include
  @GraphQLQuery(name = "id")
  protected Integer id;

  @GraphQLQuery(name = "anotherModels")
  protected Set<AnotherModel> anotherModels;
}

@Data
@EqualsAndHashCode(callSuper = true)
public class SomeModelInput extends SomeModel {

  protected List<AnotherModelInput> anotherModelInputs;

  @GraphQLQuery(name = "id")
  @Override
  public SomeModelInput setId(Integer id) {
    this.id = id;
    return this;
  }

  @GraphQLQuery(name = "anotherModels")
  public SomeModelInput setAnotherModelInputs(List<AnotherModelInput> mappingRuleInputs) {
    this.anotherModelInputs = mappingRuleInputs;
    return this;
  }

  public List<AnotherModelInput> getAnotherModelInputs() {
    return this.anotherModelInputs;
  }

  @GraphQLIgnore
  @Override
  public SomeModelInput setAnotherModels(Set<AnotherModel> anotherModels) {
    this.anotherModels = anotherModels;
    return this;
  }
}

@Data
public class AnotherModel {
  private Long id;
}

@Data
public class AnotherModelInput {
  private Long id;
}
kaqqao commented 5 months ago

Oof. This is a catch 22 type of situation. In earlier versions, SPQR had its own logic for detecting bean property elements (related field, getter, setter, constructor parameter), which was causing problems. Version 0.12.4 more directly delegates to Jackson to detect these elements, which it then filters based on its own inclusion rules (e.g. skip elements with @GraphQLIgnore). And here Jackson detects a conflict and throws an exception before SPQR even gets a chance to filter the ignored element out. As a quick workaround, you can add @JsonIgnore as that will make Jackson skip the element and avoid the conflict altogether. And I'll see how to improve @GraphQLIgnore handling to prevent conflicts like this...