We should have a common template for all services in our API. This allows for easier comprehension and a cleaner code base overall.
All Services should contain:
Transactions
- The @Transactional annotation should be at the top of the service interface
- Methods that don't write data should also put before their methods
@Transactional( readOnly = true)
Authentication
- Each method in the interface should have an @Authorized annotation on it
@Authorized(OpenmrsConstants.PRIV_EDIT_THINGS)
- Data should have these privileges (users, encounters, orders):
- PRIV_ADD_THINGS
- PRIV_EDIT_THINGS
- PRIV_VIEW_THINGS
- PRIV_DELETE_THINGS
- PRIV_PURGE_THINGS
- Metadata should have these privileges (encounterType, concepts, program, location):
- PRIV_MANAGE_THING_TYPES (covers add/edit/delete)
- PRIV_VIEW_THING_TYPES
- PRIV_PURGE_THING_TYPES
Parent classes
- All service interfaces should extend org.openmrs.api.OpenmrsService
- All service implementation classes should extend org.openmrs.api.BaseOpenmrsService
Commenting
In Eclipse, Alt-Shift-J is your friend...or Alt-Command-J on a Mac.
- Service interface:
- Should have class level comment explaining a typical use case
- Each method should have comment about what it does, each parameter explained, and return value explained
- Deprecated methods should be marked in the javadoc and given a comment about why and what replaces it.
- Service implementation:
- The class comment on the should explain and point back at the interface
- Each method should simply point at the interface
- DAO interface
- Should have class level comment should point back at the interface
- Each method comment should simply point at the interface
- DAO implementation
- Class level comment should point at DAO and service interface
- Each method should point at service interface (and DAO?)
General
- Enumerations should be used instead of numerical constants
Retired vs Voided
- DATA is defined as what is contained in the database as patient information or user defined explanation of metadata
- META DATA is defined as those tables that describe how to store and what is the DATA
- Data should be voided. Meta data should be retired
Data |
Meta Data |
---|---|
|
|
Thing Service
@Transactional public interface ThingService extends OpenmrsService { /** * @see #saveThing(Thing) * @deprecated replaced by saveThing(Thing) */ @Authorized({OpenmrsConstants.PRIV_CREATE_THINGS}) public void createThing(Thing thing) throws APIException; /** * Gets a Thing by identifier * @param thindId identifier of desired Thing */ @Authorized({OpenmrsConstants.PRIV_VIEW_THINGS}) public Thing getThing(Integer thingId) throws APIException; /** * Get a Thing by name * @param thingName is the exact name of the desired Thing */ @Authorized({OpenmrsConstants.PRIV_VIEW_THINGS}) public Thing getThing(String thingName) throws APIException; // if this is meaningful /** * Get a Thing by GUID * @param guid is the guid of the desired Thing */ @Authorized({OpenmrsConstants.PRIV_VIEW_THINGS}) public Thing getThingByGuid(String guid) throws APIException; // add this later for sync /** * Gets all Things, [including retired|not including voided] */ @Authorized({OpenmrsConstants.PRIV_VIEW_THINGS}) public List<Thing> getAllThings() throws APIException; // includes retired, but not voided /** * Gets all Things * @param include[Retired|Voided] whether or not to include [retired|voided] Things */ @Authorized({OpenmrsConstants.PRIV_VIEW_THINGS}) public List<Thing> getAllThings(boolean includeRetired) throws APIException; // only one of these public List<Thing> getAllThings(boolean includeVoided) throws APIException; // only one of these /** * Find Things * @param query partial name * @returns matching Things */ @Authorized({OpenmrsConstants.PRIV_VIEW_THINGS}) public List<Thing> getThings(String query); /** * Get Things that match the given properties * @param ... */ @Authorized({OpenmrsConstants.PRIV_VIEW_THINGS}) public List<Thing> getThings( ... all properties ... ) throws APIException; /** * Get Things by XXX * @param xxx */ @Authorized({OpenmrsConstants.PRIV_VIEW_THINGS}) public List<Thing> getThingsByXXX(XXX xxx) throws APIException; // where XXX is metadata // in most instances, will delegate to getThings(...all properties...) /** * @see #saveThing(Thing) * @deprecated replaced by saveThing(Thing) **/ @Authorized({OpenmrsConstants.PRIV_EDIT_THINGS}) public void updateThing(Thing thing) throws APIException; /** * Save Thing in database * @param thing Thing to be saved */ @Authorized({OpenmrsConstants.PRIV_EDIT_THINGS}) public Thing saveThing(Thing thing) throws APIException; /** * [Retires|Voids] Thing * @param thing Thing to be [retired|voided] */ @Authorized({OpenmrsConstants.PRIV_EDIT_THINGS}) public Thing retireThing(Thing thing) throws APIException; // for metadata, retired returned in getAll public Thing voidThing(Thing thing) throws APIException; // for data, voided not returned in getAll /** * [Unretired|Unvoid] Thing * @param thing to be recovered */ @Authorized({OpenmrsConstants.PRIV_EDIT_THINGS}) public Thing unretireThing(Thing thing) throws APIException; // for metadata public Thing unvoidThing(Thing thing) throws APIException; // for data /** * Remove Thing completely (not reversible) * @param thing Thing to be purged from the database */ @Authorized({OpenmrsConstants.PRIV_PURGE_THINGS}) public Thing purgeThing(Thing thing) throws APIException; /** * Remove Thing completely (not reversible) * @param thing Thing to be purged from the database * @param cascase if <code>true</code>, related data are purged as well */ @Authorized({OpenmrsConstants.PRIV_PURGE_THINGS}) public Thing purgeThing(Thing thing, boolean cascade) throws APIException; /** * Remove Thing completely (not reversible) * @param thing Thing to be purged from the database * @deprecated use #purgeThing(Thing) */ @Authorized({OpenmrsConstants.PRIV_PURGE_THINGS}) public void deleteThing(Thing thing) throws APIException;