Thanks ben. I'd still like to do this without adding anything to core in the short term, because i don't have the bandwidth to update the war file on our servers before our 1.8 migration in a few months. At that point, however, it would be nice to have the key property to use in 1.8.
I also don't know how widely used namephonetics is -- if PIH is the only consumer, then putting a stack trace in the logs at startup for the next 2 months before implementing a proper solution bothers me a lot less. I'm going to wait for Mark G. to get back from vacation first, before doing anything anyway. d On Fri, Aug 26, 2011 at 12:06 PM, Ben Wolfe <[email protected]> wrote: > Well, its mostly independent. Sync still knows about name phonetics in > this case, which is not ideal or scalable. > > You should be able to use the generic handler interface and method on the > Context to get all handlers for the NamePhonetics class. The only question > is how to get a specific class of handler so you're sure the handler you > found is the one specified for sync. The simplest solution would be to add > another option to the @Handler annotation for a "key" or something else. > It'd just be a string that is known between SyncService and NamePhonetics to > know whether it has the right handler or not. > > Ben > > On Wed, Aug 24, 2011 at 5:11 PM, Dave Thomas <[email protected]> wrote: >> >> Hi. AOP'ing around SyncService from NamePhonetics seems to work, >> although if you're using NamePhonetics without sync, you get a >> ClassNotFoundException stack trace in the logs during module start, >> which is a little sloppy. Also, I'm not totally sure what the rules >> are: against all versions of OpenMRS past 1.6.x, is this still how >> exceptions while loading an AOP advice class from a module will be >> handled (i.e. other AOP advice classes from the module will still be >> loaded even if the first AOP errors during loading)? Also, there's >> no danger of NamePhonetics being loaded before Sync causing this to >> fail, right? This is why the context is built twice at startup? >> >> -------------- >> >> Here's short-term option 2: >> >> in NamePhonetics, you add this handler: >> >> >> @Handler(supports = PersonName.class) >> public class PersonNamePhoneticsSaveHandler implements >> RequiredDataHandler<PersonName>{ >> >> public void handle(PersonName personName, User currentUser, Date >> currentDate, String other) { >> >> NamePhoneticsUtil.savePhoneticsForPersonName(personName); >> >> } >> } >> >> >> and in sync, you add the following postProcessor to SyncService: >> >> >> public class NamePhoneticsSaveOrUpdateAfterAdvice implements >> AfterReturningAdvice { >> >> >> private static final String NAME_PHONETICS_HANDLER_CLASS = >> "org.openmrs.module.namephonetics.advice.PersonNamePhoneticsSaveHandler"; >> private RequiredDataHandler<PersonName> handler = null; >> >> >> @SuppressWarnings("unchecked") >> public synchronized void afterReturning(Object returnVal, Method >> method, Object[] arg2, Object arg3) throws Throwable { >> String methodName = method.getName(); >> if (methodName.equals("saveOrUpdate")) { >> //check for valid main argument >> if (arg2 == null || arg2.length == 0) >> return; >> Object mainArgument = arg2[0]; >> if (mainArgument == null) >> return; >> if (mainArgument instanceof PersonName){ >> Class<?> handlerClass = null; >> try { >> handlerClass = >> Class.forName(NAME_PHONETICS_HANDLER_CLASS, false, >> OpenmrsClassLoader.getInstance()); >> } catch (Exception ex){ >> handler = null; >> //pass on doing anything else... >> } >> if (handlerClass != null){ >> if (handler == null) >> handler = >> (RequiredDataHandler<PersonName>) >> HandlerUtil.getPreferredHandler(handlerClass, PersonName.class); >> handler.handle((PersonName) >> mainArgument, null, null, null); >> } >> } >> } >> } >> } >> >> >> This approach creates true module independence with no changes to >> core, and no problems just running sync, or just running >> namephonetics. >> >> The sort-of-lame thing about this is that the Sync post-processor >> looks for a handler that's an implementation of RequiredDataHandler, >> which was the most useful handler interface i could find in core. It >> might be useful to create a new core handler interface with the single >> argument handle method: handle(OpenmrsObject o). Maybe >> GenericOpenmrsObjectHandler? This interface could also require Ben's >> toBeUsedBy(String className) method (where in this case this method >> would return org.openmrs.module.sync). >> >> Another lesser sort-of-lame thing is having such a specific item in >> Sync at all -- this was the best i could come up with without pushing >> something new into 1.6.x core, which i don't want to do because i >> don't have the bandwidth for upgrading the war file on all of our >> remote servers. >> >> Thoughts? I have both of these implemented, and i can apply either one. >> >> I really like the new handler interface idea, but it depends on an >> upgrade to core, which then introduces core version dependencies to >> both NamePhonetics and Sync. >> >> d >> >> On 8/24/11, Ben Wolfe <[email protected]> wrote: >> > The quickest solution would be to AOP around the sync save, check to see >> > if >> > its a personname, and update accordingly. >> > >> > Ben >> > >> > On Wed, Aug 24, 2011 at 2:40 AM, Darius Jazayeri >> > <[email protected]>wrote: >> > >> >> I agree that this seems like something that's more appropriate to >> >> calculate >> >> on the child after importing a name. >> >> >> >> The problem is that we do our post-service-call actions using AOP >> >> rather >> >> than having a proper event-notification mechanism. (Dave, I assume that >> >> namephonetics does AOP-after savePerson, savePersonName, etc.) >> >> >> >> Ideally sync would be able to call a core method like this: >> >> save(importedObject); // uses something internal to sync. >> >> publishEvent(importedObject, "saved"); >> >> >> >> Another solution (also requiring core changes) would be if there was a >> >> generic save method would would delegate to the appropriate service >> >> method. >> >> So that sync doesn't have to do its own hibernateSession.save(object) >> >> which >> >> bypasses the service layer. >> >> >> >> I know that Rafal implemented something like this in the Metadata >> >> Sharing >> >> Module--it actually scans everything that implements OpenmrsService on >> >> the >> >> classpath, and finds save(OpenmrsObject) methods. >> >> >> >> -Darius >> >> >> >> On Mon, Aug 22, 2011 at 12:41 AM, Ben Wolfe <[email protected]> wrote: >> >> >> >>> Am I remembering right that NamePhonetics has a table listing out the >> >>> manipulations of the names? That it doesn't do that calculation on >> >>> the >> >>> fly? It does sound a lot like conceptword in that you don't need to >> >>> send >> >>> those back an forth, instead have them re-calculated on the receiving >> >>> server. >> >>> >> >>> We do have the @Handler annotation in trunk. But its usually used as >> >>> an >> >>> @Handler(supports=NamePhonetics.class) PLUS extended a special class. >> >>> This >> >>> class would have to be in the sync module. Its possible that you >> >>> extend >> >>> a >> >>> class from sync and then use the aware_of_module tag in config.xml. >> >>> This >> >>> is >> >>> a loose module requirement that Daniel did >> >>> https://tickets.openmrs.org/browse/TRUNK-1587 (but its 1.9+ :-/) >> >>> >> >>> The generic Handler class you're proposing would just have a handle(T) >> >>> method? What happens when there are two or more classes defined as: >> >>> @Handler(supports=NamePhonetics.class) >> >>> public class NamePhoneticsHandler extends Handler >> >>> ? >> >>> >> >>> How is sync supposed to know which one to use? Or which one is for >> >>> sync >> >>> and which one is for some other handler? >> >>> >> >>> Answering my own question after pondering for a few minutes: >> >>> add another method called "whoAmIIntendedToBeUsedBy()" that returns a >> >>> String key that the sync module can use to distinguish different >> >>> handler >> >>> classes. (Obviously the name of the method needs some work...) >> >>> >> >>> Ben >> >>> >> >>> >> >>> On Sat, Aug 20, 2011 at 12:10 PM, Dave Thomas <[email protected]> >> >>> wrote: >> >>> >> >>>> Hi. At PIH, we have a few implementations using both sync and >> >>>> namephonetics. Right now, namephonetics objects aren't propagating >> >>>> through sync which is something we want to fix. >> >>>> >> >>>> Option 1 is to have NamePhonetics extend OpenmrsObject, and let sync >> >>>> propogate these. The downsides are 1) extra stuff in the payloads, >> >>>> and 2) I have a feeling there would need to be explicit handling of >> >>>> NamePhonetics objects during ingest, because PersonName objects are >> >>>> explicitly marked as getting processed last (because of the famous >> >>>> TreeSet re-ordering bug). This, i think, would mean that there >> >>>> would >> >>>> have to be a new category of 'things to process last' in the ingest >> >>>> routine, which in sync we could maybe define safely as anything >> >>>> associated with a module (org.openmrs.module*)? Not sure about this >> >>>> though. It is definitely true that we don't want to build any module >> >>>> dependencies between Sync and NamePhonetics. >> >>>> >> >>>> Option 2 is to treat NamePhonetics like ConceptWords in that they're >> >>>> calculated locally. NamePhonetics currently AOPs around savePerson >> >>>> to >> >>>> do this on a local system, but the saving of PersonNames during sync >> >>>> ingest uses the SyncService.saveOrUpdate method. Here's what I was >> >>>> thinking -- what if SyncService had a postInterceptor modeled on >> >>>> RequiredDataAdvice? This postInterceptor class would check for >> >>>> registered handlers for incoming sync items. The only architectural >> >>>> piece that would be needed to do this and avoid building in module >> >>>> dependencies between Sync and NamePhonetics would be a truly default >> >>>> Handler interface in OpenMRS core. NamePhonetics would then register >> >>>> a PersonName handler. Sync ingest would then look for a handler in >> >>>> the saveOrUpdate postProcessor, and if found, apply it. If not found >> >>>> (for sync implementations not using namephonetics), no problem. >> >>>> >> >>>> Option 2 seems like overkill, maybe, although i like the idea of >> >>>> keeping sync packages as simple and small as possible... thoughts on >> >>>> this would be appreciated. >> >>>> >> >>>> Also, generally, would it be useful to have a truly generic handler >> >>>> interface in core (or actually, maybe generic pre-processor and >> >>>> post-processor handler interfaces)? This would be kind of a cool >> >>>> way >> >>>> for modules to register handlers against each other, without building >> >>>> in module dependencies. >> >>>> >> >>>> d >> >>>> >> >>>> _________________________________________ >> >>>> >> >>>> To unsubscribe from OpenMRS Developers' mailing list, send an e-mail >> >>>> to >> >>>> [email protected] with "SIGNOFF openmrs-devel-l" in the >> >>>> body >> >>>> (not the subject) of your e-mail. >> >>>> >> >>>> [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l] >> >>>> >> >>> >> >>> ------------------------------ >> >>> Click here to >> >>> >> >>> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >> >>> OpenMRS Developers' mailing list >> >> >> >> >> >> ------------------------------ >> >> Click here to >> >> >> >> unsubscribe<[email protected]?body=SIGNOFF%20openmrs-devel-l>from >> >> OpenMRS Developers' mailing list >> > >> > _________________________________________ >> > >> > To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to >> > [email protected] with "SIGNOFF openmrs-devel-l" in the body >> > (not >> > the subject) of your e-mail. >> > >> > [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l] >> > >> >> _________________________________________ >> >> To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to >> [email protected] with "SIGNOFF openmrs-devel-l" in the body (not >> the subject) of your e-mail. >> >> [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l] > > ________________________________ > Click here to unsubscribe from OpenMRS Developers' mailing list _________________________________________ To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to [email protected] with "SIGNOFF openmrs-devel-l" in the body (not the subject) of your e-mail. [mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l]
