A module resource was added to the RESTWS module, because resources in REST are primarily identified by uuid in REST which modules don’t have, the resource was written such that it generates a reproduceable uuid on the fly based on the module’s name. I thought resource identification in REST isn’t really restricted to uuid, the getByUniqueId(String) method can be implemented to use any unique identifier for a given resource which in this case would be a module id, and even if we wanted to generate reproduce-able uuids from a string, i think it would be the module id and not the name.
Since a module id is guaranteed to be unique, i see no practical value in using anything else.
We started using uuid strings because the integer ids were not unique across databases. This is not the case for module id.
More context: as part of RESTWS-627, this pull request was merged, but the ticket was left open for discussion. This Talk thread is supposed to be that discussion. (And the next RESTWS release until we resolve this question.)
So basically the question is, which of the following should the module resource support?
GET .../ws/rest/v1/module/uiframework
GET .../ws/rest/v1/module/org.openmrs.module.uiframework
GET .../ws/rest/v1/module/some-obscure-uuid
The best long-run answer would be to support the fully-qualified module name. However I believe that OpenMRS Core doesn’t let you run two modules with the same short moduleId at the same time, and it’s nowhere on our roadmap to change this. Further, our Spring MVC configuration wrong* so anything after the final dot will be treated as a file extension, thus you’d actually need to add a “/” or a “.anything” to the URL, e.g. GET .../ws/rest/v1/module/org.openmrs.module.uiframework/.
[*] a fix for this is to set useRegisteredPatternSuffixMatch=true as mentioned here. I haven’t looked into where we’d need to make this fix in openmrs-core, but in any case it would require a lot of regression testing and backporting. I can do the RESTWS fix now, but I can’t take on the bigger piece of work. (I can ticket it though.)
My view was that modules should be identified by their moduleIds(not sure if this has to be a full name) and we shouldn’t be auto generating some fake uuids for them.
I vote for using the moduleid (xforms, reporting, uiframework, etc) instead of module package. I base this on the fact that our module framework uses moduleid with the assumption that it is unique.
I would prefer that we support fully qualified module IDs and allow non-qualified module IDs – i.e., ideally, either of these should work:
GET .../ws/rest/v1/module/uiframework
GET .../ws/rest/v1/module/org.openmrs.module.uiframework
I’m not suggesting we support a single module being simultaneously reachable at either URI; rather, someone making a registration module should be able to set its module ID to “com.example.registration” (ensuring they will not conflict with any existing/future modules) without breaking things.
If we can only support non-qualified module IDs now and have to ticket support for fully qualified module IDs, that’s okay, but if @raff’s comment is true and we can support fully qualified module IDs too with this change, then I would prefer we do it now.
<rant>
Version 4 UUIDs (what we should be using when we create UUIDs) are, [by definition] (Universally unique identifier - Wikipedia), randomly generated. If it’s reproducibly generated, then we shouldn’t be referring to them as UUIDs.
In brief, if you are creating UUIDs in any other way than calling a library/function to generate a random Version 4 UUID, then you’re doing it wrong. We’ve violated the standard for concept UUIDs, let’s not continue to make the same mistakes in the future.
@Burke the standard UUID generator class that comes with java has a function that lets you generate a ‘reproduce-able’ uuid from a specified String, I might be wrong but the last time I read its javadocs this is what they implied. Currently the REST module is using this function to auto create reproduce-able uuids from the moduleId string with the goal of keeping the uuid of a module constant.
FWIW, the dots issue started happening with the spring upgrade in platform 1.11
To confirm this, without the above fix, this test passes in 1.9, 1.10 but fails starting from 1.11 and above.
Okay, I thought we had a bespoke algorithm, which would defeat the main reasons of using RFC 4122. If we’re using a Java feature to generate them, that makes them Version 5 UUIDs (namespace name-based), which is a viable solution for getting UUIDs if we were going to use them. Since we’re going with module IDs (not UUIDs), it doesn’t matter anyway.
In the big picture we now have a bean in the REST module which overrides path handling for core and all other modules. (And this is a core module, distributed with every platform release.) We should make the fix in core, so that webservices only needs to conditionally apply this fix for “old” openmrs-core versions, and we can eventually sunset this configuration hack.
(@dkayiwa, @adamg, FYI, let’s make sure we’re constantly improving openmrs-core in small ways, even if we don’t get to see the benefits of this until a couple years down the road…)
We started using uuid strings because the integer ids were not unique across databases. This is not the case for module id.