Thanks for detailing these cases, Tim. I'll look at this further to see if I can suggest any improvements. I have a couple of further areas that it'd be great to hear from you about: Threading: As using ThreadLocal is one possible way to implement the spec, should the spec possibly require that the Async.call invocation is performed on the same thread as the preceeding mediated method invocation? Or how do you otherwise use the API in a multi-threaded scenario? Chaining: The Promise API allows chaining of asynchronous calls. Could you show a code example where first a method void foo() is called asynchronously and upon its completion asynchronously calling void bar()? Considered alternatives (chapter 8): Do I understand correctly that there was one considered alternative (builder model), and the different sub-chapters (8.1 and 8.2) describe different aspects of that? Thanks Mike Timothy Ward wrote:
On 17 Mar 2014, at 23:03, Mike Wilson <[email protected]> wrote: Hi Tim, replies inline, Timothy Ward wrote: In my view Mockito mocks are frequently about record first / invoke later. In my tests I often create a mock, run my test, and then validate that a certain action has happened, e.g. /* Setup */ MyListener listener = Mockito.mock(MyListener.class); Manager manager = new Manager(); manager.addListener(listener); /* End of setup */ manager.manage("foo"); /* Test assertions */ Mockito.verify(listener).startedManaging("foo"); I think we are misunderstanding each other's terminology. I'm assuming that your example looks like this: manager.manage("foo"); ... listener.startedManaging("foo"); // #1 ... Mockito.verify(listener).startedManaging("foo"); // #2 The point I'm making is that the invocation of the target method happens at #1, ie immediately when it is called. The second call to the startedManaging method in #2 is not the invocation, even though it makes use of recorded data from #1. The system changes state in #1, not in #2. That is why I'm not labling this as record first / invoke later, but rather invoke(+record) first / verify later. I'm not sure I completely agree with your point about when the system changes state. In this case I see the test as the system, and the state of the test is either passing or failing. The invocation in #1 doesn't contribute to the state of the test, instead the state change occurs at #2, where the test will either "pass" or "fail". I do see where you are coming from though. So if we mirror Mockito's invoke first pattern: listener.startedManaging("foo"); // perform+record invocation, system changes state Mockito.verify(listener).startedManaging("foo"); // use recording for verification with your invoke later pattern in Async: listenerMediator.startedManaging("foo"); // record invocation asyncService.call(); // use recording to perform invocation, system changes state then I hope you see what I mean. 1) Did you choose the record first / invoke later design because of implementation considerations/compromises, or because you feel it is more natural and intuitive for the user of the API? Obviously there is a balance to be struck here. Primarily an API needs to work well for a given set of common use cases, but it does also need to be implementable in a moderately sane way. Creating a fire-and-forget call against a service is relatively easy with a threadpool and a runnable, so the particular target use case we are optimising for is the one where a client wants to obtain a Promise for the asynchronous work that they're starting. If you take this as a starting point then I think that the current API feels pretty natural, although I understand that you may disagree. Ok, so how would the Async API behave in the following exceptional scenarios: This is a difficult question to answer using only the exact behaviour that is expected to be in the specification. I will do my best to answer these questions, making it clear where the behaviour would be required by the specification and where implementations have a little latitude. Where appropriate I will use MUST and SHOULD to indicate this. Method call without following Async.call: asyncList.clear(); Firstly, and most crucially for this discussion, no asynchronous invocation would start. If this is the first time that this Async service has been used then there is no way to know that this is an error case (i.e. that the client isn't just about to call asyncService.call()). The specification therefore cannot require an Exception to be thrown in this case. If the async service implementation has already been used, and is able to detect that this call is happening repeatedly, then it SHOULD detect the usage error and throw IllegalStateException. Async.call without preceeding method call: Promise<Void> p = asyncService.call(); In this case the Async service MUST throw an IllegalStateException to indicate that the API is being misused. Multiple method calls before Async.call: asyncList.clear(); asyncList.clear(); ### Promise<Void> p = asyncService.call(); This case is a more obvious version of the repeating call I described earlier. This is easy to detect and so async service implementations SHOULD detect the usage error at line ### and throw IllegalStateException. or similar but with further complexity, assuming there is first an event from System 1 and 10 minutes later an event from System 2: class Main { Async asyncService = ... void start() { a = new A(asyncService); registerForEventsFromSystem1(a); b = new B(asyncService); registerForEventsFromSystem2(b); } } class A { ... void receiveEvent() { List asyncList = asyncService.mediate(list); asyncList.clear(); ### // programmer forgot asyncService.call() } } class B { ... void receiveEvent() { List anotherAsyncList = asyncService.mediate(anotherList); anotherAsyncList.clear(); ### Promise<Void> p = asyncService.call(); } } In this case the exact failure mode will depend on the underlying implementation details, however any implementation that implements the SHOULD clause above should eventually throw an IllegalStateException at one of the lines marked ###. For the prototype RI this will definitely occur if the same thread ever: * Calls A.receiveEvent() then later calls B.receiveEvent() * Calls A.receiveEvent() more than once If the underlying implementation never reuses threads then the current RI prototype would not detect this usage error. One hopes that all of these situations would be picked up pretty early in testing anyway, but in almost all of these cases I would expect the Async service implementation to noisily fail. The main reason that the error detection statements are listed as SHOULD, rather than MUST, is that it is difficult to enumerate the cases that MUST fail without dictating the entire implementation. The only really easy to detect failure is your second example, which is why that failure is a MUST, rather than a SHOULD. The specification will expect, however, that implementations perform API usage validation where possible. 2) What mechanisms do you have in mind that implementations could use for transferring the recorded calls to Async.call? I'm guessing ThreadLocal is the main alternative? Obviously ThreadLocal is an option, but the OSGi specifications try to avoid dictating details at this level. I can see a model where an implementation uses the value returned by the mediator to track state, although this does get interesting with the zero-args call method. [assuming you meant void return type] I think this also gets interesting (=hard) with methods that return instances of final classes? (can't use dynamically generated subclasses of the return type to carry call state) You could use the returned instance to key into an identity map, although this gets messy for primitives if/when they get unwrapped. Is there a particular reason that you're interested in this detail? Just trying to find the borders of the system, knowing what rules the API must adher to not to make implementation impossible in the desired ways. Even if the mediator were responsible for immediately starting the asynchronous invocation it would still be necessary for the Async implementation to track the call so that a Promise could be obtained for it later. As I mention above, obtaining a Promise for the invocation is probably the most important use case as far as we're concerned Here we are in full agreement ;-) Best regards Mike Timothy Ward wrote: Hi Mike, Your interpretation of the behaviour described in the RFC is correct, including the roles of the mediator object and the async service. I'll outline the design process that we went through, and hopefully that will answer your question. One of the core problems associated with making this sort of asynchronous invocation is that you need to intercept a method call, immediately return a value that isn't the real result, eventually provide the result through some other means. It turns out that these requirements are very similar to those of a test mocking framework, for example Mockito. In Mockito you create a mock (or mediated) object, and the invocations made upon it are recorded. Configuring one of these mocks typically involves making the method call you wish to mock, e.g. Mockito.when(mock.doStuff()). This pattern works very well for the mediators produced by the Async Service too. The return type information from the mediator allows the generic type of the Promise to be determined. Given that the pattern was a natural fit, and the fact that mocking frameworks are widely used and understood, it was logical to reuse it. In addition to the parallels with mocking, there were several other aspects. * The return value from a method call is rarely a Promise/Future, which means that there must be some other mechanism to acquire the result. If the mediator calls directly began the asynchronous work then it would be easy to accidentally overwrite the Promise representing an earlier execution. The current API flow allows Async implementations to verify the API usage, and to throw exceptions if the mediator is used incorrectly. * Mediated objects are not thread safe, and should not be shared between bundles. The reason that they shouldn't be shared is because they would allow other bundles to act using your bundle context. Requiring them to be used with the Async service encourages safer coding patterns. In summary the mediated object isn't actually a proxy (which is why we chose the method name mediate, rather than proxy). The mediator simply records the method calls made upon it, and has nothing to delegate to. The Async implementation is the service providing the asynchronous behaviour, which is why it is the one that gets called to begin the execution. I hope that makes sense. Regards, Tim On 14 Mar 2014, at 13:28, Mike Wilson <[email protected]> wrote: Async RFC 206 (from March 03) at: https://github.com/osgi/design/tree/master/rfcs/rfc0206 >From section 5.4 we learn how to start an asynchronous invocation that has a return value: List asyncList = asyncService.mediate(listRef); Promise<Boolean> promise = asyncService.call(asyncList.contains("badEntry")); and one that has no return value (void method): mediator.clear(); Promise<Boolean> promise = asyncService.call(); The text explains that the actual asynchronous invocation doesn't take place until calling Async.call(), ie: asyncService.call(asyncList.contains("badEntry")); ^ send ^ prepare async async call call asyncList.clear(); // prepare async call asyncService.call(); // send async call Why did you choose this design, and did not choose to let a proxied method send its own async call? Best regards Mike _______________________________________________ OSGi Developer Mail List [email protected] https://mail.osgi.org/mailman/listinfo/osgi-dev _______________________________________________ OSGi Developer Mail List [email protected] https://mail.osgi.org/mailman/listinfo/osgi-dev _______________________________________________ OSGi Developer Mail List [email protected] https://mail.osgi.org/mailman/listinfo/osgi-dev
_______________________________________________ OSGi Developer Mail List [email protected] https://mail.osgi.org/mailman/listinfo/osgi-dev
