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

[Miniflare 3] Implement more of R2 and add tests #524

Merged
merged 15 commits into from
Mar 11, 2023
Merged

Conversation

mrbbot
Copy link
Contributor

@mrbbot mrbbot commented Feb 27, 2023

This PR brings over some of the improvements from #470, and adds tests for the R2 implementation.

Specifically we now: (see commit descriptions for more details)

  • Validate requests from workerd using zod to ensure type safety in the rest of the implementation (chunky)
  • Support deleting multiple keys in R2Bucket#delete()
  • Support ranged-gets with a Range header in R2Bucket#get()
  • Return range information from R2Bucket#head()
  • Support the R2Bucket#list() startAfter option
  • Support sha* checksums in R2Bucket#put() and return these from R2Bucket#{get,head}
  • Support conditional R2Bucket#put()
  • Support secondsGranularity option in R2Conditionals
  • Remove unpaired surrogate pair key validation
  • Make R2Object#version a hex string to match the real implementation
  • Validate customMetadata size
  • Test the R2 implantation (chunky)

Best reviewed commit-by-commit... 😅

@mrbbot mrbbot requested a review from a team February 27, 2023 20:04
@changeset-bot
Copy link

changeset-bot bot commented Feb 27, 2023

⚠️ No Changeset found

Latest commit: 66cf2ec

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mrbbot mrbbot force-pushed the tre-r2-fixes-tests branch 5 times, most recently from 0b6fa4a to c72c983 Compare February 27, 2023 22:32
packages/tre/package.json Show resolved Hide resolved
v4Code: number;
object?: R2Object;

constructor(status: number, message: string, v4Code: number) {
Copy link
Contributor

Choose a reason for hiding this comment

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

status: number should probably also be renamed to code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

packages/tre/src/plugins/r2/schemas.ts Show resolved Hide resolved
packages/tre/src/plugins/r2/gateway.ts Show resolved Hide resolved
packages/tre/src/plugins/r2/gateway.ts Show resolved Hide resolved
.github/workflows/test.yml Show resolved Hide resolved
package.json Show resolved Hide resolved
packages/tre/src/plugins/r2/validator.ts Show resolved Hide resolved
@mrbbot mrbbot requested a review from penalosa March 7, 2023 14:00
mrbbot added 12 commits March 11, 2023 16:55
We made a similar change in Miniflare 2 recently
Validate incoming R2 requests from `workerd` with `zod`, increasing
type safety. This removes some implicit `any`s from `JSON.parse`, and
allows us to use `method` as a type-level union discriminator.

Validation of types has also been removed from the `Validator` class,
as we don't need to duplicate this. `workerd` will handle validating
user input here.
Calling `head()` should return the full-range here
Note this is returning slightly different, but still correct, results
to the real implementation. This is due a restriction of Miniflare's
storage abstraction, see the comment in `list()` for more details.
We're planning to replace this very soon though, so this shouldn't be
a problem for long.
This restriction no longer seems to apply
@mrbbot mrbbot force-pushed the tre-r2-fixes-tests branch from ace5152 to 851c479 Compare March 11, 2023 16:56
@mrbbot mrbbot force-pushed the tre-r2-fixes-tests branch from 851c479 to 66cf2ec Compare March 11, 2023 16:58
@mrbbot mrbbot merged commit 6ec5ee3 into tre Mar 11, 2023
@mrbbot mrbbot deleted the tre-r2-fixes-tests branch March 11, 2023 17:01
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.

2 participants