Request: Convenience Method GetCurrentContext on ContextManager

rasupit posted on Saturday, February 16, 2008

Rocky,

I propose to add a static method to get current data context.  This method will allow child object(s) to participate in current data context without having to know the connection string of working context.  perhaps the signature could be like:
    public static C GetCurrentContext() {}

Thanks,
Ricky

rasupit replied on Sunday, February 17, 2008

Just to add with real example; The following code:
        private void Child_DeleteSelf(System.Data.Linq.EntitySet<PTracker.Linq2Sql.Assignment> dataSet)
        {
            var data = dataSet.Single(r => r.ProjectId == ReadProperty<Guid>(ProjectIdProperty)
                            && r.ResourceId == ReadProperty<int>(ResourceIdProperty));

            using (var mgr = Csla.Data.ContextManager<PTracker.Linq2Sql.PTrackerDataContext>
                        .GetManager(Database.PTrackerConnection))
            {
                data.LastChanged = _lastChanged;
                mgr.DataContext.Assignments.DeleteOnSubmit(data);
            }//using
        }

Could be simplified to just:

        private void Child_DeleteSelf(System.Data.Linq.EntitySet<PTracker.Linq2Sql.Assignment> dataSet)
        {
            var data = dataSet.Single(r => r.ProjectId == ReadProperty<Guid>(ProjectIdProperty)
                            && r.ResourceId == ReadProperty<int>(ResourceIdProperty));

            var db = Csla.Data.ContextManager<PTracker.Linq2Sql.PTrackerDataContext>.GetCurrentContext();
            data.LastChanged = _lastChanged;
            db.Assignments.DeleteOnSubmit(data);
        }

rasupit replied on Thursday, February 21, 2008

Rocky, any issue or comment regarding this proposed method?

RockfordLhotka replied on Thursday, February 21, 2008

The problem is that there could be multiple "current" contexts. The ContextManager (and ConnectionManager) store a list of objects, keyed on the connection string. So there's only one for a given connection string, but there could be many at once.

I did just add an overload (requested by someone on the forum) to accept a database name instead of connection string

GetManager("PTracker", true)

This does translation of PTracker to the connection string by pulling the value from the config file.

rasupit replied on Thursday, February 21, 2008

Rocky,
I see the problem... Perhaps by making the list keyed by its type name will work.  Something like using key typeof(C).FullName to return ContextManager for context C

I think I will still prefer to call the new overload with Database.PTracker.   I got burned so many times by passing a hand typing string.  I much prefered help by intellisense ;) D<tab>.P<tab> is much quicker to get:
GetManager(Database.PTracker, true) ,

Thanks.

RockfordLhotka replied on Thursday, February 21, 2008

I don't think the type name is reliably different - you could use the same db schema against multiple databases, or with multiple security credentials and have two instances of the same type of context (or connection).

You could still avoid the string literal by declaring a constant with the database name and using that - perhaps as a public element of a Database class? Especially if that Database class moved to the DAL project, so even that bit of knowledge (the name of the database) would be kept out of the business assembly and thus (potentially) off the client.

rasupit replied on Thursday, February 21, 2008

Rocky,
Although is theoretically possible an application can call multiple databases of the same schema or calling same db with different different security credential within a local context (or HttpContext.Current) and within a parent "using" scope;  I think the scenario is very unlikely.

I have to admit not having to have access to the current db context directly within an object tree/graph is realy not a big issue or something that will make/break.  I was just find the steps could be more straight forward and not so awkward especially when implement delay/pend child execution to parent object. ex:

DataPortal_Update() {
   using(var mgr = ContextManager<PTDataContext>.GetManager(connString) {
     //update members here....
    
      DataPortal.UpdateChild(ReadProperty..., this);
      mgr.DataContext.SubmitChanges();
  }
}

Child_Update() {
   using(var mgr = ContextManager<PTDataContext>.GetManager(connString) {
      var data = new ChildEntity() { .... };
      mgr.DataContext.ChildEntitySet.Attach(data);
      //update members here....
      //pending submit, let parent submit the changes.
  }
}

The Child_Update could be much simpler if we can access the current data context directly.  The awkwardness is because the "using" statement give brief impression that the changes have been finalized within this scope.

It will be great if you can find a way to allow access to current DataContext directly, but if not then I would not be a big of a deal.

Thanks for your time,
Ricky

RockfordLhotka replied on Thursday, February 21, 2008

rasupit replied on Friday, February 22, 2008

Rocky,

Yes, I see the problem you described when people try to use it like this:
using (var mgr = ContextManger<PTDataContext>.GetCurrentManager(connString)) { }

I think ContextManager<PTDataContex>.GetCurrentDataContext(connString) might work.  It would prevent the multiple database issue although it may not really address the consistency issue you've described when doing data access where data context be accessed after getting ContextManager. 

However the pattern that is introduced here that ALL root objects would use GetManager in which would rip the benefit of ContextManager when multiple (root) objects working together (interact) when calling within data access layer. GetCurrentDataContext could be used in child objects which would only return the current DataContext or thown an error when no current context exist. 

There might be an issue with using a using block on DataContext, but this would be the same issue when user use a using block after getting the ContextManger from GetManager in child DP.

Ricky.

RockfordLhotka replied on Friday, February 22, 2008

I've added a GetCurrentManager() concept to the C# code as an experiement. Take a look and let me know if you think this is good - I'm still a bit skeptical honestly.

The usage is for child objects only and is:

var ctx =
  ContextManager<PTDataContext>.GetCurrentManager(Database.PTracker).DataContext;
var q = from x in ctx ...

I still think the consistency of my original approach is preferable.

The only way I could see really liking this is if we somehow pushed the creation of the context up into the data portal so it is entirely outside all business objects (including the root level). That would allow all objects to simply tap into the current manager(s).

I'm not entirely sure how to do that though. Perhaps using an attribute on the DP_XYZ methods? You could apply an attribute specifying the type and database name or connection string.

[DataContext(typeof(PTDataContext), "PTracker")]
protected override void DataPortal_Insert()

This would tell SimpleDataPortal to wrap this call in an appropriate using block before your code is called, thus allowing consistent use of GetCurrentManager() in all data portal code.

This actually seems like a really interesting idea - perhaps for 3.5.1.

rasupit replied on Tuesday, February 26, 2008

Rocky, sorry for slow getting back to you on this.

I tested the addition you made, and as expected they work.  The object collaboration scenario is also work correctly.

However, this method will only work when it's not wrap with using block.  When it accidentally wraps with using block the ContextManager will get disposed prematurely and the upstream using block will generate ObjectDisposedException when trying to dispose the disposed object. 

Having play with this method a little bit, I think it user may easily get tripped with this.  The intellisense does not help either as this method is the first to select before GetMethod.

Ricky

RockfordLhotka replied on Tuesday, February 26, 2008

rasupit replied on Wednesday, February 27, 2008

Rocky, I agree that we should remove this method as I think it may cause more harm than good.  While I wish the usage on child objects could be more straight forward,  this merely just an inconvenience.

On the other side, not using the using block on the GetMethod will cause the ref counting get out of whack.  Violating the pattern on this is not that harmful because the context will only live the life of a user requests (in web) and eventually will get recreated on next user request.