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

Fix files_external:export so the output can be successfully imported #37513

Merged
merged 1 commit into from
Jun 11, 2020

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Jun 10, 2020

Description

The output of php occ files_external:export was containing problems like:

  • "storage" had a human-friendly text like "Local", when it needs the more technical \OC\Files\Storage\Local class name
  • "authentication_type" had a human-friendly text like "None", when it needs the more technical null::null or password::password
  • "applicable_users" was being output as "All" (when no specific users were set), or a comma-separated string of applicable users. For import, it needs to be an array of usernames (empty if there are no users specified)
  • "applicable_groups" was being output as "" (when no specific groups were set), or a comma-separated string of applicable groups. For import, it needs to be an array of group names (empty if there are no groups specified)

A new option to files_external:list has been added - -i or --importable-format - when this is specified and a JSON output is requested, the items above are corrected in the JSON to be in a format that can be used for import.

files_external:export calls list with this new option set. (It already called list to do its work anyway).

Extra unit and acceptance tests have been added to cover the new list option, and the export and import of various combinations of mounts.

Related Issue

How Has This Been Tested?

CI

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@codecov
Copy link

codecov bot commented Jun 10, 2020

Codecov Report

Merging #37513 into master will decrease coverage by 0.00%.
The diff coverage is 65.38%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #37513      +/-   ##
============================================
- Coverage     64.68%   64.68%   -0.01%     
  Complexity    19336    19336              
============================================
  Files          1277     1277              
  Lines         75535    75556      +21     
  Branches       1331     1331              
============================================
+ Hits          48860    48872      +12     
- Misses        26283    26292       +9     
  Partials        392      392              
Flag Coverage Δ Complexity Δ
#javascript 54.14% <ø> (ø) 0.00 <ø> (ø)
#phpunit 65.84% <65.38%> (-0.01%) 19336.00 <0.00> (ø)
Impacted Files Coverage Δ Complexity Δ
apps/files_external/lib/Command/Export.php 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
apps/files_external/lib/Command/ListCommand.php 66.66% <68.00%> (-0.79%) 0.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 890ef6e...c693ebb. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jun 10, 2020

Codecov Report

Merging #37513 into master will decrease coverage by 0.00%.
The diff coverage is 65.38%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #37513      +/-   ##
============================================
- Coverage     64.68%   64.68%   -0.01%     
  Complexity    19336    19336              
============================================
  Files          1277     1277              
  Lines         75535    75556      +21     
  Branches       1331     1331              
============================================
+ Hits          48860    48872      +12     
- Misses        26283    26292       +9     
  Partials        392      392              
Flag Coverage Δ Complexity Δ
#javascript 54.14% <ø> (ø) 0.00 <ø> (ø)
#phpunit 65.84% <65.38%> (-0.01%) 19336.00 <0.00> (ø)
Impacted Files Coverage Δ Complexity Δ
apps/files_external/lib/Command/Export.php 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
apps/files_external/lib/Command/ListCommand.php 66.66% <68.00%> (-0.79%) 0.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 890ef6e...c693ebb. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 10, 2020

Codecov Report

Merging #37513 into master will decrease coverage by 0.00%.
The diff coverage is 82.75%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #37513      +/-   ##
============================================
- Coverage     64.68%   64.68%   -0.01%     
  Complexity    19336    19336              
============================================
  Files          1277     1278       +1     
  Lines         75535    75562      +27     
  Branches       1331     1331              
============================================
+ Hits          48860    48876      +16     
- Misses        26283    26294      +11     
  Partials        392      392              
Flag Coverage Δ Complexity Δ
#javascript 54.14% <ø> (ø) 0.00 <ø> (ø)
#phpunit 65.84% <82.75%> (-0.01%) 19336.00 <0.00> (ø)
Impacted Files Coverage Δ Complexity Δ
apps/files_external/lib/Command/Export.php 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
apps/files_external/lib/Command/ListCommand.php 68.78% <85.71%> (+1.32%) 0.00 <0.00> (ø)
tests/acceptance/features/bootstrap/bootstrap.php 0.00% <0.00%> (ø) 0.00% <0.00%> (?%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 890ef6e...a9b8cf5. Read the comment docs.

@mmattel
Copy link
Contributor

mmattel commented Jun 10, 2020

Please file a doc issue, because this is a change in the occ command set.
(Yes, draft, but easy to be overseen 😄 )

@phil-davis
Copy link
Contributor Author

Doc issue owncloud/docs#2674

@jvillafanez
Copy link
Member

I think it's more consistent to rely only on the "importable-format" flag. I mean, I don't mind whether the information is shown in a table or in json, but it could be weird if things change just because a different format is used.
While the "importable + json" is expected to be used, "importable + table" should show the same information to keep consistency.

@phil-davis
Copy link
Contributor Author

"importable + text table" does show the storage class and technical authentication_type string. So that is consistent.

The "text table" format does not lend itself to displaying an array! So for that, I do not display the special case "All" text when there are no applicable users/groups. But when there are applicable users/groups I display them as text in the same way that they are currently displayed (comma-separated string)

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

Successfully merging this pull request may close these issues.

files_external:export and import of local mounts
3 participants