Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes failing Php integration tests because of wrong parameter types #5843

Merged
merged 32 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
0fbb488
Updated discriminator method to ensure that primitive types don't def…
Nov 27, 2024
d9caaa9
Checked for null type definition
Nov 27, 2024
b553e9e
Default to collection of primitive types for testing
Nov 27, 2024
4dd6a01
More updates on code method writer
Nov 27, 2024
88c7ad2
Rolled back the default to object values
Nov 27, 2024
de7c924
Added back enum check
Nov 27, 2024
35551a1
Defaulted any null type definitions to collection of object values
Nov 27, 2024
803e377
Rolled back
Nov 27, 2024
0f05da2
Ensured that parameter passed to getCollectionOfPrimitiveValues is a …
Nov 28, 2024
aba8d81
Merge branch 'main' into fix/php-integration-tests
timayabi2020 Nov 28, 2024
a5fa774
Updted method write
Nov 29, 2024
17a56af
Updated code method writer for php
Nov 29, 2024
122a646
More experimental updates
Nov 29, 2024
55047ba
Updated parseNodeMethod to be a collection of primitive values if typ…
Nov 29, 2024
d892c51
Defaulted parseNode method to a string incase type definition is null
Nov 29, 2024
f7b2070
Fixed undefined property error
Nov 29, 2024
e38b2b0
Updated method writer during null checks in typedefinition
Nov 29, 2024
32c2757
Updated code writer
Nov 29, 2024
ea585a4
Updates
Nov 29, 2024
a7cab36
Rectified typo
Nov 29, 2024
3af85e6
More tests
Nov 29, 2024
50de6be
Merge branch 'main' into fix/php-integration-tests
timayabi2020 Dec 2, 2024
a11a427
Default to get String values
Dec 2, 2024
994b96c
Revert changes
Dec 3, 2024
6e0bcc6
Added expected data type for setString method which takes a primitive…
Dec 4, 2024
07b21e0
Updated regex in phpstan.neon
Dec 4, 2024
a4f4306
Merge branch 'main' into fix/php-integration-tests
timayabi2020 Dec 4, 2024
8c6e84f
Updated CodeMethodWriter test file for php
Dec 4, 2024
dfc7c49
Merge branch 'main' into fix/php-integration-tests
timayabi2020 Dec 4, 2024
8886b19
$finalValue validation
Dec 5, 2024
694129c
Merge branch 'fix/php-integration-tests' of https://github.com/micros…
Dec 5, 2024
66c738c
Merge branch 'main' into fix/php-integration-tests
timayabi2020 Dec 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions it/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@
{
"Language": "ruby",
"Rationale": "https://github.com/microsoft/kiota/issues/1816"
},
{
"Language": "php",
"Rationale": "https://github.com/microsoft/kiota/issues/5779"
}
],
"ExcludePatterns": [
Expand Down
2 changes: 1 addition & 1 deletion it/php/phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ parameters:
- src
ignoreErrors:
- '#Parameter [\w\W]+ \$errorMappings of method [\w\\]+RequestAdapter::send[\w]+\(\) expects [\w\W\s]+ given\.#'
- '#getFieldDeserializers\(\) should return array\<string, callable\(Microsoft\\Kiota\\Abstractions\\Serialization\\ParseNode\)\: void\> but returns array\{[\d]+\: Closure\(Microsoft\\Kiota\\Abstractions\\Serialization\\ParseNode\)\: void.+#'
- '#getFieldDeserializers\(\) should return array\<string, callable\(Microsoft\\Kiota\\Abstractions\\Serialization\\ParseNode\)\: void\> but returns array\{.+\: Closure\(Microsoft\\Kiota\\Abstractions\\Serialization\\ParseNode\)\: void.+#'
55 changes: 35 additions & 20 deletions src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -475,35 +475,42 @@ _ when conventions.PrimitiveTypes.Contains(lowerCaseProp) => $"write{lowerCasePr
}

private const string ParseNodeVarName = "$parseNode";
private string GetDeserializationMethodName(CodeTypeBase propType, CodeMethod method)
private (string, string) GetDeserializationMethodName(CodeTypeBase propType, CodeMethod method)
{
var isCollection = propType.CollectionKind != CodeTypeBase.CodeTypeCollectionKind.None;
var propertyType = conventions.GetTypeString(propType, method, false);
var parseNodeMethod = string.Empty;
var parseNodeMethod = (string.Empty, string.Empty);
if (propType is CodeType currentType)
{
if (isCollection)
parseNodeMethod = currentType.TypeDefinition switch
{
CodeEnum enumType => $"getCollectionOfEnumValues({enumType.Name.ToFirstCharacterUpperCase()}::class)",
_ => $"getCollectionOfObjectValues([{conventions.TranslateType(propType)}::class, '{CreateDiscriminatorMethodName}'])"
CodeEnum enumType => (string.Empty, $"getCollectionOfEnumValues({enumType.Name.ToFirstCharacterUpperCase()}::class)"),
CodeClass => (string.Empty, $"getCollectionOfObjectValues([{conventions.TranslateType(propType)}::class, '{CreateDiscriminatorMethodName}'])"),
_ => (conventions.TranslateType(propType), $"getCollectionOfPrimitiveValues('{conventions.TranslateType(propType)}')")
};
else if (currentType.TypeDefinition is CodeEnum)
parseNodeMethod = $"getEnumValue({propertyType.ToFirstCharacterUpperCase()}::class)";
parseNodeMethod = (string.Empty, $"getEnumValue({propertyType.ToFirstCharacterUpperCase()}::class)");
}

var lowerCaseType = propertyType.ToLowerInvariant();
return string.IsNullOrEmpty(parseNodeMethod) ? lowerCaseType switch
{
"int" => "getIntegerValue()",
"bool" => "getBooleanValue()",
"number" => "getIntegerValue()",
"decimal" or "double" => "getFloatValue()",
"streaminterface" => "getBinaryContent()",
"byte" => "getByteValue()",
_ when conventions.PrimitiveTypes.Contains(lowerCaseType) => $"get{propertyType.ToFirstCharacterUpperCase()}Value()",
_ => $"getObjectValue([{propertyType.ToFirstCharacterUpperCase()}::class, '{CreateDiscriminatorMethodName}'])",
} : parseNodeMethod;
var res = parseNodeMethod;
if (string.IsNullOrEmpty(parseNodeMethod.Item2))
res = (string.Empty, lowerCaseType switch
{
"int" => "getIntegerValue()",
"bool" => "getBooleanValue()",
"number" => "getIntegerValue()",
"decimal" or "double" => "getFloatValue()",
"streaminterface" => "getBinaryContent()",
"byte" => "getByteValue()",
_ when conventions.PrimitiveTypes.Contains(lowerCaseType) =>
$"get{propertyType.ToFirstCharacterUpperCase()}Value()",
_ =>
$"getObjectValue([{propertyType.ToFirstCharacterUpperCase()}::class, '{CreateDiscriminatorMethodName}'])",
});

return res;
}

private void WriteSetterBody(LanguageWriter writer, CodeMethod codeElement, CodeClass parentClass)
Expand Down Expand Up @@ -672,7 +679,7 @@ private void WriteDeserializerPropertyCallback(CodeProperty property, CodeMethod
writer.WriteLine("},");
return;
}
writer.WriteLine($"'{property.WireName}' => fn(ParseNode $n) => $o->{property.Setter!.Name.ToFirstCharacterLowerCase()}($n->{GetDeserializationMethodName(property.Type, method)}),");
writer.WriteLine($"'{property.WireName}' => fn(ParseNode $n) => $o->{property.Setter!.Name.ToFirstCharacterLowerCase()}($n->{GetDeserializationMethodName(property.Type, method).Item2}),");
}

private static void WriteDeserializerBodyForIntersectionModel(CodeClass parentClass, LanguageWriter writer)
Expand Down Expand Up @@ -903,7 +910,8 @@ private void WriteFactoryMethodBodyForIntersectionModel(CodeMethod codeElement,
{
if (property.Type is CodeType propertyType)
{
var deserializationMethodName = $"{ParseNodeVarName}->{GetDeserializationMethodName(propertyType, codeElement)}";
var methodName = GetDeserializationMethodName(propertyType, codeElement);
var deserializationMethodName = $"{ParseNodeVarName}->{methodName.Item2}";
writer.StartBlock($"{(includeElse ? "} else " : string.Empty)}if ({deserializationMethodName} !== null) {{");
writer.WriteLine($"{ResultVarName}->{property.Setter!.Name.ToFirstCharacterLowerCase()}({deserializationMethodName});");
writer.DecreaseIndent();
Expand Down Expand Up @@ -973,9 +981,16 @@ private void WriteFactoryMethodBodyForUnionModelForUnDiscriminatedTypes(CodeMeth
.ToArray();
foreach (var property in otherProps)
{
var serializationMethodName = $"{ParseNodeVarName}->{GetDeserializationMethodName(property.Type, currentElement)}";
var methodName = GetDeserializationMethodName(property.Type, currentElement);
var serializationMethodName = $"{ParseNodeVarName}->{methodName.Item2}";
const string finalValueName = "$finalValue";
writer.StartBlock($"{(includeElse ? "} else " : string.Empty)}if ({serializationMethodName} !== null) {{");
writer.WriteLine($"{ResultVarName}->{property.Setter!.Name.ToFirstCharacterLowerCase()}({serializationMethodName});");
if (!string.IsNullOrEmpty(methodName.Item1))
{
writer.WriteLine($"/** @var array<{methodName.Item1}> {finalValueName} */");
}
writer.WriteLine($"{finalValueName} = {serializationMethodName};");
writer.WriteLine($"{ResultVarName}->{property.Setter!.Name.ToFirstCharacterLowerCase()}({finalValueName});");
writer.DecreaseIndent();
if (!includeElse)
includeElse = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1474,9 +1474,9 @@ public void WritesModelFactoryBodyForUnionModels()
Assert.Contains("if ('#kiota.complexType1' === $mappingValue) {", result);
Assert.Contains("$result->setComplexType1Value(new ComplexType1())", result);
Assert.Contains("if ($parseNode->getStringValue() !== null) {", result);
Assert.Contains("$result->setStringValue($parseNode->getStringValue())", result);
Assert.Contains("$result->setStringValue($finalValue)", result);
baywet marked this conversation as resolved.
Show resolved Hide resolved
Assert.Contains("else if ($parseNode->getCollectionOfObjectValues([ComplexType2::class, 'createFromDiscriminatorValue']) !== null) {", result);
Assert.Contains("$result->setComplexType2Value($parseNode->getCollectionOfObjectValues([ComplexType2::class, 'createFromDiscriminatorValue']))", result);
Assert.Contains("$result->setComplexType2Value($finalValue)", result);
Assert.Contains("return $result", result);
Assert.DoesNotContain("return new UnionTypeWrapper()", result);
AssertExtensions.Before("$parseNode->getStringValue()", "getCollectionOfObjectValues([ComplexType2::class, 'createFromDiscriminatorValue'])", result);
Expand Down
Loading