Refactor dav: Adopt two-layer architecture to match other storage providers#98
Merged
Conversation
cf-foundation-community-automation
Bot
moved this to Inbox
in Foundational Infrastructure Working Group
…oviders Restructure the DAV client to align with S3, Azure, GCS, and AliOSS provider patterns in storage-cli. This is the first step in modernizing the DAV implementation, with advanced features to follow in subsequent PRs. **Structural Changes:** - Split client into two layers: - `client.go`: High-level DavBlobstore implementing storage.Storager - `storage_client.go`: Low-level StorageClient for HTTP/WebDAV operations - Remove cmd/ and app/ layers (now integrated directly via storage/factory.go) - Remove test_assets/ directory (replaced by Docker-based testing) - Remove old fakes/fake_client.go (replaced by clientfakes/fake_storage_client.go) - Add helpers.go for shared utilities (getCertPool) **Stub Methods (to be implemented in follow-up PRs):** - List: List blobs with optional prefix filter - Copy: Server-side blob copying via WebDAV COPY - DeleteRecursive: Delete all blobs matching prefix - Properties: Retrieve blob metadata - EnsureStorageExists: Initialize WebDAV directory structure **Path Handling:** - Remove automatic 2-character path fan-out. Previously "abcdef..." was stored as "ab/cd/abcdef...". Now object IDs are passed through unchanged. - This aligns with other storage-cli providers (S3/Azure/GCS/AliOSS) and matches existing CCNG usage (BaseClient#partitioned_key already handles partitioning before calling storage-cli). - Callers are responsible for any path layout or partitioning strategy. **Testing:** - Update tests to use mocked StorageClient with counterfeiter - Add comprehensive test coverage for Exists (found, not found, error cases) - Add docker-compose.yml for local WebDAV testing - Update README with breaking change notice and usage examples **Code Quality:** - Align logging style with other providers (lowercase, Debug for success messages) - All linter checks pass (golangci-lint, go vet, gofmt) - 100% test pass rate **Breaking Changes:** - Removed automatic path partitioning. Object IDs are used exactly as provided. - Callers must include any desired directory structure in the object ID. - See dav/README.md "Path Handling Contract" section for migration guidance.
kathap
force-pushed
the
refactor-dav-two-layer-structure
branch
from
e035a02 to
3756a9c
Compare
kathap
force-pushed
the
refactor-dav-two-layer-structure
branch
from
f57155b to
bd414ce
Compare
serdarozerr
reviewed
May 20, 2026
| return nil, err | ||
| } | ||
| func (d *DavBlobstore) Exists(dest string) (bool, error) { | ||
| slog.Debug("checking if file exists on webdav", "dest", dest) |
Contributor
There was a problem hiding this comment.
Is there a reason this log is debug but not info
Contributor
Author
There was a problem hiding this comment.
Not really. I will change it to info.
|
|
||
| blobPrefix := getBlobPrefix(blobID) | ||
| func (d *DavBlobstore) Sign(dest string, action string, expiration time.Duration) (string, error) { | ||
| slog.Debug("signing url for webdav", "dest", dest, "action", action, "expiration", expiration) |
github-project-automation
Bot
moved this from Inbox
to Pending Merge | Prioritized
in Foundational Infrastructure Working Group
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Done
in Foundational Infrastructure Working Group
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Restructure the DAV client to align with S3, Azure, GCS, and AliOSS provider patterns in storage-cli. This is the first step in
modernizing the DAV implementation, with advanced features to follow in subsequent PRs.
Structural Changes:
client.go: High-level DavBlobstore implementing storage.Storagerstorage_client.go: Low-level StorageClient for HTTP/WebDAV operationsStub Methods (to be implemented in follow-up PRs):
Path Handling:
Breaking Changes: