C# client: fix default values for DateTime etc, numeric and boolean defaults were not applied

[WolfgangHG]

This is an attempt to fix #7404

If the OpenAPI spec contains model fields with format date-time, date, time or uuid and default values, the generated client did not compile because the string value was assigned directly to the property.

Default values for numeric types and boolean were not applied at all, because the code ignored all default values that did not start with a quote.

I also tried to add a test and placed my json file in "Kiota.Builder.IntegrationTests" and added a simple test to "GenerateSample.cs", but only for C#. Other languages apparently handle the default values differently.
This test just runs code generating. It might also make sense to test the generated file. But I don't have an idea how you would do it. It would probably perfect if the client code could be compiled using Roslyn and then the model class could be loaded from the dynamic assembly and the field values could be verified. Or I could take at least a look in the generated class, as e.g. test "GeneratesUritemplateHintsAsync" does. What do you think?

[baywet]

Thanks for the contribution!

Added a comment to help drive this to completion.
In addition, could you please add unit tests for the code method writer?

[WolfgangHG]

In addition, could you please add unit tests for the code method writer?

@baywet The code that I changed is completely covered by GenerateSamples.GeneratesModelWithDefaultValuesAsync - but it is only tested that it does not throw an exception. I could add code to parse the generated file for some specific lines (as is done in other tests in this class).

Could you give me a hint what kind of testing you expect and where I could copy code?

[baywet]

This is essentially an integration test. Thank you for initially adding it.

Did you come across the default value tests in coemethodwritertests? I'm suggesting to add more there.

Let me know if you have additional questions

[WolfgangHG]

Did you come across the default value tests in coemethodwritertests? I'm suggesting to add more there.

Perfect, that's what I actually was looking for but didn't find ;-).

I added the test for "float value needs the suffix f" to WritesConstructor because it felt correct to place it there - those are all basic tests.
All Parse related tests are placed in a new method WritesConstructorWithDefaultValuesThatRequireParsing

Is the integration test (which parses the full json file) still required? I think so, because it brings the full demo file for this problem.

[baywet]

Thanks for adding the unit tests! I think we're getting really close at this stage.

[WolfgangHG]

I added another small commit. Some integration tests (including my new one) did not set CleanOutput = true, thus the tests would not have re-created the client when running the testsuite multiple times in Visual Studio.

I also experimented a bit (not committed) and added Roslyn compilation to the generated clients of the "Kiota.Builder.IntegrationTests" project. Later I found that there is a big integration tests suite in the "it" directory that is only run during Github pull request verification. Probably, there compilation is also done. But this big test probably does not cover the default value handling of this pull request.

What do you think? Is the approach to compile the "Kiota.Builder.IntegrationTests" clients worth the effort? Or is this unnecessary?

[baywet]

What do you think? Is the approach to compile the "Kiota.Builder.IntegrationTests" clients worth the effort? Or is this unnecessary?

Not worth the time. Both in terms of setup, and then in terms of additional runtime of the tests. This is why we set this up as integration tests run by the CI, and not integration tests run as the local test suite. But thank you for considering it.

[WolfgangHG]

@baywet I see that a lot of tests are failing, most in java clients. Is this caused by my code? Maybe because I detect default values for basic data types now and they are not handled properly when generating the java client?

E.g. here:
https://github.com/microsoft/kiota/actions/runs/22629867053/job/65577135366?pr=7414#step:17:256

[baywet]

this is causing a regression for Java/Go/Dart.
Similar changes that were made for the CSharp writer need to be made for those languages before we can accept the PR.

[WolfgangHG]

OK, I will take a look later.

[WolfgangHG]

I hope I fixed the java problems - a client created from my sample JSON file works again, and the client from https://developers.pipedrive.com/docs/api/v1/openapi.yaml also compiles.

The sample model "WeatherForecast" model looks like this:

public WeatherForecast() {
    this.setBoolValue(true);
    this.setDateOnlyValue(LocalDate.parse("1900-01-01"));
    this.setDateValue(OffsetDateTime.parse("1900-01-01T15:00:00+00:00"));
    this.setDecimalValue(25.5d);
    this.setDoubleValue(25.5d);
    this.setFloatValue(25.5f);
    this.setGuidValue(UUID.fromString("00000000-0000-0000-0000-000000000000"));
    this.setSummary("Test");
    this.setTemperatureC(15);
    this.setTimeValue(LocalTime.parse("00:00:00"));
}

Specials compared to C#:

• double values require "d" suffix always.
• float values require "f" suffix if they don't contain a decimal point
• datetime values must have the format DateTimeFormatter.html.ISO_OFFSET_DATE_TIME. In my Swashbuckle generated OpenAPI file, the date had no timezone, which failed to parse in Java. So I add "+00:00" if the default value does not contain a timezone ("+" char).

Now to the other problems - as I don't know Go or Dart, this might be harder...

[WolfgangHG]

About GO:

Currently, it generates this:

m.SetBoolValue(true)
m.SetDecimalValue(25.5)
m.SetDoubleValue(25.5)
m.SetFloatValue(25.5)

...which results in errors like cannot use true (untyped bool constant) as *bool value in argument to m.SetBoolValue

Following https://www.codegenes.net/blog/how-to-set-bool-pointer-to-true-in-struct-literal/, this could be done like this:

m.SetBoolValue(func() *bool { 
        b := true
        return &b 
    }())

m.SetDecimalValue(func() *float64 { 
        b := 25.5
        return &b 
    }())

m.SetFloatValue(func() *float32 { 
        b := float32(25.5)
        return &b
    }())

The float value literal causes a compilation error, and the float64 value would do the same if the default has no decimal point. Is there any suffix/prefix to define a float32/float64 literal, or must it be done with float32(25.5) or float64(25)?

@baywet What do you think about my suggestion?

By the way: due to my lack of knowledge of theses languages, I would prefer to ignore the date/time default handling hat I added for C#/Java, as they probably never were an issue with GO or Dart previously ;-).

[WolfgangHG]

Also a PHP error: https://github.com/microsoft/kiota/actions/runs/22629867053/job/65577135791?pr=7414#step:17:139

Error: Parameter #1 $value of class Integration\Test\Client\Repos\Item\Item\Releases\Item\WithRelease_PatchRequestBody_make_latest constructor expects string, true given.
Error: Parameter #1 $value of class Integration\Test\Client\Repos\Item\Item\Releases\ReleasesPostRequestBody_make_latest constructor expects string, true given.

The DART error: https://github.com/microsoft/kiota/actions/runs/22629867053/job/65577135767?pr=7414#step:17:75

Analyzing lib...

  error - models/tweet_create_request.dart:35:33 - Expected to find ';'. - expected_token
  error - models/tweet_create_request.dart:35:38 - Expected an identifier. - missing_identifier
  error - models/tweet_create_request.dart:35:38 - Unexpected text ';'. Try removing the text. - unexpected_token

[baywet]

Thanks for looking into this!

For dart maybe we can ask help from @ricardoboss on this.

For go, this is verbose, but I don't have a better solution unless we add short hands in the abstractions library.

As for which types are supported, I believe we should strive for parity between stable languages here.

[ricardoboss]

Dart requires default values to be compile time constant, so you can't actually have a default parameter like DateTime.now() or DateTime.parse(...). The parameter must be nullable with null as the default and can then be set using parameter ??= DateTime.now(); (for example).

[WolfgangHG]

Making some slow progress with Go and requesting feedback on the current result.

This is the generated Go code for my "WeatherForecast" sample class. I verified that it works and all fields are assigned the expected values.
In contrary to the previous suggestion, I followed the approach already implemented to assign the default value to a variable, then set the pointer to this var to the field.

func NewWeatherForecast()(*WeatherForecast) {
    m := &WeatherForecast{
    }
    boolValueValue := true
    m.SetBoolValue(&boolValueValue)
    dateOnlyValueValue, _ := i878a80d2330e89d26896388a3f487eef27b0a0e6c010c493bf80be1452208f91.ParseDateOnly("1900-01-01")
    m.SetDateOnlyValue(dateOnlyValueValue)
    dateValueValue, _ := i336074805fc853987abe6f7fe3ad97a6a6f3077a16391fec744f671a015fbd7e.Parse(i336074805fc853987abe6f7fe3ad97a6a6f3077a16391fec744f671a015fbd7e.RFC3339, "1900-01-01T00:00:00+00:00")
    m.SetDateValue(&dateValueValue)
    decimalValueValue := float64(25.5)
    m.SetDecimalValue(&decimalValueValue)
    doubleValueValue := float64(25.5)
    m.SetDoubleValue(&doubleValueValue)
    floatValueValue := float32(25.5)
    m.SetFloatValue(&floatValueValue)
    guidValueValue, _ := i561e97a8befe7661a44c8f54600992b4207a3a0cf6770e5559949bc276de2e22.Parse("00000000-0000-0000-0000-000000000000")
    m.SetGuidValue(&guidValueValue)
    summaryValue := "Test"
    m.SetSummary(&summaryValue)
    temperatureCValue := int32(15)
    m.SetTemperatureC(&temperatureCValue)
    timeValueValue, _ := i878a80d2330e89d26896388a3f487eef27b0a0e6c010c493bf80be1452208f91.ParseTimeOnly("00:00:00")
    m.SetTimeValue(timeValueValue)
    return m
}

Attached is the full file:
weather_forecast.go.txt

Question 1: is there anything reusable? I see that the generated GetFieldDeserializers calls a lot of methods like ParseNode.GetDateOnlyValue. Could the code generator somehow generate calls to this class? If yes: how to parse a string value?
Question 2: all Parse methods return a second error object. My sample snippet just discards it. How to handle it?

[baywet]

Thank you for the additional information.

Yes, I like this approach! It's verbose, but consistent at least.

Q1: I don't think you'll be able to easily reuse the parse node infrastructure because that not only requires you to have access to the parse node factories, but also get bytes from string, and the representation might vary based on the mime type/available implementation. All of which add layers of complexity that calling a static factory is much simpler. Maybe it'd be worth refactoring things to make sure the parts that emit those lines are defined once and not duplicated.

Q2: I've always found that part to be odd in Go's design. Essentially "constructors" don't typically return an error, but the way errors work, they require an extra return value. It does not fit well. Since we don't want to introduce breaking changes, this effectively leaves us with two choices:

• panic on error
• swallow/ignore the errors

My opinion is that failing to set a default because the spec is odd, or because the static factory fails to parse a value, is not a huge deal which should lead the application to crash. So panicking is probably too aggressive. Of course that's something that can always be updated later on if people have issues with errors being ignored.

Let me know if you have any additional comments or questions.

[WolfgangHG]

Here we go with the Go client.

Hope I didn't break it - not having seen Go before three days ago...

@baywet Thanks for the feedback about error handling. I agree completely - a parse error while applying the defaults should be avoided. If a user runs into a parse problem, she could still add error logging in the generated code.

I am very confused that the Go test CodeMethodWriterTests.WritesConstructorWithEnumValue expected a line SetSize(TENTWENTYFOUR_BY_TENTWENTYFOUR) that should not have compiled in reality as the setter is generated so that the enum parameter is a pointer to an enum value. And this test did not fail before I starting changing code generation. Seems someone wrote a test for a non-compiling method.

kiota/tests/Kiota.Builder.Tests/Writers/Go/CodeMethodWriterTests.cs

Line 1944 in fb6ab13

Assert.Contains($"Set{propName.ToFirstCharacterUpperCase()}({defaultValue})", result);//ensure symbol is cleaned up

@baywet Could you start the Github CI test run so that I can see whether my changes fix the failing Java and Go tests?

PHP and Dart are still on my TODO list, but not before next Tuesday. I also don't know those two languages ;-)

[WolfgangHG]

Are those GO errors also caused by me?

For example https://github.com/microsoft/kiota/actions/runs/22796339086/job/66295047630?pr=7414#step:17:35:

Error: client/models/component1.go:23:25: cannot use &additionalDataValue (value of type *map[string]any) as map[string]any value in argument to m.SetAdditionalData

[baywet]

Are those GO errors also caused by me?

For example https://github.com/microsoft/kiota/actions/runs/22796339086/job/66295047630?pr=7414#step:17:35:

Error: client/models/component1.go:23:25: cannot use &additionalDataValue (value of type *map[string]any) as map[string]any value in argument to m.SetAdditionalData

I'd say yes since one of the files it's erroring with is local (not pulled from an external repo) and since this is only happening on this branch.

Let us know if you have any additional comments or questions.

[WolfgangHG]

Yes, it was me ;-). Previously, a default value was always set as m.setProperty(defaultValue), except if the default value started with a quote. In this case, it was first assigned to a variable, then the setter was invoked as m.setProperty(&defaultValue).
Since my change, also default values not starting with a quote are detected (numeric values, boolean), thus I removed the check "default value starts with quote" and handle all those default values the same way. Because of this change, the code for additionalData assignd the default to a variable and called the setter with a reference.

With the latest commit, I brought back the old behavior:

m.SetAdditionalData(make(map[string]any))

[WolfgangHG]

@baywet There is one failing Java test:

https://github.com/microsoft/kiota/actions/runs/22796339086/job/66295047215?pr=7414#step:17:201

The error is:
WithReleasePatchRequestBody.java:[53,75] incompatible types: boolean cannot be converted to java.lang.String

The reason is this snippet:

  "make_latest": {
    "default": true,
    "description": "Specifies whether this release should be set as the latest release for the repository. ....",
    "enum": [
      "true",
      "false",
      "legacy"
    ],

Shouldn't the default for the enum value be placed in quotes here? Before, it was not detected.

Kiota now creates this java snippet:

public WithReleasePatchRequestBody() {
    this.setAdditionalData(new HashMap<>());
    this.setMakeLatest(WithReleasePatchRequestBodyMakeLatest.forValue(true));
}

forValue expects a string.

I could work around by adding quotes to enum default values if they are missing:

if (propWithDefault.Type is CodeType propertyType && propertyType.TypeDefinition is CodeEnum enumDefinition)
{
     if (!defaultValue.StartsWith('"'))
     {
         defaultValue = $"\"{defaultValue}\"";
     }
    defaultValue = $"{enumDefinition.Name}.forValue({defaultValue})";
}

How to continue? Add the workaround? Or if my understanding is correct and the syntax is invalid, would it be worth a bug report?

Could you also trigger another run of the tests to verify that the Go problems are fixed?

[baywet]

In this example, the default value (in the OpenAPI description) is wrong IMHO.

That's because the enum value is "true" (a string) but the default is true (a boolean). And the default would not validate against the enum keyword according to JSON schema only.

I'd be ok suppressing the specific operation that's bringing this type in the configuration here for that reason.

[WolfgangHG]

I created github/rest-api-description#6100. Seems they fixed one of the problems already. But I am a bit irritated because the file that Kiota downloads differs a lot from the github file. Did I look at the proper file ;-)?

@baywet Why not use the approach of the C# client generator instead of calling a parsing method?

kiota/src/Kiota.Builder/Writers/CSharp/CodeMethodWriter.cs

Lines 256 to 259 in e19028c

	if (propWithDefault.Type is CodeType propertyType && propertyType.TypeDefinition is CodeEnum)
	{
	defaultValue = $"{conventions.GetTypeString(propWithDefault.Type, currentMethod).TrimEnd('?')}.{defaultValue.Trim('"').CleanupSymbolName().ToFirstCharacterUpperCase()}";
	}

Currently generated Java code:

public WithReleasePatchRequestBody() {
    this.setAdditionalData(new HashMap<>());
    this.setMakeLatest(WithReleasePatchRequestBodyMakeLatest.forValue("true"));
}

The C# approach would result in this code:

public WithReleasePatchRequestBody() {
    this.setAdditionalData(new HashMap<>());
    this.setMakeLatest(WithReleasePatchRequestBodyMakeLatest.True);
}

Here, the generated code compiles without errors. Could we use this code?

By the way: the "config.json" that you linked contain some suppressions for apisguru::github.com:api.github.com that link to fixed issues. They could probably restored?

[baywet]

Seems they fixed one of the problems already. But I am a bit irritated because the file that Kiota downloads differs a lot from the github file. Did I look at the proper file ;-)?

Yes, most descriptions come from the APIs.guru index. Which is often out of date sadly. This is a bit outside of the scope of this pull request but you could do a could of things:

• reach out to apis.guru and tell them their sync is broken/behind for github
• reach out to github and tell them to add their description to the "github based index" https://learn.microsoft.com/en-us/openapi/kiota/add-api (+ add an entry in the APIs guru search provider here to exclude github descriptions and avoid duplicates)
• update the test configuration to use that description instead of the APIs.guru one.

In any case, if any changes are required in kiota's repo, please open a separate pull request to avoid mixing concerns/spead up the review process.

Why not use the approach of the C# client generator instead of calling a parsing method?

I'm not sure/can't remember why we'd have a difference in the approach between languages here. Could be as simple as the initial language implementers not coordinating. Happy to see things getting aligned assuming it doesn't introduce regressions.

to fixed issues. They could probably restored?

yes, it's probably an oversight. Please do that in a separate PR though.

Thanks for the continuous work here. Let me know if you have any additional comments or questions.

[ricardoboss]

Ah yeah right, you don't need late because they aren't final. They all have an implicit default value of null then.

[baywet]

I would also have to define this directory as "MockServerITFolder" in "config.json".

Is it reasonable to do so? Would be quite a bit of work to do it for all relevant languages ;-)

I think this is how we could add an additional aspect here. But I haven't touched any of that in a LONG time. Maybe @andreaTP can help confirm. (he set all of this up back in the days)

[WolfgangHG]

I think I made another turn around with the dart generator.... Sorry, my last screenshot showing the dart error messages had the error popup positioned over an important information - the opening bracket...

Previously, Kiota generated this code (already with fixed parsing of date values):

    WeatherForecast() {
        boolValue = true,
        dateOnlyValue = DateOnly.fromDateTimeString('1900-01-01'),
        dateValue = DateTime.parse('1900-01-01T00:00:00'),
        decimalValue = 25.5,
        doubleValue = 25.5,
        enumValue = WeatherForecastEnumValue.one,
        floatValue = 25.5,
        guidValue = UuidValue.fromString('00000000-0000-0000-0000-000000000000'),
        longValue = 255,
        summary = 'Test',
        temperatureC = 15,
        timeValue = TimeOnly.fromDateTimeString('00:00:00');
    }

This fails, because it combines the comma separator (usable only in initializer list) and curly brackets. Probably not noticed before, because Kiota detected only string default values, and there was no class with more than one default value property.

My first attempt was to create this:

    WeatherForecast() {
        boolValue = true;
        dateOnlyValue = DateOnly.fromDateTimeString('1900-01-01');
        dateValue = DateTime.parse('1900-01-01T00:00:00');
        decimalValue = 25.5;
        doubleValue = 25.5;
        enumValue = WeatherForecastEnumValue.one;
        floatValue = 25.5;
        guidValue = UuidValue.fromString('00000000-0000-0000-0000-000000000000');
        longValue = 255;
        summary = 'Test';
        temperatureC = 15;
        timeValue = TimeOnly.fromDateTimeString('00:00:00');
    }

Perfectly fine, but it fails in IT with the twitter api, because here "additionalData" and a property with default value exist, but no curly bracket is written.

    ListCreateRequest() :  
        additionalData = {};
        private = false;

Adding a curly bracket resulted in another error because additionalData is not nullable and would have required the late keyword.

So I went back to the initializer approach and added more checks to

kiota/src/Kiota.Builder/Writers/Dart/CodeMethodWriter.cs

Lines 693 to 694 in 3224884

	else if (isConstructor && parentClass.Properties.Where(static x => x.IsOfKind(CodePropertyKind.AdditionalData)).Any() && !parentClass.IsErrorDefinition && !parentClass.Properties.Where(static x => x.IsOfKind(CodePropertyKind.BackingStore)).Any())
	{

and

kiota/src/Kiota.Builder/Writers/Dart/CodeMethodWriter.cs

Lines 40 to 41 in 3224884

	if (isConstructor && !inherits && parentClass.Properties.Where(static x => x.Kind is CodePropertyKind.AdditionalData).Any() && !parentClass.IsErrorDefinition && !parentClass.Properties.Where(static x => x.Kind is CodePropertyKind.BackingStore).Any())
	{

to create a initializer list if "additionalData" is available or any property has a default value.

 x.Kind is CodePropertyKind.AdditionalData || (x.Kind is CodePropertyKind.Custom && !string.IsNullOrEmpty(x.DefaultValue))

[WolfgangHG]

About the APIGurus question: I created #7495

Next step for me: fix the typescript error.

=============

I would also have to define this directory as "MockServerITFolder" in "config.json".

Is it reasonable to do so? Would be quite a bit of work to do it for all relevant languages ;-)

I think this is how we could add an additional aspect here. But I haven't touched any of that in a LONG time.

And I will take another look the next few days. I had a working prototype for C#. With some polish, I could commit it.

[andreaTP]

Sorry for the delay @baywet , I'm on the phone, but from a quick skim I think that the E2E tests is not the best place to test those things e.g. they are too expensive and too far from the root issue.

This should be a shorter feedback loop cycle that should cover the use case: https://github.com/microsoft/kiota/tree/main/tests%2FKiota.Builder.IntegrationTests

Edit: this has been done already apparently

Overall I'd try to move as much logic as possible in the refiners other than plumbing in the code writers as it is risky and hardly scalable.

[WolfgangHG]

The last failure seems to be a hiccup?

https://github.com/microsoft/kiota/actions/runs/23215179582/job/67575279590?pr=7414#step:7:828

  Failed Kiota.Builder.Tests.KiotaSearcherTests.GetsMicrosoftGraphBothVersionsAsync [1 s]
  Error Message:
   Assert.Equal() Failure: Values differ
Expected: 2
Actual:   0
  Stack Trace:
     at Kiota.Builder.Tests.KiotaSearcherTests.GetsMicrosoftGraphBothVersionsAsync() in /home/runner/work/kiota/kiota/tests/Kiota.Builder.Tests/KiotaSearcherTests.cs:line 40

And:

https://github.com/microsoft/kiota/actions/runs/23215179582/job/67575279590?pr=7414#step:7:1425

  Failed Kiota.Builder.Tests.KiotaSearcherTests.GetsMicrosoftGraphBetaAsync [578 ms]
  Error Message:
   Assert.Single() Failure: The collection was empty
  Stack Trace:
     at Kiota.Builder.Tests.KiotaSearcherTests.GetsMicrosoftGraphBetaAsync() in /home/runner/work/kiota/kiota/tests/Kiota.Builder.Tests/KiotaSearcherTests.cs:line 57

[baywet]

The last failure seems to be a hiccup?

yes those two have been flaky for years due to GitHub throttling. We still need to test it somehow. Maybe adding a auto-rerun in the test settings would help reducing the impact though.
Another option would be to make sure this runs authenticated with the CI, GitHub would give us a bigger quota.

Either way, consider this outside of the scope of this PR.

[baywet]

Quickly put together this #7499

[WolfgangHG]

Working on TypeScript...

For my sample api, Kiota created this snippet (which failed because date/dateonly/timeony/guid defaults are not even quoted):

export function deserializeIntoWeatherForecast(weatherForecast: Partial<WeatherForecast> | undefined = {}) : Record<string, (node: ParseNode) => void> {
    return {
        "boolValue": n => { weatherForecast.boolValue = n.getBooleanValue() ?? true; },
        "dateOnlyValue": n => { weatherForecast.dateOnlyValue = n.getDateOnlyValue() ?? 1900-01-01; },
        "dateValue": n => { weatherForecast.dateValue = n.getDateValue() ?? 1900-01-01T00:00:00; },
        "decimalValue": n => { weatherForecast.decimalValue = n.getNumberValue() ?? 25.5; },
        "doubleValue": n => { weatherForecast.doubleValue = n.getNumberValue() ?? 25.5; },
        "enumValue": n => { weatherForecast.enumValue = n.getEnumValue<WeatherForecast_enumValue>(WeatherForecast_enumValueObject) ?? WeatherForecast_enumValueObject.One; },
        "floatValue": n => { weatherForecast.floatValue = n.getNumberValue() ?? 25.5; },
        "guidValue": n => { weatherForecast.guidValue = n.getGuidValue() ?? 00000000-0000-0000-0000-000000000000; },
        "longValue": n => { weatherForecast.longValue = n.getNumberValue() ?? 255; },
        "summary": n => { weatherForecast.summary = n.getStringValue() ?? "Test"; },
        "temperatureC": n => { weatherForecast.temperatureC = n.getNumberValue() ?? 15; },
        "temperatureF": n => { weatherForecast.temperatureF = n.getNumberValue(); },
        "timeValue": n => { weatherForecast.timeValue = n.getTimeOnlyValue() ?? 00:00:00; },
    }
}

A fixed version could look like this:

export function deserializeIntoWeatherForecast(weatherForecast: Partial<WeatherForecast> | undefined = {}) : Record<string, (node: ParseNode) => void> {
    return {
        "boolValue": n => { weatherForecast.boolValue = n.getBooleanValue() ?? true; },
        "dateOnlyValue": n => { weatherForecast.dateOnlyValue = n.getDateOnlyValue() ?? DateOnly.parse("1900-01-01"); },
        "dateValue": n => { weatherForecast.dateValue = n.getDateValue() ?? new Date("1900-01-01T00:00:00"); },
        "decimalValue": n => { weatherForecast.decimalValue = n.getNumberValue() ?? 25.5; },
        "doubleValue": n => { weatherForecast.doubleValue = n.getNumberValue() ?? 25.5; },
        "enumValue": n => { weatherForecast.enumValue = n.getEnumValue<WeatherForecast_enumValue>(WeatherForecast_enumValueObject) ?? WeatherForecast_enumValueObject.One; },
        "floatValue": n => { weatherForecast.floatValue = n.getNumberValue() ?? 25.5; },
        "guidValue": n => { weatherForecast.guidValue = n.getGuidValue() ?? "00000000-0000-0000-0000-000000000000"; },
        "longValue": n => { weatherForecast.longValue = n.getNumberValue() ?? 255; },
        "summary": n => { weatherForecast.summary = n.getStringValue() ?? "Test"; },
        "temperatureC": n => { weatherForecast.temperatureC = n.getNumberValue() ?? 15; },
        "temperatureF": n => { weatherForecast.temperatureF = n.getNumberValue(); },
        "timeValue": n => { weatherForecast.timeValue = n.getTimeOnlyValue() ?? TimeOnly.parse("00:00:00"); },
    }
}

This brings another problem: Kiota creates this "import":

import { type DateOnly, type Guid, type Parsable, type ParseNode, type SerializationWriter, type TimeOnly } from '@microsoft/kiota-abstractions';

Which results in an error 'TimeOnly' cannot be used as a value because it was imported using 'import type'.

I could resolve it here:

kiota/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs

Lines 793 to 810 in c6f4143

	private static readonly Dictionary<string, (string, CodeUsing?)> DateTypesReplacements = new(StringComparer.OrdinalIgnoreCase) {
	{"DateTimeOffset", ("Date", null)},
	{"TimeSpan", ("Duration", new CodeUsing {
	Name = "Duration",
	Declaration = new CodeType {
	Name = AbstractionsPackageName,
	IsExternal = true,
	},
	IsErasable = true, // the import is used only for the type, not for the value
	})},
	{"DateOnly", (string.Empty, new CodeUsing {
	Name = "DateOnly",
	Declaration = new CodeType {
	Name = AbstractionsPackageName,
	IsExternal = true,
	},
	IsErasable = true, // the import is used only for the type, not for the value
	})},

by setting IsErasable = false for DateOnly and TimeOnly.

This has this result:

import { DateOnly, TimeOnly, type Guid, type Parsable, type ParseNode, type SerializationWriter } from '@microsoft/kiota-abstractions';

Is it OK to change this? I assume it would have an effect not only for model classes, but also for request/response classes.

The TypeScript client does not seem to support the concept of default values for fields of model classes. So, in the client, I cannot create an instance of the "WeatherForecast" interface and the defaults are populated. It applies the defaults only when serializing/deserializing.
I found this article about the spread syntax, but it would still require someone creating an instance of a class to use the "...defaults" term.

[baywet]

It's ok to do this only when we have a default value to parse.
And yes, that's going to impact the definitive size of the bundle, but at some point we need the code to parse the default value.

[baywet]

As for the default values in a "new" scenario, we're using interfaces for models to avoid the bundle size to get out of control. For that reason, we don't have a constructor either.

That effectively means there's no "place" for those default values.

We could potentially introduce a static factory or default object to get and spread the default values in an object... But that doesn't feel super intuitive for the user, and would add a lot of work to this PR.

Unless someone has a much better idea, I'm ok with putting the defaults in the deserialization for now.

[WolfgangHG]

OK, I tried to find the correct place to check whether a type DateOnly or a DateOnly import should be written. The latter is needed only if a DateOnly property with a default value is found.

Problem: when the default values are handled (CodeFunctionWriter.WriteDeserializerFunction which calls GetDefaultValueLiteralForProperty), the import was already written.

I found two places where I could make this check:

Version 1:

In Kiota.Builder.Refiners.CommonLanguageRefiner.AddPropertiesAndMethodTypesImports, I could add the following code snippet before those lines:

kiota/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs

Lines 805 to 806 in 7e04625

	if (usingsToAdd.Length != 0)
	(currentClass.Parent as CodeClass ?? currentClass).AddUsing(usingsToAdd); //lots of languages do not support imports on nested classes

foreach (CodeProperty prop in currentClass.Properties)
{
    //Only properties with default value
    if (!string.IsNullOrEmpty(prop.DefaultValue))
    {
        string erasableUsing = string.Empty;
        if (prop.Type.Name.Equals(TypeScriptConventionService.TYPE_DATE_ONLY, StringComparison.OrdinalIgnoreCase))
        {
            erasableUsing = "DateOnly";
        }
        else if (prop.Type.Name.Equals(TypeScriptConventionService.TYPE_TIME_ONLY, StringComparison.OrdinalIgnoreCase))
        {
            erasableUsing = "TimeOnly";
        }
        if (!string.IsNullOrEmpty(erasableUsing))
        {
            //Set using to "Enabled":
            foreach (CodeUsing currentUsing in currentClass.Usings)
            {
                if (currentUsing.Name == erasableUsing)
                {
                    currentUsing.IsErasable = false;
                }
            }
        }
    }
}

(yes, it is definitively not your code style, but it is easier to read for discussing it ;-))

This sounds quite logic, as it happens in the moment when the usings are picked from classes/properties.
But this is the common refiner, this code is invoked for all languages. Though CodeUsing.IsErasable is only used for TypeScript, it makes no sense to do TypeScript specific checks here.

I probably could add a Action callback parameter to AddPropertiesAndMethodTypesImports that could manipulate the usingsToAdd for TypeScript and is null for all other languages.

Update: following this approach and having two model classes, one with a DateOnly property with default, the other one with a DateOnly property without default, it might render the import twice:

import { DateOnly, type DateOnly, ...} ...

Possible workaround:

In Kiota.Builder.Writers.TypeScript.CodeUsingWriter.WriteCodeElement I could add code to set IsErasable = false to all usings of this name is any of them is not erasable.

kiota/src/Kiota.Builder/Writers/TypeScript/CodeUsingWriter.cs

Lines 15 to 18 in 6a0acc6

	public void WriteCodeElement(IEnumerable<CodeUsing> usings, CodeNamespace parentNamespace, LanguageWriter writer)
	{
	ArgumentNullException.ThrowIfNull(writer);
	var enumeratedUsings = usings.ToArray();

var groups = enumeratedUsings.GroupBy(x => x.Name);
foreach (var group in groups)
{
    bool anyNotErasable = group.Any(x => !x.IsErasable);
    if (anyNotErasable)
    {
        foreach (CodeUsing usingX in group)
        {
            usingX.IsErasable = false;
        }
    }
}

Version 2:
Kiota.Builder.Writers.TypeScript.CodeFileDeclarationWriter.WriteCodeElement is invoked to write the import statement.

Here I could add code beetween those two blocks:

kiota/src/Kiota.Builder/Writers/TypeScript/CodeFileDeclarationWriter.cs

Lines 38 to 45 in 7e04625

	var filteredUsing = enumeratedUsing.Where(static x => x.IsExternal)
	.Union(enumeratedUsing
	.Where(static x => x is { IsExternal: false, Declaration.TypeDefinition: not null })
	.GroupBy(static x =>
	$"{x.Declaration!.TypeDefinition!.GetImmediateParentOfType<CodeNamespace>().Name}.{x.Declaration?.Name.ToLowerInvariant()}")
	.Select(static x => x.OrderByDescending(static x => x.Alias, StringComparer.OrdinalIgnoreCase).ThenBy(static x => x.Parent?.Name, StringComparer.OrdinalIgnoreCase).First()));
	_codeUsingWriter.WriteCodeElement(filteredUsing, ns, writer);

The code is similar to the one above:

foreach (CodeInterface ci in cf.Interfaces)
{
    foreach (CodeProperty cp in ci.Properties)
    {
        //Only properties with default value
        if (!string.IsNullOrEmpty(cp.DefaultValue))
        {
            string erasableUsing = string.Empty;
            if (cp.Type.Name.Equals(TypeScriptConventionService.TYPE_DATE_ONLY, StringComparison.OrdinalIgnoreCase))
            {
                erasableUsing = "DateOnly";
            }
            else if (cp.Type.Name.Equals(TypeScriptConventionService.TYPE_TIME_ONLY, StringComparison.OrdinalIgnoreCase))
            {
                erasableUsing = "TimeOnly";
            }
            if (!string.IsNullOrEmpty(erasableUsing))
            {
                //Set using to "Enabled":
                foreach (CodeUsing currentUsing in filteredUsing)
                {
                    if (currentUsing.Name == erasableUsing)
                    {
                        currentUsing.IsErasable = false;
                    }
                }
            }
        }
    }
}

This does not feel correct, as this is far away from the place where the CodeUsings were collected.

What do you think? Hope you have a better place.

[WolfgangHG]

OK, I followed my suggestion "Version 1" and added a callback to CommonLanguageRefiner.AddPropertiesAndMethodTypesImports that sets CodeUsing.IsErasable = false for DateOnly/TimeOnly properties with default values.
In TypeScriptRefiner.AddAliasToCodeFileUsings a cleanup happens before rendering: if any CodeUsing has IsErasable == false, this is copied to all other usings for this type in the same CodeFile.

[WolfgangHG]

With the latest commit, all tests should work again and the pull request is ready for review.

I wonder why "ruby" is excluded in all tests in "config.json" - I also copied this to my api description test. Anyway, the test is executed in IT: https://github.com/microsoft/kiota/actions/runs/23022145076/job/66971518192?pr=7414

I will try to add a prototyp commit for running my api description with the mock server - in this code I could create an instance of my model class and thus check that parsing of date values does not fail. The mock server is not required here, but I consider it helpful