Cannot Ignore Protected Property

[SuricateCan]

First of all, great lib and great work!

The issue I'm having is as described. I cannot opt to ignore a property if that property is protected. In fact, I cannot do anything to a hidden property. The method .Ignore() (and all others) only allows for an expression to be passed in. That makes it dificult to handle hidden members.

My suggestion is that these methods have an overload with a string parameter (like EF does) to allow a property name to be passed in, like .Ignore("MyProtectedProperty") or .RuleFor<string>("MyProtectedProperty", f =>f.Random.Word()).

To work around this issue, I manage to get it working by using .RuleForType<TType>(Type, Func) after some try and error to get the types correct, however, if the type is shared between multiple properties, there is no way to configure the target property alone.

That's all. Keep up the good work!

Thanks,

Cleverson Nascimento

[nickdodd79]

Hey @SuricateCan,

I have been meaning to add a string version of the Ignore capability, so I will see what I can do.

In terms of the RuleFor request, that is something the Bogus library would need to add. AutoBogus inherits that functionality via the Faker class. @bchavez do you have any comments around adding the above or may be it can already be done?

Nick.

[SuricateCan]

@nickdodd79 Now that I think about it, it really seems to be an issue/implementation for the Bogus library.

Let's see what @bchavez thinks about it. If that's the case, I can open this issue there.

PS.: I'm not stucked because of this issue. It's more a QoL change, at least for me right now, so, no hurries guys!

[bchavez]

Hey friends,

Apologies for the late reply. I don't see any immediate blocking issue to allow property or field names as strings. Essentially, all .RuleFor(x => prop, f => f...) methods end up calling .RuleFor("prop", f => f...) as shown here:

https://github.com/bchavez/Bogus/blob/cc7feabc8aad91604433723eb5f0590e4fe384ea/Source/Bogus/Faker%5BT%5D.cs#L211-L225

The .RuleFor("prop", f => f...) method is already declared protected virtual as shown in the link above. Technically, one could do the following right now with the current release:

void Main()
{
   var orderFaker = new MyFaker<Order>()
          .RuleForByName(nameof(Order.OrderId), f => f.IndexVariable++)
          .RuleForByName(nameof(Order.Quantity), f => f.Random.Number(1, 3))
          .RuleForByName(nameof(Order.Item), f => f.Commerce.Product());

   var order = orderFaker.Generate();
   order.Dump();
}

public class MyFaker<T> : Faker<T> where T : class
{  
   public MyFaker<T> RuleForByName<TProperty>(string propertyOrField, Func<Faker, TProperty> setter)
   {
      return this.RuleFor(propertyOrField, setter) as MyFaker<T>;
   }
}

public class Order
{
   public int OrderId { get; set; }
   public string Item { get; set; }
   public int Quantity { get; set; }
}

I agree it is a bit kludgy, but it works.

I enabled public virtual on .RuleFor("prop", f => f...) and unit tests seemed to indicate nothing broke; so that's a good sign. :+1: The following unit test demonstrates a public .RuleFor("prop", f => f..):

[Fact]
public void can_use_rulefor_by_string_value()
{
   var orderFaker = new Faker<Order>()
      .RuleFor(nameof(Order.OrderId), f => f.IndexVariable++)
      .RuleFor(nameof(Order.Quantity), f => f.Random.Number(1, 3))
      .RuleFor(nameof(Order.Item), f => f.Commerce.Product());

   var order = orderFaker.Generate();

   order.OrderId.Should().Be(0);
   order.Quantity.Should().Be(2);
   order.Item.Should().Be("Computer");
}

However, I have a few reservations thus far mostly concerning API aesthetics and semantics.

Does .RuleFor("prop", f => f...) fit well here? Should .RuleFor("prop", f => f...) be alongside other first-class APIs at the same visibility and rank as the current .RuleFor(x => x.prop, f => f...)? Maybe. Maybe not. Either way, it would be great to get both of your feedback on this very topic.

My thoughts are described below and come with two major concerns right now:

1. Is this an advanced scenario?

My feeling is this .RuleFor("prop", f => f...) method would be used for more advanced scenarios that would fall outside the main usage scenarios.

The angle I'm looking at this comes from Framework Design Guidelines: Conventions, Idioms, and Patterns for Reusable .NET Libraries by Krzysztof Cwalina and Brad Abrams. The important points are outlined below (emphasis mine):

Pg 9, Framework Design Fundamentals

:heavy_check_mark: DO design frameworks that are both powerful and easy to use. A well-designed framework makes implementing simple scenarios easy. At the same time, it does not prohibit implementing more advanced scenarios, though these might be more difficult. As Alan Kay said, "Simple things should be simple and complex things should be possible."

Pg 14, Framework Design Fundamentals - Fundamental Principals of Framework Design

Rico Mariani: The flip side of this is that it must not only be easy to use the API; it must also be easy to use the API in the best possible way. Think carefully about what patterns you offer and be sure that the most natural way to use your system gives results that are correct, is secure against attacks, and has great performance. Make it hard to do things the wrong way.

The Pit of Success In stark contrast to a summit, a peak, or a journey across a desert to find victory through many trials and surprises, we want our customers to simply fall into winning practices by using our platform and frameworks. To the extent that we make it easy to get into trouble we fail. [...] Build a pit of success.

Pg 24, Framework Design Fundamentals - Fundamental Principals of Framework Design

:x: DO NOT have members intended for advanced scenarios on types intended for mainline scenarios.

Pg 34-36, Framework Design Fundamentals - Fundamental Principals of Framework Design

The general guideline for building a single framework that targets the breadth of developers is to factor your API set into low-level types that expose all the richness and power and into high-level types that wrap the lower layer with convenience APIs.

2.2.4.1 Exposing Layers in Separate Namespaces One way to factor a framework is to put the high-level and low-level types in different but related namespaces. This has the advantage of hiding the low-level types from the mainstream scenarios without putting them too far out of reach when developers need to implement more complex scenarios.

:heavy_check_mark: CONSIDER using a layered framework with high-level APIs optimized for productivity, and using low-level APIs optimized for power and expressiveness.

Pg 164, Member Design - Extension Methods

Phil Haack: Section 2.2.4.1 covers the topic of layering namespaces, which I think applies well to extension methods. One scenario I've seen is to put functionality that is more advanced or esoteric in a separate namespace. That way, for core scenarios, these extra methods do not 'pollute' the API. But for those who know about the namespace, or need these alternate scenarios, they can add the namespace and gain access to these extra methods. The drawback, of course, is that this results in a 'hidden' API that is not very discoverable.

If we decide Faker<T>.RuleFor("prop", f => f...) is more part of an advanced scenario, we could hide .RuleFor("prop", f => f...) as an internal method on Faker<T>, and expose an additional extension method in a Bogus.Advanced namespace that wires into that internal method on Faker<T>. For example,

namespace Bogus.Advanced
{
   public static class ExtensionsForAdvancedFakerTUsage
   {
      public static Faker<T> RuleFor<T, TProperty>(this Faker<T> faker, string propertyOrField, Func<Faker, TProperty> setter)
         where T : class
      {
         return faker.RuleFor(propertyOrField, setter);
      }
   }
}

Usage

Maybe I'm thinking too much about it and maybe it's not a big deal. I don't know. But I do care very much about developers falling into the pit of success when using Bogus from an ergonomics and semantics standpoint.

However, I do agree, it would be nice to have something either as public or internal+extension method as described above.

2. What do we do when StrictMode(true) and a rule is defined for a non-existent member?

The next pressing issue regarding enabling an advanced API like this is what does it mean when .StrictMode(true) and we have someone define a rule for a member of T that does not exist. See the following:

[Fact]
public void what_happens_when_having_a_rulefor_a_field_that_doesnt_exist()
{
   var orderFaker = new Faker<Order>()
      .StrictMode(true)
      .RuleFor(nameof(Order.OrderId), f => f.IndexVariable++)
      .RuleFor(nameof(Order.Quantity), f => f.Random.Number(1, 3))
      .RuleFor(nameof(Order.Item), f => f.Commerce.Product())
      .RuleFor("hhhhh", f=> f.Random.Number());

   orderFaker.AssertConfigurationIsValid();

   var order = orderFaker.Generate();

   order.OrderId.Should().Be(0);
   order.Quantity.Should().Be(2);
   order.Item.Should().Be("Computer");
}

Should AssertConfigurationIsValid() throw because hhhhh doesn't exist? Or should we not care? At first glance, I'm thinking strict means strict; so, yes it should be considered an invalid configuration when a member that is being defined doesn't exist (from the binder's perspective); in the same way that using .Rules with .StrictMode(true) throws due to not being able to validate each member on T having a rule.

Faker<T>.Rules(): https://github.com/bchavez/Bogus/blob/cc7feabc8aad91604433723eb5f0590e4fe384ea/Source/Bogus/Faker%5BT%5D.cs#L240-L246

.Validation of .Rules() in StrictMode(true): https://github.com/bchavez/Bogus/blob/cc7feabc8aad91604433723eb5f0590e4fe384ea/Source/Bogus/Faker%5BT%5D.cs#L715

---

So, those are my initial thoughts.

Also, feel free to create a GitHub issue in Bogus if you all feel we should move forward with changes in Bogus and enabling this scenario.

Thanks, Brian

:car: :blue_car: "Let the good times roll..."

[SuricateCan]

Hey @bchavez, Thanks for joining the thread. Perhaps I should've thought a little bit more about it before opening an issue in this repository. However, here are my thoughts about your considerations:

The .RuleFor("prop", f => f...) method is already declared protected virtual as shown in the link above. Technically, one could do the following right now with the current release: ... I agree it is a bit kludgy, but it works.

Yes, it is a bit toilsome but gets the job done. My problem with it is that it forces me into a path of advanced scenario when, in reality, all I want to do is a simple rule. Which brings me to your next point:

1. Is this an advanced scenario?

IMO this is not an advanced scenario. Let me give you an example:

new Faker<MyClass>()
    .RuleFor(a => a.PublicProperty, f => f.Random.Int())
    .RuleFor<string>("ProtectedProperty", f.Random.Word())
    .Generate();

Note that I'm not entering rule sets or other specific scenarios, I'm just setting up a property for generation. One of the things I love about this library (and by extension, about Bogus) is that I can simple declare a private faker in my test class, set it up with rules for the scenarios I'm about to test, and use it. No need to inherit or extend the default behavior. This is one of the reasons I think this does not classify as advanced.

Another example of this being "expected" from the lib is that I'm used to configure my EF Core entities using Fluent API, and the overloads allowing hidden properties or fields to be set up are all over the place, so I imagined that it would be "natural" to have these methods exposed. However I understand your concerns.

If we decide Faker.RuleFor("prop", f => f...) is more part of an advanced scenario, we could hide .RuleFor("prop", f => f...) as an internal method on Faker, and expose an additional extension method in a Bogus.Advanced namespace that wires into that internal method on Faker.

As I said before, IMO, using the new exposed methods is not an advanced scenario and having to rely on "hidden" APIs (as you emphasized) is not a good idea.

2. What do we do when StrictMode(true) and a rule is defined for a non-existent member?

I totally agree. Strict means strict. It should throw when the property is not found (or have incompatible types), allowing for the developers to catch possible refactoring remnants in their tests.

Also, feel free to create a GitHub issue in Bogus if you all feel we should move forward with changes in Bogus and enabling this scenario.

Yes, I'll close the issue here and bring the discussion over to Bogus. I think that it's the best place to continue forward with this.

Summing up:

Does .RuleFor("prop", f => f...) fit well here? Should .RuleFor("prop", f => f...) be alongside other first-class APIs at the same visibility and rank as the current .RuleFor(x => x.prop, f => f...)?

IMO, yes.

Maybe I'm thinking too much about it...

Yes. 😄 But that's ok!

@nickdodd79 Feel free to continue the discussion here or over in Bogus.

Thanks guys!

[nickdodd79]

Hey @SuricateCan and @bchavez,

I have just pushed v2.7.0 which upgrades to the latest version of Bogus and adds a string version of the skip config:

.WithSkip<Person>("Age");

Nick.

[bchavez]

Great work Nick! Thanks for keeping AutoBogus up to date! Also, thanks again for your help! If you need anything else, please don't hesitate to let me know. :+1:

[SuricateCan]

Great work! I think this is a nice addition to the lib. Thanks for your quick response.