ObjectContainerDataSource reflection behavior

Topics: Web Client Software Factory
Feb 6, 2007 at 10:56 PM
According to the doco
“ObjectContainerDataSourceStatusEventArgs class contains the following members:
Instance. This is the data object. The ObjectContainerDataSource control uses reflection to create this object (on every update operation, delete operation, and insert operation) and populates it with the input data from the data-bound control……”

In my scenario I have a person class that looks something like this

public class Person
{
public string UserId{...}
public string Title{...}
public string GivenName{...}
public string MiddleName{...}
public string FamilyName{...}
}

I am only attempting to update GivenName and FamilyName and hence those are the only two fields that I databind using a DetailsView and an ObjectContainerDataSource. I attached to the Updated event of the ObjectContainerDataSource. When I look at the Instance I discover that it is only populated with the GivenName and FamilyName and all the other fields are null.

Is this by design or is it a bug? Would it not make more sense to populate all the non databound properties with the original data?
Feb 6, 2007 at 11:41 PM
Perhaps something like this in the ObjectContainerDataSourceView?
note the region "New Code to populate old values"

    protected override int ExecuteUpdate(IDictionary keys, IDictionary values, IDictionary oldValues)
		{
		  Guard.CollectionNotNullNorEmpty(keys, String.Format(CultureInfo.CurrentCulture, Properties.Resources.NoKeysSpecified), "keys");
		  Guard.ArgumentNotNull(values, "values");
 
		  ObjectContainerDataSourceUpdatingEventArgs updatingEventArgs =
		    new ObjectContainerDataSourceUpdatingEventArgs(DictionaryHelper.GetReadOnlyDictionary(keys), values, oldValues);
		  OnUpdating(updatingEventArgs);
		  if (updatingEventArgs.Cancel)
		    return 0;
 
		  object newInstance = CreateInstance();
		  TypeDescriptionHelper.BuildInstance(keys, newInstance);
		  TypeDescriptionHelper.BuildInstance(values, newInstance);
		  int rowsAffected;
		  object oldInstance = FindInstance(keys);
		  if (oldInstance != null)
		  {
		    #region New Code to populate old values
 
		    PropertyDescriptorCollection properties = TypeDescriptor.GetProperties(oldInstance);
		    foreach (PropertyDescriptor property in properties)
		    {
		      if (!keys.Contains(property.Name) && !values.Contains(property.Name))
		      {
		        property.SetValue(newInstance, property.GetValue(oldInstance));
		      }
		    }
 
		    #endregion
 
		    int index = Data.IndexOf(oldInstance);
		    Data[index] = newInstance;
		    rowsAffected = 1;
		  }
		  else
		  {
		    rowsAffected = 0;
		  }
		  OnDataSourceViewChanged(EventArgs.Empty);
 
		  ObjectContainerDataSourceStatusEventArgs updatedEventArgs = new ObjectContainerDataSourceStatusEventArgs(newInstance, rowsAffected);
		  OnUpdated(updatedEventArgs);
 
		  return rowsAffected;
		}
Feb 7, 2007 at 1:48 AM
Hi, you bring to the board an interesting point of view. The only drawbacks I see in the solution you propose are:

- In the case you have read-only properties in your data object (for example, a value calculated from other properties), the code will throw an exception because it will try to set it. You can easily avoid this by checking that the property can be written before setting it.
- The values in your data object will potentially differ depending on whether you are using custom paging or not. If you are using the built-in paging functionality, your data object will hold the old values, but if you are using custom paging, it won’t hold them (because the old instance is not present when the Updated event is fired). This weird behavior may result in confusions for some users.

As a workaround for your situation, you could bind all the properties of your object and make some data-bound controls read-only so users are not allowed to update them. See the ObjectContainerDataSource QuickStart for an example of this.

If you think that this should be part of the code in the ObjectContainerDataSource control, feel free to open a work item in the Issue Tracker for it.

Thanks for the feedback.
Mariano Szklanny
http://staff.southworks.net/mariano
Mar 12, 2007 at 8:15 AM
For an update operation, could there be an option of not using reflection to create the (empty) object but to allow the developer to supply one?

This would allow the developer to go away and fetch the object from the database before the new values are taken from the view. The key fields for the object being edited would have to passed through to the developer as well...

What do you think?

Thanks,

Scott.


Oct 8, 2007 at 3:40 PM
Hi,

I am having the same issue here. I would also like to see the oldValues populate my newInstance. For me the data object is empty too (So object oldInstance = FindInstance(keys); gives oldInstance null), not because I am using custom paging, but simply because I did not see the point in getting the old values from the database before the update, when the old values are returned in ViewState by the DetailsView / GridView on PostBack. And that's exactly what's in the oldValues. So why not do

object newInstance = CreateInstance();
TypeDescriptionHelper.BuildInstance(oldValues, newInstance);
TypeDescriptionHelper.BuildInstance(keys, newInstance);
TypeDescriptionHelper.BuildInstance(values, newInstance);

And then at the same time change TypeDescriptionHelper.BuildInstance to not throw an InvalidOperationException for ReadOnly properties, but simply skip ReadOnly properties.

Also, the updatedEventArgs should in principle contain both the oldInstance and the newInstance. This is so that you could do optimistic concurrency checking in the database against any value (I prefer to use a timestamp key, in which case I don't need this, but in principle both oldInstance and newInstance should be available).

Was a workitem created for this in the Issue Tracker? I think it should be.

cheers

Remco
Oct 11, 2007 at 10:42 AM
further to my comment on the updatedEventArgs should in principle contain both the oldInstance and the newInstance. Does linq-to-sql not require both the oldInstance and the newInstance to re-attach to the DataContext in a disconnected scenario (asp.net postbacks) in order for its change-tracking to work?

remco wrote:
Also, the updatedEventArgs should in principle contain both the oldInstance and the newInstance. This is so that you could do optimistic concurrency checking in the database against any value (I prefer to use a timestamp key, in which case I don't need this, but in principle both oldInstance and newInstance should be available).

Oct 17, 2007 at 8:55 AM
Looks to me that two work items need to be added:
1) Readonly properties should be skipped rather than throw an exception
2) An option of not using reflection to create the (empty) object but to allow the developer to supply one

What about adding another property to the argument of the RowUpdating event that can be assigned by a developer. ObjectContainerDataSource looks to see whether this property has been assigned and if it has it updates this one instead of creating a new instance of the type that is being databound?

I'm happy to add these WorkItems's unless there are some better suggestions, anyone?

Thanks
Christian
Oct 21, 2007 at 11:31 PM
I have created two work items:

ObjectContainerDataSource should skip ReadOnly properties: http://www.codeplex.com/websf/WorkItem/View.aspx?WorkItemId=13416
ObjectContainerDataSource should allow a developer to supply instances of the bound type: http://www.codeplex.com/websf/WorkItem/View.aspx?WorkItemId=13417

I made very minor changes to the source for the ObjectContainerDataSource on my machine to support these work items so I at the moment this looks like viable solution.
Oct 21, 2007 at 11:31 PM
Edited Oct 21, 2007 at 11:32 PM
I have created two work items:

ObjectContainerDataSource should skip ReadOnly properties: http://www.codeplex.com/websf/WorkItem/View.aspx?WorkItemId=13416
ObjectContainerDataSource should allow a developer to supply instances of the bound type: http://www.codeplex.com/websf/WorkItem/View.aspx?WorkItemId=13417

I made very minor changes to the source for the ObjectContainerDataSource on my machine to support these work items - so far this looks like viable solution.
Oct 23, 2007 at 11:14 AM
and I created these two:

ObjectContainerDataSource UpdateEventArgs must have OldInstance as well as newInstance: http://www.codeplex.com/websf/WorkItem/View.aspx?WorkItemId=13454
ObjectContainerDataSource Updated must use oldValues as well as keys and values to construct Instance: http://www.codeplex.com/websf/WorkItem/View.aspx?WorkItemId=13455

please vote for these issues to be actioned
Oct 23, 2007 at 11:44 AM
christianacca,

I don't understand why you would want to supply your own instance of the bound type. The ObjectDataSourceView ExecuteUpdate method gets the oldValues, keys and values dictionaries passed in from which you can construct your oldInstance and newInstance. These dictionaries are all populated by the bound control when it has ViewState enabled. No need to get the oldInstance from the database on postback. Also you would want to perform optimistic concurrency checks against the old values you updated on the page which are round-tripped by ViewState. Not against the current values in the database after you post back your updates. You could end up overwriting another user's changes that were submitted in between when you recevied your page and you posted it back.


christianacca wrote:
2) An option of not using reflection to create the (empty) object but to allow the developer to supply one


Nov 26, 2007 at 1:03 AM
Edited Nov 26, 2007 at 1:04 AM
Remco:

I was checking your suggestion, but the oldvalues is an IOrderedDictionary of bound items, as seen in the HandleUpdate() from the FormView code(using reflector), so that takes us back to square one:

foreach (DictionaryEntry entry in this.BoundFieldValues)
{
e.OldValues.Add(entry.Key, entry.Value);
}

So I think the best option is to add a new property like OriginalValues in the ObjectContainerDataSource and in the ObjectContainerDataSourceView ExecuteUpdate do something like this:

if (_originalValues != null)
{
// start of with the original values passed by the user
newInstance = _originalValues;
}
else
{
newInstance = CreateInstance();
}
Mar 11, 2008 at 3:43 PM
Actually I was expecting the oldInstance to just be updated with the new values coming in from the control.
Something like:

...

int rowsAffected;
object oldInstance = FindInstance(keys);
if (oldInstance != null)
{
TypeDescriptionHelper.BuildInstance(values, oldInstance);
rowsAffected = 1;
}
else
{
rowsAffected = 0;
}

What exactly is the advantage of throwing the old object away, building a new one and then bothering to update a new object with the new values? Is there a benefit to this that I'm missing?
This approach leaves me with default values on my object for everything that was not bound to the control. What I would expect is to have my old instance be updated with values from the control - so if I'm using an ORM framework I can just persist that object without worrying exactly what properties have been changed.
I think such a change would solve a lot of problems people on this forum are having with complex hierarchical objects.

The problem I've been having with implementing this though is that FindInstance is private - which is a problem with a class that's supposed to a part of a class library - so I had to use reflection :(