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

Private readonly fields - are overwritten/generated automatically #175

Closed
rizi opened this issue May 24, 2024 · 9 comments
Closed

Private readonly fields - are overwritten/generated automatically #175

rizi opened this issue May 24, 2024 · 9 comments

Comments

@rizi
Copy link

rizi commented May 24, 2024

Hi,

With this fix: #143, private readonly fields are now also set automatically, is there a way to prevent/controll this with some kind of configuration (per type)?

This can cause issues when the field is initilized via constructor, because afterwards it's getting overwritten bei Autobogus.

Here is a very simple sample.

public class AnotherObjectToFake
{
    private readonly string _key;

    public AnotherObjectToFake(string key)
    {
        _key = key;
    }

    public string GetKey()
    {
        return _key;
    }
}

//test class
 [TestMethod]
 public void PrivateReadOnlyField_Should_Not_Be_Overwritten()
 {
     //arrange
     const string key = "someKey";

     //act
     AnotherObjectToFake objectToFake = new AutoFaker<AnotherObjectToFake>()
         .CustomInstantiator(_ => new AnotherObjectToFake(key));

     //assert
     objectToFake.GetKey().Should().Be(key);
 } 

image

Br

@rizi rizi changed the title Private readonly fields - are set Private readonly fields - are overwritten/generated automatically May 24, 2024
@soenneker
Copy link
Owner

soenneker commented May 26, 2024

I looked into this... there doesn't seem to be a straightforward way of solving this.

CustomInstantiator builds a 'CreateAction' which is for the 'Create' action within Bogus, and 'Finalize' is used to populate after the variables have been created with AutoBogus.

Since it's private, a rule can't be created within Faker. An override could be a solution that works now.

Other options:

  • Create a toggle for disabling private variable population
  • Create a string lookup type of ignore list

@rizi
Copy link
Author

rizi commented May 26, 2024

I looked into this... there doesn't seem to be a straightforward way of solving this.

CustomInstantiator builds a 'CreateAction' which is for the 'Create' action within Bogus, and 'Finalize' is used to populate after the variables have been created with AutoBogus.

Since it's private, a rule can't be created within Faker. An override could be a solution that works now.

Other options:

  • Create a toggle for disabling private variable population
  • Create a string lookup type of ignore list

Thx for your answer, the override is working, but I couldn't find a way (tried it the last two days) to create a general AutoFakerOverride that can be used to ingore all fields (regardless of the type) --> maybe you could point me in the right direction (if there's a way).

So I think the best way would be to:

  • Create a toggle for disabling private variable population

How much effort do you think this would be?

I think it would also be great solution, that when a CustomInstantiator is used that no fields are initialized from AutoBogus, in other words if a CustomInstantiator is used the the toogle should be set to "ignore fields" automatically --> don't know if this is possible, what do you think?

Br

@soenneker
Copy link
Owner

@rizi

Create a toggle for disabling private variable population

Do you want to make this change? The main changes would need to be done in the reflection cache library. Probably should just have a bool excluding private fields or just pass in the reflection flags into the cache.

Then in this library add a config option and then configure the cache.

I think it would also be great solution, that when a CustomInstantiator is used that no fields are initialized from AutoBogus

I am not certain that this actually is possible without some changes within Bogus.. I remember looking into this but want to check again.

@rizi
Copy link
Author

rizi commented May 28, 2024

@rizi

Create a toggle for disabling private variable population

Do you want to make this change? The main changes would need to be done in the reflection cache library. Probably should just have a bool excluding private fields or just pass in the reflection flags into the cache.

Then in this library add a config option and then configure the cache.

I think it would also be great solution, that when a CustomInstantiator is used that no fields are initialized from AutoBogus

I am not certain that this actually is possible without some changes within Bogus.. I remember looking into this but want to check again.

@soenneker I would happily make the change if you tell me how I get the adapted version (after my change) of your cache library in your AutoFaker Library --> is there NuGet involved or something like that?

About what kind of config do you think, a config per type or a global one?

Br

@soenneker
Copy link
Owner

is there NuGet involved or something like that?

Yes, check soenneker.reflection.cache

About what kind of config do you think, a config per type or a global one?

I think a global one would be good to start out with

@rizi
Copy link
Author

rizi commented May 29, 2024

is there NuGet involved or something like that?

Yes, check soenneker.reflection.cache

About what kind of config do you think, a config per type or a global one?

I think a global one would be good to start out with

@soenneker here is a first draft to add the possibility to configure which properties/fields should be taken into account.
soenneker/soenneker.reflection.cache#94

Could you please also tell me how to push the latest version of soenneker.reflection.cache to nuget so I can use it once you are happy with the solution.

br

@rizi
Copy link
Author

rizi commented May 31, 2024

@soenneker
I found a few places where ReflectionCache is used (see the list below):

private Lazy<ReflectionCache> _cacheLazy = new(() => new ReflectionCache(), true);

public static ReflectionCache ReflectionCache { get; set; } = new ReflectionCache();

private static readonly Lazy<ReflectionCache> _cacheLazy = new(() => new ReflectionCache(), true);

CacheService = new CacheService();

I think the correct place to pass the config to the Reflection Cache is within the AutoFaker class

What do you think, is this the correct place to pass the config to the Reflection Cache?

What should I do with the other usages (of Reflection Cache), pass the config there as well?

I would add the new option(s) to the AutoFakerConfig, do you think that's O.K?

br

@soenneker
Copy link
Owner

soenneker commented Jun 1, 2024

@rizi

I appreciate you calling out the various usages of ReflectionCache... I was able to consolidate that nicely in this commit: e724a45

I then added the ReflectionCacheOptions to AutoFakerConfig. Along with that I implemented delayed initialization so that Config can be set either within the constructor or just before Generate.

I'll also add that the example in your first comment, since the variable is set from the constructor (and that's public), it will still get set from Autobogus.

Let me know how it works. Thanks again for your help, please keep at it!

@rizi
Copy link
Author

rizi commented Jun 1, 2024

@soenneker it's woking fine, thx for your help!

@rizi rizi closed this as completed Jun 1, 2024
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

No branches or pull requests

2 participants