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 >> >>
