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

Option to return fileStat info during listFiles #249

Merged
merged 9 commits into from Jul 30, 2019

Conversation

@zone117x
Copy link
Member

commented Jul 1, 2019

Goal of PR

Related to #220, #240

Provide ability to query file metadata (primarily lastModifiedDate) with the listFiles endpoint.

This may be needed by a future Collections historical file restoration tool -- pending discussion on historical naming scheme in #248:

If a [gaiahub server] timestamp cannot be used [in the historical file name] then I have a couple alternative proposals that avoid bucket-wide index file race conditions and conflict resolution ... Use a random ID in this historical file name. Future file history restoration tooling could use the cloud driver provided lastModifiedDate that is now available in the fileStat operation. This field has integration testing showing accuracy and behavioral consistency across all currently supported cloud storage providers.

This feature does not cause additional request overhead with the current cloud providers -- their list file APIs already return this metadata alongside the filenames. The only additional overhead should be the additional data returned to clients that request it.

Implementation

Extends the existing POST ${hubUrl}/list-files/${address} endpoint.
If the POST body contains a stat: true field then the returned JSON includes the fileStat metadata:

{
  entries: [
    { name: string, lastModifiedDate: number, contentLength: number },
    { name: string, lastModifiedDate: number, contentLength: number },
    ...
  ]
  page: string // optional pagination marker
}

Without stat: true the previous behavior is preserved -- the returned JSON format remains:

{
  entries: string[], // file name strings
  page: string // optional pagination marker
}

Testing

Integration tests for all supported cloud providers -- all showing behavioral consistency.

Submission Checklist

  • Based on correct branch: feature submissions should be on develop, hotfixes should be on master

  • The code passes our eslint definitions, unit tests, and
    contains correct TypeFlow annotations.

  • Submission contains tests that cover any and all new functionality or code changes.

  • Submission documents any new features or endpoints, and describes how developers
    would be expected to interact with them.

  • Author has agreed to our contributor's agreement.

zone117x added 2 commits Jul 1, 2019
@zone117x zone117x requested review from jcnelson and kantai Jul 1, 2019
@codecov

This comment has been minimized.

Copy link

commented Jul 1, 2019

Codecov Report

Merging #249 into develop will increase coverage by 0.42%.
The diff coverage is 95.41%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #249      +/-   ##
===========================================
+ Coverage    79.65%   80.08%   +0.42%     
===========================================
  Files           21       21              
  Lines         1440     1486      +46     
  Branches       263      264       +1     
===========================================
+ Hits          1147     1190      +43     
- Misses         217      221       +4     
+ Partials        76       75       -1
Impacted Files Coverage Δ
hub/src/server/utils.ts 92% <100%> (+0.33%) ⬆️
hub/src/server/drivers/GcDriver.ts 92.48% <100%> (+0.54%) ⬆️
hub/src/server/http.ts 72.65% <100%> (+0.21%) ⬆️
hub/src/server/drivers/S3Driver.ts 92.96% <100%> (+2.21%) ⬆️
hub/src/server/server.ts 97.26% <50%> (-2.74%) ⬇️
hub/src/server/drivers/diskDriver.ts 85.98% <94.59%> (-0.13%) ⬇️
hub/src/server/drivers/AzDriver.ts 91.6% <95%> (-0.21%) ⬇️

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 a5dcfbd...42e4ed9. Read the comment docs.

@jcnelson

This comment has been minimized.

Copy link
Member

commented Jul 3, 2019

I'm in favor of this regardless of the eventual collections history naming scheme.

@larrysalibra

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

Agree with @jcnelson

zone117x added 2 commits Jul 23, 2019
* feature/read-file:
  Add docs for performRead to driver model definition in README
  Add docs for performRename and performStat to driver model definition in README
  Update blockstack.js dep out of beta version
  Implement decodeTokenForPayload helper function
  Update to blockstack.js beta package to fix typing collisions with bitcoinjs-lib v5
  Add tests for schema validation warnings
  Add additional config param schema descriptions, cleanup sample configs
  Config schema validation that logs console warnings
@zone117x zone117x marked this pull request as ready for review Jul 23, 2019
@moxiegirl moxiegirl referenced this pull request Jul 29, 2019
* develop:
  Re-order the mutex registration statement to prevent syncronous reentrant locking issues
  Fix race condition from accidental promise yielding in mutex implementation
  Grammar update on http path mutex
  Use single `topLevelStorage` arg in file rename
  Update driver model README with a notice about path mutex.
  Simplify mutx with Set<string>
  Implement mutex around http endpoints for file mutations

# Conflicts:
#	hub/src/server/drivers/GcDriver.ts
#	hub/src/server/drivers/diskDriver.ts
@zone117x zone117x changed the base branch from feature/read-file to develop Jul 30, 2019
zone117x added 3 commits Jul 30, 2019
@kantai
kantai approved these changes Jul 30, 2019
Copy link
Member

left a comment

Looks good to me, thanks @zone117x !

Copy link
Member

left a comment

LGTM!

@zone117x zone117x merged commit 5d4173f into develop Jul 30, 2019
3 checks passed
3 checks passed
ci/circleci Your tests passed on CircleCI!
Details
codecov/project 80.08% (+0.42%) compared to a5dcfbd
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.