RESTWS-688: Enable installation of modules through rest calls #316
Conversation
|
Looks good 👍 |
|
Looks good:) |
|
Hello @malmike, why are the test failing? |
malmike
force-pushed
the
RESTWS-688_2
branch
from
854de20
to
53da780
Compare
|
The tests pass, just the coverage that dropped by 0.1 |
malmike
force-pushed
the
RESTWS-688_2
branch
from
53da780
to
05a94bf
Compare
malmike
force-pushed
the
RESTWS-688_2
branch
from
05a94bf
to
3d8f3f6
Compare
|
Looks good now. |
| return tempModule; | ||
| } | ||
| catch (MalformedURLException e) { | ||
| throw new RuntimeException(e.getMessage()); |
There was a problem hiding this comment.
It is not good good to throw RuntimeException in this case. Throw an APIException
But also remember to pass on the original exception as the second parameter.
| throw new RuntimeException(e.getMessage()); | ||
| } | ||
| catch (IOException e) { | ||
| throw new RuntimeException(e.getMessage()); |
| throw new RuntimeException(e.getMessage()); | ||
| } | ||
| finally { | ||
| if (tempModule == null && moduleFile != null) { |
There was a problem hiding this comment.
Since it is moduleFile that you are deleting, isn't it enough to just check for it alone?
malmike
force-pushed
the
RESTWS-688_2
branch
2 times, most recently
from
d6580a5
to
9bb73f2
Compare
| @@ -109,17 +127,63 @@ public Object create(SimpleObject post, RequestContext context) throws ResponseE | |||
| case UNLOAD: | |||
| unloadModules(modules, servletContext); | |||
| break; | |||
| case INSTALL: | |||
| Module module = installModule(modules, installUri, servletContext); | |||
| List<Module> listModule = new ArrayList<Module>(); | |||
There was a problem hiding this comment.
Is there any reason why you do not just call listModule.add(module)?
|
|
||
|
|
||
| private Module installModule(Collection<Module> modules, String installUri, ServletContext servletContext) { | ||
| List<Module> moduleList = new ArrayList<Module>(modules); |
There was a problem hiding this comment.
What happens if modules is empty?
| File moduleFile = null; | ||
|
|
||
| try { | ||
| //Module existingModule = moduleFactoryWrapper.getModuleById(module.getModuleId()); |
There was a problem hiding this comment.
For the above, did you see this? https://wiki.openmrs.org/display/docs/Java+Conventions#JavaConventions-Donotcommentoutcode
JIRA TICKET NAME:
RESTWS-688: Enable installation of modules through rest calls
SUMMARY:
Currently a module could not be installed through a REST call. When a REST call is made the module is to be downloaded on to the server, loaded and started.