A little update. I had forgot to change the AnnotationDeployer class to merge the deployment info from the xml file. The issue OPENEJB-1144 has now a new patch file. The newest one is the valid one. tkx, Thiago.
On Tue, Jun 1, 2010 at 5:02 PM, Thiago Veronezi <[email protected]> wrote: > Hi, devs. > Thats just to say that I've uploaded a patch for the issues below (here: > https://issues.apache.org/jira/browse/OPENEJB-1144) > When you have time, please take a look on that for me. > tkx, > Thiago. > > > On Mon, May 31, 2010 at 3:38 PM, David Blevins <[email protected]>wrote: > >> >> On May 31, 2010, at 5:13 AM, Thiago Veronezi wrote: >> >> > Hi, David! >> > I've just finished the coding and unit-testing for the ejb-jar stuff: >> > http://issues.apache.org/jira/browse/OPENEJB-1144 and >> > http://issues.apache.org/jira/browse/OPENEJB-1146. >> >> Great! >> >> > I'de like to create a >> > patch for that, but Im not sure how to do it. >> > Actually, what is a patch? Is it a screenshot of my code with no hidden >> svn >> > files for you (or the other devs) to verify commit? Where do I publish >> it? >> > tkx! >> >> Basically if you type 'svn diff' it will show all the lines of code that >> were changed. There's a corresponding program, called 'patch', that will >> read the patch file and edit all the code accordingly. >> >> So you can do something like: >> >> svn diff > OPENEJB-1144.patch >> >> And attach that OPENEJB-1144.patch to the OPENEJB-1144 JIRA issue. >> >> One patch for the two JIRAs is fine. No need to attach the same one >> twice. >> >> This page has a tiny bit more info: >> >> http://wiki.creativecommons.org/HOWTO_Patch >> >> -David >> >> > >> > >> > On Fri, May 28, 2010 at 3:44 PM, David Blevins <[email protected] >> >wrote: >> > >> >> Wow, that was quick. I'm quite shocked actually :) >> >> >> >> You've definitely got the big picture. >> >> >> >> On May 28, 2010, at 6:56 AM, Thiago Veronezi wrote: >> >> >> >>> Hi David, >> >>> thanks for the directions. I think thats a good task for me to have a >> >> global >> >>> view of the system. I think its becoming more clear how to implement >> the >> >>> fix. Please, verify it for me... >> >>> >> >>> ************************************ >> >>> add new class org.apache.openejb.jee.StatefulTimeoutConfig >> >>> >> >>> class StatefulTimeoutConfig { >> >>> @XmlEnumValue("timeout") TIMEOUT, >> >>> @XmlEnumValue("unit") UNIT; >> >>> } >> >> >> >> Note that @StatefulTimeout is a different annotation then >> @AccessTimeout -- >> >> see spec for details. It has the exact xml schema details too. >> >> >> >> We will have to support both, so feel free to add that JAXB item while >> your >> >> in the code. Perhaps a "Timeout" class that could be reused by both. >> >> >> >> Note also that <access-timeout> is a list as it's possible to specify >> the >> >> access timeout for each individual method. I'm totally fine if we want >> to >> >> ignore that in the in the first patch and come back and do it in a >> second >> >> revision. It's already a lot to chew on and "per method" metadata is >> >> tougher to pull through the system. >> >> >> >> On naming conventions, we typically go with the xml element name with >> no >> >> added suffix. >> >> >> >>> ************************************ >> >>> TransactionType >> >>> >> >>> Add a new property (say statefulTimeout) to >> >>> the org.apache.openejb.jee.SessionBean class (below) >> >>> >> >>> @XmlElement(name = "stateful-timeout") >> >>> protected StatefulTimeoutConfig statefulTimeout; >> >>> >> >>> ************************************ >> >>> Add the new entry (on bold - same org.apache.openejb.jee.SessionBean >> >> class) >> >>> to the "session-beanType" annotation >> >>> >> >>> @XmlType(name = "session-beanType", propOrder = { >> >>> "descriptions", >> >>> "displayNames", >> >>> "icon", >> >>> "ejbName", >> >>> "mappedName", >> >>> "home", >> >>> "remote", >> >>> "localHome", >> >>> "local", >> >>> "businessLocal", >> >>> "businessRemote", >> >>> "localBean", >> >>> "serviceEndpoint", >> >>> "ejbClass", >> >>> "sessionType", >> >>> "loadOnStartup", >> >>> "timeoutMethod", >> >>> "initMethod", >> >>> "removeMethod", >> >>> "transactionType", >> >>> "concurrencyType", >> >>> "aroundInvoke", >> >>> "envEntry", >> >>> "ejbRef", >> >>> "ejbLocalRef", >> >>> "serviceRef", >> >>> "resourceRef", >> >>> "resourceEnvRef", >> >>> "messageDestinationRef", >> >>> "persistenceContextRef", >> >>> "persistenceUnitRef", >> >>> "postConstruct", >> >>> "preDestroy", >> >>> "postActivate", >> >>> "prePassivate", >> >>> "securityRoleRef", >> >>> "securityIdentity", >> >>> "dependsOn", >> >>> *"statefulTimeout"* >> >>> }) >> >> >> >> Exactly. Ditto for "acccessTimeout". >> >> >> >>> ************************************ >> >>> Create a new Class... >> >>> class AccessTimeoutValue { >> >>> long time; >> >>> TimeUnit unit; >> >>> } >> >>> >> >>> ************************************ >> >>> add new property to org.apache.openejb.core.CoreDeploymentInfo class >> >>> >> >>> private AccessTimeoutValue accessTimeoutValue; >> >>> >> >>> AccessTimeoutValue getAccessTimeoutValue() { >> >>> return this.accessTimeoutValue; >> >>> } >> >>> >> >>> void setAccessTimeoutValue(AccessTimeoutValue accessTimeoutValue) { >> >>> this.accessTimeoutValue = accessTimeoutValue; >> >>> } >> >> >> >> For the CoreDeploymentInfo class we can use an existing class called >> >> org.apache.openejb.util.Duration. It does some other fancy stuff, but >> >> basically it's a long/TimeUnit tuple. >> >> >> >> As noted above, feel free to ignore the fact that AccessTimeout can be >> done >> >> on a per method basis. Eventually we'll need some map here, but it's >> fine >> >> to ignore because I have some fancy ideas for what we should use as the >> >> default when they haven't specified the AccessTimeout for a specific >> method. >> >> >> >> Maybe better to get something basic done and checked in. I suspect the >> >> code will be changing kind of fast with all the other EJB 3.1 work we >> have >> >> going on and holding big changes can be hard. In fact, feel free to >> submit >> >> patches for individual little parts of this while chunk of work. I >> tried to >> >> break it up into sort of logical "bites" in our road map: >> >> >> >> http://openejb.apache.org/ejb-31-roadmap.html >> >> >> >> Feel free to attach patches to those individual jiras. >> >> >> >>> ************************************ >> >>> Class org.apache.openejb.assembler.classic.EnterpriseBeanInfo >> >>> >> >>> add new property >> >>> public AccessTimeoutValue accessTimeout; >> >>> >> >>> ************************************ >> >>> >> >>> Class org.apache.openejb.config.EjbJarInfoBuilder (line 564) >> >>> >> >>> bean.accessTimeout = new AccessTimeoutValue( >> >>> Long.valueof(s.geStatefulTimeout().TIMEOUT), >> >>> Long.valueof(s.geStatefulTimeout().UNIT)); >> >> >> >> To match the Timeout class of the jaxb tree, we can make a TimeoutInfo >> >> class. See OpenEjbConfigurationValidationTest for the rules we have in >> >> place. >> >> >> >>> ************************************ >> >>> Class org.apache.openejb.core.stateful.StatefulContainer. >> >> [...] >> >> >> >>> Use... >> >>> synchronized (instance) { >> >>> *final AccessTimeoutValue accessTimeout = >> >>> instance.deploymentInfo.getAccessTimeoutValue();* >> >>> >> >>> Lock currLock = instance.getLock(); >> >>> final boolean lockAcquired; >> >>> if(accessTimeout == null) { >> >>> // returns immediately true if the lock is available >> >>> lockAcquired = currLock.tryLock(); >> >>> } else { >> >>> // AccessTimeout annotation found. >> >>> // Trying to get the lock within the specified period. >> >>> try { >> >>> lockAcquired = currLock.tryLock(accessTimeout.value(), >> >>> accessTimeout.unit()); >> >>> } catch (InterruptedException e) { >> >>> throw new ApplicationException("Unable to get lock.", e); >> >>> } >> >>> } >> >>> // Did we acquire the lock to the current execution? >> >>> if (!lockAcquired) { >> >>> throw new ApplicationException( >> >>> new ConcurrentAccessTimeoutException("Unable to get >> lock.")); >> >>> } >> >> >> >> This is the part that is a little tricky. Basically all of that >> >> synchronized block has to get rewritten. The "gotcha" is that if >> someone is >> >> waiting (tryAquire(1, MINUTE)) inside a synchronized block, then all >> other >> >> threads trying to access that instance will also have to wait just to >> enter >> >> the synchronized block and get their chance to call "tryAquire", which >> if >> >> course involves more waiting. >> >> >> >> I'm not entirely sure we need the synchronized block anymore. At best >> we >> >> need it after the lock aquire. That of course makes the "tryLock()" >> call >> >> already happening a bit strange. >> >> >> >> Will probably take some experimentation/testing to get it nailed down. >> >> >> >> -David >> >> >> >> >> >> >
