Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for resource id to the archiver #2100

Merged
merged 16 commits into from
Oct 1, 2021

Conversation

gmgigi96
Copy link
Member

@gmgigi96 gmgigi96 commented Sep 23, 2021

Before the archiver only supported resources provided by a path.
Now also the resources ID are supported in order to specify the content of the archive to download. The parameters accepted by the archiver are two:

  • an optional list of path (containing the paths of the resources)
  • an optional list of id (containing the resources IDs of the resources)

Closes #2097

@update-docs
Copy link

update-docs bot commented Sep 23, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

glpatcern
glpatcern previously approved these changes Sep 24, 2021
Copy link
Member

@glpatcern glpatcern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the latest fixes it looks good to go for me.

…and changed the interface of the archvier service, to accept a list of `path` and `id`
@kulmann
Copy link
Member

kulmann commented Sep 28, 2021

On a mac I get the following error when trying to extract the downloaded tarball with 7z x download.tar:

7-Zip [64] 16.02 : Copyright (c) 1999-2016 Igor Pavlov : 2016-05-21
p7zip Version 16.02 (locale=utf8,Utf16=on,HugeFiles=on,64 bits,12 CPUs x64)

Scanning the drive for archives:
1 file, 453241 bytes (443 KiB)

Extracting archive: download.tar

ERRORS:
Headers Error


WARNINGS:
There are data after the end of archive

--
Path = download.tar
Type = tar
ERRORS:
Headers Error
WARNINGS:
There are data after the end of archive
Physical Size = 250880
Tail Size = 202361
Headers Size = 1536
Code Page = UTF-8



Archives with Errors: 1

Warnings: 1

Open Errors: 1

It extracts it, the folder structure is correct, but the files are corrupted. Does this work for you?

@pascalwengerter
Copy link
Contributor

Can confirm that on Linux/Ubuntu archives that contain PNG and JPG files don't work as expected - top level files can't be opened, while a folder containing images didn't even make it into the *.tar

@gmgigi96
Copy link
Member Author

@kulmann @pascalwengerter I tried on both Mac and Arch and I didn't manage to reproduce the errors you had. For me it works on both systems, could you please provide me the steps to reproduce it? Thanks

@kulmann
Copy link
Member

kulmann commented Sep 28, 2021

Maybe it's an issue with the current state of owncloud/web#5832
I'm running an ocis with
a) a local replace to reva which is checked out to your branch AND rebased to reva master
b) with WEB_ASSET_PATH=path/to/my/web/dist/folder

@gmgigi96
Copy link
Member Author

Can you please try doing an HTTP GET request directly to the archiver service?

@gmgigi96 gmgigi96 marked this pull request as ready for review September 30, 2021 07:57
@gmgigi96
Copy link
Member Author

@kulmann @pascalwengerter I added some unit tests into the archiver and all are passing, are you having the same issues?

Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The id based version now works like a charm 🥳

@kulmann
Copy link
Member

kulmann commented Sep 30, 2021

@kulmann @pascalwengerter I added some unit tests into the archiver and all are passing, are you having the same issues?

Resolved in the web PR. Sorry for the noise 😅

@gmgigi96
Copy link
Member Author

@kulmann @pascalwengerter I added some unit tests into the archiver and all are passing, are you having the same issues?

Resolved in the web PR. Sorry for the noise 😅

Glad it works! @labkode @glpatcern can we merge it now?

@glpatcern
Copy link
Member

Great, thanks all! Merging right away

@glpatcern glpatcern self-requested a review October 1, 2021 06:36
@individual-it
Copy link
Contributor

@gmgigi96 when using both id and path in the same request e.g. curl -k -u admin:admin 'https://localhost:9200/archiver?path=/home/image.jpg&id=MTI4NGQyMzgtYWE5Mi00MmNlLWJkYzQtMGIwMDAwMDA5MTU3OmZlZTVhNjZlLWI4MzYtNDY2OS05ZDJhLTliNGEwMDQwYmY0OQ==' --output /tmp/6.tar

I get whatever is requested by path in the folder called home and whatever is requested by id in the folder users/<uuid>/
is that intended?

$ tar -tvf /tmp/6.tar 
drwxr-xr-x 0/0               0 2021-11-11 16:10 users/ddc2004c-0977-11eb-9d3f-a793888cd0f8/my_data
-rw-r--r-- 0/0          472800 2011-03-07 21:41 users/ddc2004c-0977-11eb-9d3f-a793888cd0f8/my_data/11-02.JPG
-rw-r--r-- 0/0         1278511 2020-08-17 15:39 home/image.jpg

glpatcern pushed a commit to glpatcern/reva that referenced this pull request Nov 12, 2021
* Add support for resource id when requesting a download of an archive and changed the interface of the archvier service, to accept a list of `path` and `id`

* Add tests

* Add changelog

* Add Walker interface

* Add walker mock

* The mock downloader implements the Downlaoder interface for downloading of local files

* The Reva downloader implements the Downloader interface for downloading files from reva

* Refactored archiver to be testable

* HTTP handler of the archiver

* Some useful functions for tests

* Add unit tests

* Fix typo

* Fix linter

* Fix linter2

* Reorganized files

* Fix config parsing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change archiver to work resource id based
5 participants