Extend TokenHandlingBindingElement/IServiceProviderTokenManager to support regular get/set based storages

[http://oyvind.kinsey.no/]

As it is now, the TokenHandlingBindingElement depends on the objects maintaining and saving their own state as it is modifying the tokens without calling any sort of update/save methods on the !IServiceProviderTokenManager.

The IServiceProviderTokenManager should be extended with a
void UpdateRequestToken(IServiceProviderRequestToken token);
and TokenHandlingBindingElement.ProcessOutgoingMessage should be modified to call this after modifying the token.

The would not alter existing functionality except for requiring an extra method implemented (could have an empty body).

Modified files are attached.

[http://blog.nerdbank.net/]

Thanks for the suggestion. While we could add this method, it is anticipated that most implementations of the IServiceProviderTokenManager interface will use Linq to SQL, Linq to Entities, or direct SQL access to apply the changes to the tokens, none of which require special update methods to be called on an individual object, but rather a general SaveChanges type method at the end of an HTTP request.

I think having an Update method might be awkward because some changes to tokens are made with method calls on the interface while others are made with property accessors. It seems unclear at an API inspection level which calls must be followed with a call to UpdateToken.

The existing design is simply that any property setter calls or method call implementations should immediately save the changes to the database (within a transaction scoped to the incoming HTTP request, preferably). Delaying the db save operation till a following call to Update could introduce some bugs that are difficult to discover if, for example, a token is queried for twice using this interface and different token instances are returned with different versions of the database data.

[http://oyvind.kinsey.no/]

I dont think I can agree with you here, I think it would be wrong to anticipate such a narrow use case. First of all, the library should not demand that the infrastructure and data access layer needs to keep track of all objects, and delay commiting, so that they can be stored back on RequestEnd?. This is not compatible with how many data access layers works.

For instance, if all changes are commited on RequestEnd?, how would you safely roll back single transactions and notify the user of it? In stead of the first step failing, and the user getting notified, all steps would have to be run through before the exeception being thrown at RequestEnd?.

This method would not change the existing behaviour, it would just give us developers an extra way of maintaining state. As it is now I am not able to use your library unmodified, and I would think this is the case for others as well.

Could perhaps some kind of voting system be used to get a consensus on such matters?

[http://oyvind.kinsey.no/]

And about it being unclear which methods needs a call to UpdateRequestToken?, I dont see this as unclear as it should ALWAYS be called after a token retrieved using the interface has been modified. And as I can see it, all methods using GetRequestToken? resides in TokenHandlingBindingElement? and in OAuthServiceProviderMessageFactor, and the token is only modified in TokenHandlingBindingElement?.ProcessOutgoingMessage?.

So all in all, I dont see it as being difficult to implement this. As it is doable using one additional method on the interface and two small alterations in TokenHandlingBindingElement?.ProcessOutgoingMessage?.

[http://blog.nerdbank.net/]

If it is important to call a method after setting a property, why not let the property setter call that method?

[http://oyvind.kinsey.no/]

Now that would be a bad design!

But am I missing something here? I dont see why this should pose as a problem, it would only allow EVERYONE that is NOT using a data layer framework, or who DONT want to have data layer specific code tied into the entities.

Having the update method means that you would have an data layer agnostic aproach - it would be entirely up to the class implementing the IServiceProviderTokenManager interface to choose HOW and WHEN objects should be persisted.

Again, am I missing something here?

[http://blog.nerdbank.net/]

Ah! I finally realized you were referring to put the Update method on the manager interface (not the token interface). That makes sense. Fixed.

master 30c6c06eac