Netflix / dgs-framework

GraphQL for Java with Spring Boot made easy.
https://netflix.github.io/dgs
Apache License 2.0
3.03k stars 286 forks source link

bug: Java Optional<> - Failed to convert from type LinkedHasMap<?, ?> to type #1925

Closed filippocozzini closed 6 days ago

filippocozzini commented 1 month ago

Hello everyone. I need to retrieve data from a graphql db and load it into a SetPointParametersInput object. When I try to convert it to Optional, it gives the following error:

**Failed to convert from type [java.util.LinkedHashMap<?, ?>] to type [@com.netflix.graphql.dgs.InputArgument @jakarta.validation.Valid cloud.smartfish.writedown.inputs.SetPoint.SetPointInput] for value [{...}]**

It works without the Optional conversion. Bean of Jackson Object Mapper is setted. If i convert with ObjectMapper.convertValue function all works.

SetPointInput

@Getter
@Setter
@NoArgsConstructor
@AllArgsConstructor
@SuperBuilder(toBuilder = true)
public class SetPointParametersInput {

    private Optional<String> idFeedTable;

    private Optional<SetPointParameters.Rule> feeding;

    private Optional<SetPointParameters.Rule> conversion;

    private SetPointParameters.TimedTriggerRule loss;

    private Optional<SetPointParameters.TimedTriggerRule> selfGrow;

    private SetPointParameters.TimedTriggerRule starvation;

    private Optional<String> idBreedingStage;

    private Optional<String> idBreedingProcess;

    private Optional<String> idGradingStage;

    private Optional<String> idPathologicStage;

}
@Getter
    @Setter
    @NoArgsConstructor
    @AllArgsConstructor
    @SuperBuilder(toBuilder = true)
    public static class Rule implements Serializable {

        private String id;

        private RuleType ruleType;

    }

    @Getter
    @Setter
    @NoArgsConstructor
    @AllArgsConstructor
    @SuperBuilder(toBuilder = true)
    public static class TimedTriggerRule extends Rule {

        private Duration triggerAfter;

        private Duration frequency;

        public Duration getMaxPeriod() {
            return triggerAfter.getSeconds() >= frequency.getSeconds() ? triggerAfter : frequency;
        }

    }
    public Boolean isSetPointParametersEmpty() {
        return  this.getIdFeedTable() == null &&
                this.getFeeding() == null &&
                this.getConversion() == null &&
                this.getLoss() == null &&
                this.getSelfGrow() == null &&
                this.getStarvation() == null &&
                this.getIdBreedingStage() == null &&
                this.getIdBreedingProcess() == null &&
                this.getIdGradingStage() == null &&
                this.getIdPathologicStage() == null;
    }
srinivasankavitha commented 1 month ago

Hi - Thanks for reporting. Using Optionals should generally work and I confirmed it works fine, in general, with a much simpler schema. To help debug this better, could you pinpoint the field that is causing the issue, along with the input values? It would also be really helpful to simplify the number of fields in this example you provided to identify where the problem is occurring.

filippocozzini commented 1 month ago

Thank you fast reply.

So, the fields that generate that error are (basically all fields with custom class type, for example string works fine):

private Optional<SetPointParameters.Rule> feeding;

private Optional<SetPointParameters.Rule> conversion;

private Optional<SetPointParameters.TimedTriggerRule> selfGrow;

private SetPointParameters.TimedTriggerRule starvation;

This is GraphQL mutation used for debug:

mutation {
  addSetPoint(data: {
    code: "AAA"
    idFeedTable: "idFeedTable1"
    feeding: {
      id: "idFeeding1"
      ruleType: TABLE
    }
    loss: {
      id: "idLoss1"
      ruleType: FORMULA
      triggerAfter: "PT4H30M"
      frequency: "PT6H15M"
    }
    idBreedingStage: "idBreedingStage1"
  })
}
srinivasankavitha commented 1 month ago

Ok, I see the issue. The existing converters don't handle cases with source type map and target type Optional<*>. We'll investigate further and fix. This might not be feasible to fix in the next few weeks, but should be able to prioritize in the next month. In the meantime, please workaround the issue by extracting manually as you have already described.

kilink commented 6 days ago

Fixed in #1951.