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

Possible regression: ConfigurationBinder cannot bind private properties of base classes #71040

Closed
lateapexearlyspeed opened this issue Jun 21, 2022 · 4 comments · Fixed by #71039

Comments

@lateapexearlyspeed
Copy link
Contributor

lateapexearlyspeed commented Jun 21, 2022

Description

In recent main branch, I found seems ConfigurationBinder cannot bind private properties of base classes, but before that, it can (for example .net 3, 5, 6).

In .net 3, 5, 6:
image

Recent main branch (I add an test case to show result):
image

        [Fact]
        public void CanBindPrivatePropertiesFromBaseClass()
        {
            ConfigurationBuilder configurationBuilder = new();
            configurationBuilder.AddInMemoryCollection(new Dictionary<string, string>
            {
                { "PrivateProperty", "a" }
            });
            IConfiguration config = configurationBuilder.Build();

            var test = new ClassOverridingVirtualProperty();
            config.Bind(test, b => b.BindNonPublicProperties = true);
            Assert.Equal("a", test.ExposePrivatePropertyValue());
        }

        public class BaseClassWithVirtualProperty
        {
            private string? PrivateProperty { get; set; }

            public string? ExposePrivatePropertyValue() => PrivateProperty;
        }

        public class ClassOverridingVirtualProperty : BaseClassWithVirtualProperty
        {
        }

I raised PR to try to fix that, meantime can handle virtual properties.

Reproduction Steps

As above.

Expected behavior

ConfigurationBinder may should be able to bind private properties of base classes.

Actual behavior

ConfigurationBinder cannot bind private properties of base classes in recent main branch.

Regression?

Yes, it did.

Known Workarounds

No response

Configuration

Found this from main branch with commit version: 2ce8130, at least.

Other information

I raised PR to try to fix that, meantime can handle virtual properties.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 21, 2022
@ghost
Copy link

ghost commented Jun 21, 2022

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

In recent main branch, I found seems ConfigurationBinder cannot bind private properties of base classes, but before that, it can (for example .net 3, 5, 6).

In .net 3, 5, 6:
image

Recent main branch (I add an test case to show result):
image

        [Fact]
        public void CanBindPrivatePropertiesFromBaseClass()
        {
            ConfigurationBuilder configurationBuilder = new();
            configurationBuilder.AddInMemoryCollection(new Dictionary<string, string>
            {
                { "PrivateProperty", "a" }
            });
            IConfiguration config = configurationBuilder.Build();

            var test = new ClassOverridingVirtualProperty();
            config.Bind(test, b => b.BindNonPublicProperties = true);
            Assert.Equal("a", test.ExposePrivatePropertyValue());
        }

        public class BaseClassWithVirtualProperty
        {
            private string? PrivateProperty { get; set; }

            public string? ExposePrivatePropertyValue() => PrivateProperty;
        }

        public class ClassOverridingVirtualProperty : BaseClassWithVirtualProperty
        {
        }`

### Reproduction Steps

As above.

### Expected behavior

`ConfigurationBinder` may should be able to bind private properties of base classes.

### Actual behavior

`ConfigurationBinder` cannot bind private properties of base classes in recent main branch.

### Regression?

Yes, it did.

### Known Workarounds

_No response_

### Configuration

Found this from main branch with commit version: 2ce8130f4bf4b02393b95719edb0f9d5c6ee1776, at least.

### Other information

I raised [PR](https://github.com/dotnet/runtime/pull/71039) to try to fix that, meantime can handle virtual properties.

<table>
  <tr>
    <th align="left">Author:</th>
    <td>lateapexearlyspeed</td>
  </tr>
  <tr>
    <th align="left">Assignees:</th>
    <td>-</td>
  </tr>
  <tr>
    <th align="left">Labels:</th>
    <td>

`area-Extensions-Configuration`

</td>
  </tr>
  <tr>
    <th align="left">Milestone:</th>
    <td>-</td>
  </tr>
</table>
</details>

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 21, 2022
@eerhardt
Copy link
Member

cc @pedrobsaila. Looks like this might be a regression from #70592.

@lateapexearlyspeed
Copy link
Contributor Author

lateapexearlyspeed commented Jun 21, 2022

@eerhardt Thanks. Yes I noticed that PR before I raise my PR to fix. In short, I turn to keep using loop to retrieve all layers of properties (as .net 3/5/6) to be able to get also private properties (I guess this is reason to loop before). Meantime take care virtual and non virtual ones (even if with same names). Please review, thanks!

@eerhardt eerhardt added this to the 7.0.0 milestone Jun 21, 2022
@eerhardt eerhardt removed the untriaged New issue has not been triaged by the area owner label Jun 21, 2022
@lateapexearlyspeed
Copy link
Contributor Author

Hi @eerhardt, any chance to have a look at PR above ? :)

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 14, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants