# GSCoC 2018 / 'Attachments enhancements' **URL:** \ **Category:** GSoC\ **Tags:** attachments, gsoc2018\ **Created:** [April 24, 2018, 6:01am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701 "2018-04-24T06:01:52Z")\ **Posts on this page:** 20\ **Page:** 5
**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\ **Post date:** [June 12, 2018, 12:25pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/81 "2018-06-12T12:25:09Z")
@ridmal I think the last bit of Java API that we need is to modify the signature of `getAttachments(..)` like this: ```java List getAttachments(Patient patient, boolean includeIsolated, boolean includeRetired); ``` So that we can toggle whether we want isolated attachments or not. Could you extend the API as above and update the Javadoc accordingly? Don’t forget the unit test that goes with it! ---
**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\ **Post date:** [June 12, 2018, 12:39pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/82 "2018-06-12T12:39:40Z")
Ok i will start working on this. I have a small question. If user set includeIsolated = true , is it means that API will only fetch the isolated attachments ? ( or fetch both isolated and not isolated attachments ? ) ---
**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\ **Post date:** [June 12, 2018, 2:28pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/83 "2018-06-12T14:28:36Z")
You should read it the same as with what `includeRetired` does. Meaning: “what will be returned should _also_ include the isolated attachments”. In summary: > attachments with `includeIsolated=true` = attachments with `includeIsolated=false` + isolated attachments By the way you can also keep the existing signature and do this for its implementation: ```java public List getAttachments(Patient patient, boolean includeRetired) { return getAttachments(patient, true, includeRetired); } ``` This means that the default behaviour is to return **all** attachments, and this should be stated in the Javadoc for that signature. ---
**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\ **Post date:** [June 12, 2018, 9:52pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/84 "2018-06-12T21:52:12Z")
I add the changes and updated the PR. 🙂 ---
**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\ **Post date:** [June 13, 2018, 8:31pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/85 "2018-06-13T20:31:53Z")
Hi @mksd, I am getting “Obs.error.trying.write.complex” error in sometimes when i run the test classes. But sometimes it works properly. This error coming from the methods which are used , Obs obs = obsSaver.saveOtherAttachment(null, patient, null, “File caption #” + i + 1, multipartRandomFile, ValueComplex.INSTRUCTIONS\_DEFAULT); method. Any suggestions about this [error](https://pastebin.com/kwsBvaEs). ---
**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\ **Post date:** [June 13, 2018, 10:29pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/86 "2018-06-13T22:29:29Z")
Can you give me some steps to reproduce this, how do you launch those tests exactly? ---
**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\ **Post date:** [June 14, 2018, 5:46am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/87 "2018-06-14T05:46:43Z")
I launch the test cases using this command . **mvn test -Dtest=AttachmentsServiceTest -pl api-2.0/** ---
**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\ **Post date:** [June 14, 2018, 9:30am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/88 "2018-06-14T09:30:59Z")
Did you always experience this, or do you remember when this started happening? ---
**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\ **Post date:** [June 14, 2018, 11:32am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/89 "2018-06-14T11:32:33Z")
No it is not occur in every time. When i run mvn clean install this error is not occur in most times.I think that this error start to occour when i save the attachments without encounters using ComplexObsSaver. ---
**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\ **Post date:** [June 14, 2018, 12:50pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/90 "2018-06-14T12:50:27Z")
Could you try narrowing down the issue the specific test method(s) that might be affected? You could do that by temporarily using the [`@Ignore`](http://junit.sourceforge.net/javadoc/org/junit/Ignore.html) annotation on the other methods. And then run the test case class via Maven multiple times to validate that the problem indeed occurs. I’m trying to pinpoint which exact test method does that, or at least find a scenario that can be replayed to reproduce the issue more or less consistently. ---
**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\ **Post date:** [June 15, 2018, 5:28am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/91 "2018-06-15T05:28:30Z")
@mksd I tested using @Ignore annotation. But i can’t find exact method which is give above mentioned error. But all the newly added test class give error in different times. I can’t find exactly which method from that methods. I think all the following methods have common thing for occur this error. getAttachments\_shouldReturnIsolatedAttachments(), getAttachments\_shouldNotReturnIsolatedAttachments(), getAttachments\_shouldReturnAllAttachments() But maven clean install will give this error only i run that command several time. It works perfectly in first time. Is this can be a problem in cache of the memory ? ---
**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\ **Post date:** [June 15, 2018, 1:56pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/92 "2018-06-15T13:56:05Z")
@ridmal could you add me as a collaborator (see [here](https://help.github.com/articles/inviting-collaborators-to-a-personal-repository/) how) on your fork of Attachments. I will push a commit to unlock the situation. I have found the exact reason why this is happening, but in the meantime I took the time to refactor quite a few things so better let me insert a full commit and take it from there. I’m busy now, going into a meeting, but I will explain what happened in my next message. _P.S._ I would be great if you could connect on IRC Freenode `#openmrs`. ---
**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\ **Post date:** [June 15, 2018, 1:58pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/93 "2018-06-15T13:58:39Z")
Ok , I will connect to IRC 🙂 ---
**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\ **Post date:** [June 15, 2018, 5:09pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/94 "2018-06-15T17:09:25Z")
As part of my commit I also changed the actual implementation a little bit. In particular it does not get upset right away if no concepts complex are configured, a simple warning is then given (see [here](https://github.com/ridmal/openmrs-module-attachments/blob/f2fe6bb9b7d439cac41104f8ba1c34d80998dc34/api/src/main/java/org/openmrs/module/attachments/AttachmentsServiceImpl.java#L126-L128)). _However_ it will get upset if, for some reason (such as a misconfiguration), non-complex obs are returned, see - [https://github.com/ridmal/openmrs-module-attachments/blob/f2fe6bb9b7d439cac41104f8ba1c34d80998dc34/api/src/main/java/org/openmrs/module/attachments/AttachmentsServiceImpl.java#L41-L43](https://github.com/ridmal/openmrs-module-attachments/blob/f2fe6bb9b7d439cac41104f8ba1c34d80998dc34/api/src/main/java/org/openmrs/module/attachments/AttachmentsServiceImpl.java#L41-L43) - [https://github.com/ridmal/openmrs-module-attachments/blob/f2fe6bb9b7d439cac41104f8ba1c34d80998dc34/api/src/main/java/org/openmrs/module/attachments/AttachmentsServiceImpl.java#L69-L71](https://github.com/ridmal/openmrs-module-attachments/blob/f2fe6bb9b7d439cac41104f8ba1c34d80998dc34/api/src/main/java/org/openmrs/module/attachments/AttachmentsServiceImpl.java#L69-L71) Could you add a unit test that verifies that an API exception is thrown when the module is misconfigured? So the test scenario would be: - Configure a concept complex UUID through the ad-hoc global property that in fact points to a non-complex concept. - Choose a concept UUID for which some obs are already saved (as per the standard test data set). - Attempt to fetch the attachments for the relevant encounter(s) or visit(s). - Verify that an API exception is thrown. ---
**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\ **Post date:** [June 17, 2018, 8:13pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/95 "2018-06-17T20:13:53Z")
@mksd , I am really grateful for your positive feedback and advises given in the 1st evaluation and thankful for all the support and guidance you have given me to complete my tasks. 🙂 ---
**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\ **Post date:** [June 17, 2018, 9:52pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/96 "2018-06-17T21:52:16Z")
@mksd , I update the PR with required changes. There is a error occurred in sometimes ( very rarely occured ) . I think this is because order of the fetching attachments.What Can we about that ? getAttachments\_shouldReturnAllAttachments(org.openmrs.module.attachments.AttachmentsServiceTest): **arrays first differed at element [0]; expected**:\<[0be9476d-17dc-4e01- be71-51b81e3ec594]\> but was:\<[1186503c-74c2-43c3-b771-6035ccbe439a]\> ---
**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\ **Post date:** [June 18, 2018, 9:37am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/97 "2018-06-18T09:37:52Z")
Could it be because of [this](https://github.com/ridmal/openmrs-module-attachments/blob/252ce5c655b72643db4094115bc57b4727b555a0/api-2.0/src/test/java/org/openmrs/module/attachments/AttachmentsServiceTest.java#L274-L275)? Each routine of the service implementation ensures that the obs are returned in descending order of updated timestamps. However in this very specific place the properly sorted obs from two method calls might get mixed up when the two arrays are added. On the other hand the actual attachments are sorted correctly and the assert fails on the first mismatching element (element 0). That’s my guess of what might be happening. Can you `@Ignore` that test routine and launch the test case multiple times to confirm that that’s where the problem come from? If you can confirm that, the solution would be to sort `expectedAllAttachments` by update date prior to asserting against `actualAllAttachments`. Does that make sense? ---
**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\ **Post date:** [June 18, 2018, 9:20pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/98 "2018-06-18T21:20:56Z")
I run test cases by putting @Ignore in every test cases individually. Like you said [testIncludeIsolatedParameter()](https://github.com/ridmal/openmrs-module-attachments/blob/252ce5c655b72643db4094115bc57b4727b555a0/api-2.0/src/test/java/org/openmrs/module/attachments/AttachmentsServiceTest.java#L250) unit test occurred previous mentioned error many times. But when i ignore that method and run the test cases , sometimes getAttachments\_shouldReturnAllAttachments() , getAttachments\_shouldNotReturnIsolatedAttachments() , getAttachments\_shouldReturnIsolatedAttachments() tests also give the same error ( ration of occurring is 1 for 10 times.) I used **mvn test -Dtest=AttachmentsServiceTest -pl api-2.0/** for testings. And for overcome this error can i used this method , use **Collections.sort(expectedAttachments, Comparator.comparing(Attachment::getUuid));** which will sort the **expectedAttachments** list based on the **Uuid** of attachements.( i didn’t able to get getDateCreated() for sorting because it return null vlaues ). And aslo sort the **actualAttachments** list using same method and used the **Assert.assertArrayEquals** method for both sorted lists. ---
**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\ **Post date:** [June 19, 2018, 7:12am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/99 "2018-06-19T07:12:39Z")
@ridmal I added [this](https://github.com/ridmal/openmrs-module-attachments/commit/1f39b9964e03d62f039aa825766c00131c615ae1) commit to your branch. This ensures that you can now obtain the attachment/obs date-time out of any `Attachment` instance. I also added some other minor changes. Can you try to sort _expected_ attachments by descending date-time in a test method where you know that the problem occurs? ---
**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\ **Post date:** [June 19, 2018, 7:16am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/100 "2018-06-19T07:16:16Z")
ok i will try that. Thank you. 🙂 [Previous page](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701.md?page=4) [Next page](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701.md?page=6)