Skip to content

GG-377 Service input validation#378

Open
folkef6 wants to merge 1 commit intomainfrom
gg-377
Open

GG-377 Service input validation#378
folkef6 wants to merge 1 commit intomainfrom
gg-377

Conversation

@folkef6
Copy link
Collaborator

@folkef6 folkef6 commented Feb 26, 2026

Fixed validation for service inputs (they need either table or record) and that the signature of the service matches. If not options of overloads are listed.

…) and that the signature of the service matches. If not options of overloads are listed.
@folkef6 folkef6 requested review from Erkelinux and jenskm February 26, 2026 13:07
Comment on lines +783 to +786
allFields
.stream()
.filter(ObjectField::isGenerated)
.filter(ObjectField::hasServiceReference)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ser at disse fire linjene er duplisert 4 ganger. Kanskje innføre en hjelpemetode som returnrerer streamen?

validateRecordParameterTypes(field, serviceWrapper, method, parser, resolverKeyOffset);
}

private boolean hasParameterCountMismatch(ObjectField field, CodeReferenceWrapper serviceWrapper,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude fant en feil som jeg ikke så:

Validerings-gap ved overloads med riktig antall, men feil type

Gitt en service med:
public String checkOverload(String wrongType) { ... } // 1 param, feil type
public String checkOverload(CustomerRecord a, AddressRecord b) { ... } // 2 params

Og et skjema som forventer 1 CustomerRecord-parameter — ingen av overloadene er gyldige, men valideringen slipper det gjennom.

Årsaken er todelt:

  1. hasParameterCountMismatch() hopper over type-sjekk når primærmetoden har feil antall, selv om overloaden med riktig antall har inkompatibel type.
  2. validateRecordParameterTypes() sjekker type mot alle overloads uten å filtrere på parameterantall, så en overload med feil antall men riktig type på indeksen passerer.

Det ser ut til å stemme at dette blir feil

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Her er forslaget den har til fiks

I stedet for den nåværende to-stegs-tilnærmingen (hasParameterCountMismatch → validateRecordParameterTypes), filtrer overloads til de med riktig antall parametere først, og sjekk deretter typer kun mot det
subsettet. Dette fikser overload-buggen og forenkler flyten:

private void checkServiceParameterTypes(ObjectField field) {
>       var serviceWrapper = field.getExternalMethod();
>       var overloads = getAllOverloads(serviceWrapper);
>       if (overloads.isEmpty()) return;
> 
>       var parser = new InputParser(field, schema);
>       var resolverKeyOffset = field.isRootField() ? 0 : 1;
>       var expectedCount = resolverKeyOffset + parser.getMethodInputNames(true, true, true).size();
> 
>       var candidates = overloads.stream()
>               .filter(m -> m.getParameterTypes().length == expectedCount)
>               .toList();
> 
>       if (candidates.isEmpty()) {
>           addErrorMessage("...count mismatch...", ...);
>           return;
>       }
> 
>       // Type-sjekk kun mot kandidater med riktig antall parametere
>       for (var entry : parser.getRecords().entrySet()) {
>           // ...sjekk mot candidates...
>       }
>   }

Dette fjerner hasParameterCountMismatch helt og fjerner avhengigheten til serviceWrapper.getMethod() som "primærmetode" — den er vilkårlig (findFirst()-rekkefølge). Ved å filtrere på antall parametere først,
unngår man at en overload med feil antall men riktig type på en gitt indeks slipper gjennom.

Comment on lines +929 to +956
private String formatOverloads(List<Method> overloads, String methodName) {
if (overloads.isEmpty()) {
return " (no overloads found)";
}
return overloads.stream()
.map(m -> " " + formatMethodSignature(m, methodName))
.collect(Collectors.joining("\n"));
}

private String formatMethodSignature(Method method, String methodName) {
var params = Arrays.stream(method.getGenericParameterTypes())
.map(this::formatTypeName)
.collect(Collectors.joining(", "));
return methodName + "(" + params + ")";
}

private String formatTypeName(Type type) {
if (type instanceof ParameterizedType pt) {
var raw = ((Class<?>) pt.getRawType()).getSimpleName();
var args = Arrays.stream(pt.getActualTypeArguments())
.map(this::formatTypeName)
.collect(Collectors.joining(", "));
return raw + "<" + args + ">";
}
if (type instanceof Class<?> cls) {
return cls.getSimpleName();
}
return type.getTypeName();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kanskje flytte disse 3 metodene og formatExpectedType som har med tekstformatering til en egen klasse?

ProcessedDefinitionsValidator begynner å bli i største laget, så vi bør se på måter å splitte den litt på.

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

Successfully merging this pull request may close these issues.

2 participants