Skip to content

fix(gcs): preserve encoded names and preconditions - #97

Open
aniketshukla1 wants to merge 3 commits into
floci-io:mainfrom
aniketshukla1:fix/gcs-encoded-names-preconditions
Open

fix(gcs): preserve encoded names and preconditions#97
aniketshukla1 wants to merge 3 commits into
floci-io:mainfrom
aniketshukla1:fix/gcs-encoded-names-preconditions

Conversation

@aniketshukla1

@aniketshukla1 aniketshukla1 commented Jul 28, 2026

Copy link
Copy Markdown

Summary

  • Preserve literal URI-encoded GCS object names by avoiding a second decode of JAX-RS path parameters.
  • Apply generation and metageneration preconditions inside a shared per-object lock for uploads, object updates, and deletes.
  • Release idle lock entries and allocate unique object generations under concurrent writes.
  • Add regression coverage for encoded object names, conditional put/patch/delete races, and lock cleanup.

Related issue: no dedicated open issue; this fixes a GCS compatibility regression identified during review.

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

GCP Compatibility

JAX-RS path parameters are already decoded once. Decoding them again turns a literal encoded sequence such as %2F into a path separator. This change preserves the object name consistently across JSON metadata, JSON download, and XML download endpoints. It also makes object preconditions atomic with the mutation they guard.

Checklist

  • ./mvnw test passes locally on JDK 25
  • New or updated automated tests added
  • Commit messages follow Conventional Commits

Comment thread src/main/java/io/floci/gcp/services/gcs/GcsService.java Outdated
Comment thread src/main/java/io/floci/gcp/services/gcs/GcsService.java Outdated
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown

Greptile Summary

Fixes GCS object mutation races and lock lifecycle for conditional writes.

  • Move generation/metageneration precondition checks under a shared per-object lock for put, patch, and delete.
  • Release idle per-object lock map entries via holder counting; allocate unique object generations under concurrent writes.
  • Wire delete/patch controllers to pass preconditions into the service; add regression tests for encoded names, atomic conditionals, and lock cleanup.

Confidence Score: 5/5

Safe to merge; prior conditional-mutation lock and unbounded lock-map issues are addressed in the current code.

put, patch, and delete (including version delete) take the same per-object lock around precondition checks and mutations, and objectLocks entries are removed when the holder count reaches zero, with tests covering races and cleanup.

Important Files Changed

Filename Overview
src/main/java/io/floci/gcp/services/gcs/GcsService.java Shared withObjectLock now wraps put/patch/delete (and version delete); locks are released when holders hit zero; generation IDs use an AtomicLong.
src/main/java/io/floci/gcp/services/gcs/GcsObjectController.java Patch and delete endpoints pass ObjectPreconditions into the service instead of checking outside the mutation.
src/main/java/io/floci/gcp/services/gcs/GcsUploadController.java Media/multipart uploads pass preconditions into putObject; resumable start still checks at session start only.
src/test/java/io/floci/gcp/services/gcs/GcsServiceTest.java Concurrent put/patch/delete precondition races and lock-churn cleanup coverage.
src/test/java/io/floci/gcp/services/gcs/GcsObjectControllerTest.java HTTP regression that literal encoded object names round-trip on JSON and download paths.

Reviews (4): Last reviewed commit: "fix(gcs): resolve main branch conflicts" | Re-trigger Greptile

@aniketshukla1
aniketshukla1 force-pushed the fix/gcs-encoded-names-preconditions branch from 56b8c49 to ae4ebc8 Compare July 28, 2026 14:36
…dispatch

GcsBatchController.rewriteToLocalhost fed getRawPath() into the multi-argument
URI constructor, which treats its path argument as decoded and re-encodes the
'%' — turning an already-encoded %2F into %252F. Batch sub-request URLs are
absolute, so object names with reserved characters (e.g. the slash in
"batch/object1.txt") arrived double-encoded and 404'd once the redundant second
decode was removed from the object controller.

Build the rewritten URI from the raw path via URI.create (as the method's own
fallback already did), preserving percent-encoded segments. Fixes GcsBatchTest.
@aniketshukla1

Copy link
Copy Markdown
Author

@hectorvent please review again, fixed the failing issue.
Thanks

@hectorvent hectorvent added bug Something isn't working storage Cloud Storage (GCS) waiting-contributor labels Aug 1, 2026
@hectorvent

Copy link
Copy Markdown
Contributor

Thanks @aniketshukla1,
please resolve conflicts when you have a chance.

@aniketshukla1

Copy link
Copy Markdown
Author

@hectorvent done. Please check. Thanks.

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

Labels

bug Something isn't working storage Cloud Storage (GCS) waiting-contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants