Conversation
…he processing of a ImageTask
…urce for processed ImageTasks and another resource for email
…ave had its ids specified
thiagomanel
left a comment
thiagomanel
left a comment
There was a problem hiding this comment.
Nice, thank @velmer some suggestions for improvement
| jsonObject.put(FILES, filesJSONArray); | ||
| } catch (JSONException e) { | ||
| jsonObject.put(STATUS, UNAVAILABLE); | ||
| throw e; |
There was a problem hiding this comment.
why do we need to call this put if we throw the exception the line below?
There was a problem hiding this comment.
I was deciding between two approaches, but ended up doing none oh them. I wanted to built the json object even if there was an error, but I didn't want to put logging in an instance method of a pojo, then I added the throw so the caller method can log the error... But both strategies cannot coexist. I'll fix it
| private static final String STATUS = "status"; | ||
| private static final String UNAVAILABLE = "UNAVAILABLE"; | ||
|
|
||
| private ImageTask imageTask; |
There was a problem hiding this comment.
add this declarations finals
| private String name; | ||
| private String URL; | ||
|
|
||
| public ImageTaskFile(String path, String name) { |
There was a problem hiding this comment.
do we need (sorry, I did not checkout the code) both these constructor (can we keep only one?)
There was a problem hiding this comment.
The other constructor isn't used externally, I added just for precaution, so yes, it can be removed
| * Service that provides operations for communication with Object Store. | ||
| */ | ||
| public class ObjectStoreService { | ||
|
|
There was a problem hiding this comment.
this class seems very coupled with SWIFT logic. I think it would be better to call it SwiftStorageService (or something like that). overall, I think it is a very nice idea to hide the complexity.
Also, why no passing the properties file to a constructor as a regular object instead of the more staless design
There was a problem hiding this comment.
Yes, it is, I'll change it.
About the properties file, it's because that conversation we had about services of restlet's architecture, but because of the deadline I couldn't use them. So, my strategy was to simplify the utilization of services, making all of its methods as static, in order to avoid instantiate the services.
The expected strategy was to create an interface to define the contract of the services and the instantiations of its implementations be done by injection (or some like that, I don't know how restlet handle this)
I'm open for suggestions, should I change the methods to be instance's and instantiate the services in constructors of its users?
| /** | ||
| * Service the provides operations with processed {@link ImageTask}. | ||
| */ | ||
| public class ProcessedImagesService { |
There was a problem hiding this comment.
also very couple with Swift. I'd be better to add Swift in the class name
2 participants
Purpose of this PR:
Create an API route to return the URLs of every file generated by execution of a ImageTask. The client must specify a list of IDs of ImageTasks through query parameter.
What problem will be solved:
Now a SAPS instance can request to one of its neighbors the URLs of files of a ImageTask, when sending an email of processed ImageTasks and at least one of these was processed in other SAPS instance.
Modifications: