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 children bucket creation #233

Merged
merged 3 commits into from
Dec 12, 2023
Merged

Conversation

jujaga
Copy link
Member

@jujaga jujaga commented Dec 9, 2023

Description

This PR focuses on implementing the ability to create children buckets without necessarily knowing the raw S3 bucket credentials.

  • 83f2ace Resolve all max-len lint warnings
  • c4d715b Update OpenAPI spec with createBucketChild and add unit tests
  • 1e7c498 Implement createBucketChild endpoint

SHOWCASE-3399

Types of changes

New feature (non-breaking change which adds functionality)
Documentation (non-breaking change with enhancements to documentation)

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

As this PR also does a complete codebase update to remove the max-len lint warnings, there are many inconsequential differences in the changelog. The core changes of this PR can be found in the following files:

  • app/src/controllers/bucket.js
  • app/src/docs/v1.api-spec.yaml
  • app/src/routes/v1/bucket.js
  • app/src/validators/bucket.js
  • app/tests/unit/controllers/bucket.spec.js
  • app/tests/unit/validators/bucket.spec.js

@jujaga jujaga added documentation Improvements or additions to documentation enhancement New feature or request labels Dec 9, 2023
@jujaga jujaga self-assigned this Dec 9, 2023
app/src/routes/v1/bucket.js Show resolved Hide resolved
app/src/controllers/object.js Show resolved Hide resolved
app/src/controllers/object.js Show resolved Hide resolved
app/src/routes/v1/bucket.js Show resolved Hide resolved
app/src/controllers/bucket.js Show resolved Hide resolved
Copy link

codeclimate bot commented Dec 9, 2023

Code Climate has analyzed commit 496d385 and detected 10 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3
Duplication 6
Bug Risk 1

The test coverage on the diff in this pull request is 62.2% (50% is the threshold).

This pull request will bring the total coverage in the repository to 65.7% (0.1% change).

View more on Code Climate.

Copy link

github-actions bot commented Dec 9, 2023

Coverage Report

Totals Coverage
Statements: 59.18% ( 2731 / 4615 )
Methods: 49.13% ( 309 / 629 )
Lines: 65.75% ( 1641 / 2496 )
Branches: 52.42% ( 781 / 1490 )

@jujaga jujaga force-pushed the feature/createBucketChild branch from 83f2ace to 2b1861e Compare December 11, 2023 20:14
@@ -703,7 +741,8 @@ const controller = {

// Download via service proxy
if (req.query.download && req.query.download === DownloadMode.PROXY) {
// TODO: Consider if we need a HEAD operation first before doing the actual read on large files for pre-flight caching behavior?
// TODO: Consider if we need a HEAD operation first before doing the actual read on large files
Copy link

Choose a reason for hiding this comment

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

TODO found

Copy link
Contributor

@TimCsaky TimCsaky left a comment

Choose a reason for hiding this comment

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

when i call the endpoint with body:

{"bucketName":"abc"}

i get the 409 conflict response:

{"type":"https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/409","title":"Conflict","status":409,"bucketId":"5078dd24-c6a6-43bd-9205-42393d9c7eb4","detail":"Requested bucket already exists","instance":"/api/v1/bucket/5078dd24-c6a6-43bd-9205-42393d9c7eb4/child","key":"coms/tim"}

maybe you just need to put 'required()` on the joi schema for subKey

Also ensure that Joi.trim() is properly enforced with Joi.strict()

Signed-off-by: Jeremy Ho <jujaga@gmail.com>
Signed-off-by: Jeremy Ho <jujaga@gmail.com>
Signed-off-by: Jeremy Ho <jujaga@gmail.com>
@jujaga jujaga force-pushed the feature/createBucketChild branch from 2b1861e to 496d385 Compare December 12, 2023 18:38
@TimCsaky TimCsaky merged commit 4904784 into master Dec 12, 2023
15 of 16 checks passed
@TimCsaky TimCsaky deleted the feature/createBucketChild branch December 12, 2023 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants