[CSHARP-4374] Add SetPropertyGetter and SetPropertySetter methods to BsonMemberMap Created: 20/Oct/22  Updated: 10/Nov/22

Status: Backlog
Project: C# Driver
Component/s: Serialization
Affects Version/s: 2.18.0
Fix Version/s: None

Type: New Feature Priority: Unknown
Reporter: David Mannion Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
is duplicated by CSHARP-3531 Init properties sometimes are not ser... Closed
Quarter: FY23Q2

 Description   

A common paradigm is to create a private backing field for a public property on a C# class (POCO).  Often times there is logic in the property setter to validate input, modify a related field or record an event log/history of the change.  This logic should not be performed when deserializing the object from the database.  Currently, the only alternative is to provide a creator/constructor method.  This can be painful for classes with many properties, especially if only one ( or a few) property(ies) have backing fields and setter logic that should be skipped on re-hydration.  It can also lead to bugs when new properties are added but developers forget to add them to the constructor.

By allowing callers to set a custom setter(and getter) actions, developers can still AutoMap the class, but customize individual properties that require special handling.  Something like:

public BsonMemberMap SetPropertySetter(Action<object, object> setter) 

Ideally, there would also be a overload like this:

public BsonMemberMap SetPropertySetter(string memberName)

This overload will allow developers to simply specify an alternate member, like a private backing field,  to use when setting the value during deserialization.
 

There are probably fewer use cases for customizing the Getter, but it could still be valuable and would maintain symmetry:

public BsonMemberMap SetPropertyGetter(Func<object, object> getter) 

public BsonMemberMap SetPropertyGetter(string memberName)



 Comments   
Comment by James Kovacs [ 03/Nov/22 ]

Hi, David,

Thank you for the feedback and continued discussion. You raise many valid points. The team is going to consider implementation options (including the one that you proposed) and decide on a direction forward. We plan to implement this feature in an upcoming release. Please follow this ticket for updates.

Sincerely,
James

Comment by David Mannion [ 02/Nov/22 ]

Your proposed workaround suffers from a similar drawback as the the constructor solution:  database persistence code is leaking into the domain model.  A big advantage of this driver is that it can all be configured outside the domain model, so we don't have to muck up our entities with persistence or serialization code/attributes.

My proposed feature is optional.  For cases where it could cause inconsistent state, it doesn't have to be used.  But keep in mind it would only cause inconsistent state when the database is already in an inconsistent state with respect to the class logic.  And I don't see how the current implementation helps with this scenario.  By respecting the class logic, you risk having a mismatch between the instantiated object instance and what's in the database.  My proposed feature actually gives developers more options for handing these cases (i.e. a custom Action that checks for inconsistent state and decided how to handle it, on a per field basis, without changing the domain model).

This feature would also give us more options for handling things like immutable fields, that should only be specified at creation time.  Currently, read-only fields are written to the database, but not read back when deserializing.  This makes sense for calculated fields, but not immutable fields like a "Created" timestamp.  Right now we have to create a private settter for such fields just for the MongoDB driver - another case of having to modify our class to fit the persistence layer.

 

Comment by Dmitry Lukyanov (Inactive) [ 01/Nov/22 ]

Hey dmannion@propractice.com,

Validation typically happens before the data is persisted to the database.

typically yes, but not necessary in all cases

how did it get there?

I see a number of options, but the easiest way is when some other process has changed it. And with your suggestion, your data level won't even notice it and will think that provided data is valid.

As a workaround for your issue, you can inherit your class from ISupportInitialize (see for details) that can allow you to determine whether your class is called from deserializer or not. Pay attention that BeginInit is called right after a ctor call. It can look similar too:

        public class Test : ISupportInitialize
        {
            private TrainingStatus status;
            private bool _deserialization;
 
            public TrainingStatus Status
            {
                get
                {
                    return this.status;
                }
                private set
                {
                    if (!_deserialization)
                    {
                        this.History.Add(new TrainingHistory()
                        {
                            Event = TrainingEvent.StatusChanged,
                            Description = $"Status changed from {this.status} to {value}",
                        });
                    }
                    this.status = value;
                }
            }
 
            public void BeginInit() => _deserialization = true;
            public void EndInit() => _deserialization = false;
        }

Comment by David Mannion [ 24/Oct/22 ]

The deserialized object should be identical to the serialized object.  Validation typically happens before the data is persisted to the database.  If the data is in an invalid state in the database. 

  1. how did it get there?
  2. how would it be fixed?

The same problem you purpose could exist if I used a constructor, which I'm guessing is how most developers are solving this problem.  The problem with the constructor solution is:

  1. It forces me to add a constructor method that should only be used by the persistence layer.  Its not prescriptive for the users of my class since they shouldn't really use it to create new instances.
  2. I have to remember to add new fields to it as the class evolves over time.  If my class has 6 properties and only one of them as setter logic, I have to create a constructor will all 7 properties and keep adding all new fields to that constructor going forward.  It creates a maintenance burden all because of the one field.  And the constructor gets very ugly over time.

The purpose of the database is to persist the state of the object, exactly as it is.  The state of an object is represented by its data, which is stored in fields.  I don't want to "process" or validate the data on the way in and out of the database, so I want the private field data serialized and deserialized.  This is essentially the same as Json.NET's (Newtonsoft)  "fields mode" (https://www.newtonsoft.com/json/help/html/SerializationGuide.htm).    But I also need to be able to run LINQ queries using the public properties as a representation for the object. This is what breaks if I map the private field and unmap the corresponding public property.

Here is the code for the property that I motived this: 

public TrainingStatus Status {
            get
            {
                return this.status;
            }
            private set
            {
                this.History.Add(new TrainingHistory()
                {
                    Event = TrainingEvent.StatusChanged,
                    Description = $"Status changed from {this.status} to {value}",
                     
                });
                this.status = value;
            }
        }

 Obviously, I don't want new history records every time the object is deserilaized from the database.  I can fix this by mapping the status field and unmapping the Status property.  But then this code errors out:

var query = from training in trainingStore
            where training.UserId == userId && training.Status != TrainingStatus.Completed && training.Status != TrainingStatus.Submitted
             orderby training.DueBy ascending
             select training; 

My current solution, which seems to work well,  is to use .NET reflection to modify the BsonMemberMap._setter for the public Status property to get the value from the private status field.  I'm much prefer an officially supported solution and I think others would find this really valuable and help support a lot more use cases.

Comment by Dmitry Lukyanov (Inactive) [ 21/Oct/22 ]

Hey dmannion@propractice.com , thank you for your feature request.

Can you please provide more details about your use case to better understand your suggestion.

Meanwhile, please see some initial thoughts about this request.

This logic should not be performed when deserializing the object from the database.

This doesn't look like good behavior. If your properties have some validation, then this validation should be important to have consistent state of the deserialized class. For example, let's say you disallow the value 0 for property A, the rest of logic in the class can rely on that restriction. So when the property A became 0, then the whole class state can be affected by this which will lead to make the deserialized instance in unexpected state that even won't be visible outside this class (ie for consumers of this class). 

 

Generated at Wed Feb 07 21:48:02 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.