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

Readonly properties are no longer readonly when using backing store #499

Open
rangp opened this issue Dec 19, 2024 · 1 comment
Open

Readonly properties are no longer readonly when using backing store #499

rangp opened this issue Dec 19, 2024 · 1 comment
Labels
priority:p2 Medium. For a p2 bug, generally have a work-around. Bug SLA <=30 days type: bug A broken experience type:regression A bug from previous release

Comments

@rangp
Copy link

rangp commented Dec 19, 2024

While generating a c# client with kiota version 1.21.0, I noticed that readonly properties are no longer readonly when using a backing store.

My readonly property was generated like this:

public double? AveragePriceGross { get; private set; }

After re-generating the client with a backing store, it now looks like this and I can set the property, even though it is marked as readonly in my api definition

        public double? AveragePriceGross
        {
            get { return BackingStore?.Get<double?>("average_price_gross"); }
            set { BackingStore?.Set("average_price_gross", value); }
        }

I understand that with backing store enabled, one can always change the property by manually changing the value in the underlying store, but I think the behavior should be consistent and the generated properties should either always or never be readonly, independent of whether a backing store is used.

@baywet
Copy link
Member

baywet commented Dec 23, 2024

Hi @rangp
Thank you for using kiota and for reaching out.

I'd like to mention that we're operating with a reduced team at the moment, expect delays in responses, thank you for your patience!

This is where it's written

https://github.com/microsoft/kiota/blob/1b9c9f9f6dfd0b873cf158994d2805bb8ff93520/src/Kiota.Builder/Writers/CSharp/CodePropertyWriter.cs#L58

This is where it's being tested

https://github.com/microsoft/kiota/blob/1b9c9f9f6dfd0b873cf158994d2805bb8ff93520/tests/Kiota.Builder.Tests/Writers/CSharp/CodePropertyWriterTests.cs#L107

The only challenge here is that reducing the accessibility of the setter is a source breaking change. So we'll have to have an additional condition to only apply the access modifier when Exclude Backwards Compatible is passed (--ebc).

Is this something you'd like to submit a pull request for provided some guidance?

@baywet baywet added type: bug A broken experience type:regression A bug from previous release priority:p2 Medium. For a p2 bug, generally have a work-around. Bug SLA <=30 days labels Dec 23, 2024
@baywet baywet moved this from Needs Triage 🔍 to New📃 in Kiota Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:p2 Medium. For a p2 bug, generally have a work-around. Bug SLA <=30 days type: bug A broken experience type:regression A bug from previous release
Projects
Status: New📃
Development

No branches or pull requests

2 participants