-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43508/#review119469
-----------------------------------------------------------
Hello Keta,
Why did you remove code change introduced via AMBARI-14228? Have you tested if
that'd introduce regression?
If you look at DownloadService.java, methods mapped to @Path("/zip")
neither is setting allowed to true. as the "allowed" is only set when url is
/browse, have you tried distinguishing when to and not to check the
"data.allowed" at the front end?
Please also add Jaimin Jetly as a reviewer?
- Di Li
On Feb. 12, 2016, 5:34 p.m., Keta Patel wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43508/
> -----------------------------------------------------------
>
> (Updated Feb. 12, 2016, 5:34 p.m.)
>
>
> Review request for Ambari, Di Li, DIPAYAN BHOWMICK, Pallav Kulshreshtha, and
> Srimanth Gunturi.
>
>
> Bugs: AMBARI-14932
> https://issues.apache.org/jira/browse/AMBARI-14932
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Steps to Reproduce:
> 1. go to "View" - "FILES" - " Create Instance"
> 2. go to hdfs file view
> 3. click "download zip" icon
> The click doesn't download any file. The REST call returns 200 OK status.
>
> Detailed steps on configuring Views can be found in the link below:
> http://docs.hortonworks.com/HDPDocuments/Ambari-2.1.0.0/bk_ambari_views_guide/bk_ambari_views_guide-20150721.pdf
>
>
> Diffs
> -----
>
>
> contrib/views/files/src/main/java/org/apache/ambari/view/filebrowser/DownloadService.java
> 749174a
> contrib/views/files/src/main/resources/ui/app/controllers/file.js 88fa5fb
>
> Diff: https://reviews.apache.org/r/43508/diff/
>
>
> Testing
> -------
>
> FIX:
> The check for "allowed" in the controller, from the response of
> zipByRequestId() ("/zip" API call for downloading zip file) resulted in no
> action after clicking the download zip button. This was because there was no
> "allowed" paramter in the response of zip download. This "allowed" property
> was added in the response of browse() ("/browse" API call for downloading
> individual file) to check if the user had permissions for the file or not.
> This was verified by opening the file. If that operation didn't throw an
> error, then it would mean that the user had the required permissions to
> download the individual file. But the fact that the point of execution
> reaches past the statement of opening the file verifies that the user has the
> permission. The check with "checkperm" and setting the response with
> "allowed" attribute in the response is not necessary.
> So, for the fix I simply removed the check for "allowed" from the controller
> and also removed the check for "checkperm" which sets the "allowed" attribute
> in the response.
>
> TESTS:
> There are already exisitng tests for DownloadService, so I haven't added any
> new ones.
>
>
> Thanks,
>
> Keta Patel
>
>
