Closed Daniel-Svensson closed 6 years ago
[ColinBlair@2014-02-28] That is a good suggestion and from the code I think there may have been a plan to support this originally. Instead of AllowMultipleInvocations=true I would suggest having a Priority value that controls in what order the custom method are called. By default the priority would be -1 which would enforce only allowing the single custom method call.
[papaneko@2014-03-03] Sounds good, it solves my problem, but consequently it would also rule out the possibility to prioritize single invocations of custom method calls. They feel like separate ideas, how about both?
[Update(IsCustomMethod=true, Priority=1, AllowMultipleInvocations=false)]
[ColinBlair@2014-03-03] How would just priority "rule out the possibility to prioritize single invocations"?
[papaneko@2014-03-04] Assume that I have three custom methods, A(), B() and C(). I would like to make sure that A() and C() is limited to one call only and I would like to able to call B() any number of times. But I would like to perform all calls in order A() -> all B()'s -> C(). How would I be able to do that?
[ColinBlair@2014-03-04] Good point, and after further thought on the matter I have decided that having a built in priority goes against the basic, nondeterministic design of the DomainService. It also seems unneeded. If the custom operations are setup as a queue in the entity then we just run the custom methods in the order that they were originally called on the entity.
I think the original reason it was limited to one is that during testing the RIA Services team decided that the resulting code was too confusing and resulted in bad code. Every time you call a custom method the entity becomes "dirty" until that custom method is resolved. Since the object is dirty and no values in the entity currently can be trusted, theoretically it would be invalid to make any additional changes. I am wondering if the solution would be to make all custom methods calls client side return Task. That way it is clearly an async call that is happening which eventually could change the state of the object. That solves the bad code issue.
[danneesset@2014-03-04] I think we should start with adding the possibility of having multiple different custom actions queued and add a setting similar to the suggested AllowMultipleInvocations in order to control if multiple calls to the same custom method can be queued.
The idea of executing custom methods asynchronously directly seems quite interesting as it would also allow them to return values, but we should definitely keep the current approach as it allows lots of methods to be executed in batch mode while making it very easy to make sure that either all changes or none are executed.
I'll try to look into implementing this since we would have much use of it at work, I only have to convince my boss that I should be allowed to work with it work hours which should be fairly easy.
What do you think about a new CustomUpdateAttribute (derived from UpdateAttribute), the reason is that it could become confusing if you were able to specify the new setting (and any future settings) on normal update methods such as this.
[Update(AllowMultipleInvocations=false)]
void UpdateEntityA(A a) {...}
But maybe it is better to just throw an exception if trying the set the setting unless IsCustomMethod is sett (if it is possible).
Pros with CustomUpdateAttribute:
[ColinBlair@2014-03-04] Historically, custom (or named) updates originally had its own attribute. The original RIA Services team had a policy of having the bare minimum number of attributes that they could, a policy I generally agree with. That is why the attribute was removed and named/custom update got merged into the main Update attribute.
I am fine with a new attribute for this, but then I don't think we need AllowMultipleInvocations anymore. If you don't want multiple invocations then you use the original Update attribute. If you do want them then you use the new attribute. We should also get away from the word Update with the new method, that is a sore point for me since the existing custom "update" can also be used for inserts. We just need to come with a new terminology that means "method that will be execute asynchronously as part of the unit of work during submit".
I think https://openriaservices.codeplex.com/workitem/7 needs to be looked at in conjunction with the new functionality.
[Daniel_Svensson@2014-04-09] I was thinking about using [CustomMethodAttribute()), but after reading your last comment how do you feel about:
[EntityAction(AllowMultipleInvocations=false)))
It seems that "action" is the odata (and (ebapi) name for server side invoke operations including operations on "entities".
[ColinBlair@2014-04-09] EntityAction is interesting. I think that is a good starting place unless someone comes along and tells us that Action should only be GET, POST, etc. and that using the word for something else is bad.
[danneesset@2014-04-12] I've been working on this and have a working implementation togheter with updated tests at https://openriaservices.codeplex.com/SourceControl/network/forks/danneesset/openriaservices?branch=multipleinvoke
As soon as I have come around to move some new strings to Resources and add some new unit tests I will submit a pull request. My goal is to do so before next thursday.
[danneesset@2014-04-16] I've added a pull request at https://openriaservices.codeplex.com/SourceControl/network/forks/danneesset/openriaservices/contribution/6618 . I've been running the code for some days at my developer box at work without any issues.
@ColinBlair, I will be away without access to any of my developer machines for approximately a week so if you are planning to publish a new prerelease (of 4.3.1.0) then I would really appreciate if you (or somebody else) would consider merging this pull request first.
[ColinBlair@2014-04-16] I am concerned about the breaking changes. When I said I was fine with a new attribute, I meant that I was fine having a new attribute in addition to the existing attribute. I will take a look at the code.
[danneesset@2014-04-18] Some clarifications about the breaking changes:
There are no required changes on the server: [Update(IsCustomMethod=true)] still works perfectly fine. The obsole attribute on IsCustomMethod was added in order to hint people into using the EntityAction attribute instead, but the code definitly work.
On the client it is only the generated code which has changed, there should be no need to modify any user code, unless someone have modified their code gen in order to utilize the methods inquestion in any non-standard way.
The client can be made backwards compatible with code generated using the old/current code generator but I have not had the time into making such changes since it took much more time to get the tests going and then update them than anticipated. My idea for implementing this would be to add a fallback in InvokeAction where OnActionStateChanged is called once per entity type so that the MetaType can be updated with information about all entity actions for a given type. It this is very important I can look into adding this fix later before the final release of the next version.
[ColinBlair@2014-04-21] I am ok having the breaking change in the prerelease version, but I would like it fixed before we change it to a full release. I know it is an insignificant breaking change, but any breaking change in the 4 branch is a bug and I don't want to start the 5 branch yet unless we have to.
[danneesset@2014-04-29] I've think I managed to implement compability with the 4.3.0 codegen.
I've added a test and Everything seems to work as expected.
It is currently not possible to have multiple custom method invocations pending on a single entity, the client has to perform a submit after each operation. This is a big drawback since it becomes impossible to have a "none-or-all" changes performed when calling multiple custom method invocations on a large set of entities.
I suggest that the client should be able to perform multiple custom method invocations by setting an AllowMultipleInvocations property on the attribute, e.g.:
[Update(IsCustomMethod=true, AllowMultipleInvocations=true)]
This work item was migrated from CodePlex
CodePlex work item ID: '31' Vote count: '1'