For example https://github.com/openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/Person.java#L485-L583 has getAttribute for:
but no reference to the Uuid of the Attribute type, which makes it more difficult to use via custom methods in REST.
This is global to all domain objects but is an entry level ticket for a new per community
Isn’t it sufficient to do this?
person.getAttributes(service.getPersonAttributeTypeByUuid(patUuid))
Since we already have methods that take a String (and interpret this as name) if we were to add methods that take a UUID we’d have to give it a different name, like Person.getAttributeByTypeUuid(String)…
@darius This utility method is what I am talking about for all domain objects which simplifies access to collections across relationships
I think @ssmusoke would like to see 2 things happen, i.e to be able to retrieve any persistent openmrs object by a uuid, and also in this particular case he wants to be able to get a person attribute when he calls Person.getAttribute(String arg) where the argument value is a person attribute type uuid , currently this only supports attribute type name.
I personally would love to see our API services evolve and become more generic to minimize code duplication, we have a BaseOpenmrsService that is inherited by all services, we could add to it methods like below:
<T extends BaseOpenmrsObject> T getById(Class<T> clazz, Long id) {
return genericDAO.getById(clazz, id);
}
<T extends BaseOpenmrsObject> T getByUuid(Class<T> clazz, String uuid) {
return genericDAO.getByUuid(clazz, uuid);
}
Alternatively we could introduce a ServiceUtil class that contains the methods above along with more like:
<T extends BaseOpenmrsMetadata> T getByName(Class<T> clazz, String name) {
return genericDAO.getByName(clazz, name);
}And instead of adding Person.getAttributeByAttributeTypeUuid, Person.getAttributeByAttributeTypeId etc to Person, we could add a new interface that can be implemented by all domain objects that have attributes and implement these getAttributeByAttributeTypeXXX methods as java 8’s default methods in there because soon or later another developer might require similar methods on Location, Visit etc
Note this ticket, whose output may be helpful here: https://issues.openmrs.org/browse/TRUNK-3782
I don’t like “genericDAO” as a concept. What you’re describing here @wyclif effectively removes the distinction between Service and DAO…I don’t want to refactor our codebase this way (basically, to tie our service layer more closely to hibernate magic) when in the long-term we should be aiming towards a polyglot persistence approach.
@ssmusoke note that we have https://github.com/openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/customdatatype/Customizable.java and base implementations for data and metadata, which work for our “newer” (as of 1.9) attributable interface. The problem is that PersonAttribute has not been updated to use the new attribute mechanism.
Out of curiousity are you suggesting Util.getAttributeByTypeUuid(person, uuid) or Util.getCollectionMembersWithPropertyAndValue(person, "attributes", "attributeType.uuid", uuid) ?
As I mentioned, this code can always reside in BaseOpenmrsService, we don’t necessarily have to add a generic DAO, it was just an example, and in any case genericDAO would be an interface with a hibernate implementation as the default just like we do for all our DAOs therefore we wouldn’t be tying the service layer to hibernate.
Instead of a generic DAO, I would create an abstract DAO and make all the DAOs extend from it. And then move the duplicated methods up the hierarchy. No refactoring needed on the clients. A few years ago I was working on this solution but was delayed because Spring Data was preferred. And now we have none.
