Designing REST endpoint for starting all modules

Hi,

I am working on https://issues.openmrs.org/browse/RESTWS-627, as part of migrating Manage Modules section to OWA. Most of functionality (like starting/stopping single module) is pretty straightforward, but problematic question is, how to expose endpoint to start all modules?

After quick consulatation with @raff, we see 2 options:

  1. Create new resource, modulemanagement to handle this operation. It is a bit out of convention, but Swagger would document it.

  2. Modify systemsettings resource, so sending POST request with json body e.g. {'property':'modulesmanagement.startAll','value':'true'}(analogic to creating new GlobalProperty), would trigger starting all modules. Swagger wouldn’t document this behaviour, but we would avoid creating new endpoint for single action.

Any thoughts? Maybe you have some better ideas?

Can’t you do this?

  • Add a new Java class that encapsulates a module along with its resource class

    Class MyModuleClass { enum Action { START, STOP} String moduleId; Action action; }

    @Resource(supportedClass=MyModuleClass.class…) Class MyModuleClass Resource…

Rest calls:

POST /ws/rest/v1/module {module:moduleId, action: START} -> Start a single module POST /ws/rest/v1/module {module:moduleId, action: STOP} -> Stop a single module POST /ws/rest/v1/module {action: START} -> Start all Modules POST /ws/rest/v1/module {action: STOP} -> Stop all Modules

To fetch the modules:

GET /ws/rest/v1/module - > List all module GET /ws/rest/v1/module?q=search_phrase - > Search modules

I would prefer “modulemanagement” to “module”. @gutkowski do you also plan to implement uploading new and updating existing modules?

I really care less about the name of the resource, I was just thinking that we could leverage the functionality around the existing module resource.

I’ve been wishing we could make the module service more intuitive since 2010:

Maybe we can take this opportunity to make it better via the REST API. Thank you, @gutkowski!

modulemanagement is fairly abstract. Best practice in REST design is to use nouns for resources rather than verbs or abstract concepts. Here’s a good article from ThoughtWorks on REST API Design - Resource Modeling (see sections on “Nouns versus Verbs” and “Reification of abstract concept”).

Drawing from the article and from @wyclif’s suggestion:

GET /ws/rest/v1/module → get info on all modules GET /ws/rest/v1/module/:moduleuuid → get info on a single module POST /ws/rest/v1/module → upload a new module

Take action on all modules:

POST /ws/rest/v1/moduleaction
{
  "modules": "all",
  "action": "restart"
}

Take an action on specific modules:

POST /ws/rest/v1/moduleaction
{
  "modules": [
    "8ea49007-6d40-4419-acf5-9612af9661a2",
    "df0058c6-3b51-4c1a-aa2b-b54a9afa05ce"
  ],
  "action": "stop"
}

Shortcut for a single module:

GET /ws/rest/v1/moduleaction?module=8ea49007-6d40-4419-acf5-9612af9661a2&action=restart

A module resource would include status and links to take actions on the module. For example:

GET /ws/rest/v1/module/:moduleuuid
{
  …
  "status": "started",
  "links": [
    { "rel": "self", "uri": "/ws/rest/v1/:moduleuuid" },
    { "rel": "start", "uri": "/ws/rest/v1/moduleaction?module=:moduleuuid&action=start" },
    { "rel": "stop", "uri": "/ws/rest/v1/moduleaction?module=:moduleuuid&action=stop" },
    { "rel": "restart", "uri": "/ws/rest/v1/moduleaction?module=:moduleuuid&action=restart" }
  ]
  …
}
1 Like

Thank you all for your feedback. I really like @burke proposition, it looks clean and intuitive.

Yes, I didn’t open PR yet, because starting all modules was a blocker, but I choose to add new interface - Uploadable with single method:

public Object upload(MultipartFile file, RequestContext context) throws ResponseException, IOException;

which is implemented by ModuleResource1_8, and add new method to MainResourceController:

@RequestMapping(value = "/{resource}", method = RequestMethod.POST, headers = "Content-Type=multipart/form-data")
@ResponseBody
public Object upload(@PathVariable("resource") String resource, @RequestParam("file") MultipartFile file,
        HttpServletRequest request, HttpServletResponse response) throws IOException, ResponseException

Which executes lookup for resource and checks if it implements Uploadable. If so, it invokes upload method.

One comment regarding Burke’s proposal is that it’s not nice to change between an array and a string for the modules property on the moduleaction resource. I’d say if you want to run an action on all modules, do not include the modules property in the payload.

I’ve started work on implementing moduleaction resource, and I’m stuck on:

MainResourceController treats module and action as search parameters and tries to run search. Implementing Searchable to execute actions seems to be ugly hack. I thought about introducing new interface, e.g. Executable, but I don’t know how we could cleanly differ in controller, whether request parameters are execution or search parameters.

What do you think? @raff, @burke, @dkayiwa, @wyclif

I’ve overlooked that shortcut. I don’t think we should support updates via request parameters. The same way we do not support POSTing changes via parameters. Updates have to be in the POST body.

Okay, it will make this much easier.

Modules have no uuid, do we mean moduleId when we say uuid? @gutkowski module and action are only used when posting and they are embedded in the payload, therefore the rest controller can’t be treating them as search parameters unless you are making a get request

I agree it’s not nice to change between array and a string when giving data to a client, but it’s convenient for clients when an API does the extra work to support it. For example, Mongoose SchemaType Options. While I favor making things easy for the client, if you take a slightly less convenient approach to make things easier for the server, that’s fine.

On the other hand, I think it would be a mistake to default to operating on all modules. The client should have to explicitly request that an operation affect all modules. Consider the following pseudocode:

var moduleaction = { "actions": "restart" };
for (module in modulesToRestart) {
  moduleactions.modules.push(module);
}

The developer wouldn’t expect an error that results in an empty modulesToRestart to possibly take down the server as all modules are restarted. Instead, a moduleaction without any specified modules should either do nothing or, probably better, return a 400 Bad Request.

The purpose of the shortcuts was to support links to take actions on the module.

Yet, you write server code once and many clients. It’s especially inconvenient for mapping json to strongly typed languages like Java. I’d go with:

POST /ws/rest/v1/moduleaction
{
  "allModules": "true",
  "action": "restart"
}

POST /ws/rest/v1/moduleaction
{
  "modules": [
    "8ea49007-6d40-4419-acf5-9612af9661a2",
    "df0058c6-3b51-4c1a-aa2b-b54a9afa05ce"
  ],
  "action": "stop"
}

Regarding the shortcut. If we want to support passing parameters via URIs for POST, I feel we should make it work for all resources. Probably deserves a separate JIRA issue? Also for the links to be useful for consumers we would have to make it clear they are POSTs and not GETs i.e

{ "rel": "start", "method":"post", "uri": "/ws/rest/v1/moduleaction?module=:moduleuuid&action=start" }

[quote=“raff, post:14, topic:8411”] Regarding the shortcut. If we want to support passing parameters via URIs for POST, I feel we should make it work for all resources. Probably deserves a separate JIRA issue? Also for the links to be useful for consumers we would have to make it clear they are POSTs and not GETs i.e

{ "rel": "start", "method":"post", "uri": "/ws/rest/v1/moduleaction?module=:moduleuuid&action=start" }
```[/quote]

I'd prefer we try to stay true to HATEOS instead of trying to make our custom version by [adding HTTP verbs](http://stackoverflow.com/questions/19959284/where-in-a-hateoas-architecture-do-you-specify-the-http-verbs). If we don't like the shortcuts (I felt a little ashamed suggesting them anyway), then we should just leave them off, perhaps:

```json
GET /ws/rest/v1/module/:moduleuuid
{
  …
  "status": "started",
  "links": [
    { "rel": "self", "uri": "/ws/rest/v1/:moduleuuid" },
    { "rel": "action", "uri": "/ws/rest/v1/moduleaction" }
  ]
  …
}