[jboss-jira] [JBoss JIRA] Commented: (AS7-834) Implement a deterministic way of adding interceptors and configurators to a view and component

[ https://issues.jboss.org/browse/AS7-834?page=com.atlassian.jira.plugin.sy... ] jaikiran pai commented on AS7-834: ---------------------------------- Some IRC discussion about this: (http://echelog.matzon.dk/logs/browse/jboss-as7/1304978400) {quote} [09:55:11] <stuartdouglas> another idea I had was that maybe we should introduce something like Phase.java for configurations, so we have one place that we set the Configurator order [09:56:06] <Jaikiran> it would help with readability but other than that probably won't make much of a difference [09:56:07] <Jaikiran> would it/ [09:56:08] <Jaikiran> ? [09:56:12] <stuartdouglas> because I think as we start adding more interceptors etc figuring out the order is just going to turn into a mess [09:56:26] <Jaikiran> yeah, I agree with that [09:59:09] <stuartdouglas> how hard is it to add the transaction interceptor back in? because I think that would fix all the EPCPropagationTestCase failures [09:59:10] *** jamezp_afk has quit IRC [10:00:06] <Jaikiran> the tx interceptors are already in [10:00:12] <stuartdouglas> and other than that there is only really the weld failures I mentioned before, and the multiple bindings failures, which I think dmlloyd is working on [10:00:24] <Jaikiran> the EPCpropagation tests are failing due to this EJBContext issue that i'm fixing [10:00:43] <Jaikiran> yeah, the multiple bindings thing is affect the persistencecontext/unit test [10:00:58] <stuartdouglas> I see those tests failing with javax.persistence.TransactionRequiredException [10:01:09] <stuartdouglas> I don't think the transaction interceptor is actually enabled [10:01:10] * Jaikiran looks again [10:01:38] <stuartdouglas> that is after my changes, so your problem is probably hiding the transaction one [10:02:23] <Jaikiran> oh might be [10:02:27] <Jaikiran> because mine is failing with http://pastebin.com/k4BDxyhY [10:02:39] <Jaikiran> i'll take a look at those tests while i work on this fix [10:02:42] *** rmaucher has joined #jboss-as7 [10:02:51] <Jaikiran> can you pastebin your log [10:03:03] <Jaikiran> just to see what i might have missed with tx interceptors [10:04:18] *** rawbdor has quit IRC [10:04:21] <stuartdouglas> http://pastebin.com/jqp9cMXA [10:05:23] <stuartdouglas> actually it could be something else [10:05:28] * stuartdouglas looks at test again [10:06:18] *** AndyTaylor has joined #jboss-as7 [10:06:18] *** ChanServ sets mode: +v AndyTaylor [10:09:09] <stuartdouglas> I'm not really sure what is going on, I will try and debug it [10:09:54] *** wolfc has joined #jboss-as7 [10:09:54] *** ChanServ sets mode: +v wolfc [10:10:14] *** davidbos has joined #jboss-as7 [10:11:09] <Jaikiran> brb after lunch [10:11:13] *** Jaikiran is now known as Jaikiran|Lunch [10:23:42] *** alesj has joined #jboss-as7 [10:26:06] <stuartdouglas> Jaikiran|Lunch: It seems to be an ordering issue the transaction interceptor and the SessionInvocationContextInterceptor appear after the ComponentDispatcherInterceptor, and so never get invoked [10:26:51] *** aslak has quit IRC [10:27:22] *** aslak has joined #jboss-as7 [10:27:22] *** ChanServ sets mode: +v aslak [10:27:40] *** aslak has quit IRC [10:28:02] *** stalep has joined #jboss-as7 [10:31:30] *** hardy has joined #jboss-as7 [10:31:46] *** Nihility has quit IRC [10:41:04] *** galderz has joined #jboss-as7 [10:41:04] *** ChanServ sets mode: +v galderz [10:41:45] *** zroubali has joined #jboss-as7 [10:41:45] *** ChanServ sets mode: +v zroubali [10:54:47] *** kkhan has joined #jboss-as7 [10:54:47] *** ChanServ sets mode: +v kkhan [11:01:45] *** davidbos1 has joined #jboss-as7 [11:02:24] *** galderz has quit IRC [11:02:35] *** davidbos has quit IRC [11:05:20] <stuartdouglas> Jaikiran|Lunch: You there? [11:06:15] *** davidbos1 has quit IRC [11:25:14] *** Jaikiran|Lunch is now known as Jaikiran [11:25:16] <Jaikiran> stuartdouglas: yep [11:26:00] <stuartdouglas> I changed the order round so the interceptor runs, but now I am getting a NPE as EjbComponent.txAttrs is null [11:26:19] <stuartdouglas> and I am not 100% sure how to build that up from the info that is in the component description [11:26:46] <Jaikiran> i'll take that up, since it all requires rewiring the components like we had in the previous ee framework [11:31:45] *** aslak has joined #jboss-as7 [11:31:45] *** ChanServ sets mode: +v aslak [11:34:23] <stuartdouglas> Jaikiran: I have pushed my (minor) changes to my ejb branch [11:34:43] <Jaikiran> ok, i'll get them into my workspace [11:34:50] *** sannegrinovero has joined #jboss-as7 [11:34:50] *** sannegrinovero has quit IRC [11:34:50] *** sannegrinovero has joined #jboss-as7 [11:34:50] *** ChanServ sets mode: +v sannegrinovero [11:36:08] *** miclorb_ has joined #jboss-as7 [11:37:24] <stuartdouglas> I have a feeling method level interceptors are also broken at the moment [11:37:38] * stuartdouglas starts writing a test [11:38:29] * stuartdouglas accidentally just rubbed chilli in his eyes [11:38:54] *** pmuir has joined #jboss-as7 [11:38:54] *** pmuir has joined #jboss-as7 [11:38:54] *** ChanServ sets mode: +v pmuir [11:41:01] *** alesj has quit IRC [11:41:43] *** alesj has joined #jboss-as7 [11:42:26] *** davidbos has joined #jboss-as7 [11:42:29] <Jaikiran> ha ha :D [11:45:26] *** alesj has quit IRC [11:46:06] *** alesj has joined #jboss-as7 [11:46:50] <Jaikiran> well all these addLast/addFirst are relative at a given point in time [11:46:59] <stuartdouglas> I know, it's horrible [11:47:11] <stuartdouglas> that why I want to get to a Phase.java type setup [11:47:14] <Jaikiran> yeah [11:47:24] <Jaikiran> without that, it's going to be very brittle [11:48:18] <stuartdouglas> yea, I asked david about it a few days ago, and he wanted to see how it turned out the way it currently is [11:48:32] <stuartdouglas> but I think that the way it turned out is a mess :-( [11:49:24] <Jaikiran> yeah, we'll for sure have to change this ordering approach [11:50:19] <stuartdouglas> do you think we should specify the ordering for the interceptors, the configurators or both? I think that just the configurators is probably enough [11:50:43] <stuartdouglas> though as they can still add to the front and back it may still have potential for mess [11:52:04] <Jaikiran> i think it should be both [11:52:20] <Jaikiran> without that, we'll end up with a similar issue like now {quote} (http://echelog.matzon.dk/logs/browse/jboss-as7/1305064800) {quote} [01:18:52] <stuartdouglas> You know how I asked about using a Phase.java type setup for interceptor / configurator ordering a few days ago? [01:19:33] <stuartdouglas> I talked to Jaikiran about it last night, and he thought it was a good idea, because at the moment the ordering is pretty fragile, and it is hard to figure out exactly where a given interceptor is in the chain [01:24:21] <dmlloyd> well if we want an interceptor order why not just use DUP phase numbers directly? [01:24:46] <dmlloyd> I mean we can do it the other way but then we'll have to sort them [01:25:16] *** jwulf has joined #jboss-as7 [01:26:24] <stuartdouglas> If we try and force the dup order to reflect the interceptor order we will probably end up with a heap of DUP's that only add a single interceptor [01:27:11] <dmlloyd> so we'd only sort once on deployment though? [01:27:17] <stuartdouglas> and sometimes DUP's have their own ordering considerations that are different to the interceptor order [01:27:31] <dmlloyd> I guess it'd be OK though I don't have the cycles to do the work [01:27:49] <dmlloyd> or as the perl guys used to say, I don't have a "round tuit" [01:27:56] <stuartdouglas> I can do it, it is not a big change [01:28:20] *** rmaucher has quit IRC [01:28:25] <stuartdouglas> here you go: http://suziedoscher.com/?page_id=126 [01:29:03] <dmlloyd> yessss [01:30:18] <stuartdouglas> anyway, the interceptor ordering is not as high a priority as getting the remaining tests to pass [01:32:09] <dmlloyd> I'm all done with interceptors at this point so you can just coordinate it with jaikiran if you want to make the change [01:32:55] <stuartdouglas> ok {quote} -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira