From d9c26671f486a203867dda104ea8ea94c6e4105b Mon Sep 17 00:00:00 2001 From: Matteo Date: Fri, 14 Aug 2026 09:23:30 +0200 Subject: [PATCH 1/3] refactor(app): establish domain boundaries - isolate admin, archive, share, and sync policy behind narrow ports - add compiler-enforced dependency guards and merged app coverage - verify non-empty Space archive downloads in the live integration suite - split authentication dispatch into focused operation handlers --- ARCHITECTURE.md | 50 ++- CONTRIBUTING.md | 9 +- .../group.go} | 56 ++-- internal/app/admin/helpers.go | 123 ++++++++ internal/app/admin/helpers_test.go | 73 +++++ .../{admin_role_service.go => admin/role.go} | 47 +-- .../{admin_service.go => admin/service.go} | 121 ++++--- internal/app/admin/types.go | 129 ++++++++ .../user.go} | 74 ++--- internal/app/admin_adapter.go | 70 +++++ internal/app/admin_guard.go | 140 +-------- internal/app/admin_mutation_service_test.go | 70 ----- internal/app/admin_service_test.go | 9 +- internal/app/api.go | 294 +++++------------- internal/app/architecture_test.go | 61 ++++ .../service.go} | 101 ++++-- internal/app/archive/service_test.go | 130 ++++++++ internal/app/archive/test_helpers_test.go | 7 + internal/app/archive_api.go | 76 ++++- internal/app/auth_service.go | 269 +++++++--------- internal/app/doctor.go | 5 +- .../overview.go} | 50 +-- internal/app/share/recipient.go | 126 ++++++++ .../{share_service.go => share/service.go} | 182 +++++------ internal/app/share/service_test.go | 112 +++++++ internal/app/share/types.go | 201 ++++++++++++ internal/app/share_adapter.go | 50 +++ internal/app/space_recipient.go | 24 -- .../bidirectional.go} | 40 +-- .../conflict.go} | 6 +- internal/app/sync/helpers.go | 22 ++ .../app/{sync_job_service.go => sync/job.go} | 64 ++-- .../recovery.go} | 54 ++-- .../app/{sync_service.go => sync/service.go} | 98 +++--- internal/app/sync/service_test.go | 65 ++++ .../{sync_state_service.go => sync/state.go} | 38 +-- internal/app/sync/types.go | 134 ++++++++ internal/app/sync_adapter.go | 50 +++ internal/app/sync_job_service_test.go | 14 +- internal/app/sync_service_test.go | 46 --- internal/app/sync_state_service_test.go | 17 +- test/integration/integration_test.go | 78 +++++ tools/covercheck/main.go | 61 ++++ tools/covercheck/main_test.go | 18 ++ 44 files changed, 2333 insertions(+), 1131 deletions(-) rename internal/app/{admin_group_mutation_service.go => admin/group.go} (80%) create mode 100644 internal/app/admin/helpers.go create mode 100644 internal/app/admin/helpers_test.go rename internal/app/{admin_role_service.go => admin/role.go} (90%) rename internal/app/{admin_service.go => admin/service.go} (79%) create mode 100644 internal/app/admin/types.go rename internal/app/{admin_user_mutation_service.go => admin/user.go} (82%) create mode 100644 internal/app/admin_adapter.go create mode 100644 internal/app/architecture_test.go rename internal/app/{archive_service.go => archive/service.go} (83%) create mode 100644 internal/app/archive/service_test.go create mode 100644 internal/app/archive/test_helpers_test.go rename internal/app/{share_overview_service.go => share/overview.go} (80%) create mode 100644 internal/app/share/recipient.go rename internal/app/{share_service.go => share/service.go} (84%) create mode 100644 internal/app/share/service_test.go create mode 100644 internal/app/share/types.go create mode 100644 internal/app/share_adapter.go rename internal/app/{bidirectional_sync_service.go => sync/bidirectional.go} (90%) rename internal/app/{sync_conflict_service.go => sync/conflict.go} (98%) create mode 100644 internal/app/sync/helpers.go rename internal/app/{sync_job_service.go => sync/job.go} (86%) rename internal/app/{sync_recovery_service.go => sync/recovery.go} (86%) rename internal/app/{sync_service.go => sync/service.go} (92%) create mode 100644 internal/app/sync/service_test.go rename internal/app/{sync_state_service.go => sync/state.go} (92%) create mode 100644 internal/app/sync/types.go create mode 100644 internal/app/sync_adapter.go create mode 100644 tools/covercheck/main_test.go diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 2ea6838..cf8aa10 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -11,7 +11,11 @@ cmd/ internal/ activities/ authenticated oCIS Graph activity-history client command/ Cobra command tree and input validation - app/ application use-case orchestration + app/ public application facade and runtime composition + admin/ account and global Space administration policy + archive/ archive-download application policy + share/ direct, received, and public-link share policy + sync/ sync execution, state, jobs, and recovery policy apperror/ stable error categories and exit-code mapping archiver/ authenticated archive-download protocol client auth/ OIDC protocol implementation @@ -40,10 +44,16 @@ test/ ## Dependency direction -`cmd/ocis` depends on `internal/command`, which depends on `internal/app`. -The application layer may depend on focused infrastructure packages under `internal`, -but application and infrastructure packages never depend on Cobra or the -command package. +`cmd/ocis` depends on `internal/command`, which depends on the public +`internal/app` facade. Large application domains live in subpackages such as +`internal/app/admin`, `internal/app/archive`, `internal/app/share`, and +`internal/app/sync`. The facade composes runtime +clients into their narrow ports and preserves the public request and result +types used by Cobra. Domain subpackages never import the parent `internal/app` +package, which makes the dependency boundary compiler-enforced and prevents +them from reaching unrelated package-level helpers. The application layer may +depend on focused infrastructure packages under `internal`, but application +and infrastructure packages never depend on Cobra or the command package. The executable entrypoint is intentionally small. It translates an application error to a message and non-zero exit code; business behavior remains testable @@ -55,19 +65,32 @@ without starting a subprocess. map errors to exit codes. - `internal/command`: define Cobra commands, flags, aliases, help, completion, and syntactic validation. -- `internal/app`: expose typed use-case requests, select profiles, and coordinate - authentication and protocol operations. Focused services such as - `bidirectional_sync_service.go`, `config_service.go`, - `batch_service.go`, `filesystem_service.go`, `filesystem_tree_service.go`, +- `internal/app`: expose the compatibility facade used by Cobra, select + profiles, compose authenticated runtime clients, classify errors, and adapt + narrow domain ports. Focused services such as + `config_service.go`, `batch_service.go`, `filesystem_service.go`, `filesystem_tree_service.go`, `filesystem_du_service.go`, `filesystem_touch_service.go`, `filesystem_walk.go`, `metadata_service.go`, `activity_service.go`, `event_service.go`, `notification_service.go`, - `share_overview_service.go`, `space_member_service.go`, `space_update_service.go`, - `space_lifecycle_service.go`, and - the split `admin_*_service.go` files keep each use case independent; + `space_lifecycle_service.go` keep the remaining use cases independent; `admin_guard.go` owns account-admin and MFA preflights, while `runtime.go` contains shared application wiring. +- `internal/app/archive`: own archive selection, recursive preflight, limits, + output, and safe local installation through a narrow client factory. It + cannot access unrelated authentication, administration, sync, or sharing + helpers in the parent package. +- `internal/app/admin`: own account inventory and mutation, advertised role + assignment, MFA-gated administration policy, and global Space inventory + through narrow Graph and OCS capability ports. +- `internal/app/share`: own direct, federated, received, overview, and + public-link application policy through a narrow authenticated client port. + It cannot access unrelated archive, administration, sync, or configuration + helpers in the parent package. +- `internal/app/sync`: own one-way and bidirectional execution, conflict + policy, named jobs, local state, and interrupted-run recovery through narrow + WebDAV and persistence ports. It cannot access authentication secrets, + configuration storage, administration, or sharing policy in the parent. - `internal/apperror`: classify usage, authentication, not-found, and conflict errors without coupling application services to Cobra. - `internal/archiver`: validate same-origin server-advertised archive endpoints, @@ -169,6 +192,9 @@ Fast package tests remain Docker-independent. ## Design rules - Dependencies point inward toward use cases. +- New large application domains belong in `internal/app/` with a + narrow client or repository port. Do not grow the parent package when a use + case can be isolated without creating an import cycle. - Configuration I/O is isolated and tested. - Destructive commands fail closed. - Destructive Space operations require explicit intent in both the Cobra and diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c6d9e69..c4f1d25 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,7 +25,10 @@ debugging targets. ## Design rules - Keep Cobra code in `internal/command` thin. -- Put use-case orchestration in `internal/app`. +- Put small cross-domain orchestration in the `internal/app` facade. Put large + domain policy in `internal/app/` behind narrow injected ports; domain + packages must not import the parent `internal/app` package. Current domain + boundaries are `admin`, `archive`, `share`, and `sync`. - Keep authentication and WebDAV protocol details out of commands. - Pass contexts, dependencies, and output streams explicitly. - Add tests at the narrowest package boundary. @@ -34,8 +37,8 @@ debugging targets. - Format with `gofmt`. - Run `make check`, including per-package coverage gates, golangci-lint v2.12.2, and `gosec`. -- Keep `app`, `auth`, `graph`, `httpapi`, `sharing`, `transfer`, and `webdav` - at or above 75% statement coverage. +- Keep the complete `app/...` tree, plus `auth`, `graph`, `httpapi`, `sharing`, + `transfer`, and `webdav`, at or above 75% statement coverage. - Every `//nolint` directive must name the linter and explain why suppression is safe using a second `//`, for example `//nolint:gosec // reason`. - Write lowercase, contextual errors without trailing punctuation. diff --git a/internal/app/admin_group_mutation_service.go b/internal/app/admin/group.go similarity index 80% rename from internal/app/admin_group_mutation_service.go rename to internal/app/admin/group.go index e0a0932..d4bda3e 100644 --- a/internal/app/admin_group_mutation_service.go +++ b/internal/app/admin/group.go @@ -1,4 +1,4 @@ -package app +package admin import ( "context" @@ -9,11 +9,11 @@ import ( "github.com/mzner/ocis-cli/internal/graph" ) -func runAdminGroupCreate( +func RunGroupCreate( ctx context.Context, - request AdminGroupCreateRequest, + request GroupCreateRequest, selectedProfile string, - options RunOptions, + options Options, ) error { request.Name = strings.TrimSpace(request.Name) if request.Name == "" { @@ -22,10 +22,13 @@ func runAdminGroupCreate( fmt.Errorf("group name is required"), ) } - selected, err := newAdminMutationClient(ctx, selectedProfile, options) + selected, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } + if err := options.RequireAccountAdmin(ctx, selected); err != nil { + return err + } if request.DryRun { return output( options, "admin-group-change", @@ -35,7 +38,7 @@ func runAdminGroupCreate( "Would create group %s\n", request.Name, ) } - created, err := selected.graphClient().CreateGroup( + created, err := selected.Graph().CreateGroup( ctx, graph.CreateGroupRequest{DisplayName: request.Name}, ) if err != nil { @@ -47,11 +50,11 @@ func runAdminGroupCreate( ) } -func runAdminGroupUpdate( +func RunGroupUpdate( ctx context.Context, - request AdminGroupUpdateRequest, + request GroupUpdateRequest, selectedProfile string, - options RunOptions, + options Options, ) error { request.Identifier = strings.TrimSpace(request.Identifier) request.Name = strings.TrimSpace(request.Name) @@ -61,10 +64,13 @@ func runAdminGroupUpdate( fmt.Errorf("group identifier and --name are required"), ) } - selected, err := newAdminMutationClient(ctx, selectedProfile, options) + selected, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } + if err := options.RequireAccountAdmin(ctx, selected); err != nil { + return err + } group, err := resolveMutationGroup(ctx, selected, request.Identifier) if err != nil { return err @@ -85,7 +91,7 @@ func runAdminGroupUpdate( ) } name := request.Name - if err := selected.graphClient().UpdateGroup( + if err := selected.Graph().UpdateGroup( ctx, group.ID, graph.UpdateGroupRequest{DisplayName: &name}, ); err != nil { return adminMutationError("group", err) @@ -102,11 +108,11 @@ func runAdminGroupUpdate( ) } -func runAdminGroupDelete( +func RunGroupDelete( ctx context.Context, - request AdminGroupDeleteRequest, + request GroupDeleteRequest, selectedProfile string, - options RunOptions, + options Options, ) error { request.Identifier = strings.TrimSpace(request.Identifier) if request.Identifier == "" { @@ -115,10 +121,13 @@ func runAdminGroupDelete( fmt.Errorf("group identifier is required"), ) } - selected, err := newAdminMutationClient(ctx, selectedProfile, options) + selected, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } + if err := options.RequireAccountAdmin(ctx, selected); err != nil { + return err + } group, err := resolveMutationGroup(ctx, selected, request.Identifier) if err != nil { return err @@ -137,7 +146,7 @@ func runAdminGroupDelete( group.DisplayName, group.ID, ) } - if err := selected.graphClient().DeleteGroup(ctx, group.ID); err != nil { + if err := selected.Graph().DeleteGroup(ctx, group.ID); err != nil { return adminMutationError("group", err) } return output( @@ -150,11 +159,11 @@ func runAdminGroupDelete( ) } -func runAdminGroupMemberMutation( +func RunGroupMemberMutation( ctx context.Context, - request AdminGroupMemberMutationRequest, + request GroupMemberMutationRequest, selectedProfile string, - options RunOptions, + options Options, ) error { request.Group = strings.TrimSpace(request.Group) request.User = strings.TrimSpace(request.User) @@ -164,10 +173,13 @@ func runAdminGroupMemberMutation( fmt.Errorf("group and user identifiers are required"), ) } - selected, err := newAdminMutationClient(ctx, selectedProfile, options) + selected, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } + if err := options.RequireAccountAdmin(ctx, selected); err != nil { + return err + } group, err := resolveMutationGroup(ctx, selected, request.Group) if err != nil { return err @@ -198,9 +210,9 @@ func runAdminGroupMemberMutation( ) } if request.Remove { - err = selected.graphClient().RemoveGroupMember(ctx, group.ID, user.ID) + err = selected.Graph().RemoveGroupMember(ctx, group.ID, user.ID) } else { - err = selected.graphClient().AddGroupMember(ctx, group.ID, user.ID) + err = selected.Graph().AddGroupMember(ctx, group.ID, user.ID) } if err != nil { return adminMutationError("group membership", err) diff --git a/internal/app/admin/helpers.go b/internal/app/admin/helpers.go new file mode 100644 index 0000000..29e094c --- /dev/null +++ b/internal/app/admin/helpers.go @@ -0,0 +1,123 @@ +package admin + +import ( + "context" + "errors" + "fmt" + "net/http" + "strings" + + "github.com/mzner/ocis-cli/internal/apperror" + "github.com/mzner/ocis-cli/internal/graph" + appoutput "github.com/mzner/ocis-cli/internal/output" + "github.com/mzner/ocis-cli/internal/sharing" +) + +func output(options Options, kind string, value any, format string, args ...any) error { + return (appoutput.Renderer{Writer: options.Out, Mode: options.OutputMode, Type: kind}).Write(value, format, args...) +} + +func writeOutput(options Options, kind string, value any) error { + return output(options, kind, value, "") +} + +func fallback(values ...string) string { + for _, value := range values { + if strings.TrimSpace(value) != "" { + return value + } + } + return "" +} + +func protocolStatus(err error) int { + var statusErr interface{ HTTPStatusCode() int } + if errors.As(err, &statusErr) { + return statusErr.HTTPStatusCode() + } + return 0 +} + +func adminCapabilities(ctx context.Context, selected Client, options Options) (sharing.Capabilities, bool) { + capabilities, err := selected.Sharing().Capabilities(ctx) + if err != nil { + options.Logger.Debug("could not read optional administration capabilities", "error", err) + return sharing.Capabilities{}, false + } + return capabilities, true +} + +func RequireWritableUserFields(capabilities sharing.Capabilities, fields ...string) error { + for _, field := range fields { + for _, readOnly := range capabilities.Graph.Users.ReadOnlyAttributes { + if strings.EqualFold(strings.TrimSpace(readOnly), field) { + return fmt.Errorf("server advertises %s as read-only", field) + } + } + } + return nil +} + +func requireWritableUserFields(capabilities sharing.Capabilities, fields ...string) error { + return RequireWritableUserFields(capabilities, fields...) +} + +func resolveMutationUser(ctx context.Context, selected Client, identifier string) (graph.DirectoryUser, error) { + user, err := selected.Graph().GetUser(ctx, identifier) + if err != nil { + return graph.DirectoryUser{}, err + } + if strings.TrimSpace(user.ID) == "" { + return graph.DirectoryUser{}, fmt.Errorf("server returned user %q without a stable ID", user.DisplayName) + } + return user, nil +} + +func resolveMutationGroup(ctx context.Context, selected Client, identifier string) (graph.DirectoryGroup, error) { + group, err := selected.Graph().GetGroup(ctx, identifier) + if err != nil { + return graph.DirectoryGroup{}, err + } + if strings.TrimSpace(group.ID) == "" { + return graph.DirectoryGroup{}, fmt.Errorf("server returned group %q without a stable ID", group.DisplayName) + } + return group, nil +} + +func RejectReadOnlyGroup(group graph.DirectoryGroup) error { + if AdminGroupAccess(group) == "read-only" { + return fmt.Errorf("group %q is managed by a read-only identity backend", group.DisplayName) + } + return nil +} + +func rejectReadOnlyGroup(group graph.DirectoryGroup) error { return RejectReadOnlyGroup(group) } + +func rejectSelfTarget(ctx context.Context, selected Client, target graph.DirectoryUser, operation string) error { + current, err := selected.Graph().GetMe(ctx) + if err != nil { + return fmt.Errorf("verify current account before %s: %w", operation, err) + } + if current.ID == target.ID { + return apperror.Wrap(apperror.KindConflict, operation, fmt.Errorf("refusing to %s the currently authenticated account %q", operation, target.Username)) + } + return nil +} + +func AdminMutationError(resource string, err error) error { + if err == nil { + return nil + } + message := strings.ToLower(err.Error()) + if strings.Contains(message, "read-only") || strings.Contains(message, "configured read only") { + return fmt.Errorf("%s is managed by a read-only identity backend", resource) + } + switch protocolStatus(err) { + case http.StatusMethodNotAllowed, http.StatusNotImplemented: + return fmt.Errorf("server does not expose the requested %s mutation", resource) + default: + return err + } +} + +func adminMutationError(resource string, err error) error { return AdminMutationError(resource, err) } diff --git a/internal/app/admin/helpers_test.go b/internal/app/admin/helpers_test.go new file mode 100644 index 0000000..952bf39 --- /dev/null +++ b/internal/app/admin/helpers_test.go @@ -0,0 +1,73 @@ +package admin + +import ( + "errors" + "net/http" + "strings" + "testing" + + "github.com/mzner/ocis-cli/internal/graph" + "github.com/mzner/ocis-cli/internal/httpapi" + "github.com/mzner/ocis-cli/internal/sharing" +) + +func TestRoleResolutionUsesAdvertisedIDsAndRejectsAmbiguity(t *testing.T) { + roles := advertisedRoles([]graph.Application{{ + ID: "app-1", DisplayName: "oCIS", + AppRoles: []graph.AppRole{ + {ID: "role-admin", DisplayName: "Admin"}, + {ID: "role-user", DisplayName: "User"}, + }, + }}) + resolved, err := ResolveAdvertisedRole(roles, "admin") + if err != nil || resolved.role.ID != "role-admin" { + t.Fatalf("resolved: %#v, %v", resolved, err) + } + roles = append(roles, advertisedRole{ + application: graph.Application{ID: "app-2"}, + role: graph.AppRole{ID: "other-admin", DisplayName: "Admin"}, + }) + if _, err := ResolveAdvertisedRole(roles, "Admin"); err == nil || + !strings.Contains(err.Error(), "ambiguous") { + t.Fatalf("ambiguity error: %v", err) + } +} + +func TestMutationGuardHelpers(t *testing.T) { + capabilities := sharing.Capabilities{} + capabilities.Graph.Users.ReadOnlyAttributes = []string{"user.mail"} + if err := RequireWritableUserFields( + capabilities, "user.displayName", "user.mail", + ); err == nil || !strings.Contains(err.Error(), "read-only") { + t.Fatalf("writable field error: %v", err) + } + if err := RejectReadOnlyGroup(graph.DirectoryGroup{ + DisplayName: "External", GroupTypes: []string{"ReadOnly"}, + }); err == nil { + t.Fatal("read-only group was accepted") + } + displayName := "Updated" + fields := SelectedUserUpdateFields(UserUpdateRequest{ + DisplayName: &displayName, SetPassword: true, + }) + if len(fields) != 2 || titleWord("delete") != "Delete" || titleWord("") != "" { + t.Fatalf("fields=%#v title=%q", fields, titleWord("delete")) + } + if err := AdminMutationError( + "group", errors.New("backend is configured read-only"), + ); err == nil || !strings.Contains(err.Error(), "read-only identity") { + t.Fatalf("read-only backend error: %v", err) + } + unsupported := &httpapi.HTTPError{ + StatusCode: http.StatusNotImplemented, Status: "501 Not Implemented", + } + if err := AdminMutationError("group", unsupported); err == nil || + !strings.Contains(err.Error(), "does not expose") { + t.Fatalf("unsupported mutation error: %v", err) + } + if err := roleServiceError(&httpapi.HTTPError{ + StatusCode: http.StatusNotFound, Status: "404 Not Found", + }); err == nil || !strings.Contains(err.Error(), "role service") { + t.Fatalf("role service error: %v", err) + } +} diff --git a/internal/app/admin_role_service.go b/internal/app/admin/role.go similarity index 90% rename from internal/app/admin_role_service.go rename to internal/app/admin/role.go index ebbd480..996d63b 100644 --- a/internal/app/admin_role_service.go +++ b/internal/app/admin/role.go @@ -1,4 +1,4 @@ -package app +package admin import ( "context" @@ -27,52 +27,55 @@ type advertisedRole struct { role graph.AppRole } -func runAdminRole( +func RunRole( ctx context.Context, - request AdminRoleRequest, + request RoleRequest, selectedProfile string, - options RunOptions, + options Options, ) error { request.User = strings.TrimSpace(request.User) request.Role = strings.TrimSpace(request.Role) - if request.Operation != AdminRoleAvailable && request.User == "" { + if request.Operation != RoleAvailable && request.User == "" { return apperror.Wrap( apperror.KindUsage, "admin user role", fmt.Errorf("user identifier is required"), ) } - if (request.Operation == AdminRoleGrant || - request.Operation == AdminRoleRevoke) && request.Role == "" { + if (request.Operation == RoleGrant || + request.Operation == RoleRevoke) && request.Role == "" { return apperror.Wrap( apperror.KindUsage, "admin user role", fmt.Errorf("role name, role ID, or assignment ID is required"), ) } - selected, err := newAdminMutationClient(ctx, selectedProfile, options) + selected, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } - applications, err := selected.graphClient().ListApplications(ctx) + if err := options.RequireAccountAdmin(ctx, selected); err != nil { + return err + } + applications, err := selected.Graph().ListApplications(ctx) if err != nil { return roleServiceError(err) } roles := advertisedRoles(applications) - if request.Operation == AdminRoleAvailable { + if request.Operation == RoleAvailable { return listAvailableAdminRoles(roles, options) } user, err := resolveMutationUser(ctx, selected, request.User) if err != nil { return err } - assignments, err := selected.graphClient().ListAppRoleAssignments(ctx, user.ID) + assignments, err := selected.Graph().ListAppRoleAssignments(ctx, user.ID) if err != nil { return roleServiceError(err) } switch request.Operation { - case AdminRoleList: + case RoleList: return listAdminRoles(assignments, roles, options) - case AdminRoleGrant: - role, err := resolveAdvertisedRole(roles, request.Role) + case RoleGrant: + role, err := ResolveAdvertisedRole(roles, request.Role) if err != nil { return err } @@ -90,7 +93,7 @@ func runAdminRole( fallback(user.Username, user.DisplayName), user.ID, ) } - created, err := selected.graphClient().AssignAppRole( + created, err := selected.Graph().AssignAppRole( ctx, graph.AppRoleAssignment{ AppRoleID: role.role.ID, PrincipalID: user.ID, ResourceID: role.application.ID, @@ -105,11 +108,11 @@ func runAdminRole( role.role.DisplayName, role.role.ID, fallback(user.Username, user.DisplayName), user.ID, ) - case AdminRoleRevoke: + case RoleRevoke: if err := rejectSelfTarget(ctx, selected, user, "revoke roles from"); err != nil { return err } - assignment, roleName, err := resolveRoleAssignment( + assignment, roleName, err := ResolveRoleAssignment( assignments, roles, request.Role, ) if err != nil { @@ -130,7 +133,7 @@ func runAdminRole( fallback(user.Username, user.DisplayName), user.ID, ) } - if err := selected.graphClient().RemoveAppRoleAssignment( + if err := selected.Graph().RemoveAppRoleAssignment( ctx, user.ID, assignment.ID, ); err != nil { return roleServiceError(err) @@ -168,7 +171,7 @@ func advertisedRoles(applications []graph.Application) []advertisedRole { return roles } -func resolveAdvertisedRole( +func ResolveAdvertisedRole( roles []advertisedRole, selector string, ) (advertisedRole, error) { var matches []advertisedRole @@ -193,7 +196,7 @@ func resolveAdvertisedRole( ) } -func resolveRoleAssignment( +func ResolveRoleAssignment( assignments []graph.AppRoleAssignment, roles []advertisedRole, selector string, @@ -223,7 +226,7 @@ func resolveRoleAssignment( func listAdminRoles( assignments []graph.AppRoleAssignment, roles []advertisedRole, - options RunOptions, + options Options, ) error { rows := make([]adminRoleAssignment, 0, len(assignments)) for _, assignment := range assignments { @@ -270,7 +273,7 @@ func listAdminRoles( func listAvailableAdminRoles( roles []advertisedRole, - options RunOptions, + options Options, ) error { type roleRow struct { Role string `json:"role"` diff --git a/internal/app/admin_service.go b/internal/app/admin/service.go similarity index 79% rename from internal/app/admin_service.go rename to internal/app/admin/service.go index 6fcac5d..456559b 100644 --- a/internal/app/admin_service.go +++ b/internal/app/admin/service.go @@ -1,4 +1,4 @@ -package app +package admin import ( "context" @@ -15,11 +15,11 @@ import ( appoutput "github.com/mzner/ocis-cli/internal/output" ) -func runAdmin( +func Run( ctx context.Context, - request AdminRequest, + request Request, selectedProfile string, - options RunOptions, + options Options, ) error { if options.Space != "" { return apperror.Wrap( @@ -32,45 +32,45 @@ func runAdmin( if err := validateAdminRequest(request); err != nil { return apperror.Wrap(apperror.KindUsage, "admin", err) } - client, err := newClientWithOptions(ctx, selectedProfile, options) + client, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } switch request.Operation { - case AdminUserList: - if err := requireAccountAdminMFA(ctx, client); err != nil { + case UserList: + if err := options.RequireAccountAdmin(ctx, client); err != nil { return err } return listAdminUsers( ctx, client, adminDirectorySearch(request), options, ) - case AdminUserInfo: - if err := requireAccountAdminMFA(ctx, client); err != nil { + case UserInfo: + if err := options.RequireAccountAdmin(ctx, client); err != nil { return err } return showAdminUser(ctx, client, request.Identifier, options) - case AdminGroupList: - if err := requireAccountAdminMFA(ctx, client); err != nil { + case GroupList: + if err := options.RequireAccountAdmin(ctx, client); err != nil { return err } return listAdminGroups( ctx, client, adminDirectorySearch(request), options, ) - case AdminGroupInfo: - if err := requireAccountAdminMFA(ctx, client); err != nil { + case GroupInfo: + if err := options.RequireAccountAdmin(ctx, client); err != nil { return err } return showAdminGroup(ctx, client, request.Identifier, options) - case AdminGroupMemberList: - if err := requireAccountAdminMFA(ctx, client); err != nil { + case GroupMemberList: + if err := options.RequireAccountAdmin(ctx, client); err != nil { return err } return listAdminGroupMembers( ctx, client, request.Identifier, options, ) - case AdminSpaceList: + case SpaceList: return listAdminSpaces(ctx, client, options) - case AdminSpaceInfo: + case SpaceInfo: return showAdminSpace(ctx, client, request.Identifier, options) default: return apperror.Wrap( @@ -80,7 +80,7 @@ func runAdmin( } } -func validateAdminRequest(request AdminRequest) error { +func validateAdminRequest(request Request) error { hasSearch := strings.TrimSpace(request.Search) != "" hasRawSearch := strings.TrimSpace(request.RawSearch) != "" if hasSearch && hasRawSearch { @@ -91,17 +91,17 @@ func validateAdminRequest(request AdminRequest) error { `--search cannot contain a double quote; use --search-raw for an exact LibreGraph expression`, ) } - if request.Operation != AdminUserList && - request.Operation != AdminGroupList && + if request.Operation != UserList && + request.Operation != GroupList && (hasSearch || hasRawSearch) { return errors.New("search is only supported for user and group lists") } switch request.Operation { - case AdminUserList, AdminGroupList: + case UserList, GroupList: return nil - case AdminSpaceList: + case SpaceList: return nil - case AdminUserInfo, AdminGroupInfo, AdminGroupMemberList, AdminSpaceInfo: + case UserInfo, GroupInfo, GroupMemberList, SpaceInfo: if strings.TrimSpace(request.Identifier) == "" { return errors.New("identifier is required") } @@ -111,7 +111,7 @@ func validateAdminRequest(request AdminRequest) error { } } -func adminDirectorySearch(request AdminRequest) graph.DirectorySearch { +func adminDirectorySearch(request Request) graph.DirectorySearch { if strings.TrimSpace(request.RawSearch) != "" { return graph.DirectorySearch{ Value: request.RawSearch, @@ -126,11 +126,11 @@ func adminDirectorySearch(request AdminRequest) graph.DirectorySearch { func listAdminUsers( ctx context.Context, - client *client, + client Client, search graph.DirectorySearch, - options RunOptions, + options Options, ) error { - users, err := client.graphClient().ListUsers(ctx, search) + users, err := client.Graph().ListUsers(ctx, search) if err != nil { return unavailableAdminList("user inventory", err) } @@ -154,7 +154,7 @@ func listAdminUsers( for _, user := range users { if _, err := fmt.Fprintf( writer, "%s\t%s\t%s\t%s\t%s\n", - adminUserStatus(user.AccountEnabled), + AdminUserStatus(user.AccountEnabled), user.Username, user.DisplayName, user.Mail, user.ID, ); err != nil { return err @@ -166,11 +166,11 @@ func listAdminUsers( func showAdminUser( ctx context.Context, - client *client, + client Client, identifier string, - options RunOptions, + options Options, ) error { - user, err := client.graphClient().GetUser(ctx, identifier) + user, err := client.Graph().GetUser(ctx, identifier) if err != nil { return err } @@ -186,7 +186,7 @@ func writeAdminUser(writer io.Writer, user graph.DirectoryUser) error { {"Username", user.Username}, {"Display name", user.DisplayName}, {"Email", user.Mail}, - {"Account", adminUserStatus(user.AccountEnabled)}, + {"Account", AdminUserStatus(user.AccountEnabled)}, {"Type", user.UserType}, {"Given name", user.GivenName}, {"Surname", user.Surname}, @@ -205,7 +205,7 @@ func writeAdminUser(writer io.Writer, user graph.DirectoryUser) error { return nil } -func adminUserStatus(enabled *bool) string { +func AdminUserStatus(enabled *bool) string { switch { case enabled == nil: return "unknown" @@ -218,11 +218,11 @@ func adminUserStatus(enabled *bool) string { func listAdminGroups( ctx context.Context, - client *client, + client Client, search graph.DirectorySearch, - options RunOptions, + options Options, ) error { - groups, err := client.graphClient().ListGroups(ctx, search) + groups, err := client.Graph().ListGroups(ctx, search) if err != nil { return unavailableAdminList("group inventory", err) } @@ -246,7 +246,7 @@ func listAdminGroups( for _, group := range groups { if _, err := fmt.Fprintf( writer, "%s\t%s\t%s\t%s\n", - adminGroupAccess(group), group.DisplayName, + AdminGroupAccess(group), group.DisplayName, group.Description, group.ID, ); err != nil { return err @@ -258,11 +258,11 @@ func listAdminGroups( func showAdminGroup( ctx context.Context, - client *client, + client Client, identifier string, - options RunOptions, + options Options, ) error { - group, err := client.graphClient().GetGroup(ctx, identifier) + group, err := client.Graph().GetGroup(ctx, identifier) if err != nil { return err } @@ -277,7 +277,7 @@ func writeAdminGroup(writer io.Writer, group graph.DirectoryGroup) error { {"ID", group.ID}, {"Name", group.DisplayName}, {"Description", group.Description}, - {"Access", adminGroupAccess(group)}, + {"Access", AdminGroupAccess(group)}, } for _, field := range fields { if field[1] == "" { @@ -299,7 +299,7 @@ func writeAdminGroup(writer io.Writer, group graph.DirectoryGroup) error { return nil } -func adminGroupAccess(group graph.DirectoryGroup) string { +func AdminGroupAccess(group graph.DirectoryGroup) string { for _, groupType := range group.GroupTypes { if strings.EqualFold(groupType, "ReadOnly") { return "read-only" @@ -310,11 +310,11 @@ func adminGroupAccess(group graph.DirectoryGroup) string { func listAdminGroupMembers( ctx context.Context, - client *client, + client Client, identifier string, - options RunOptions, + options Options, ) error { - group, err := client.graphClient().GetGroup(ctx, identifier) + group, err := client.Graph().GetGroup(ctx, identifier) if err != nil { return err } @@ -324,7 +324,7 @@ func listAdminGroupMembers( group.DisplayName, ) } - members, err := client.graphClient().ListGroupMembers(ctx, group.ID) + members, err := client.Graph().ListGroupMembers(ctx, group.ID) if err != nil { return err } @@ -363,9 +363,9 @@ func listAdminGroupMembers( } func listAdminSpaces( - ctx context.Context, client *client, options RunOptions, + ctx context.Context, client Client, options Options, ) error { - spaces, err := client.graphClient().ListDrives(ctx) + spaces, err := client.Graph().ListDrives(ctx) if err != nil { return unavailableAdminList("global Space inventory", err) } @@ -404,30 +404,23 @@ func listAdminSpaces( func showAdminSpace( ctx context.Context, - client *client, + client Client, identifier string, - options RunOptions, + options Options, ) error { - spaces, err := client.graphClient().ListDrives(ctx) + spaces, err := client.Graph().ListDrives(ctx) if err != nil { return unavailableAdminList("global Space inventory", err) } - selected, err := resolveAdminSpace(spaces, identifier) + selected, err := ResolveSpace(spaces, identifier) if err != nil { return err } - details, err := loadSpaceDetails(ctx, client, selected) - if err != nil { - return err - } - if options.OutputMode != appoutput.Human { - return writeOutput(options, "admin-space", details) - } - return writeSpaceDetails(options, details) + return options.WriteSpaceDetails(ctx, client, selected, options) } -func resolveAdminSpace(spaces []space, identifier string) (space, error) { - var matches []space +func ResolveSpace(spaces []graph.Drive, identifier string) (graph.Drive, error) { + var matches []graph.Drive for _, value := range spaces { if value.ID == identifier || strings.EqualFold(value.Name, identifier) || @@ -439,7 +432,7 @@ func resolveAdminSpace(spaces []space, identifier string) (space, error) { case 1: return matches[0], nil case 0: - return space{}, apperror.Wrap( + return graph.Drive{}, apperror.Wrap( apperror.KindUsage, "admin space", fmt.Errorf( "unknown Space %q; run ocis admin space list", @@ -447,7 +440,7 @@ func resolveAdminSpace(spaces []space, identifier string) (space, error) { ), ) default: - return space{}, apperror.Wrap( + return graph.Drive{}, apperror.Wrap( apperror.KindUsage, "admin space", fmt.Errorf("space name %q is ambiguous; use its ID", identifier), ) diff --git a/internal/app/admin/types.go b/internal/app/admin/types.go new file mode 100644 index 0000000..61b5e25 --- /dev/null +++ b/internal/app/admin/types.go @@ -0,0 +1,129 @@ +// Package admin contains account and global Space administration policy. +package admin + +import ( + "context" + "io" + + "github.com/mzner/ocis-cli/internal/graph" + "github.com/mzner/ocis-cli/internal/logging" + appoutput "github.com/mzner/ocis-cli/internal/output" + "github.com/mzner/ocis-cli/internal/sharing" +) + +type Operation string + +const ( + UserList Operation = "user-list" + UserInfo Operation = "user-info" + GroupList Operation = "group-list" + GroupInfo Operation = "group-info" + GroupMemberList Operation = "group-member-list" + SpaceList Operation = "space-list" + SpaceInfo Operation = "space-info" +) + +type Request struct { + Operation Operation + Identifier string + Search string + RawSearch string +} + +type UserCreateRequest struct { + Username, DisplayName, Mail, GivenName, Surname, Password string + Disabled, DryRun bool +} +type UserUpdateRequest struct { + Identifier string + Username, DisplayName, Mail, GivenName, Surname *string + Password string + SetPassword, DryRun bool +} +type UserStateRequest struct { + Identifier string + Enabled, DryRun bool +} +type UserDeleteRequest struct { + Identifier string + DryRun bool +} +type GroupCreateRequest struct { + Name string + DryRun bool +} +type GroupUpdateRequest struct { + Identifier, Name string + DryRun bool +} +type GroupDeleteRequest struct { + Identifier string + DryRun bool +} +type GroupMemberMutationRequest struct { + Group, User string + Remove, DryRun bool +} + +type RoleOperation string + +const ( + RoleList RoleOperation = "list" + RoleAvailable RoleOperation = "available" + RoleGrant RoleOperation = "grant" + RoleRevoke RoleOperation = "revoke" +) + +type RoleRequest struct { + Operation RoleOperation + User, Role string + DryRun bool +} + +type GraphClient interface { + CheckAdminMFA(context.Context) error + ListUsers(context.Context, graph.DirectorySearch) ([]graph.DirectoryUser, error) + GetUser(context.Context, string) (graph.DirectoryUser, error) + ListGroups(context.Context, graph.DirectorySearch) ([]graph.DirectoryGroup, error) + GetGroup(context.Context, string) (graph.DirectoryGroup, error) + ListGroupMembers(context.Context, string) ([]graph.DirectoryUser, error) + ListDrives(context.Context) ([]graph.Drive, error) + ListSpacePermissions(context.Context, string) (graph.Permissions, error) + GetMe(context.Context) (graph.Me, error) + CreateUser(context.Context, graph.CreateUserRequest) (graph.DirectoryUser, error) + UpdateUser(context.Context, string, graph.UpdateUserRequest) (graph.DirectoryUser, error) + DeleteUser(context.Context, string) error + CreateGroup(context.Context, graph.CreateGroupRequest) (graph.DirectoryGroup, error) + UpdateGroup(context.Context, string, graph.UpdateGroupRequest) error + DeleteGroup(context.Context, string) error + AddGroupMember(context.Context, string, string) error + RemoveGroupMember(context.Context, string, string) error + ListApplications(context.Context) ([]graph.Application, error) + ListAppRoleAssignments(context.Context, string) ([]graph.AppRoleAssignment, error) + AssignAppRole(context.Context, graph.AppRoleAssignment) (graph.AppRoleAssignment, error) + RemoveAppRoleAssignment(context.Context, string, string) error +} + +type SharingClient interface { + Capabilities(context.Context) (sharing.Capabilities, error) +} + +type Client interface { + Graph() GraphClient + Sharing() SharingClient + ProfileName() string +} + +type ClientFactory func(context.Context, string) (Client, error) +type AccountAdminGuard func(context.Context, Client) error +type SpaceDetailsWriter func(context.Context, Client, graph.Drive, Options) error + +type Options struct { + OutputMode appoutput.Mode + Out io.Writer + Space string + Logger logging.Logger + NewClient ClientFactory + RequireAccountAdmin AccountAdminGuard + WriteSpaceDetails SpaceDetailsWriter +} diff --git a/internal/app/admin_user_mutation_service.go b/internal/app/admin/user.go similarity index 82% rename from internal/app/admin_user_mutation_service.go rename to internal/app/admin/user.go index 60699b3..6bedda3 100644 --- a/internal/app/admin_user_mutation_service.go +++ b/internal/app/admin/user.go @@ -1,4 +1,4 @@ -package app +package admin import ( "context" @@ -9,11 +9,11 @@ import ( "github.com/mzner/ocis-cli/internal/graph" ) -func runAdminUserCreate( +func RunUserCreate( ctx context.Context, - request AdminUserCreateRequest, + request UserCreateRequest, selectedProfile string, - options RunOptions, + options Options, ) error { request.Username = strings.TrimSpace(request.Username) request.DisplayName = strings.TrimSpace(request.DisplayName) @@ -35,13 +35,14 @@ func runAdminUserCreate( "passwordProvided": request.Password != "", "dryRun": request.DryRun, } - selected, err := newAdminMutationClient( - ctx, selectedProfile, options, - ) + selected, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } - if capabilities, ok := adminCapabilities(ctx, selected); ok && + if err := options.RequireAccountAdmin(ctx, selected); err != nil { + return err + } + if capabilities, ok := adminCapabilities(ctx, selected, options); ok && capabilities.Graph.Users.CreateDisabled { return fmt.Errorf( "server advertises user creation as disabled for its identity backend", @@ -63,7 +64,7 @@ func runAdminUserCreate( enabled := false create.AccountEnabled = &enabled } - created, err := selected.graphClient().CreateUser(ctx, create) + created, err := selected.Graph().CreateUser(ctx, create) if err != nil { return adminMutationError("user", err) } @@ -74,11 +75,11 @@ func runAdminUserCreate( ) } -func runAdminUserUpdate( +func RunUserUpdate( ctx context.Context, - request AdminUserUpdateRequest, + request UserUpdateRequest, selectedProfile string, - options RunOptions, + options Options, ) error { request.Identifier = strings.TrimSpace(request.Identifier) if request.Identifier == "" { @@ -87,7 +88,7 @@ func runAdminUserUpdate( fmt.Errorf("user identifier is required"), ) } - fields := selectedUserUpdateFields(request) + fields := SelectedUserUpdateFields(request) if len(fields) == 0 { return apperror.Wrap( apperror.KindUsage, "admin user update", @@ -100,17 +101,18 @@ func runAdminUserUpdate( fmt.Errorf("replacement password cannot be empty"), ) } - selected, err := newAdminMutationClient( - ctx, selectedProfile, options, - ) + selected, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } + if err := options.RequireAccountAdmin(ctx, selected); err != nil { + return err + } user, err := resolveMutationUser(ctx, selected, request.Identifier) if err != nil { return err } - if capabilities, ok := adminCapabilities(ctx, selected); ok { + if capabilities, ok := adminCapabilities(ctx, selected, options); ok { if err := requireWritableUserFields(capabilities, fields...); err != nil { return err } @@ -136,7 +138,7 @@ func runAdminUserUpdate( if request.SetPassword { update.Password = &graph.PasswordProfile{Password: request.Password} } - updated, err := selected.graphClient().UpdateUser( + updated, err := selected.Graph().UpdateUser( ctx, user.ID, update, ) if err != nil { @@ -149,11 +151,11 @@ func runAdminUserUpdate( ) } -func runAdminUserState( +func RunUserState( ctx context.Context, - request AdminUserStateRequest, + request UserStateRequest, selectedProfile string, - options RunOptions, + options Options, ) error { request.Identifier = strings.TrimSpace(request.Identifier) if request.Identifier == "" { @@ -162,12 +164,13 @@ func runAdminUserState( fmt.Errorf("user identifier is required"), ) } - selected, err := newAdminMutationClient( - ctx, selectedProfile, options, - ) + selected, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } + if err := options.RequireAccountAdmin(ctx, selected); err != nil { + return err + } user, err := resolveMutationUser(ctx, selected, request.Identifier) if err != nil { return err @@ -179,7 +182,7 @@ func runAdminUserState( return err } } - if capabilities, ok := adminCapabilities(ctx, selected); ok { + if capabilities, ok := adminCapabilities(ctx, selected, options); ok { if err := requireWritableUserFields( capabilities, "user.accountEnabled", ); err != nil { @@ -197,7 +200,7 @@ func runAdminUserState( action, fallback(user.Username, user.DisplayName), user.ID, ) } - updated, err := selected.graphClient().UpdateUser( + updated, err := selected.Graph().UpdateUser( ctx, user.ID, graph.UpdateUserRequest{AccountEnabled: &request.Enabled}, ) @@ -212,11 +215,11 @@ func runAdminUserState( ) } -func runAdminUserDelete( +func RunUserDelete( ctx context.Context, - request AdminUserDeleteRequest, + request UserDeleteRequest, selectedProfile string, - options RunOptions, + options Options, ) error { request.Identifier = strings.TrimSpace(request.Identifier) if request.Identifier == "" { @@ -225,12 +228,13 @@ func runAdminUserDelete( fmt.Errorf("user identifier is required"), ) } - selected, err := newAdminMutationClient( - ctx, selectedProfile, options, - ) + selected, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } + if err := options.RequireAccountAdmin(ctx, selected); err != nil { + return err + } user, err := resolveMutationUser(ctx, selected, request.Identifier) if err != nil { return err @@ -238,7 +242,7 @@ func runAdminUserDelete( if err := rejectSelfTarget(ctx, selected, user, "delete"); err != nil { return err } - if capabilities, ok := adminCapabilities(ctx, selected); ok && + if capabilities, ok := adminCapabilities(ctx, selected, options); ok && capabilities.Graph.Users.DeleteDisabled { return fmt.Errorf( "server advertises user deletion as disabled for its identity backend", @@ -255,7 +259,7 @@ func runAdminUserDelete( fallback(user.Username, user.DisplayName), user.ID, ) } - if err := selected.graphClient().DeleteUser(ctx, user.ID); err != nil { + if err := selected.Graph().DeleteUser(ctx, user.ID); err != nil { return adminMutationError("user", err) } return output( @@ -269,7 +273,7 @@ func runAdminUserDelete( ) } -func selectedUserUpdateFields(request AdminUserUpdateRequest) []string { +func SelectedUserUpdateFields(request UserUpdateRequest) []string { fields := make([]string, 0, 6) for _, field := range []struct { value *string diff --git a/internal/app/admin_adapter.go b/internal/app/admin_adapter.go new file mode 100644 index 0000000..a6fcc55 --- /dev/null +++ b/internal/app/admin_adapter.go @@ -0,0 +1,70 @@ +package app + +import ( + "context" + + adminapp "github.com/mzner/ocis-cli/internal/app/admin" + "github.com/mzner/ocis-cli/internal/graph" +) + +type adminClientAdapter struct{ client *client } + +func (adapter adminClientAdapter) Graph() adminapp.GraphClient { return adapter.client.graphClient() } +func (adapter adminClientAdapter) Sharing() adminapp.SharingClient { + return adapter.client.sharingClient() +} +func (adapter adminClientAdapter) ProfileName() string { return adapter.client.name } + +func adminOptions(options RunOptions) adminapp.Options { + return adminapp.Options{ + OutputMode: options.OutputMode, + Out: options.Out, + Space: options.Space, + Logger: options.Logger, + NewClient: func(ctx context.Context, selectedProfile string) (adminapp.Client, error) { + selected, err := newClientWithOptions(ctx, selectedProfile, options) + if err != nil { + return nil, err + } + return adminClientAdapter{client: selected}, nil + }, + RequireAccountAdmin: func(ctx context.Context, selected adminapp.Client) error { + adapter, ok := selected.(adminClientAdapter) + if !ok { + return requireAccountAdminThroughPort(ctx, selected) + } + return requireAccountAdminMFA(ctx, adapter.client) + }, + WriteSpaceDetails: func(ctx context.Context, selected adminapp.Client, drive graph.Drive, _ adminapp.Options) error { + adapter, ok := selected.(adminClientAdapter) + if !ok { + return writeAdminSpaceDetailsThroughPort(ctx, selected, drive, options) + } + details, err := loadSpaceDetails(ctx, adapter.client, drive) + if err != nil { + return err + } + if options.OutputMode != "human" { + return writeOutput(options, "admin-space", details) + } + return writeSpaceDetails(options, details) + }, + } +} + +func requireAccountAdminThroughPort(ctx context.Context, selected adminapp.Client) error { + if err := selected.Graph().CheckAdminMFA(ctx); err != nil { + return err + } + return nil +} + +func writeAdminSpaceDetailsThroughPort(ctx context.Context, selected adminapp.Client, drive graph.Drive, options RunOptions) error { + // Production always uses adminClientAdapter. This fallback keeps the domain + // port independently testable without exposing the concrete runtime client. + permissions, err := selected.Graph().ListSpacePermissions(ctx, drive.ID) + if err != nil { + return err + } + return writeOutput(options, "admin-space", map[string]any{"space": drive, "permissions": permissions}) +} diff --git a/internal/app/admin_guard.go b/internal/app/admin_guard.go index 1fb8e17..eb6729f 100644 --- a/internal/app/admin_guard.go +++ b/internal/app/admin_guard.go @@ -5,36 +5,10 @@ import ( "errors" "fmt" "net/http" - "strings" "github.com/mzner/ocis-cli/internal/apperror" - "github.com/mzner/ocis-cli/internal/graph" - "github.com/mzner/ocis-cli/internal/sharing" ) -func newAdminMutationClient( - ctx context.Context, - selectedProfile string, - options RunOptions, -) (*client, error) { - if options.Space != "" { - return nil, apperror.Wrap( - apperror.KindUsage, "admin", - errors.New( - "--space cannot be used with administrative account operations", - ), - ) - } - selected, err := newClientWithOptions(ctx, selectedProfile, options) - if err != nil { - return nil, err - } - if err := requireAccountAdminMFA(ctx, selected); err != nil { - return nil, err - } - return selected, nil -} - func requireAccountAdminMFA(ctx context.Context, selected *client) error { if err := selected.graphClient().CheckAdminMFA(ctx); err != nil { if requiresMFA(err) { @@ -86,117 +60,13 @@ func requiresMFA(err error) bool { return errors.As(err, &required) && required.RequiresMFA() } -func adminCapabilities( - ctx context.Context, selected *client, -) (sharing.Capabilities, bool) { - capabilities, err := selected.sharingClient().Capabilities(ctx) - if err != nil { - selected.logger.Debug( - "could not read optional administration capabilities", - "error", err, - ) - return sharing.Capabilities{}, false - } - return capabilities, true -} - -func requireWritableUserFields( - capabilities sharing.Capabilities, - fields ...string, -) error { - for _, field := range fields { - for _, readOnly := range capabilities.Graph.Users.ReadOnlyAttributes { - if strings.EqualFold(strings.TrimSpace(readOnly), field) { - return fmt.Errorf( - "server advertises %s as read-only", field, - ) - } - } - } - return nil -} - -func resolveMutationUser( - ctx context.Context, selected *client, identifier string, -) (graph.DirectoryUser, error) { - user, err := selected.graphClient().GetUser(ctx, identifier) - if err != nil { - return graph.DirectoryUser{}, err - } - if strings.TrimSpace(user.ID) == "" { - return graph.DirectoryUser{}, fmt.Errorf( - "server returned user %q without a stable ID", - user.DisplayName, - ) - } - return user, nil -} - -func resolveMutationGroup( - ctx context.Context, selected *client, identifier string, -) (graph.DirectoryGroup, error) { - group, err := selected.graphClient().GetGroup(ctx, identifier) - if err != nil { - return graph.DirectoryGroup{}, err - } - if strings.TrimSpace(group.ID) == "" { - return graph.DirectoryGroup{}, fmt.Errorf( - "server returned group %q without a stable ID", - group.DisplayName, - ) - } - return group, nil -} - -func rejectReadOnlyGroup(group graph.DirectoryGroup) error { - if adminGroupAccess(group) == "read-only" { - return fmt.Errorf( - "group %q is managed by a read-only identity backend", - group.DisplayName, - ) - } - return nil -} - -func rejectSelfTarget( - ctx context.Context, - selected *client, - target graph.DirectoryUser, - operation string, -) error { - current, err := selected.graphClient().GetMe(ctx) - if err != nil { - return fmt.Errorf( - "verify current account before %s: %w", operation, err, - ) - } - if current.ID == target.ID { - return apperror.Wrap( - apperror.KindConflict, operation, - fmt.Errorf( - "refusing to %s the currently authenticated account %q", - operation, target.Username, - ), - ) - } - return nil -} - -func adminMutationError(resource string, err error) error { - if err == nil { - return nil - } - message := strings.ToLower(err.Error()) - if strings.Contains(message, "read-only") || - strings.Contains(message, "configured read only") { - return fmt.Errorf( - "%s is managed by a read-only identity backend", resource, - ) - } +func unavailableAdminList(capability string, err error) error { switch protocolStatus(err) { - case http.StatusMethodNotAllowed, http.StatusNotImplemented: + case http.StatusNotFound, http.StatusMethodNotAllowed, + http.StatusNotImplemented: return fmt.Errorf( - "server does not expose the requested %s mutation", resource, + "server does not expose %s through LibreGraph: %w", + capability, err, ) default: return err diff --git a/internal/app/admin_mutation_service_test.go b/internal/app/admin_mutation_service_test.go index 0d41916..36b6da5 100644 --- a/internal/app/admin_mutation_service_test.go +++ b/internal/app/admin_mutation_service_test.go @@ -3,7 +3,6 @@ package app import ( "bytes" "context" - "errors" "io" "net/http" "net/http/httptest" @@ -13,10 +12,7 @@ import ( "time" "github.com/mzner/ocis-cli/internal/apperror" - "github.com/mzner/ocis-cli/internal/graph" - "github.com/mzner/ocis-cli/internal/httpapi" appoutput "github.com/mzner/ocis-cli/internal/output" - "github.com/mzner/ocis-cli/internal/sharing" ) func TestAdminUserCreateRequiresAdminAndDoesNotExposePassword(t *testing.T) { @@ -359,72 +355,6 @@ func TestResolveMFAACRUsesServerCapability(t *testing.T) { } } -func TestRoleResolutionUsesAdvertisedIDsAndRejectsAmbiguity(t *testing.T) { - roles := advertisedRoles([]graph.Application{ - { - ID: "app-1", DisplayName: "oCIS", - AppRoles: []graph.AppRole{ - {ID: "role-admin", DisplayName: "Admin"}, - {ID: "role-user", DisplayName: "User"}, - }, - }, - }) - resolved, err := resolveAdvertisedRole(roles, "admin") - if err != nil || resolved.role.ID != "role-admin" { - t.Fatalf("resolved: %#v, %v", resolved, err) - } - roles = append(roles, advertisedRole{ - application: graph.Application{ID: "app-2"}, - role: graph.AppRole{ID: "other-admin", DisplayName: "Admin"}, - }) - if _, err := resolveAdvertisedRole(roles, "Admin"); err == nil || - !strings.Contains(err.Error(), "ambiguous") { - t.Fatalf("ambiguity error: %v", err) - } -} - -func TestAdminMutationGuardHelpers(t *testing.T) { - capabilities := sharing.Capabilities{} - capabilities.Graph.Users.ReadOnlyAttributes = []string{"user.mail"} - if err := requireWritableUserFields( - capabilities, "user.displayName", "user.mail", - ); err == nil || !strings.Contains(err.Error(), "read-only") { - t.Fatalf("writable field error: %v", err) - } - if err := rejectReadOnlyGroup(graph.DirectoryGroup{ - DisplayName: "External", - GroupTypes: []string{"ReadOnly"}, - }); err == nil { - t.Fatal("read-only group was accepted") - } - displayName := "Updated" - fields := selectedUserUpdateFields(AdminUserUpdateRequest{ - DisplayName: &displayName, SetPassword: true, - }) - if len(fields) != 2 || titleWord("delete") != "Delete" || - titleWord("") != "" { - t.Fatalf("fields=%#v title=%q", fields, titleWord("delete")) - } - if err := adminMutationError( - "group", errors.New("backend is configured read-only"), - ); err == nil || !strings.Contains(err.Error(), "read-only identity") { - t.Fatalf("read-only backend error: %v", err) - } - unsupported := &httpapi.HTTPError{ - StatusCode: http.StatusNotImplemented, - Status: "501 Not Implemented", - } - if err := adminMutationError("group", unsupported); err == nil || - !strings.Contains(err.Error(), "does not expose") { - t.Fatalf("unsupported mutation error: %v", err) - } - if err := roleServiceError(&httpapi.HTTPError{ - StatusCode: http.StatusNotFound, Status: "404 Not Found", - }); err == nil || !strings.Contains(err.Error(), "role service") { - t.Fatalf("role service error: %v", err) - } -} - func writeAdminCapabilities(writer http.ResponseWriter, capabilities string) { writer.Header().Set("Content-Type", "application/json") _, _ = io.WriteString(writer, `{"ocs":{"meta":{ diff --git a/internal/app/admin_service_test.go b/internal/app/admin_service_test.go index c6415e0..3b04b41 100644 --- a/internal/app/admin_service_test.go +++ b/internal/app/admin_service_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + adminapp "github.com/mzner/ocis-cli/internal/app/admin" "github.com/mzner/ocis-cli/internal/apperror" "github.com/mzner/ocis-cli/internal/graph" appoutput "github.com/mzner/ocis-cli/internal/output" @@ -254,7 +255,7 @@ func TestAdministrativeInventoryRejectsSpaceSelectionBeforeProfile(t *testing.T) } func TestResolveAdminSpaceFailsClosedOnAmbiguousName(t *testing.T) { - _, err := resolveAdminSpace([]space{ + _, err := adminapp.ResolveSpace([]space{ {ID: "space-1", Name: "Shared"}, {ID: "space-2", Name: "shared"}, }, "shared") @@ -290,11 +291,11 @@ func TestAdminValidationAndFormatting(t *testing.T) { } disabled := false - if adminUserStatus(nil) != "unknown" || - adminUserStatus(&disabled) != "disabled" { + if adminapp.AdminUserStatus(nil) != "unknown" || + adminapp.AdminUserStatus(&disabled) != "disabled" { t.Fatal("unexpected account status formatting") } - if adminGroupAccess(directoryGroupForTest()) != "writable" { + if adminapp.AdminGroupAccess(directoryGroupForTest()) != "writable" { t.Fatal("ordinary group was not writable") } } diff --git a/internal/app/api.go b/internal/app/api.go index d6b9135..9fab257 100644 --- a/internal/app/api.go +++ b/internal/app/api.go @@ -8,6 +8,9 @@ import ( "os" "time" + adminapp "github.com/mzner/ocis-cli/internal/app/admin" + shareapp "github.com/mzner/ocis-cli/internal/app/share" + syncapp "github.com/mzner/ocis-cli/internal/app/sync" "github.com/mzner/ocis-cli/internal/apperror" "github.com/mzner/ocis-cli/internal/graph" "github.com/mzner/ocis-cli/internal/logging" @@ -152,50 +155,30 @@ type SpaceMemberRequest struct { } // ShareOperation identifies a sharing use case. -type ShareOperation string +type ShareOperation = shareapp.Operation const ( - ShareCreate ShareOperation = "create" - ShareList ShareOperation = "list" - ShareRevoke ShareOperation = "revoke" - ShareLinkInfo ShareOperation = "link-info" - ShareLinkUpdate ShareOperation = "link-update" - ShareDirectAdd ShareOperation = "direct-add" - ShareFederatedAdd ShareOperation = "federated-add" - ShareDirectUpdate ShareOperation = "direct-update" - ShareRemove ShareOperation = "remove" - ShareOverview ShareOperation = "overview" - ShareReceived ShareOperation = "received" - ShareAccept ShareOperation = "accept" - ShareDecline ShareOperation = "decline" - ShareRoles ShareOperation = "roles" + ShareCreate = shareapp.Create + ShareList = shareapp.List + ShareRevoke = shareapp.Revoke + ShareLinkInfo = shareapp.LinkInfo + ShareLinkUpdate = shareapp.LinkUpdate + ShareDirectAdd = shareapp.DirectAdd + ShareFederatedAdd = shareapp.FederatedAdd + ShareDirectUpdate = shareapp.DirectUpdate + ShareRemove = shareapp.Remove + ShareOverview = shareapp.Overview + ShareReceived = shareapp.Received + ShareAccept = shareapp.Accept + ShareDecline = shareapp.Decline + ShareRoles = shareapp.Roles ) // ShareRequest describes one public-link or direct-sharing operation. -type ShareRequest struct { - Operation ShareOperation - Path string - ID string - Name string - Password string - UpdateName bool - UpdateExpiration bool - UpdateAccess bool - UpdatePassword bool - RemovePassword bool - Expiration string - Permissions int - Recipient string - RecipientType string - RecipientIsID bool - Role string - Direction string - State string - LinksOnly bool - Confirmed bool - DryRun bool - Federated bool -} +type ShareRequest = shareapp.Request + +// ShareOverviewItem is one stable outgoing or received share inventory row. +type ShareOverviewItem = shareapp.OverviewItem // TrashOperation identifies a recycle-bin use case. type TrashOperation string @@ -349,101 +332,62 @@ type BatchSummary struct { } // SyncDirection identifies a synchronization policy. -type SyncDirection string +type SyncDirection = syncapp.Direction const ( // SyncPush copies the local source tree into the remote destination. - SyncPush SyncDirection = "push" + SyncPush = syncapp.Push // SyncPull copies the remote source tree into the local destination. - SyncPull SyncDirection = "pull" + SyncPull = syncapp.Pull // SyncBidirectional reconciles local and remote changes through a baseline. - SyncBidirectional SyncDirection = "bidirectional" + SyncBidirectional = syncapp.Bidirectional ) // SyncRequest describes one deterministic directory reconciliation. -type SyncRequest struct { - Direction SyncDirection - LocalRoot string - RemoteRoot string - Includes []string - Excludes []string - Delete bool - Overwrite bool - DryRun bool - MaxEntries int - ConflictStrategy string - Prefer string -} +type SyncRequest = syncapp.Request // SyncStateOperation identifies a local synchronization-state operation. -type SyncStateOperation string +type SyncStateOperation = syncapp.StateOperation const ( - SyncStateList SyncStateOperation = "list" - SyncStateShow SyncStateOperation = "show" - SyncStateExport SyncStateOperation = "export" - SyncStateRemove SyncStateOperation = "remove" + SyncStateList = syncapp.StateList + SyncStateShow = syncapp.StateShow + SyncStateExport = syncapp.StateExport + SyncStateRemove = syncapp.StateRemove ) // SyncStateRequest describes inspection, export, or removal of a saved // synchronization baseline. -type SyncStateRequest struct { - Operation SyncStateOperation - ID string - Profile string - Confirmed bool - DryRun bool -} +type SyncStateRequest = syncapp.StateRequest // SyncRecoveryOperation identifies an interrupted-run journal operation. -type SyncRecoveryOperation string +type SyncRecoveryOperation = syncapp.RecoveryOperation const ( - SyncRecoveryList SyncRecoveryOperation = "list" - SyncRecoveryShow SyncRecoveryOperation = "show" - SyncRecoveryRetry SyncRecoveryOperation = "retry" - SyncRecoveryRemove SyncRecoveryOperation = "remove" + SyncRecoveryList = syncapp.RecoveryList + SyncRecoveryShow = syncapp.RecoveryShow + SyncRecoveryRetry = syncapp.RecoveryRetry + SyncRecoveryRemove = syncapp.RecoveryRemove ) // SyncRecoveryRequest describes inspection, safe retry, or removal of a // bidirectional synchronization recovery journal. -type SyncRecoveryRequest struct { - Operation SyncRecoveryOperation - ID string - Profile string - Confirmed bool - DryRun bool -} +type SyncRecoveryRequest = syncapp.RecoveryRequest // SyncJobOperation identifies a reusable synchronization-job operation. -type SyncJobOperation string +type SyncJobOperation = syncapp.JobOperation const ( - SyncJobAdd SyncJobOperation = "add" - SyncJobList SyncJobOperation = "list" - SyncJobShow SyncJobOperation = "show" - SyncJobRun SyncJobOperation = "run" - SyncJobRemove SyncJobOperation = "remove" + SyncJobAdd = syncapp.JobAdd + SyncJobList = syncapp.JobList + SyncJobShow = syncapp.JobShow + SyncJobRun = syncapp.JobRun + SyncJobRemove = syncapp.JobRemove ) // SyncJobRequest describes creation, inspection, execution, or removal of a // named synchronization configuration. -type SyncJobRequest struct { - Operation SyncJobOperation - Name string - Profile string - Space string - Direction SyncDirection - LocalRoot string - RemoteRoot string - Includes []string - Excludes []string - DeleteDestination bool - Overwrite bool - MaxEntries int - Confirmed bool - DryRun bool -} +type SyncJobRequest = syncapp.JobRequest // MetadataOperation identifies a file-metadata use case. type MetadataOperation string @@ -471,108 +415,57 @@ type MetadataRequest struct { } // AdminOperation identifies a read-only administrative use case. -type AdminOperation string +type AdminOperation = adminapp.Operation const ( - AdminUserList AdminOperation = "user-list" - AdminUserInfo AdminOperation = "user-info" - AdminGroupList AdminOperation = "group-list" - AdminGroupInfo AdminOperation = "group-info" - AdminGroupMemberList AdminOperation = "group-member-list" - AdminSpaceList AdminOperation = "space-list" - AdminSpaceInfo AdminOperation = "space-info" + AdminUserList = adminapp.UserList + AdminUserInfo = adminapp.UserInfo + AdminGroupList = adminapp.GroupList + AdminGroupInfo = adminapp.GroupInfo + AdminGroupMemberList = adminapp.GroupMemberList + AdminSpaceList = adminapp.SpaceList + AdminSpaceInfo = adminapp.SpaceInfo ) // AdminRequest describes one read-only administrative operation. -type AdminRequest struct { - Operation AdminOperation - Identifier string - Search string - RawSearch string -} +type AdminRequest = adminapp.Request // AdminUserCreateRequest describes a new server user. -type AdminUserCreateRequest struct { - Username string - DisplayName string - Mail string - GivenName string - Surname string - Password string - Disabled bool - DryRun bool -} +type AdminUserCreateRequest = adminapp.UserCreateRequest // AdminUserUpdateRequest contains explicitly selected user changes. -type AdminUserUpdateRequest struct { - Identifier string - Username *string - DisplayName *string - Mail *string - GivenName *string - Surname *string - Password string - SetPassword bool - DryRun bool -} +type AdminUserUpdateRequest = adminapp.UserUpdateRequest // AdminUserStateRequest enables or disables one user account. -type AdminUserStateRequest struct { - Identifier string - Enabled bool - DryRun bool -} +type AdminUserStateRequest = adminapp.UserStateRequest // AdminUserDeleteRequest permanently deletes one user account. -type AdminUserDeleteRequest struct { - Identifier string - DryRun bool -} +type AdminUserDeleteRequest = adminapp.UserDeleteRequest // AdminGroupCreateRequest describes a new server group. -type AdminGroupCreateRequest struct { - Name string - DryRun bool -} +type AdminGroupCreateRequest = adminapp.GroupCreateRequest // AdminGroupUpdateRequest renames one server group. -type AdminGroupUpdateRequest struct { - Identifier string - Name string - DryRun bool -} +type AdminGroupUpdateRequest = adminapp.GroupUpdateRequest // AdminGroupDeleteRequest permanently deletes one server group. -type AdminGroupDeleteRequest struct { - Identifier string - DryRun bool -} +type AdminGroupDeleteRequest = adminapp.GroupDeleteRequest // AdminGroupMemberMutationRequest adds or removes a direct user member. -type AdminGroupMemberMutationRequest struct { - Group string - User string - Remove bool - DryRun bool -} +type AdminGroupMemberMutationRequest = adminapp.GroupMemberMutationRequest // AdminRoleOperation identifies one user-role operation. -type AdminRoleOperation string +type AdminRoleOperation = adminapp.RoleOperation const ( - AdminRoleList AdminRoleOperation = "list" - AdminRoleAvailable AdminRoleOperation = "available" - AdminRoleGrant AdminRoleOperation = "grant" - AdminRoleRevoke AdminRoleOperation = "revoke" + AdminRoleList = adminapp.RoleList + AdminRoleAvailable = adminapp.RoleAvailable + AdminRoleGrant = adminapp.RoleGrant + AdminRoleRevoke = adminapp.RoleRevoke ) // AdminRoleRequest lists, grants, or revokes a server-advertised user role. -type AdminRoleRequest struct { - Operation AdminRoleOperation - User string - Role string - DryRun bool -} +type AdminRoleRequest = adminapp.RoleRequest // SearchRequest describes a read-only remote resource search. type SearchRequest struct { @@ -683,7 +576,7 @@ func RunSyncWithOptions( ) error { return classifyProtocolError( "sync "+string(request.Direction), - runSync(ctx, request, selectedProfile, options.normalized()), + syncapp.Run(ctx, request, selectedProfile, syncOptions(options.normalized())), ) } @@ -695,7 +588,7 @@ func RunSyncStateWithOptions( ) error { return classifyProtocolError( "sync state "+string(request.Operation), - runSyncState(ctx, request, options.normalized()), + syncapp.RunState(ctx, request, syncOptions(options.normalized())), ) } @@ -707,7 +600,7 @@ func RunSyncJobWithOptions( ) error { return classifyProtocolError( "sync job "+string(request.Operation), - runSyncJob(ctx, request, options.normalized()), + syncapp.RunJob(ctx, request, syncOptions(options.normalized())), ) } @@ -719,7 +612,7 @@ func RunSyncRecoveryWithOptions( ) error { return classifyProtocolError( "sync recovery "+string(request.Operation), - runSyncRecovery(ctx, request, options.normalized()), + syncapp.RunRecovery(ctx, request, syncOptions(options.normalized())), ) } @@ -745,7 +638,7 @@ func RunAdminWithOptions( ) error { return classifyProtocolError( "admin "+string(request.Operation), - runAdmin(ctx, request, selectedProfile, options.normalized()), + adminapp.Run(ctx, request, selectedProfile, adminOptions(options.normalized())), ) } @@ -771,9 +664,7 @@ func RunAdminUserCreateWithOptions( ) error { return classifyProtocolError( "admin user create", - runAdminUserCreate( - ctx, request, selectedProfile, options.normalized(), - ), + adminapp.RunUserCreate(ctx, request, selectedProfile, adminOptions(options.normalized())), ) } @@ -786,9 +677,7 @@ func RunAdminUserUpdateWithOptions( ) error { return classifyProtocolError( "admin user update", - runAdminUserUpdate( - ctx, request, selectedProfile, options.normalized(), - ), + adminapp.RunUserUpdate(ctx, request, selectedProfile, adminOptions(options.normalized())), ) } @@ -801,9 +690,7 @@ func RunAdminUserStateWithOptions( ) error { return classifyProtocolError( "admin user state", - runAdminUserState( - ctx, request, selectedProfile, options.normalized(), - ), + adminapp.RunUserState(ctx, request, selectedProfile, adminOptions(options.normalized())), ) } @@ -816,9 +703,7 @@ func RunAdminUserDeleteWithOptions( ) error { return classifyProtocolError( "admin user delete", - runAdminUserDelete( - ctx, request, selectedProfile, options.normalized(), - ), + adminapp.RunUserDelete(ctx, request, selectedProfile, adminOptions(options.normalized())), ) } @@ -831,9 +716,7 @@ func RunAdminGroupCreateWithOptions( ) error { return classifyProtocolError( "admin group create", - runAdminGroupCreate( - ctx, request, selectedProfile, options.normalized(), - ), + adminapp.RunGroupCreate(ctx, request, selectedProfile, adminOptions(options.normalized())), ) } @@ -846,9 +729,7 @@ func RunAdminGroupUpdateWithOptions( ) error { return classifyProtocolError( "admin group update", - runAdminGroupUpdate( - ctx, request, selectedProfile, options.normalized(), - ), + adminapp.RunGroupUpdate(ctx, request, selectedProfile, adminOptions(options.normalized())), ) } @@ -861,9 +742,7 @@ func RunAdminGroupDeleteWithOptions( ) error { return classifyProtocolError( "admin group delete", - runAdminGroupDelete( - ctx, request, selectedProfile, options.normalized(), - ), + adminapp.RunGroupDelete(ctx, request, selectedProfile, adminOptions(options.normalized())), ) } @@ -876,9 +755,7 @@ func RunAdminGroupMemberMutationWithOptions( ) error { return classifyProtocolError( "admin group member", - runAdminGroupMemberMutation( - ctx, request, selectedProfile, options.normalized(), - ), + adminapp.RunGroupMemberMutation(ctx, request, selectedProfile, adminOptions(options.normalized())), ) } @@ -891,9 +768,7 @@ func RunAdminRoleWithOptions( ) error { return classifyProtocolError( "admin user role", - runAdminRole( - ctx, request, selectedProfile, options.normalized(), - ), + adminapp.RunRole(ctx, request, selectedProfile, adminOptions(options.normalized())), ) } @@ -965,9 +840,10 @@ func RunSpaceMemberWithOptions( func RunShareWithOptions( ctx context.Context, request ShareRequest, selectedProfile string, options RunOptions, ) error { + options = options.normalized() return classifyProtocolError( string(request.Operation), - runShare(ctx, request, selectedProfile, options.normalized()), + shareapp.Run(ctx, request, selectedProfile, shareOptions(options)), ) } diff --git a/internal/app/architecture_test.go b/internal/app/architecture_test.go new file mode 100644 index 0000000..9228a7a --- /dev/null +++ b/internal/app/architecture_test.go @@ -0,0 +1,61 @@ +package app + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "strconv" + "strings" + "testing" +) + +func TestDomainPackagesDoNotReachOuterLayers(t *testing.T) { + root, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + for _, domain := range []string{"admin", "archive", "share", "sync"} { + directory := filepath.Join(root, domain) + entries, err := os.ReadDir(directory) + if err != nil { + t.Fatal(err) + } + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".go") || + strings.HasSuffix(entry.Name(), "_test.go") { + continue + } + name := filepath.Join(directory, entry.Name()) + parsed, err := parser.ParseFile( + token.NewFileSet(), name, nil, parser.ImportsOnly, + ) + if err != nil { + t.Fatal(err) + } + for _, imported := range parsed.Imports { + assertAllowedDomainImport(t, domain, name, imported) + } + } + } +} + +func assertAllowedDomainImport( + t *testing.T, domain, name string, imported *ast.ImportSpec, +) { + t.Helper() + path, err := strconv.Unquote(imported.Path.Value) + if err != nil { + t.Fatal(err) + } + forbidden := path == "github.com/mzner/ocis-cli/internal/app" || + strings.HasPrefix(path, "github.com/mzner/ocis-cli/internal/command") || + strings.HasPrefix(path, "github.com/spf13/cobra") + if forbidden { + t.Errorf( + "application domain %s imports outer layer %s in %s", + domain, path, name, + ) + } +} diff --git a/internal/app/archive_service.go b/internal/app/archive/service.go similarity index 83% rename from internal/app/archive_service.go rename to internal/app/archive/service.go index bcb4c49..c1834b0 100644 --- a/internal/app/archive_service.go +++ b/internal/app/archive/service.go @@ -1,9 +1,13 @@ -package app +// Package archive contains the archive-download application domain. It +// depends on a narrow authenticated client port instead of the parent app +// package, so archive policy cannot reach unrelated application helpers. +package archive import ( "context" "errors" "fmt" + "io" "math" "os" "path/filepath" @@ -18,9 +22,42 @@ import ( appoutput "github.com/mzner/ocis-cli/internal/output" "github.com/mzner/ocis-cli/internal/sharing" "github.com/mzner/ocis-cli/internal/transfer" + "github.com/mzner/ocis-cli/internal/webdav" "golang.org/x/term" ) +// Request describes one server-side archive download. +type Request struct { + Paths []string + Destination string + Format string + Overwrite bool + DryRun bool +} + +// Client is the authenticated server functionality used by this domain. +type Client interface { + SelectSpace(string) error + Capabilities(context.Context) (sharing.Capabilities, error) + Stat(string) (webdav.Item, error) + List(string) ([]webdav.Item, error) + Archiver(string) (*archiveclient.Client, error) +} + +// ClientFactory creates an account-bound client without exposing application +// runtime internals to this domain. +type ClientFactory func(context.Context, string) (Client, error) + +// Options contains the process-boundary values used by archive operations. +type Options struct { + OutputMode appoutput.Mode + Out io.Writer + Err io.Writer + Quiet bool + Space string + NewClient ClientFactory +} + // ArchiveFormat reports one usable format and the limits shared by the // selected archive service. type ArchiveFormat struct { @@ -50,10 +87,11 @@ type ArchiveResult struct { DryRun bool `json:"dryRun,omitempty"` } -func runArchiveFormats( - ctx context.Context, selectedProfile string, options RunOptions, +// RunFormats lists the preferred enabled archive service's formats. +func RunFormats( + ctx context.Context, selectedProfile string, options Options, ) error { - client, err := newClientWithOptions(ctx, selectedProfile, options) + client, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } @@ -82,11 +120,12 @@ func runArchiveFormats( return writer.Flush() } -func runArchiveDownload( +// RunDownload preflights and downloads one server-created archive. +func RunDownload( ctx context.Context, - request ArchiveDownloadRequest, + request Request, selectedProfile string, - options RunOptions, + options Options, ) error { if len(request.Paths) == 0 { return archiveUsage("select at least one remote path") @@ -111,11 +150,11 @@ func runArchiveDownload( return err } - client, err := newClientWithOptions(ctx, selectedProfile, options) + client, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } - if err := client.selectSpace(options.Space); err != nil { + if err := client.SelectSpace(options.Space); err != nil { return err } capability, err := discoverArchiver(ctx, client) @@ -142,7 +181,7 @@ func runArchiveDownload( return writeArchiveResult(result, options) } - protocol, err := client.archiverClient(capability.URL) + protocol, err := client.Archiver(capability.URL) if err != nil { return fmt.Errorf("configure archive download: %w", err) } @@ -200,18 +239,19 @@ func runArchiveDownload( } func discoverArchiver( - ctx context.Context, client *client, + ctx context.Context, client Client, ) (sharing.ArchiverCapabilities, error) { - capabilities, err := client.sharingClient().Capabilities(ctx) + capabilities, err := client.Capabilities(ctx) if err != nil { return sharing.ArchiverCapabilities{}, fmt.Errorf( "discover archive service: %w", err, ) } - return selectArchiver(capabilities.Files.Archivers) + return SelectCapabilities(capabilities.Files.Archivers) } -func selectArchiver( +// SelectCapabilities chooses the highest enabled usable archive capability. +func SelectCapabilities( capabilities []sharing.ArchiverCapabilities, ) (sharing.ArchiverCapabilities, error) { var selected *sharing.ArchiverCapabilities @@ -236,12 +276,12 @@ func selectArchiver( } func addArchiveResource( - client *client, + client Client, remote string, capability sharing.ArchiverCapabilities, result *ArchiveResult, ) error { - root, err := client.stat(remote) + root, err := client.Stat(remote) if err != nil { return err } @@ -266,8 +306,8 @@ func addArchiveResource( } func scanArchiveItem( - client *client, - value item, + client Client, + value webdav.Item, include bool, capability sharing.ArchiverCapabilities, result *ArchiveResult, @@ -299,7 +339,7 @@ func scanArchiveItem( if value.Type != "directory" { return nil } - children, err := client.list(value.Path) + children, err := client.List(value.Path) if err != nil { return err } @@ -453,7 +493,7 @@ func archiveResourceIDs(values []ArchiveResource) []string { } func archiveProgressReporter( - options RunOptions, destination string, + options Options, destination string, ) (func(int64), func(int64)) { if options.Quiet || options.OutputMode != appoutput.Human { return nil, func(int64) {} @@ -488,8 +528,9 @@ func archiveProgressReporter( return update, finish } -func archiverCapabilityDetail(capabilities sharing.Capabilities) string { - selected, err := selectArchiver(capabilities.Files.Archivers) +// CapabilityDetail describes the preferred capability for diagnostics. +func CapabilityDetail(capabilities sharing.Capabilities) string { + selected, err := SelectCapabilities(capabilities.Files.Archivers) if err != nil { return "not advertised" } @@ -510,7 +551,7 @@ func archiverCapabilityDetail(capabilities sharing.Capabilities) string { return strings.Join(details, "; ") } -func writeArchiveResult(result ArchiveResult, options RunOptions) error { +func writeArchiveResult(result ArchiveResult, options Options) error { if options.OutputMode != appoutput.Human { return writeOutput(options, "archive", result) } @@ -532,6 +573,20 @@ func writeArchiveResult(result ArchiveResult, options RunOptions) error { return err } +func writeOutput(options Options, kind string, value any) error { + return (appoutput.Renderer{ + Writer: options.Out, Mode: options.OutputMode, Type: kind, + }).Write(value, "") +} + +func cleanRemote(remote string) string { + remote = strings.TrimSpace(remote) + if remote == "" || remote == "/" { + return "/" + } + return "/" + strings.Trim(remote, "/") +} + func archiveUsage(message string) error { return apperror.Wrap( apperror.KindUsage, "archive download", errors.New(message), diff --git a/internal/app/archive/service_test.go b/internal/app/archive/service_test.go new file mode 100644 index 0000000..795c241 --- /dev/null +++ b/internal/app/archive/service_test.go @@ -0,0 +1,130 @@ +package archive + +import ( + "bytes" + "context" + "errors" + "io" + "path/filepath" + "strings" + "testing" + + archiveclient "github.com/mzner/ocis-cli/internal/archiver" + appoutput "github.com/mzner/ocis-cli/internal/output" + "github.com/mzner/ocis-cli/internal/sharing" + "github.com/mzner/ocis-cli/internal/webdav" +) + +type fakeClient struct{ capabilities sharing.Capabilities } + +func (client fakeClient) SelectSpace(string) error { return nil } + +func (client fakeClient) Capabilities(context.Context) (sharing.Capabilities, error) { + return client.capabilities, nil +} + +func (fakeClient) Stat(string) (webdav.Item, error) { + return webdav.Item{}, errors.New("unexpected stat") +} + +func (fakeClient) List(string) ([]webdav.Item, error) { + return nil, errors.New("unexpected list") +} + +func (fakeClient) Archiver(string) (*archiveclient.Client, error) { + return nil, errors.New("unexpected archive client") +} + +func TestRunFormatsUsesPreferredEnabledCapability(t *testing.T) { + capabilities := sharing.Capabilities{} + capabilities.Files.Archivers = []sharing.ArchiverCapabilities{ + {Enabled: true, Version: "1.0.0", Formats: []string{"zip"}, URL: "/old"}, + {Enabled: true, Version: "2.0.0", Formats: []string{"tar", "zip"}, URL: "/archiver", MaxNumFiles: 10, MaxSize: 100}, + } + var output bytes.Buffer + options := Options{ + OutputMode: appoutput.JSON, Out: &output, Err: io.Discard, + NewClient: func(context.Context, string) (Client, error) { + return fakeClient{capabilities: capabilities}, nil + }, + } + if err := RunFormats(context.Background(), "work", options); err != nil { + t.Fatal(err) + } + if !strings.Contains(output.String(), `"version": "2.0.0"`) || + !strings.Contains(output.String(), `"format": "tar"`) || + strings.Contains(output.String(), `"version": "1.0.0"`) { + t.Fatalf("output: %s", output.String()) + } +} + +func TestArchiveValidationHelpers(t *testing.T) { + if _, err := normalizeArchivePaths([]string{"/reports", "/reports/file"}); err == nil { + t.Fatal("nested selection accepted") + } + if _, err := normalizeArchivePaths([]string{"/reports", "reports"}); err == nil { + t.Fatal("duplicate selection accepted") + } + paths, err := normalizeArchivePaths([]string{" reports ", "/photos"}) + if err != nil || strings.Join(paths, ",") != "/reports,/photos" { + t.Fatalf("paths=%v error=%v", paths, err) + } + for _, test := range []struct { + requested string + destination string + want string + }{ + {"", "backup", "zip"}, {"", "backup.tar", "tar"}, + {"zip", "backup.zip", "zip"}, {"tar", "backup", "tar"}, + } { + got, err := resolveArchiveFormat(test.requested, test.destination) + if err != nil || got != test.want { + t.Fatalf("format %#v = %q, %v", test, got, err) + } + } + for _, invalid := range [][2]string{{"rar", "a.rar"}, {"tar", "a.zip"}} { + if _, err := resolveArchiveFormat(invalid[0], invalid[1]); err == nil { + t.Fatalf("invalid format accepted: %v", invalid) + } + } + destination := filepath.Join(t.TempDir(), "archive.zip") + if err := validateArchiveDestination(destination, false); err != nil { + t.Fatal(err) + } + if err := writeTestFile(destination); err != nil { + t.Fatal(err) + } + if err := validateArchiveDestination(destination, false); err == nil { + t.Fatal("existing destination accepted without overwrite") + } + if err := validateArchiveDestination(destination, true); err != nil { + t.Fatal(err) + } +} + +func TestCapabilitySelectionAndDetail(t *testing.T) { + values := []sharing.ArchiverCapabilities{ + {Enabled: false, Version: "9.0.0", Formats: []string{"zip"}, URL: "/disabled"}, + {Enabled: true, Version: "v2.1.0", Formats: []string{"ZIP", "tar", "rar"}, URL: "/archiver", MaxNumFiles: 12, MaxSize: 34}, + } + selected, err := SelectCapabilities(values) + if err != nil || selected.Version != "v2.1.0" || + strings.Join(selected.Formats, ",") != "tar,zip" { + t.Fatalf("selected=%#v error=%v", selected, err) + } + capabilities := sharing.Capabilities{} + capabilities.Files.Archivers = values + detail := CapabilityDetail(capabilities) + for _, expected := range []string{ + "version v2.1.0", "formats tar, zip", "maximum 12 entries", + "maximum 34 source bytes", + } { + if !strings.Contains(detail, expected) { + t.Fatalf("detail %q missing %q", detail, expected) + } + } + if _, err := SelectCapabilities(nil); err == nil || + CapabilityDetail(sharing.Capabilities{}) != "not advertised" { + t.Fatal("missing capability accepted") + } +} diff --git a/internal/app/archive/test_helpers_test.go b/internal/app/archive/test_helpers_test.go new file mode 100644 index 0000000..b55e312 --- /dev/null +++ b/internal/app/archive/test_helpers_test.go @@ -0,0 +1,7 @@ +package archive + +import "os" + +func writeTestFile(name string) error { + return os.WriteFile(name, []byte("existing"), 0600) +} diff --git a/internal/app/archive_api.go b/internal/app/archive_api.go index acee6a7..c26292c 100644 --- a/internal/app/archive_api.go +++ b/internal/app/archive_api.go @@ -1,15 +1,25 @@ package app -import "context" +import ( + "context" + + archiveapp "github.com/mzner/ocis-cli/internal/app/archive" + archiveclient "github.com/mzner/ocis-cli/internal/archiver" + "github.com/mzner/ocis-cli/internal/sharing" + "github.com/mzner/ocis-cli/internal/webdav" +) // ArchiveDownloadRequest describes one server-side archive download. -type ArchiveDownloadRequest struct { - Paths []string - Destination string - Format string - Overwrite bool - DryRun bool -} +type ArchiveDownloadRequest = archiveapp.Request + +// ArchiveFormat reports one usable server archive format. +type ArchiveFormat = archiveapp.ArchiveFormat + +// ArchiveResource is one selected archive root. +type ArchiveResource = archiveapp.ArchiveResource + +// ArchiveResult describes archive preflight and completed download state. +type ArchiveResult = archiveapp.ArchiveResult // RunArchiveDownloadWithOptions creates and downloads a server-side archive. func RunArchiveDownloadWithOptions( @@ -18,10 +28,11 @@ func RunArchiveDownloadWithOptions( selectedProfile string, options RunOptions, ) error { + options = options.normalized() return classifyProtocolError( "archive download", - runArchiveDownload( - ctx, request, selectedProfile, options.normalized(), + archiveapp.RunDownload( + ctx, request, selectedProfile, archiveOptions(options), ), ) } @@ -33,8 +44,51 @@ func RunArchiveFormatsWithOptions( selectedProfile string, options RunOptions, ) error { + options = options.normalized() return classifyProtocolError( "archive formats", - runArchiveFormats(ctx, selectedProfile, options.normalized()), + archiveapp.RunFormats(ctx, selectedProfile, archiveOptions(options)), ) } + +type archiveClientAdapter struct{ client *client } + +func (adapter archiveClientAdapter) SelectSpace(identifier string) error { + return adapter.client.selectSpace(identifier) +} + +func (adapter archiveClientAdapter) Capabilities( + ctx context.Context, +) (sharing.Capabilities, error) { + return adapter.client.sharingClient().Capabilities(ctx) +} + +func (adapter archiveClientAdapter) Stat(path string) (webdav.Item, error) { + return adapter.client.stat(path) +} + +func (adapter archiveClientAdapter) List(path string) ([]webdav.Item, error) { + return adapter.client.list(path) +} + +func (adapter archiveClientAdapter) Archiver( + endpoint string, +) (*archiveclient.Client, error) { + return adapter.client.archiverClient(endpoint) +} + +func archiveOptions(options RunOptions) archiveapp.Options { + return archiveapp.Options{ + OutputMode: options.OutputMode, Out: options.Out, Err: options.Err, + Quiet: options.Quiet, Space: options.Space, + NewClient: func( + ctx context.Context, selectedProfile string, + ) (archiveapp.Client, error) { + selected, err := newClientWithOptions(ctx, selectedProfile, options) + if err != nil { + return nil, err + } + return archiveClientAdapter{client: selected}, nil + }, + } +} diff --git a/internal/app/auth_service.go b/internal/app/auth_service.go index 79c3850..1012b75 100644 --- a/internal/app/auth_service.go +++ b/internal/app/auth_service.go @@ -30,179 +30,140 @@ func runAuth(ctx context.Context, request AuthRequest, selected string, options if err != nil { return err } - switch request.Operation { - case AuthSetup: - profileName := request.Profile - if profileName == "" { - profileName = selected - } - return setupOIDCClient(ctx, s, profileName, options) - case AuthLogin: - server, name, clientID := request.Server, request.Name, request.ClientID - if request.Profile != "" { - selected = request.Profile + handlers := map[AuthOperation]func() error{ + AuthSetup: func() error { return runAuthSetup(ctx, request, selected, s, options) }, + AuthLogin: func() error { return runAuthLogin(ctx, request, selected, s, options) }, + AuthStatus: func() error { return runAuthStatus(request, selected, s, options) }, + AuthLogout: func() error { return runAuthLogout(request, selected, s, options) }, + } + handler, found := handlers[request.Operation] + if !found { + return apperror.Wrap(apperror.KindUsage, "authentication", fmt.Errorf("unknown auth command %q", request.Operation)) + } + return handler() +} + +func runAuthSetup(ctx context.Context, request AuthRequest, selected string, s *store, options RunOptions) error { + profileName := request.Profile + if profileName == "" { + profileName = selected + } + return setupOIDCClient(ctx, s, profileName, options) +} + +func runAuthStatus(request AuthRequest, selected string, s *store, options RunOptions) error { + profileName := request.Profile + if profileName == "" { + profileName = selected + } + name, p, err := selectProfile(s, profileName) + if err != nil { + return err + } + authenticated := p.Password != "" || p.AccessToken != "" || p.RefreshToken != "" + return output(options, "authentication", map[string]any{"profile": name, "server": p.Server, "username": p.Username, "authType": p.AuthType, "authenticated": authenticated, "expiresAt": p.ExpiresAt}, "%s: %s as %s using %s (authenticated: %t)\n", name, p.Server, p.Username, p.AuthType, authenticated) +} + +func runAuthLogout(request AuthRequest, selected string, s *store, options RunOptions) error { + profileName := request.Profile + if profileName == "" { + profileName = selected + } + name, p, err := selectProfile(s, profileName) + if err != nil { + return err + } + clearAuthenticatedAccount(&p) + s.Profiles[name] = p + if err := saveStore(options.Dependencies, s); err != nil { + return err + } + return output(options, "authentication", map[string]any{"profile": name, "authenticated": false}, "Logged out from %s\n", name) +} + +func runAuthLogin(ctx context.Context, request AuthRequest, selected string, s *store, options RunOptions) error { + server, name, clientID := request.Server, request.Name, request.ClientID + if request.Profile != "" { + selected = request.Profile + } + if server != "" { + if err := validateServerURL(server, request.Insecure); err != nil { + return apperror.Wrap(apperror.KindUsage, "login", err) } - if server != "" { - if err := validateServerURL(server, request.Insecure); err != nil { - return apperror.Wrap(apperror.KindUsage, "login", err) - } + if name == "" { + u, _ := url.Parse(server) + name = strings.ReplaceAll(u.Hostname(), ".", "-") if name == "" { - u, _ := url.Parse(server) - name = strings.ReplaceAll(u.Hostname(), ".", "-") - if name == "" { - name = "ocis" - } - } - if clientID == "" { - clientID = defaultClientID + name = "ocis" } - secret := os.Getenv("OCIS_CLIENT_SECRET") - s.Profiles[name] = profile{ - Server: strings.TrimRight(server, "/"), Insecure: request.Insecure, - ClientID: clientID, ClientSecret: secret, - } - s.Current, selected = name, name - } - name, p, err := selectProfile(s, selected) - if err != nil { - return err - } - if clientID != "" { - p.ClientID = clientID - p.ClientSecret = os.Getenv("OCIS_CLIENT_SECRET") } - if request.Insecure { - p.Insecure = true + if clientID == "" { + clientID = defaultClientID } - // A login without a new --server reuses the stored URL, which a release - // before the https requirement may have saved as cleartext. Checked after - // request.Insecure is applied, so the flag still opts in, and before a new - // password is obtained, a browser is opened, discovery runs, or a probe is - // sent. - if err := validateProfileServerURL(name, p); err != nil { - return err - } - authType := request.Mode - if authType == "" { - authType = "oidc" + s.Profiles[name] = profile{Server: strings.TrimRight(server, "/"), Insecure: request.Insecure, ClientID: clientID, ClientSecret: os.Getenv("OCIS_CLIENT_SECRET")} + s.Current, selected = name, name + } + name, p, err := selectProfile(s, selected) + if err != nil { + return err + } + if clientID != "" { + p.ClientID = clientID + p.ClientSecret = os.Getenv("OCIS_CLIENT_SECRET") + } + if request.Insecure { + p.Insecure = true + } + if err := validateProfileServerURL(name, p); err != nil { + return err + } + mode := request.Mode + if mode == "" { + mode = "oidc" + } + switch mode { + case "oidc": + if request.ACR != "" && !request.MFA { + return apperror.Wrap(apperror.KindUsage, "login", errors.New("--acr requires --mfa")) } - switch authType { - case "oidc": - if request.ACR != "" && !request.MFA { - return apperror.Wrap( - apperror.KindUsage, "login", - errors.New("--acr requires --mfa"), - ) - } - acr := "" - if request.MFA { - acr, err = resolveMFAACR( - ctx, p, request.ACR, options, - ) - if err != nil { - return err - } - } - if err := oidcLogin( - ctx, &p, request.NoBrowser, acr, options, - ); err != nil { - return explainOIDCLoginError(name, p.ClientID, err) - } - p.AuthType, p.Password = "oidc", "" - case "basic": - if request.MFA || request.ACR != "" { - return apperror.Wrap( - apperror.KindUsage, "login", - errors.New( - "MFA step-up requires OIDC authentication", - ), - ) - } - if request.Username == "" { - return apperror.Wrap( - apperror.KindUsage, "login", - errors.New("--username is required with --auth basic"), - ) - } - password, err := obtainPassword(options) + acr := "" + if request.MFA { + acr, err = resolveMFAACR(ctx, p, request.ACR, options) if err != nil { return err } - p.AuthType, p.Username, p.Password = "basic", request.Username, password - p.Subject = "" - p.AccessToken, p.RefreshToken, p.ExpiresAt = "", "", 0 - probe := &client{ - name: name, profile: p, http: httpClientFor(p, options.Timeout), - store: s, ctx: ctx, retries: options.Retries, logger: options.Logger, - } - if _, err := probe.list("/"); err != nil { - return fmt.Errorf("basic authentication failed: %w", err) - } - default: - return apperror.Wrap( - apperror.KindUsage, "login", - fmt.Errorf("unsupported auth mode %q; use oidc or basic", authType), - ) } - clearDefaultSpaceAfterIdentityChange(&p) - s.Profiles[name], s.Current = p, name - if err := saveStore(options.Dependencies, s); err != nil { - return err + if err := oidcLogin(ctx, &p, request.NoBrowser, acr, options); err != nil { + return explainOIDCLoginError(name, p.ClientID, err) } - return output( - options, "authentication", - map[string]any{ - "authenticated": true, "profile": name, "server": p.Server, - "username": p.Username, "authType": p.AuthType, - }, - "Authenticated with %s as %s using %s\n", - p.Server, p.Username, p.AuthType, - ) - case AuthStatus: - profileName := request.Profile - if profileName == "" { - profileName = selected + p.AuthType, p.Password = "oidc", "" + case "basic": + if request.MFA || request.ACR != "" { + return apperror.Wrap(apperror.KindUsage, "login", errors.New("MFA step-up requires OIDC authentication")) } - name, p, err := selectProfile(s, profileName) - if err != nil { - return err + if request.Username == "" { + return apperror.Wrap(apperror.KindUsage, "login", errors.New("--username is required with --auth basic")) } - authenticated := p.Password != "" || p.AccessToken != "" || p.RefreshToken != "" - return output( - options, "authentication", - map[string]any{ - "profile": name, "server": p.Server, "username": p.Username, - "authType": p.AuthType, "authenticated": authenticated, - "expiresAt": p.ExpiresAt, - }, - "%s: %s as %s using %s (authenticated: %t)\n", - name, p.Server, p.Username, p.AuthType, authenticated, - ) - case AuthLogout: - profileName := request.Profile - if profileName == "" { - profileName = selected - } - name, p, err := selectProfile(s, profileName) + password, err := obtainPassword(options) if err != nil { return err } - clearAuthenticatedAccount(&p) - s.Profiles[name] = p - if err := saveStore(options.Dependencies, s); err != nil { - return err + p.AuthType, p.Username, p.Password = "basic", request.Username, password + p.Subject = "" + p.AccessToken, p.RefreshToken, p.ExpiresAt = "", "", 0 + probe := &client{name: name, profile: p, http: httpClientFor(p, options.Timeout), store: s, ctx: ctx, retries: options.Retries, logger: options.Logger} + if _, err := probe.list("/"); err != nil { + return fmt.Errorf("basic authentication failed: %w", err) } - return output( - options, "authentication", - map[string]any{"profile": name, "authenticated": false}, - "Logged out from %s\n", name, - ) default: - return apperror.Wrap( - apperror.KindUsage, "authentication", - fmt.Errorf("unknown auth command %q", request.Operation), - ) + return apperror.Wrap(apperror.KindUsage, "login", fmt.Errorf("unsupported auth mode %q; use oidc or basic", mode)) + } + clearDefaultSpaceAfterIdentityChange(&p) + s.Profiles[name], s.Current = p, name + if err := saveStore(options.Dependencies, s); err != nil { + return err } + return output(options, "authentication", map[string]any{"authenticated": true, "profile": name, "server": p.Server, "username": p.Username, "authType": p.AuthType}, "Authenticated with %s as %s using %s\n", p.Server, p.Username, p.AuthType) } func oidcLogin( diff --git a/internal/app/doctor.go b/internal/app/doctor.go index 33c12f7..b851e1c 100644 --- a/internal/app/doctor.go +++ b/internal/app/doctor.go @@ -6,6 +6,7 @@ import ( "fmt" "strings" + archiveapp "github.com/mzner/ocis-cli/internal/app/archive" "github.com/mzner/ocis-cli/internal/apperror" "github.com/mzner/ocis-cli/internal/credentials" "github.com/mzner/ocis-cli/internal/sharing" @@ -98,12 +99,12 @@ func RunDoctorWithOptions( Detail: resumableUploadCapabilityDetail(features), }) archiveStatus := "unsupported" - if _, err := selectArchiver(features.Files.Archivers); err == nil { + if _, err := archiveapp.SelectCapabilities(features.Files.Archivers); err == nil { archiveStatus = "ok" } checks = append(checks, DoctorCheck{ Name: "archive downloads", Status: archiveStatus, - Detail: archiverCapabilityDetail(features), + Detail: archiveapp.CapabilityDetail(features), }) eventStatus := "unsupported" if features.Core.SupportSSE { diff --git a/internal/app/share_overview_service.go b/internal/app/share/overview.go similarity index 80% rename from internal/app/share_overview_service.go rename to internal/app/share/overview.go index 7befab2..0720921 100644 --- a/internal/app/share_overview_service.go +++ b/internal/app/share/overview.go @@ -1,4 +1,4 @@ -package app +package share import ( "context" @@ -8,6 +8,7 @@ import ( "text/tabwriter" "github.com/mzner/ocis-cli/internal/apperror" + "github.com/mzner/ocis-cli/internal/graph" appoutput "github.com/mzner/ocis-cli/internal/output" "github.com/mzner/ocis-cli/internal/sharing" ) @@ -24,24 +25,7 @@ const ( shareStateDeclined = "declined" ) -// ShareOverviewItem is one stable outgoing or received share inventory row. -type ShareOverviewItem struct { - ShareID string `json:"shareId"` - Direction string `json:"direction"` - State string `json:"state"` - SpaceID string `json:"spaceId,omitempty"` - SpaceName string `json:"spaceName"` - Path string `json:"path,omitempty"` - Type string `json:"type"` - PartyID string `json:"partyId,omitempty"` - PartyName string `json:"partyName"` - Permissions int `json:"permissions"` - Permission string `json:"permission"` - Expiration string `json:"expiration,omitempty"` - PublicLinkURL string `json:"publicLinkUrl,omitempty"` -} - -func validateShareOverviewFilters(request ShareRequest) error { +func validateShareOverviewFilters(request Request) error { direction := normalizeOverviewDirection(request.Direction) state := normalizeOverviewState(request.State) if direction != shareDirectionAll && @@ -71,16 +55,16 @@ func validateShareOverviewFilters(request ShareRequest) error { } func listShareOverview( - ctx context.Context, client *client, request ShareRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { direction := normalizeOverviewDirection(request.Direction) state := normalizeOverviewState(request.State) - spaces, err := client.graphClient().ListMyDrives(ctx) + spaces, err := client.Graph().ListMyDrives(ctx) if err != nil { return err } - var selected *space + var selected *graph.Drive if strings.TrimSpace(options.Space) != "" { value, resolveErr := resolveSpace(spaces, options.Space) if resolveErr != nil { @@ -96,13 +80,13 @@ func listShareOverview( includeOutgoing = false } - items := make([]ShareOverviewItem, 0) + items := make([]OverviewItem, 0) if includeOutgoing { listRequest := sharing.ShareListRequest{} if selected != nil { listRequest.SpaceID = selected.ID } - shares, listErr := client.sharingClient().ListShares(ctx, listRequest) + shares, listErr := client.Sharing().ListShares(ctx, listRequest) if listErr != nil { return listErr } @@ -113,7 +97,7 @@ func listShareOverview( } } if includeReceived { - shares, listErr := client.sharingClient().ListShares( + shares, listErr := client.Sharing().ListShares( ctx, sharing.ShareListRequest{Received: true, AllStates: true}, ) if listErr != nil { @@ -183,7 +167,7 @@ func overviewReceivedStateMatches(value sharing.Share, state string) bool { } } -func overviewSpaceMatches(value sharing.Share, selected *space) bool { +func overviewSpaceMatches(value sharing.Share, selected *graph.Drive) bool { if selected == nil || isReceivedSharesDrive(*selected) { return true } @@ -195,15 +179,15 @@ func shareSpaceMatches(shareSpaceID, selectedSpaceID string) bool { strings.HasPrefix(shareSpaceID, selectedSpaceID+"!") } -func isReceivedSharesDrive(value space) bool { +func isReceivedSharesDrive(value graph.Drive) bool { return value.DriveType == "virtual" && (strings.EqualFold(value.DriveAlias, "virtual/shares") || strings.EqualFold(value.Name, "Shares")) } func overviewItem( - value sharing.Share, direction string, spaces []space, -) ShareOverviewItem { + value sharing.Share, direction string, spaces []graph.Drive, +) OverviewItem { state := "active" partyID := value.RecipientID partyName := fallback(value.RecipientName, value.RecipientID) @@ -225,18 +209,18 @@ func overviewItem( if spaceName == "" { spaceName = "unknown" } - return ShareOverviewItem{ + return OverviewItem{ ShareID: value.ID, Direction: direction, State: state, SpaceID: value.SpaceID, SpaceName: spaceName, Path: value.Path, Type: value.Type, PartyID: partyID, PartyName: partyName, Permissions: value.Permissions, - Permission: permissionName(value.Permissions), + Permission: PermissionName(value.Permissions), Expiration: value.Expiration, PublicLinkURL: value.URL, } } -func overviewSpaceName(spaceID string, spaces []space) string { +func overviewSpaceName(spaceID string, spaces []graph.Drive) string { bestName, bestID := "", "" for _, candidate := range spaces { if shareSpaceMatches(spaceID, candidate.ID) && @@ -247,7 +231,7 @@ func overviewSpaceName(spaceID string, spaces []space) string { return bestName } -func writeShareOverview(items []ShareOverviewItem, options RunOptions) error { +func writeShareOverview(items []OverviewItem, options Options) error { if options.OutputMode != appoutput.Human { return writeOutput(options, "share-overview", items) } diff --git a/internal/app/share/recipient.go b/internal/app/share/recipient.go new file mode 100644 index 0000000..3baebc0 --- /dev/null +++ b/internal/app/share/recipient.go @@ -0,0 +1,126 @@ +package share + +import ( + "context" + "fmt" + "sort" + "strings" + + "github.com/mzner/ocis-cli/internal/graph" +) + +type recipient struct { + ID string + DisplayName string + Username string + Mail string +} + +func resolveRecipient( + ctx context.Context, + client Client, + recipientType string, + identifier string, + isID bool, + usage func(string) error, +) (recipient, error) { + if isID { + return recipient{ID: identifier, DisplayName: identifier}, nil + } + var candidates []recipient + switch recipientType { + case "user": + users, err := client.Graph().SearchUsers(ctx, identifier) + if err != nil { + return recipient{}, err + } + for _, user := range users { + candidates = append(candidates, recipientFromUser(user)) + } + case "group": + groups, err := client.Graph().SearchGroups(ctx, identifier) + if err != nil { + return recipient{}, err + } + for _, group := range groups { + candidates = append(candidates, recipient{ + ID: group.ID, DisplayName: group.DisplayName, + }) + } + } + return selectRecipient(candidates, identifier, recipientType, usage) +} + +func resolveFederatedRecipient( + ctx context.Context, + client Client, + identifier string, + isID bool, +) (recipient, error) { + if isID { + return recipient{ID: identifier, DisplayName: identifier}, nil + } + users, err := client.Graph().SearchFederatedUsers(ctx, identifier) + if err != nil { + return recipient{}, err + } + candidates := make([]recipient, 0, len(users)) + for _, user := range users { + if strings.EqualFold(user.UserType, "Federated") { + candidates = append(candidates, recipientFromUser(user)) + } + } + return selectRecipient(candidates, identifier, "federated user", usageShare) +} + +func recipientFromUser(user graph.DirectoryUser) recipient { + return recipient{ + ID: user.ID, DisplayName: user.DisplayName, + Username: user.Username, Mail: user.Mail, + } +} + +func selectRecipient( + candidates []recipient, + identifier string, + recipientType string, + usage func(string) error, +) (recipient, error) { + var exact []recipient + for _, candidate := range candidates { + if strings.EqualFold(candidate.ID, identifier) || + strings.EqualFold(candidate.DisplayName, identifier) || + strings.EqualFold(candidate.Username, identifier) || + strings.EqualFold(candidate.Mail, identifier) { + exact = append(exact, candidate) + } + } + switch len(exact) { + case 1: + return exact[0], nil + case 0: + if len(candidates) == 1 { + return candidates[0], nil + } + default: + candidates = exact + } + if len(candidates) == 0 { + return recipient{}, usage(fmt.Sprintf( + "no %s matched %q; use --recipient-id with an opaque Graph ID", + recipientType, identifier, + )) + } + labels := make([]string, 0, len(candidates)) + for _, candidate := range candidates { + labels = append(labels, fmt.Sprintf( + "%s (%s)", fallback(candidate.DisplayName, candidate.Username), + candidate.ID, + )) + } + sort.Strings(labels) + return recipient{}, usage(fmt.Sprintf( + "%s %q is ambiguous: %s; use --recipient-id with the intended ID", + recipientType, identifier, strings.Join(labels, ", "), + )) +} diff --git a/internal/app/share_service.go b/internal/app/share/service.go similarity index 84% rename from internal/app/share_service.go rename to internal/app/share/service.go index 1383331..33e5065 100644 --- a/internal/app/share_service.go +++ b/internal/app/share/service.go @@ -1,4 +1,4 @@ -package app +package share import ( "context" @@ -14,38 +14,39 @@ import ( "github.com/mzner/ocis-cli/internal/sharing" ) -func runShare( - ctx context.Context, request ShareRequest, selectedProfile string, options RunOptions, +// Run validates and executes one sharing operation. +func Run( + ctx context.Context, request Request, selectedProfile string, options Options, ) error { options.Logger.Debug("run share operation", "operation", request.Operation) - if request.Operation == ShareCreate { + if request.Operation == Create { if err := validateShareCreateRequest(request); err != nil { return err } } - if request.Operation == ShareLinkUpdate { + if request.Operation == LinkUpdate { if err := validatePublicLinkUpdate(request); err != nil { return err } } - if request.Operation == ShareReceived { + if request.Operation == Received { if _, _, err := receivedShareStateFilter(request.State); err != nil { return err } } - if request.Operation == ShareOverview { + if request.Operation == Overview { if err := validateShareOverviewFilters(request); err != nil { return err } } - if request.Operation == ShareRemove && !request.Confirmed { + if request.Operation == Remove && !request.Confirmed { return usageShare( "removing a share requires explicit confirmation", ) } if request.DryRun { switch request.Operation { - case ShareCreate: + case Create: permissions := request.Permissions if permissions == 0 { permissions = 1 @@ -58,7 +59,7 @@ func runShare( }, "Would create public link for %s\n", cleanRemote(request.Path), ) - case ShareRevoke: + case Revoke: return output( options, "share", map[string]any{"operation": "revoke", "id": request.ID, "dryRun": true}, @@ -66,26 +67,26 @@ func runShare( ) } } - client, err := newClientWithOptions(ctx, selectedProfile, options) + client, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } switch request.Operation { - case ShareCreate, ShareList, ShareDirectAdd, ShareFederatedAdd, ShareRoles: - if err := client.selectSpace(options.Space); err != nil { + case Create, List, DirectAdd, FederatedAdd, Roles: + if err := client.SelectSpace(options.Space); err != nil { return err } - case ShareReceived: + case Received: if options.Space != "" { return usageShare( "--space cannot filter received shares; use the optional received path", ) } - case ShareOverview: + case Overview: // An overview is cross-Space by default. Its optional --space value is // a filter and must not activate or mutate the saved Space selection. - case ShareLinkInfo, ShareLinkUpdate, ShareDirectUpdate, ShareRemove, - ShareAccept, ShareDecline: + case LinkInfo, LinkUpdate, DirectUpdate, Remove, + Accept, Decline: if options.Space != "" { return usageShare( "--space cannot filter an operation addressed by share ID", @@ -93,37 +94,37 @@ func runShare( } } switch request.Operation { - case ShareCreate: + case Create: return createPublicLink(ctx, client, request, options) - case ShareList: + case List: if request.LinksOnly { return listPublicLinks(ctx, client, request, options) } return listOutgoingShares(ctx, client, request, options) - case ShareLinkInfo: + case LinkInfo: return showPublicLink(ctx, client, request.ID, options) - case ShareLinkUpdate: + case LinkUpdate: return updatePublicLink(ctx, client, request, options) - case ShareDirectAdd: + case DirectAdd: return addDirectShare(ctx, client, request, options) - case ShareFederatedAdd: + case FederatedAdd: return addFederatedShare(ctx, client, request, options) - case ShareDirectUpdate: + case DirectUpdate: return updateDirectShare(ctx, client, request, options) - case ShareRemove: + case Remove: return removeShare(ctx, client, request, options) - case ShareReceived: + case Received: return listReceivedShares(ctx, client, request, options) - case ShareOverview: + case Overview: return listShareOverview(ctx, client, request, options) - case ShareAccept, ShareDecline: + case Accept, Decline: return respondToReceivedShare(ctx, client, request, options) - case ShareRoles: + case Roles: return listShareRoles( ctx, client, request.Path, request.Federated, options, ) - case ShareRevoke: - if err := client.sharingClient().RevokeLink(ctx, request.ID); err != nil { + case Revoke: + if err := client.Sharing().RevokeLink(ctx, request.ID); err != nil { return err } return output( @@ -139,7 +140,7 @@ func runShare( } func addDirectShare( - ctx context.Context, client *client, request ShareRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { request.Recipient = strings.TrimSpace(request.Recipient) request.RecipientType = strings.ToLower( @@ -154,7 +155,7 @@ func addDirectShare( request.RecipientType, )) } - capabilities, err := client.sharingClient().Capabilities(ctx) + capabilities, err := client.Sharing().Capabilities(ctx) if err != nil { return fmt.Errorf("check sharing capabilities: %w", err) } @@ -172,7 +173,7 @@ func addDirectShare( ) } remote := cleanRemote(request.Path) - metadata, err := client.stat(remote) + metadata, err := client.Stat(remote) if err != nil { return err } @@ -209,7 +210,7 @@ func addDirectShare( fallback(recipient.DisplayName, recipient.ID), role.DisplayName, ) } - permission, err := client.graphClient().InviteItem( + permission, err := client.Graph().InviteItem( ctx, metadata.ResourceID, graph.InviteRequest{ Recipients: []graph.Recipient{{ @@ -236,13 +237,13 @@ func addDirectShare( } func addFederatedShare( - ctx context.Context, client *client, request ShareRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { request.Recipient = strings.TrimSpace(request.Recipient) if request.Recipient == "" { return usageShare("federated recipient must not be empty") } - capabilities, err := client.sharingClient().Capabilities(ctx) + capabilities, err := client.Sharing().Capabilities(ctx) if err != nil { return fmt.Errorf("check federation capabilities: %w", err) } @@ -253,7 +254,7 @@ func addFederatedShare( ) } remote := cleanRemote(request.Path) - metadata, err := client.stat(remote) + metadata, err := client.Stat(remote) if err != nil { return err } @@ -288,7 +289,7 @@ func addFederatedShare( remote, fallback(recipient.DisplayName, recipient.ID), role.DisplayName, ) } - permission, err := client.graphClient().InviteItem( + permission, err := client.Graph().InviteItem( ctx, metadata.ResourceID, graph.InviteRequest{ Recipients: []graph.Recipient{{ObjectID: recipient.ID, Type: "user"}}, @@ -312,7 +313,7 @@ func addFederatedShare( } func updateDirectShare( - ctx context.Context, client *client, request ShareRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { selected, err := resolveOutgoingShare(ctx, client, request.ID, true) if err != nil { @@ -337,7 +338,7 @@ func updateDirectShare( selected.ID, selected.Path, role.DisplayName, ) } - permission, err := client.graphClient().UpdateItemPermission( + permission, err := client.Graph().UpdateItemPermission( ctx, selected.ResourceID, selected.ID, graph.PermissionUpdateRequest{Roles: []string{role.ID}}, ) @@ -361,7 +362,7 @@ func updateDirectShare( } func removeShare( - ctx context.Context, client *client, request ShareRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { selected, err := resolveOutgoingShare(ctx, client, request.ID, false) if err != nil { @@ -380,11 +381,11 @@ func removeShare( ) } if selected.Type == "public_link" { - if err := client.sharingClient().RevokeLink(ctx, selected.ID); err != nil { + if err := client.Sharing().RevokeLink(ctx, selected.ID); err != nil { return err } } else { - if err := client.graphClient().RemoveItemPermission( + if err := client.Graph().RemoveItemPermission( ctx, selected.ResourceID, selected.ID, ); err != nil { return err @@ -403,11 +404,11 @@ func removeShare( } func listOutgoingShares( - ctx context.Context, client *client, request ShareRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { - values, err := client.sharingClient().ListShares( + values, err := client.Sharing().ListShares( ctx, sharing.ShareListRequest{ - Path: request.Path, SpaceID: client.selectedSpaceID(), + Path: request.Path, SpaceID: client.SelectedSpaceID(), }, ) if err != nil { @@ -417,13 +418,13 @@ func listOutgoingShares( } func listReceivedShares( - ctx context.Context, client *client, request ShareRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { state, allStates, err := receivedShareStateFilter(request.State) if err != nil { return err } - values, err := client.sharingClient().ListShares( + values, err := client.Sharing().ListShares( ctx, sharing.ShareListRequest{ Path: request.Path, Received: true, State: state, AllStates: allStates, @@ -436,7 +437,7 @@ func listReceivedShares( } func writeShares( - values []sharing.Share, received bool, options RunOptions, + values []sharing.Share, received bool, options Options, ) error { if options.OutputMode != appoutput.Human { return writeOutput(options, "share", values) @@ -463,7 +464,7 @@ func writeShares( if _, err := fmt.Fprintf( options.Out, "%-12s %-12s %-10s %-8s %-24s %s\n", value.ID, value.Type, state, - permissionName(value.Permissions), + PermissionName(value.Permissions), value.Path, target, ); err != nil { return err @@ -499,9 +500,9 @@ func receivedShareStateFilter(value string) (*int, bool, error) { } func respondToReceivedShare( - ctx context.Context, client *client, request ShareRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { - values, err := client.sharingClient().ListShares( + values, err := client.Sharing().ListShares( ctx, sharing.ShareListRequest{Received: true, AllStates: true}, ) if err != nil { @@ -521,7 +522,7 @@ func respondToReceivedShare( ) } action, nextState := "accept", "accepted" - if request.Operation == ShareDecline { + if request.Operation == Decline { action, nextState = "decline", "declined" } value := map[string]any{ @@ -536,10 +537,10 @@ func respondToReceivedShare( action, selected.ID, selected.Path, ) } - if request.Operation == ShareAccept { - err = client.sharingClient().AcceptShare(ctx, request.ID) + if request.Operation == Accept { + err = client.Sharing().AcceptShare(ctx, request.ID) } else { - err = client.sharingClient().DeclineShare(ctx, request.ID) + err = client.Sharing().DeclineShare(ctx, request.ID) } if err != nil { return err @@ -553,13 +554,13 @@ func respondToReceivedShare( func listShareRoles( ctx context.Context, - client *client, + client Client, remote string, federated bool, - options RunOptions, + options Options, ) error { if federated { - capabilities, err := client.sharingClient().Capabilities(ctx) + capabilities, err := client.Sharing().Capabilities(ctx) if err != nil { return fmt.Errorf("check federation capabilities: %w", err) } @@ -571,7 +572,7 @@ func listShareRoles( } } remote = cleanRemote(remote) - metadata, err := client.stat(remote) + metadata, err := client.Stat(remote) if err != nil { return err } @@ -582,11 +583,11 @@ func listShareRoles( } var permissions graph.Permissions if federated { - permissions, err = client.graphClient().ListFederatedItemPermissions( + permissions, err = client.Graph().ListFederatedItemPermissions( ctx, metadata.ResourceID, ) } else { - permissions, err = client.graphClient().ListItemPermissions( + permissions, err = client.Graph().ListItemPermissions( ctx, metadata.ResourceID, ) } @@ -608,13 +609,13 @@ func listShareRoles( } func resolveOutgoingShare( - ctx context.Context, client *client, shareID string, directOnly bool, + ctx context.Context, client Client, shareID string, directOnly bool, ) (sharing.Share, error) { shareID = strings.TrimSpace(shareID) if shareID == "" { return sharing.Share{}, usageShare("share ID must not be empty") } - values, err := client.sharingClient().ListShares( + values, err := client.Sharing().ListShares( ctx, sharing.ShareListRequest{}, ) if err != nil { @@ -653,7 +654,7 @@ func resolveOutgoingShare( } func resolveDirectRole( - ctx context.Context, client *client, resourceID string, requested string, + ctx context.Context, client Client, resourceID string, requested string, federated bool, ) (graph.Permissions, graph.RoleDefinition, error) { requested = strings.TrimSpace(requested) @@ -664,11 +665,11 @@ func resolveDirectRole( var permissions graph.Permissions var err error if federated { - permissions, err = client.graphClient().ListFederatedItemPermissions( + permissions, err = client.Graph().ListFederatedItemPermissions( ctx, resourceID, ) } else { - permissions, err = client.graphClient().ListItemPermissions(ctx, resourceID) + permissions, err = client.Graph().ListItemPermissions(ctx, resourceID) } if err != nil { return graph.Permissions{}, graph.RoleDefinition{}, err @@ -756,9 +757,9 @@ type directShareOutput struct { } func createPublicLink( - ctx context.Context, client *client, request ShareRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { - capabilities, err := client.sharingClient().Capabilities(ctx) + capabilities, err := client.Sharing().Capabilities(ctx) if err != nil { return fmt.Errorf("check sharing capabilities: %w", err) } @@ -786,8 +787,8 @@ func createPublicLink( if permissions == 0 { permissions = 1 } - value, err := client.sharingClient().CreateLink(ctx, sharing.CreateRequest{ - Path: request.Path, SpaceID: client.selectedSpaceID(), + value, err := client.Sharing().CreateLink(ctx, sharing.CreateRequest{ + Path: request.Path, SpaceID: client.SelectedSpaceID(), Name: request.Name, Password: request.Password, Expiration: request.Expiration, Permissions: permissions, }) @@ -800,7 +801,7 @@ func createPublicLink( ) } -func validateShareCreateRequest(request ShareRequest) error { +func validateShareCreateRequest(request Request) error { if request.Expiration == "" { return nil } @@ -813,7 +814,7 @@ func validateShareCreateRequest(request ShareRequest) error { return nil } -func validatePublicLinkUpdate(request ShareRequest) error { +func validatePublicLinkUpdate(request Request) error { if !request.UpdateName && !request.UpdateExpiration && !request.UpdateAccess && !request.UpdatePassword { return usageShare("select at least one public-link property to update") @@ -836,7 +837,7 @@ func validatePublicLinkUpdate(request ShareRequest) error { } func showPublicLink( - ctx context.Context, client *client, id string, options RunOptions, + ctx context.Context, client Client, id string, options Options, ) error { value, err := loadPublicLink(ctx, client, id) if err != nil { @@ -846,7 +847,7 @@ func showPublicLink( } func updatePublicLink( - ctx context.Context, client *client, request ShareRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { selected, err := loadPublicLink(ctx, client, request.ID) if err != nil { @@ -888,7 +889,7 @@ func updatePublicLink( changes["expiration"] = request.Expiration } if request.UpdateAccess { - changes["permissions"] = permissionName(request.Permissions) + changes["permissions"] = PermissionName(request.Permissions) } if request.UpdatePassword { changes["password"] = "set" @@ -909,14 +910,14 @@ func updatePublicLink( passwordSet := request.UpdatePassword && !request.RemovePassword if passwordSet { - if _, err := client.graphClient().SetItemPermissionPassword( + if _, err := client.Graph().SetItemPermissionPassword( ctx, selected.ResourceID, selected.ID, request.Password, ); err != nil { return err } } if !update.Empty() { - if _, err := client.graphClient().UpdateLinkPermission( + if _, err := client.Graph().UpdateLinkPermission( ctx, selected.ResourceID, selected.ID, update, ); err != nil { if passwordSet { @@ -929,7 +930,7 @@ func updatePublicLink( } } if request.UpdatePassword && request.RemovePassword { - if _, err := client.graphClient().SetItemPermissionPassword( + if _, err := client.Graph().SetItemPermissionPassword( ctx, selected.ResourceID, selected.ID, "", ); err != nil { if !update.Empty() { @@ -955,13 +956,13 @@ func updatePublicLink( } func loadPublicLink( - ctx context.Context, client *client, id string, + ctx context.Context, client Client, id string, ) (sharing.Link, error) { id = strings.TrimSpace(id) if id == "" { return sharing.Link{}, usageShare("share ID must not be empty") } - value, err := client.sharingClient().GetLink(ctx, id) + value, err := client.Sharing().GetLink(ctx, id) if err != nil { return sharing.Link{}, err } @@ -970,7 +971,7 @@ func loadPublicLink( "server did not return a resource ID for public link %s", id, ) } - permission, err := client.graphClient().GetItemPermission( + permission, err := client.Graph().GetItemPermission( ctx, value.ResourceID, value.ID, ) if err != nil { @@ -998,7 +999,7 @@ func loadPublicLink( return value, nil } -func writePublicLink(value sharing.Link, options RunOptions) error { +func writePublicLink(value sharing.Link, options Options) error { if options.OutputMode != appoutput.Human { return writeOutput(options, "share", value) } @@ -1015,7 +1016,7 @@ func writePublicLink(value sharing.Link, options RunOptions) error { "ID: %s\nPath: %s\nURL: %s\nName: %s\nAccess: %s\n"+ "Expiration: %s\nPassword protected: %t\n", value.ID, value.Path, value.URL, name, - permissionName(value.Permissions), expiration, + PermissionName(value.Permissions), expiration, value.PasswordProtected, ) return err @@ -1050,10 +1051,10 @@ func publicLinkPermissions(linkType string) int { } func listPublicLinks( - ctx context.Context, client *client, request ShareRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { - values, err := client.sharingClient().ListLinks(ctx, sharing.ListRequest{ - Path: request.Path, SpaceID: client.selectedSpaceID(), + values, err := client.Sharing().ListLinks(ctx, sharing.ListRequest{ + Path: request.Path, SpaceID: client.SelectedSpaceID(), }) if err != nil { return err @@ -1068,14 +1069,15 @@ func listPublicLinks( } _, _ = fmt.Fprintf( options.Out, "%-12s %-8s %-12s %-24s %s\n", - value.ID, permissionName(value.Permissions), expiration, + value.ID, PermissionName(value.Permissions), expiration, value.Path, value.URL, ) } return nil } -func permissionName(permissions int) string { +// PermissionName returns a stable human label for an OCS permission mask. +func PermissionName(permissions int) string { switch permissions { case 1: return "read" diff --git a/internal/app/share/service_test.go b/internal/app/share/service_test.go new file mode 100644 index 0000000..9563737 --- /dev/null +++ b/internal/app/share/service_test.go @@ -0,0 +1,112 @@ +package share + +import ( + "strings" + "testing" + + "github.com/mzner/ocis-cli/internal/apperror" + "github.com/mzner/ocis-cli/internal/graph" + "github.com/mzner/ocis-cli/internal/sharing" +) + +func TestRecipientSelection(t *testing.T) { + candidates := []recipient{ + {ID: "one", DisplayName: "Alice", Username: "alice"}, + {ID: "two", DisplayName: "Alice", Username: "alice-two"}, + } + selected, err := selectRecipient(candidates, "alice-two", "user", usageShare) + if err != nil || selected.ID != "two" { + t.Fatalf("selected=%#v error=%v", selected, err) + } + if _, err := selectRecipient(candidates, "Alice", "user", usageShare); err == nil || + !strings.Contains(err.Error(), "ambiguous") { + t.Fatalf("ambiguous error=%v", err) + } + if _, err := selectRecipient(nil, "missing", "user", usageShare); err == nil || + !apperror.IsKind(err, apperror.KindUsage) { + t.Fatalf("missing error=%v", err) + } + selected, err = selectRecipient(candidates[:1], "partial", "user", usageShare) + if err != nil || selected.ID != "one" { + t.Fatalf("single selected=%#v error=%v", selected, err) + } +} + +func TestShareStateAndRoleHelpers(t *testing.T) { + for value, want := range map[string]string{ + "": "current", " rejected ": "declined", "ALL": "all", + } { + if got := normalizeOverviewState(value); got != want { + t.Fatalf("state %q = %q, want %q", value, got, want) + } + } + for _, value := range []string{"", "all", "accepted", "pending", "declined"} { + if _, _, err := receivedShareStateFilter(value); err != nil { + t.Fatalf("state %q: %v", value, err) + } + } + if _, _, err := receivedShareStateFilter("future"); err == nil { + t.Fatal("future state accepted") + } + for permissions, want := range map[int]string{ + 1: "read", 3: "edit", 4: "upload", 5: "upload", 15: "edit", 7: "7", + } { + if got := PermissionName(permissions); got != want { + t.Fatalf("permission %d = %q, want %q", permissions, got, want) + } + } + for value, want := range map[string]string{ + "Viewer": "view", "Can read": "view", "Editor": "edit", + "Uploader": "upload", "Manager": "manage", "Custom": "", + } { + if got := canonicalShareRole(value); got != want { + t.Fatalf("role %q = %q, want %q", value, got, want) + } + } +} + +func TestOverviewAndSpaceHelpers(t *testing.T) { + spaces := []graph.Drive{ + {ID: "personal", Name: "Home", DriveAlias: "personal/alice"}, + {ID: "project", Name: "Project", DriveAlias: "project/work"}, + } + selected, err := resolveSpace(spaces, "project/work") + if err != nil || selected.ID != "project" { + t.Fatalf("selected=%#v error=%v", selected, err) + } + if _, err := resolveSpace(spaces, "missing"); err == nil { + t.Fatal("missing Space accepted") + } + state := 1 + received := sharing.Share{ + ID: "share", State: &state, SpaceID: "project!item", Owner: "owner", + OwnerName: "Owner", Path: "/report.pdf", Type: "user", Permissions: 1, + } + if !overviewReceivedStateMatches(received, shareStatePending) || + overviewReceivedStateMatches(received, shareStateDeclined) { + t.Fatalf("received state mismatch: %#v", received) + } + item := overviewItem(received, shareDirectionReceived, spaces) + if item.SpaceName != "Project" || item.PartyName != "Owner" || + item.Permission != "read" { + t.Fatalf("overview=%#v", item) + } + if !shareSpaceMatches("project!item", "project") || + shareSpaceMatches("another!item", "project") { + t.Fatal("Space matching failed") + } +} + +func TestPublicLinkTypeHelpers(t *testing.T) { + for permissions, want := range map[int]string{ + 1: "view", 5: "upload", 15: "edit", + } { + got, err := publicLinkType(permissions) + if err != nil || got != want || publicLinkPermissions(got) == 0 { + t.Fatalf("permissions %d = %q, %v", permissions, got, err) + } + } + if _, err := publicLinkType(2); err == nil || publicLinkPermissions("future") != 0 { + t.Fatal("unsupported link type accepted") + } +} diff --git a/internal/app/share/types.go b/internal/app/share/types.go new file mode 100644 index 0000000..7c3e21d --- /dev/null +++ b/internal/app/share/types.go @@ -0,0 +1,201 @@ +// Package share contains direct-share, received-share, and public-link +// application policy behind a narrow authenticated client port. +package share + +import ( + "context" + "fmt" + "io" + "strings" + + "github.com/mzner/ocis-cli/internal/apperror" + "github.com/mzner/ocis-cli/internal/graph" + "github.com/mzner/ocis-cli/internal/logging" + appoutput "github.com/mzner/ocis-cli/internal/output" + "github.com/mzner/ocis-cli/internal/sharing" + "github.com/mzner/ocis-cli/internal/webdav" +) + +// Operation identifies one sharing use case. +type Operation string + +const ( + Create Operation = "create" + List Operation = "list" + Revoke Operation = "revoke" + LinkInfo Operation = "link-info" + LinkUpdate Operation = "link-update" + DirectAdd Operation = "direct-add" + FederatedAdd Operation = "federated-add" + DirectUpdate Operation = "direct-update" + Remove Operation = "remove" + Overview Operation = "overview" + Received Operation = "received" + Accept Operation = "accept" + Decline Operation = "decline" + Roles Operation = "roles" +) + +// Request describes one public-link or direct-sharing operation. +type Request struct { + Operation Operation + Path string + ID string + Name string + Password string + UpdateName bool + UpdateExpiration bool + UpdateAccess bool + UpdatePassword bool + RemovePassword bool + Expiration string + Permissions int + Recipient string + RecipientType string + RecipientIsID bool + Role string + Direction string + State string + LinksOnly bool + Confirmed bool + DryRun bool + Federated bool +} + +// OverviewItem is one stable outgoing or received share inventory row. +type OverviewItem struct { + ShareID string `json:"shareId"` + Direction string `json:"direction"` + State string `json:"state"` + SpaceID string `json:"spaceId,omitempty"` + SpaceName string `json:"spaceName"` + Path string `json:"path,omitempty"` + Type string `json:"type"` + PartyID string `json:"partyId,omitempty"` + PartyName string `json:"partyName"` + Permissions int `json:"permissions"` + Permission string `json:"permission"` + Expiration string `json:"expiration,omitempty"` + PublicLinkURL string `json:"publicLinkUrl,omitempty"` +} + +// GraphClient is the LibreGraph functionality used by sharing use cases. +// Keeping this port small makes additions to the protocol client independent +// from the application-domain boundary. +type GraphClient interface { + ListMyDrives(context.Context) ([]graph.Drive, error) + SearchUsers(context.Context, string) ([]graph.DirectoryUser, error) + SearchFederatedUsers(context.Context, string) ([]graph.DirectoryUser, error) + SearchGroups(context.Context, string) ([]graph.DirectoryGroup, error) + ListItemPermissions(context.Context, string) (graph.Permissions, error) + ListFederatedItemPermissions(context.Context, string) (graph.Permissions, error) + InviteItem( + context.Context, string, graph.InviteRequest, + ) (graph.Permission, error) + UpdateItemPermission( + context.Context, string, string, graph.PermissionUpdateRequest, + ) (graph.Permission, error) + GetItemPermission( + context.Context, string, string, + ) (graph.Permission, error) + UpdateLinkPermission( + context.Context, string, string, graph.LinkPermissionUpdateRequest, + ) (graph.Permission, error) + SetItemPermissionPassword( + context.Context, string, string, string, + ) (graph.Permission, error) + RemoveItemPermission(context.Context, string, string) error +} + +// SharingClient is the OCS sharing functionality used by sharing use cases. +type SharingClient interface { + CreateLink(context.Context, sharing.CreateRequest) (sharing.Link, error) + ListLinks(context.Context, sharing.ListRequest) ([]sharing.Link, error) + GetLink(context.Context, string) (sharing.Link, error) + RevokeLink(context.Context, string) error + Capabilities(context.Context) (sharing.Capabilities, error) + ListShares( + context.Context, sharing.ShareListRequest, + ) ([]sharing.Share, error) + AcceptShare(context.Context, string) error + DeclineShare(context.Context, string) error +} + +// Client is the authenticated server functionality used by this domain. +type Client interface { + SelectSpace(string) error + SelectedSpaceID() string + Stat(string) (webdav.Item, error) + Graph() GraphClient + Sharing() SharingClient +} + +// ClientFactory creates an account-bound client without exposing parent app +// runtime internals. +type ClientFactory func(context.Context, string) (Client, error) + +// Options contains the process-boundary values used by share operations. +type Options struct { + OutputMode appoutput.Mode + Out io.Writer + Space string + Logger logging.Logger + NewClient ClientFactory +} + +func output( + options Options, kind string, value any, format string, args ...any, +) error { + return (appoutput.Renderer{ + Writer: options.Out, Mode: options.OutputMode, Type: kind, + }).Write(value, format, args...) +} + +func writeOutput(options Options, kind string, value any) error { + return (appoutput.Renderer{ + Writer: options.Out, Mode: options.OutputMode, Type: kind, + }).Write(value, "") +} + +func cleanRemote(remote string) string { + remote = strings.TrimSpace(remote) + if remote == "" || remote == "/" { + return "/" + } + return "/" + strings.Trim(remote, "/") +} + +func fallback(values ...string) string { + for _, value := range values { + if strings.TrimSpace(value) != "" { + return value + } + } + return "" +} + +func resolveSpace( + spaces []graph.Drive, identifier string, +) (graph.Drive, error) { + var matches []graph.Drive + for _, value := range spaces { + if value.ID == identifier || strings.EqualFold(value.Name, identifier) || + strings.EqualFold(value.DriveAlias, identifier) { + matches = append(matches, value) + } + } + switch len(matches) { + case 1: + return matches[0], nil + case 0: + return graph.Drive{}, apperror.Wrap( + apperror.KindUsage, "space", + fmt.Errorf("unknown space %q; run ocis space list", identifier), + ) + default: + return graph.Drive{}, apperror.Wrap( + apperror.KindUsage, "space", + fmt.Errorf("space name %q is ambiguous; use its ID", identifier), + ) + } +} diff --git a/internal/app/share_adapter.go b/internal/app/share_adapter.go new file mode 100644 index 0000000..0552ee1 --- /dev/null +++ b/internal/app/share_adapter.go @@ -0,0 +1,50 @@ +package app + +import ( + "context" + + shareapp "github.com/mzner/ocis-cli/internal/app/share" + "github.com/mzner/ocis-cli/internal/webdav" +) + +type shareClientAdapter struct{ client *client } + +func (adapter shareClientAdapter) SelectSpace(identifier string) error { + return adapter.client.selectSpace(identifier) +} + +func (adapter shareClientAdapter) SelectedSpaceID() string { + return adapter.client.selectedSpaceID() +} + +func (adapter shareClientAdapter) Stat(path string) (webdav.Item, error) { + return adapter.client.stat(path) +} + +func (adapter shareClientAdapter) Graph() shareapp.GraphClient { + return adapter.client.graphClient() +} + +func (adapter shareClientAdapter) Sharing() shareapp.SharingClient { + return adapter.client.sharingClient() +} + +func shareOptions(options RunOptions) shareapp.Options { + return shareapp.Options{ + OutputMode: options.OutputMode, Out: options.Out, + Space: options.Space, Logger: options.Logger, + NewClient: func( + ctx context.Context, selectedProfile string, + ) (shareapp.Client, error) { + selected, err := newClientWithOptions(ctx, selectedProfile, options) + if err != nil { + return nil, err + } + return shareClientAdapter{client: selected}, nil + }, + } +} + +func permissionName(permissions int) string { + return shareapp.PermissionName(permissions) +} diff --git a/internal/app/space_recipient.go b/internal/app/space_recipient.go index 5e25208..87bb7ef 100644 --- a/internal/app/space_recipient.go +++ b/internal/app/space_recipient.go @@ -65,30 +65,6 @@ func resolveRecipient( return selectRecipient(candidates, identifier, recipientType, usage) } -func resolveFederatedRecipient( - ctx context.Context, - client *client, - identifier string, - isID bool, -) (spaceRecipient, error) { - if isID { - return spaceRecipient{ID: identifier, DisplayName: identifier}, nil - } - users, err := client.graphClient().SearchFederatedUsers(ctx, identifier) - if err != nil { - return spaceRecipient{}, err - } - candidates := make([]spaceRecipient, 0, len(users)) - for _, user := range users { - if strings.EqualFold(user.UserType, "Federated") { - candidates = append(candidates, recipientFromUser(user)) - } - } - return selectRecipient( - candidates, identifier, "federated user", usageShare, - ) -} - func recipientFromUser(user graph.DirectoryUser) spaceRecipient { return spaceRecipient{ ID: user.ID, DisplayName: user.DisplayName, diff --git a/internal/app/bidirectional_sync_service.go b/internal/app/sync/bidirectional.go similarity index 90% rename from internal/app/bidirectional_sync_service.go rename to internal/app/sync/bidirectional.go index 3ff4d88..551711f 100644 --- a/internal/app/bidirectional_sync_service.go +++ b/internal/app/sync/bidirectional.go @@ -1,4 +1,4 @@ -package app +package sync import ( "context" @@ -17,23 +17,23 @@ import ( func runPreparedBidirectionalSync( ctx context.Context, prepared preparedSyncRequest, - client *client, - options RunOptions, + client Client, + options Options, ) error { request := prepared.request - accountID := profileIdentity(client.profile) + accountID := client.AccountID() if accountID == "" { return errors.New("cannot bind sync state to an unauthenticated account") } binding := syncmodel.Binding{ - Profile: client.name, AccountID: accountID, - SpaceID: client.selectedSpaceID(), + Profile: client.ProfileName(), AccountID: accountID, + SpaceID: client.SelectedSpaceID(), Direction: syncmodel.Bidirectional, LocalRoot: prepared.localRoot, RemoteRoot: prepared.remoteRoot, Includes: request.Includes, Excludes: request.Excludes, } stateKey := binding.Key() - previous, found, err := options.Dependencies.SyncStates.Load(stateKey) + previous, found, err := options.SyncStates.Load(stateKey) if err != nil { return fmt.Errorf("load sync state: %w", err) } @@ -72,7 +72,7 @@ func runPreparedBidirectionalSync( ) journal.Status = syncrecovery.Conflict journal.Failure = "automatic keep-both resolution was not safe; both trees were left unchanged" - if saveErr := options.Dependencies.SyncRecoveries.Save(journal); saveErr != nil { + if saveErr := options.SyncRecoveries.Save(journal); saveErr != nil { return errors.Join(err, fmt.Errorf("save sync conflict report: %w", saveErr)) } } @@ -94,12 +94,12 @@ func runPreparedBidirectionalSync( journal.Status = syncrecovery.Conflict journal.Failure = "the plan contains unresolved conflicts" journal.UpdatedAt = time.Now().UTC() - if err := options.Dependencies.SyncRecoveries.Save(journal); err != nil { + if err := options.SyncRecoveries.Save(journal); err != nil { return fmt.Errorf("save sync conflict report: %w", err) } return bidirectionalSyncConflict(plan) } - if err := options.Dependencies.SyncRecoveries.Save(journal); err != nil { + if err := options.SyncRecoveries.Save(journal); err != nil { return fmt.Errorf("create sync recovery journal: %w", err) } if err := applyBidirectionalSyncPlan( @@ -160,7 +160,7 @@ func runPreparedBidirectionalSync( ) } state := syncmodel.NewState(binding, postLocal, postRemote) - if err := options.Dependencies.SyncStates.Save( + if err := options.SyncStates.Save( stateKey, state, ); err != nil { _ = updateSyncRecovery( @@ -170,7 +170,7 @@ func runPreparedBidirectionalSync( ) return fmt.Errorf("save sync state: %w", err) } - if _, err := options.Dependencies.SyncRecoveries.Delete(journal.ID); err != nil { + if _, err := options.SyncRecoveries.Delete(journal.ID); err != nil { return fmt.Errorf("remove completed sync recovery journal: %w", err) } result.Applied = true @@ -179,12 +179,12 @@ func runPreparedBidirectionalSync( func applyBidirectionalSyncPlan( ctx context.Context, - client *client, + client Client, localRoot string, remoteRoot string, plan syncmodel.Plan, journal *syncrecovery.Journal, - options RunOptions, + options Options, ) error { uploadCapabilities := webdav.TUSCapabilities{} for _, action := range plan.Actions { @@ -209,7 +209,7 @@ func applyBidirectionalSyncPlan( current := action journal.Current = ¤t journal.UpdatedAt = time.Now().UTC() - if err := options.Dependencies.SyncRecoveries.Save(*journal); err != nil { + if err := options.SyncRecoveries.Save(*journal); err != nil { return fmt.Errorf("record pending sync action: %w", err) } var direction syncmodel.Direction @@ -237,7 +237,7 @@ func applyBidirectionalSyncPlan( journal.Completed = append(journal.Completed, action) journal.Current = nil journal.UpdatedAt = time.Now().UTC() - if err := options.Dependencies.SyncRecoveries.Save(*journal); err != nil { + if err := options.SyncRecoveries.Save(*journal); err != nil { return fmt.Errorf("record completed sync action: %w", err) } } @@ -248,16 +248,16 @@ func updateSyncRecovery( journal *syncrecovery.Journal, status syncrecovery.Status, failure string, - options RunOptions, + options Options, ) error { journal.Status = status journal.Failure = failure journal.UpdatedAt = time.Now().UTC() - return options.Dependencies.SyncRecoveries.Save(*journal) + return options.SyncRecoveries.Save(*journal) } func verifyBidirectionalSourcePrecondition( - client *client, + client Client, localRoot string, remoteRoot string, action syncmodel.Action, @@ -331,7 +331,7 @@ func bidirectionalSyncConflict(plan syncmodel.Plan) error { func collectBidirectionalSnapshots( ctx context.Context, - client *client, + client Client, localRoot string, remoteRoot string, maxEntries int, diff --git a/internal/app/sync_conflict_service.go b/internal/app/sync/conflict.go similarity index 98% rename from internal/app/sync_conflict_service.go rename to internal/app/sync/conflict.go index 9051c05..4417042 100644 --- a/internal/app/sync_conflict_service.go +++ b/internal/app/sync/conflict.go @@ -1,4 +1,4 @@ -package app +package sync import ( "crypto/sha256" @@ -16,7 +16,7 @@ func resolveBidirectionalConflicts( plan syncmodel.Plan, local syncmodel.Snapshot, remote syncmodel.Snapshot, - request SyncRequest, + request Request, ) (syncmodel.Plan, error) { strategy := request.ConflictStrategy if strategy == "" || strategy == "abort" || plan.Conflicts == 0 { @@ -146,7 +146,7 @@ func ensureConflictCopyPathAvailable( relative string, local syncmodel.Snapshot, remote syncmodel.Snapshot, - request SyncRequest, + request Request, ) error { if _, exists := local[relative]; exists { return apperror.Wrap( diff --git a/internal/app/sync/helpers.go b/internal/app/sync/helpers.go new file mode 100644 index 0000000..a4e146c --- /dev/null +++ b/internal/app/sync/helpers.go @@ -0,0 +1,22 @@ +package sync + +import ( + "path" + "strings" + + appoutput "github.com/mzner/ocis-cli/internal/output" +) + +func output(options Options, kind string, value any, format string, args ...any) error { + return (appoutput.Renderer{Writer: options.Out, Mode: options.OutputMode, Type: kind}).Write(value, format, args...) +} +func writeOutput(options Options, kind string, value any) error { + return output(options, kind, value, "") +} +func cleanRemote(remote string) string { + remote = strings.TrimSpace(remote) + if remote == "" || remote == "/" { + return "/" + } + return path.Clean("/" + strings.Trim(remote, "/")) +} diff --git a/internal/app/sync_job_service.go b/internal/app/sync/job.go similarity index 86% rename from internal/app/sync_job_service.go rename to internal/app/sync/job.go index 724ba03..901ab0d 100644 --- a/internal/app/sync_job_service.go +++ b/internal/app/sync/job.go @@ -1,4 +1,4 @@ -package app +package sync import ( "context" @@ -19,36 +19,36 @@ type syncJobRemoval struct { DryRun bool `json:"dryRun"` } -func runSyncJob( +func RunJob( ctx context.Context, - request SyncJobRequest, - options RunOptions, + request JobRequest, + options Options, ) error { if err := ctx.Err(); err != nil { return err } - store, err := options.Dependencies.SyncJobs.Load() + store, err := options.SyncJobs.Load() if err != nil { return fmt.Errorf("load sync jobs: %w", err) } switch request.Operation { - case SyncJobAdd: + case JobAdd: return addSyncJob(ctx, request, store, options) - case SyncJobList: + case JobList: return listSyncJobs(request.Profile, store, options) - case SyncJobShow: + case JobShow: job, err := findSyncJob(request.Name, store) if err != nil { return err } return writeSyncJob(job, options) - case SyncJobRun: + case JobRun: job, err := findSyncJob(request.Name, store) if err != nil { return err } return executeSyncJob(ctx, request, job, options) - case SyncJobRemove: + case JobRemove: return removeSyncJob(request, store, options) default: return apperror.Wrap( @@ -60,9 +60,9 @@ func runSyncJob( func addSyncJob( ctx context.Context, - request SyncJobRequest, + request JobRequest, store syncjob.Store, - options RunOptions, + options Options, ) error { if err := syncjob.ValidateName(request.Name); err != nil { return apperror.Wrap(apperror.KindUsage, "sync job add", err) @@ -76,7 +76,7 @@ func addSyncJob( ), ) } - prepared, err := prepareSyncRequest(SyncRequest{ + prepared, err := prepareSyncRequest(Request{ Direction: request.Direction, LocalRoot: request.LocalRoot, RemoteRoot: request.RemoteRoot, Includes: request.Includes, Excludes: request.Excludes, @@ -86,20 +86,20 @@ func addSyncJob( if err != nil { return err } - client, err := newClientWithOptions(ctx, request.Profile, options) + client, err := options.NewClient(ctx, request.Profile) if err != nil { return err } - if err := client.selectSpace(request.Space); err != nil { + if err := client.SelectSpace(request.Space); err != nil { return err } - accountID := profileIdentity(client.profile) + accountID := client.AccountID() if accountID == "" { return errors.New("cannot bind a sync job to an unauthenticated account") } job := syncjob.Job{ - Name: request.Name, Profile: client.name, AccountID: accountID, - SpaceID: client.selectedSpaceID(), Direction: prepared.direction, + Name: request.Name, Profile: client.ProfileName(), AccountID: accountID, + SpaceID: client.SelectedSpaceID(), Direction: prepared.direction, LocalRoot: prepared.localRoot, RemoteRoot: prepared.remoteRoot, Includes: append([]string(nil), prepared.request.Includes...), Excludes: append([]string(nil), prepared.request.Excludes...), @@ -111,7 +111,7 @@ func addSyncJob( } store = cloneSyncJobStore(store) store.Jobs[job.Name] = job - if err := options.Dependencies.SyncJobs.Save(store); err != nil { + if err := options.SyncJobs.Save(store); err != nil { return fmt.Errorf("save sync jobs: %w", err) } return output( @@ -124,7 +124,7 @@ func addSyncJob( func listSyncJobs( profile string, store syncjob.Store, - options RunOptions, + options Options, ) error { names := make([]string, 0, len(store.Jobs)) for name, job := range store.Jobs { @@ -167,7 +167,7 @@ func listSyncJobs( return writer.Flush() } -func writeSyncJob(job syncjob.Job, options RunOptions) error { +func writeSyncJob(job syncjob.Job, options Options) error { if options.OutputMode != appoutput.Human { return writeOutput(options, "sync-job", job) } @@ -203,9 +203,9 @@ func writeSyncJob(job syncjob.Job, options RunOptions) error { func executeSyncJob( ctx context.Context, - request SyncJobRequest, + request JobRequest, job syncjob.Job, - options RunOptions, + options Options, ) error { if request.Profile != "" && request.Profile != job.Profile { return apperror.Wrap( @@ -224,8 +224,8 @@ func executeSyncJob( ), ) } - prepared, err := prepareSyncRequest(SyncRequest{ - Direction: SyncDirection(job.Direction), + prepared, err := prepareSyncRequest(Request{ + Direction: Direction(job.Direction), LocalRoot: job.LocalRoot, RemoteRoot: job.RemoteRoot, Includes: job.Includes, Excludes: job.Excludes, Delete: job.Delete, Overwrite: job.Overwrite, @@ -234,11 +234,11 @@ func executeSyncJob( if err != nil { return fmt.Errorf("invalid saved sync job %q: %w", job.Name, err) } - client, err := newClientWithOptions(ctx, job.Profile, options) + client, err := options.NewClient(ctx, job.Profile) if err != nil { return err } - currentIdentity := profileIdentity(client.profile) + currentIdentity := client.AccountID() if currentIdentity != job.AccountID { return apperror.Wrap( apperror.KindAuthentication, "sync job run", @@ -250,23 +250,23 @@ func executeSyncJob( ) } if job.SpaceID != "" { - if err := client.selectSpace(job.SpaceID); err != nil { + if err := client.SelectSpace(job.SpaceID); err != nil { return fmt.Errorf( "sync job %q Space %q is unavailable: %w", job.Name, job.SpaceID, err, ) } } - if client.selectedSpaceID() != job.SpaceID { + if client.SelectedSpaceID() != job.SpaceID { return errors.New("resolved sync-job Space does not match its binding") } return runPreparedSync(ctx, prepared, client, options) } func removeSyncJob( - request SyncJobRequest, + request JobRequest, store syncjob.Store, - options RunOptions, + options Options, ) error { if !request.Confirmed && !request.DryRun { return apperror.Wrap( @@ -282,7 +282,7 @@ func removeSyncJob( if !request.DryRun { store = cloneSyncJobStore(store) delete(store.Jobs, job.Name) - if err := options.Dependencies.SyncJobs.Save(store); err != nil { + if err := options.SyncJobs.Save(store); err != nil { return fmt.Errorf("save sync jobs: %w", err) } result.Removed = true diff --git a/internal/app/sync_recovery_service.go b/internal/app/sync/recovery.go similarity index 86% rename from internal/app/sync_recovery_service.go rename to internal/app/sync/recovery.go index 4b87ac4..4bbab92 100644 --- a/internal/app/sync_recovery_service.go +++ b/internal/app/sync/recovery.go @@ -1,4 +1,4 @@ -package app +package sync import ( "context" @@ -20,30 +20,30 @@ type syncRecoveryRemoval struct { DryRun bool `json:"dryRun"` } -func runSyncRecovery( +func RunRecovery( ctx context.Context, - request SyncRecoveryRequest, - options RunOptions, + request RecoveryRequest, + options Options, ) error { if err := ctx.Err(); err != nil { return err } switch request.Operation { - case SyncRecoveryList: + case RecoveryList: return listSyncRecoveries(request.Profile, options) - case SyncRecoveryShow: + case RecoveryShow: journal, err := loadSyncRecovery(request.ID, options) if err != nil { return err } return writeSyncRecovery(journal, options) - case SyncRecoveryRetry: + case RecoveryRetry: journal, err := loadSyncRecovery(request.ID, options) if err != nil { return err } return retrySyncRecovery(ctx, request, journal, options) - case SyncRecoveryRemove: + case RecoveryRemove: return removeSyncRecovery(request, options) default: return apperror.Wrap( @@ -53,14 +53,14 @@ func runSyncRecovery( } } -func listSyncRecoveries(profile string, options RunOptions) error { - keys, err := options.Dependencies.SyncRecoveries.Keys() +func listSyncRecoveries(profile string, options Options) error { + keys, err := options.SyncRecoveries.Keys() if err != nil { return fmt.Errorf("list sync recovery journals: %w", err) } journals := make([]syncrecovery.Journal, 0, len(keys)) for _, key := range keys { - journal, found, err := options.Dependencies.SyncRecoveries.Load(key) + journal, found, err := options.SyncRecoveries.Load(key) if err != nil { return fmt.Errorf("load sync recovery %s: %w", key, err) } @@ -100,7 +100,7 @@ func listSyncRecoveries(profile string, options RunOptions) error { func writeSyncRecovery( journal syncrecovery.Journal, - options RunOptions, + options Options, ) error { if options.OutputMode != appoutput.Human { return writeOutput(options, "sync-recovery", journal) @@ -137,9 +137,9 @@ func writeSyncRecovery( func retrySyncRecovery( ctx context.Context, - request SyncRecoveryRequest, + request RecoveryRequest, journal syncrecovery.Journal, - options RunOptions, + options Options, ) error { if request.Profile != "" && request.Profile != journal.Binding.Profile { return apperror.Wrap( @@ -150,26 +150,26 @@ func retrySyncRecovery( ), ) } - client, err := newClientWithOptions(ctx, journal.Binding.Profile, options) + client, err := options.NewClient(ctx, journal.Binding.Profile) if err != nil { return err } - if profileIdentity(client.profile) != journal.Binding.AccountID { + if client.AccountID() != journal.Binding.AccountID { return apperror.Wrap( apperror.KindAuthentication, "sync recovery retry", errors.New("the recovery journal belongs to a different authenticated account"), ) } if journal.Binding.SpaceID != "" { - if err := client.selectSpace(journal.Binding.SpaceID); err != nil { + if err := client.SelectSpace(journal.Binding.SpaceID); err != nil { return err } } - if client.selectedSpaceID() != journal.Binding.SpaceID { + if client.SelectedSpaceID() != journal.Binding.SpaceID { return errors.New("resolved recovery Space does not match its binding") } - prepared, err := prepareSyncRequest(SyncRequest{ - Direction: SyncBidirectional, + prepared, err := prepareSyncRequest(Request{ + Direction: Bidirectional, LocalRoot: journal.Binding.LocalRoot, RemoteRoot: journal.Binding.RemoteRoot, Includes: journal.Binding.Includes, @@ -184,8 +184,8 @@ func retrySyncRecovery( } func removeSyncRecovery( - request SyncRecoveryRequest, - options RunOptions, + request RecoveryRequest, + options Options, ) error { if !request.Confirmed && !request.DryRun { return apperror.Wrap( @@ -199,7 +199,7 @@ func removeSyncRecovery( } result := syncRecoveryRemoval{ID: journal.ID, DryRun: request.DryRun} if !request.DryRun { - removed, err := options.Dependencies.SyncRecoveries.Delete(journal.ID) + removed, err := options.SyncRecoveries.Delete(journal.ID) if err != nil { return fmt.Errorf("remove sync recovery journal: %w", err) } @@ -217,13 +217,13 @@ func removeSyncRecovery( func loadSyncRecovery( id string, - options RunOptions, + options Options, ) (syncrecovery.Journal, error) { key, err := resolveSyncRecoveryID(id, options) if err != nil { return syncrecovery.Journal{}, err } - journal, found, err := options.Dependencies.SyncRecoveries.Load(key) + journal, found, err := options.SyncRecoveries.Load(key) if err != nil { return syncrecovery.Journal{}, err } @@ -238,7 +238,7 @@ func loadSyncRecovery( func resolveSyncRecoveryID( query string, - options RunOptions, + options Options, ) (string, error) { query = strings.ToLower(strings.TrimSpace(query)) if len(query) < syncStateIDMinimumPrefix || @@ -251,7 +251,7 @@ func resolveSyncRecoveryID( ), ) } - keys, err := options.Dependencies.SyncRecoveries.Keys() + keys, err := options.SyncRecoveries.Keys() if err != nil { return "", err } diff --git a/internal/app/sync_service.go b/internal/app/sync/service.go similarity index 92% rename from internal/app/sync_service.go rename to internal/app/sync/service.go index 57a95ef..f4e414a 100644 --- a/internal/app/sync_service.go +++ b/internal/app/sync/service.go @@ -1,4 +1,4 @@ -package app +package sync import ( "context" @@ -38,33 +38,33 @@ type syncResult struct { } type preparedSyncRequest struct { - request SyncRequest + request Request direction syncmodel.Direction localRoot string remoteRoot string } -func runSync( +func Run( ctx context.Context, - request SyncRequest, + request Request, selected string, - options RunOptions, + options Options, ) error { prepared, err := prepareSyncRequest(request) if err != nil { return err } - client, err := newClientWithOptions(ctx, selected, options) + client, err := options.NewClient(ctx, selected) if err != nil { return err } - if err := client.selectSpace(options.Space); err != nil { + if err := client.SelectSpace(options.Space); err != nil { return err } return runPreparedSync(ctx, prepared, client, options) } -func prepareSyncRequest(request SyncRequest) (preparedSyncRequest, error) { +func prepareSyncRequest(request Request) (preparedSyncRequest, error) { direction, err := validateSyncRequest(request) if err != nil { return preparedSyncRequest{}, apperror.Wrap( @@ -95,8 +95,8 @@ func prepareSyncRequest(request SyncRequest) (preparedSyncRequest, error) { func runPreparedSync( ctx context.Context, prepared preparedSyncRequest, - client *client, - options RunOptions, + client Client, + options Options, ) error { if prepared.direction == syncmodel.Bidirectional { return runPreparedBidirectionalSync(ctx, prepared, client, options) @@ -105,18 +105,18 @@ func runPreparedSync( direction := prepared.direction localRoot := prepared.localRoot remoteRoot := prepared.remoteRoot - accountID := profileIdentity(client.profile) + accountID := client.AccountID() if accountID == "" { return errors.New("cannot bind sync state to an unauthenticated account") } binding := syncmodel.Binding{ - Profile: client.name, AccountID: accountID, - SpaceID: client.selectedSpaceID(), Direction: direction, + Profile: client.ProfileName(), AccountID: accountID, + SpaceID: client.SelectedSpaceID(), Direction: direction, LocalRoot: localRoot, RemoteRoot: remoteRoot, Includes: request.Includes, Excludes: request.Excludes, } stateKey := binding.Key() - previous, found, err := options.Dependencies.SyncStates.Load(stateKey) + previous, found, err := options.SyncStates.Load(stateKey) if err != nil { return fmt.Errorf("load sync state: %w", err) } @@ -181,7 +181,7 @@ func runPreparedSync( return err } state := syncmodel.NewState(binding, postSource, postDestination) - if err := options.Dependencies.SyncStates.Save(stateKey, state); err != nil { + if err := options.SyncStates.Save(stateKey, state); err != nil { return fmt.Errorf("save sync state: %w", err) } result.Applied = true @@ -189,15 +189,15 @@ func runPreparedSync( } func validateSyncRequest( - request SyncRequest, + request Request, ) (syncmodel.Direction, error) { var direction syncmodel.Direction switch request.Direction { - case SyncPush: + case Push: direction = syncmodel.Push - case SyncPull: + case Pull: direction = syncmodel.Pull - case SyncBidirectional: + case Bidirectional: direction = syncmodel.Bidirectional default: return "", fmt.Errorf("unknown sync direction %q", request.Direction) @@ -243,7 +243,7 @@ func validateSyncRequest( func collectSyncSnapshots( ctx context.Context, - client *client, + client Client, direction syncmodel.Direction, localRoot, remoteRoot string, maxEntries int, @@ -345,12 +345,12 @@ func snapshotLocal( func snapshotRemote( ctx context.Context, - client *client, + client Client, root string, allowMissing bool, maxEntries int, ) (syncmodel.Snapshot, error) { - rootItem, err := client.stat(root) + rootItem, err := client.Stat(root) if webdav.StatusCode(err) == 404 && allowMissing { return syncmodel.Snapshot{}, nil } @@ -368,7 +368,7 @@ func snapshotRemote( if err := ctx.Err(); err != nil { return err } - children, err := client.list(remote) + children, err := client.List(remote) if err != nil { return err } @@ -402,7 +402,7 @@ func snapshotRemote( return result, nil } -func remoteSyncEntry(relative string, value item) syncmodel.Entry { +func remoteSyncEntry(relative string, value webdav.Item) syncmodel.Entry { checksum := "" for _, candidate := range value.Checksums { if strings.EqualFold(candidate.Algorithm, "SHA1") { @@ -422,15 +422,15 @@ func remoteSyncEntry(relative string, value item) syncmodel.Entry { func applySyncPlan( ctx context.Context, - client *client, + client Client, direction syncmodel.Direction, localRoot, remoteRoot string, plan syncmodel.Plan, - options RunOptions, + options Options, ) error { uploadCapabilities := webdav.TUSCapabilities{} if direction == syncmodel.Push { - uploadCapabilities = discoverUploadCapabilities(ctx, client) + uploadCapabilities = client.DiscoverUploadCapabilities(ctx) } for _, action := range plan.Actions { if err := ctx.Err(); err != nil { @@ -455,12 +455,12 @@ func applySyncPlan( } func applySyncAction( - client *client, + client Client, direction syncmodel.Direction, localRoot, remoteRoot string, action syncmodel.Action, uploadCapabilities webdav.TUSCapabilities, - options RunOptions, + options Options, ) error { local, err := syncLocalPath(localRoot, action.Path) if err != nil { @@ -493,8 +493,8 @@ func applySyncAction( return err } if action.Replace { - if err := client.davClient().RemoveWithOptions( - client.context(), remote, webdav.RemoveOptions{ + if err := client.DAV().RemoveWithOptions( + client.Context(), remote, webdav.RemoveOptions{ Recursive: true, ExpectedETag: syncExpectedETag(action.Destination), }, @@ -504,17 +504,17 @@ func applySyncAction( } switch action.Action { case syncmodel.ActionDelete: - return client.davClient().RemoveWithOptions( - client.context(), remote, webdav.RemoveOptions{ + return client.DAV().RemoveWithOptions( + client.Context(), remote, webdav.RemoveOptions{ Recursive: true, ExpectedETag: syncExpectedETag(action.Destination), }, ) case syncmodel.ActionCreateDirectory: - return client.ensureCollection(remote) + return client.EnsureCollection(remote) case syncmodel.ActionTransfer: - return client.davClient().UploadWithOptions( - client.context(), localSource, remote, + return client.DAV().UploadWithOptions( + client.Context(), localSource, remote, webdav.TransferOptions{ NoClobber: action.Destination.Type == "", Verify: true, TUS: uploadCapabilities, @@ -541,8 +541,8 @@ func applySyncAction( if err := os.MkdirAll(filepath.Dir(local), 0750); err != nil { return err } - return client.davClient().DownloadWithOptions( - client.context(), remoteSource, local, + return client.DAV().DownloadWithOptions( + client.Context(), remoteSource, local, webdav.TransferOptions{ NoClobber: action.Destination.Type == "", Resume: true, Verify: true, @@ -555,7 +555,7 @@ func applySyncAction( } func applySyncCopy( - client *client, + client Client, direction syncmodel.Direction, localSource string, remoteSource string, @@ -577,8 +577,8 @@ func applySyncCopy( ); err != nil { return err } - return client.davClient().CopyWithOptions( - client.context(), remoteSource, remoteDestination, + return client.DAV().CopyWithOptions( + client.Context(), remoteSource, remoteDestination, webdav.MoveOptions{ ExpectedETag: syncExpectedETag(action.Source), }, @@ -630,7 +630,7 @@ func copyLocalSyncFile(source, destination string) error { } func applySyncMove( - client *client, + client Client, direction syncmodel.Direction, localRoot string, remoteRoot string, @@ -653,8 +653,8 @@ func applySyncMove( ); err != nil { return err } - return client.davClient().MoveWithOptions( - client.context(), remoteSource, remoteDestination, + return client.DAV().MoveWithOptions( + client.Context(), remoteSource, remoteDestination, webdav.MoveOptions{ ExpectedETag: syncExpectedETag(action.Destination), }, @@ -681,7 +681,7 @@ func applySyncMove( } func verifyRemoteSyncPrecondition( - client *client, + client Client, remote string, expected syncmodel.Entry, ) error { @@ -691,7 +691,7 @@ func verifyRemoteSyncPrecondition( } func verifyRemoteSyncSourcePrecondition( - client *client, + client Client, remote string, expected syncmodel.Entry, ) error { @@ -699,12 +699,12 @@ func verifyRemoteSyncSourcePrecondition( } func verifyRemoteSyncEntry( - client *client, + client Client, remote string, expected syncmodel.Entry, role string, ) error { - current, err := client.stat(remote) + current, err := client.Stat(remote) if webdav.StatusCode(err) == 404 && expected.Type == "" { return nil } @@ -862,7 +862,7 @@ func syncConflict(plan syncmodel.Plan) error { ) } -func writeSyncResult(options RunOptions, result syncResult) error { +func writeSyncResult(options Options, result syncResult) error { if options.OutputMode != appoutput.Human { return writeOutput(options, "sync-plan", result) } diff --git a/internal/app/sync/service_test.go b/internal/app/sync/service_test.go new file mode 100644 index 0000000..fe7af64 --- /dev/null +++ b/internal/app/sync/service_test.go @@ -0,0 +1,65 @@ +package sync + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + syncmodel "github.com/mzner/ocis-cli/internal/sync" + "github.com/mzner/ocis-cli/internal/syncjob" +) + +func TestDeletionFiltersAndValidation(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "report.md"), []byte("report"), 0600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(root, "report.md"), filepath.Join(root, "link")); err != nil { + t.Fatal(err) + } + if _, err := snapshotLocal(context.Background(), root, false, 100); err == nil || !strings.Contains(err.Error(), "unsupported local file type") { + t.Fatalf("symlink error: %v", err) + } + if _, err := snapshotLocal(context.Background(), root, false, 1); err == nil || !strings.Contains(err.Error(), "--max-entries") { + t.Fatalf("entry-limit error: %v", err) + } + for _, request := range []Request{ + {Direction: Direction("sideways"), LocalRoot: root, RemoteRoot: "/", MaxEntries: 1}, + {Direction: Push, RemoteRoot: "/", MaxEntries: 1}, + {Direction: Pull, LocalRoot: root, MaxEntries: 1}, + {Direction: Pull, LocalRoot: root, RemoteRoot: "/", MaxEntries: 0}, + } { + if _, err := validateSyncRequest(request); err == nil { + t.Fatalf("request accepted: %#v", request) + } + } + if direction, err := validateSyncRequest(Request{Direction: Bidirectional, LocalRoot: root, RemoteRoot: "/", MaxEntries: 1}); err != nil || direction != syncmodel.Bidirectional { + t.Fatalf("bidirectional request: direction=%q err=%v", direction, err) + } + if _, err := syncLocalPath(root, "../../escape"); err == nil { + t.Fatal("escaping local sync path accepted") + } + if err := validateSyncRemoteName("../escape"); err == nil { + t.Fatal("unsafe remote name accepted") + } +} + +func TestJobRootDisplay(t *testing.T) { + job := syncjob.Job{Direction: syncmodel.Pull, LocalRoot: "/local", RemoteRoot: "/remote"} + if got := syncJobRoots(job); got != "/remote -> /local" { + t.Fatalf("roots=%q", got) + } +} + +func TestUniqueStateID(t *testing.T) { + first := "123456789012a" + strings.Repeat("0", 51) + second := "123456789012b" + strings.Repeat("0", 51) + if got := uniqueSyncStateID(first, []string{first, second}); got != first[:13] { + t.Fatalf("unique ID=%q", got) + } + if got := uniqueSyncStateID(first, []string{first}); got != first[:12] { + t.Fatalf("single ID=%q", got) + } +} diff --git a/internal/app/sync_state_service.go b/internal/app/sync/state.go similarity index 92% rename from internal/app/sync_state_service.go rename to internal/app/sync/state.go index 2f25025..7fb83a3 100644 --- a/internal/app/sync_state_service.go +++ b/internal/app/sync/state.go @@ -1,4 +1,4 @@ -package app +package sync import ( "context" @@ -46,18 +46,18 @@ type syncStateRemoval struct { DryRun bool `json:"dryRun"` } -func runSyncState( +func RunState( ctx context.Context, - request SyncStateRequest, - options RunOptions, + request StateRequest, + options Options, ) error { if err := ctx.Err(); err != nil { return err } switch request.Operation { - case SyncStateList: + case StateList: return listSyncStates(request.Profile, options) - case SyncStateShow: + case StateShow: key, state, err := resolveAndLoadSyncState(request.ID, options) if err != nil { return err @@ -65,7 +65,7 @@ func runSyncState( return writeSyncStateSummary( options, syncStateSummaryFor(key, state, nil), ) - case SyncStateExport: + case StateExport: if options.OutputMode != appoutput.Human { return apperror.Wrap( apperror.KindUsage, "sync state export", @@ -79,7 +79,7 @@ func runSyncState( return err } return exportSyncState(options.Out, key, state) - case SyncStateRemove: + case StateRemove: return removeSyncState(request, options) default: return apperror.Wrap( @@ -89,14 +89,14 @@ func runSyncState( } } -func listSyncStates(profile string, options RunOptions) error { - keys, err := options.Dependencies.SyncStates.Keys() +func listSyncStates(profile string, options Options) error { + keys, err := options.SyncStates.Keys() if err != nil { return fmt.Errorf("list sync state: %w", err) } summaries := make([]syncStateSummary, 0, len(keys)) for _, key := range keys { - state, found, loadErr := options.Dependencies.SyncStates.Load(key) + state, found, loadErr := options.SyncStates.Load(key) if !found && loadErr == nil { continue } @@ -142,7 +142,7 @@ func listSyncStates(profile string, options RunOptions) error { } func writeSyncStateSummary( - options RunOptions, + options Options, summary syncStateSummary, ) error { if options.OutputMode != appoutput.Human { @@ -193,8 +193,8 @@ func exportSyncState( } func removeSyncState( - request SyncStateRequest, - options RunOptions, + request StateRequest, + options Options, ) error { if !request.Confirmed && !request.DryRun { return apperror.Wrap( @@ -208,7 +208,7 @@ func removeSyncState( } result := syncStateRemoval{ID: key, DryRun: request.DryRun} if !request.DryRun { - removed, err := options.Dependencies.SyncStates.Delete(key) + removed, err := options.SyncStates.Delete(key) if err != nil { return fmt.Errorf("remove sync state: %w", err) } @@ -229,13 +229,13 @@ func removeSyncState( func resolveAndLoadSyncState( query string, - options RunOptions, + options Options, ) (string, syncmodel.State, error) { key, err := resolveSyncStateID(query, options) if err != nil { return "", syncmodel.State{}, err } - state, found, err := options.Dependencies.SyncStates.Load(key) + state, found, err := options.SyncStates.Load(key) if err != nil { return "", syncmodel.State{}, fmt.Errorf( "sync state %s is unreadable: %w; remove it with "+ @@ -258,7 +258,7 @@ func resolveAndLoadSyncState( func resolveSyncStateID( query string, - options RunOptions, + options Options, ) (string, error) { query = strings.ToLower(strings.TrimSpace(query)) if len(query) < syncStateIDMinimumPrefix || @@ -271,7 +271,7 @@ func resolveSyncStateID( ), ) } - keys, err := options.Dependencies.SyncStates.Keys() + keys, err := options.SyncStates.Keys() if err != nil { return "", fmt.Errorf("list sync state: %w", err) } diff --git a/internal/app/sync/types.go b/internal/app/sync/types.go new file mode 100644 index 0000000..e2d4919 --- /dev/null +++ b/internal/app/sync/types.go @@ -0,0 +1,134 @@ +// Package sync owns synchronization planning, execution, jobs, state, and +// interrupted-run recovery policy. +package sync + +import ( + "context" + "io" + + "github.com/mzner/ocis-cli/internal/logging" + appoutput "github.com/mzner/ocis-cli/internal/output" + syncmodel "github.com/mzner/ocis-cli/internal/sync" + "github.com/mzner/ocis-cli/internal/syncjob" + "github.com/mzner/ocis-cli/internal/syncrecovery" + "github.com/mzner/ocis-cli/internal/webdav" +) + +type Direction string + +const ( + Push Direction = "push" + Pull Direction = "pull" + Bidirectional Direction = "bidirectional" +) + +type Request struct { + Direction Direction + LocalRoot, RemoteRoot string + Includes, Excludes []string + Delete, Overwrite, DryRun bool + MaxEntries int + ConflictStrategy, Prefer string +} + +type StateOperation string + +const ( + StateList StateOperation = "list" + StateShow StateOperation = "show" + StateExport StateOperation = "export" + StateRemove StateOperation = "remove" +) + +type StateRequest struct { + Operation StateOperation + ID, Profile string + Confirmed, DryRun bool +} + +type RecoveryOperation string + +const ( + RecoveryList RecoveryOperation = "list" + RecoveryShow RecoveryOperation = "show" + RecoveryRetry RecoveryOperation = "retry" + RecoveryRemove RecoveryOperation = "remove" +) + +type RecoveryRequest struct { + Operation RecoveryOperation + ID, Profile string + Confirmed, DryRun bool +} + +type JobOperation string + +const ( + JobAdd JobOperation = "add" + JobList JobOperation = "list" + JobShow JobOperation = "show" + JobRun JobOperation = "run" + JobRemove JobOperation = "remove" +) + +type JobRequest struct { + Operation JobOperation + Name, Profile, Space string + Direction Direction + LocalRoot, RemoteRoot string + Includes, Excludes []string + DeleteDestination, Overwrite bool + MaxEntries int + Confirmed, DryRun bool +} + +type StateRepository interface { + Keys() ([]string, error) + Load(string) (syncmodel.State, bool, error) + Save(string, syncmodel.State) error + Delete(string) (bool, error) +} +type JobRepository interface { + Load() (syncjob.Store, error) + Save(syncjob.Store) error +} +type RecoveryRepository interface { + Keys() ([]string, error) + Load(string) (syncrecovery.Journal, bool, error) + Save(syncrecovery.Journal) error + Delete(string) (bool, error) +} + +type DAVClient interface { + RemoveWithOptions(context.Context, string, webdav.RemoveOptions) error + UploadWithOptions(context.Context, string, string, webdav.TransferOptions) error + DownloadWithOptions(context.Context, string, string, webdav.TransferOptions) error + CopyWithOptions(context.Context, string, string, webdav.MoveOptions) error + MoveWithOptions(context.Context, string, string, webdav.MoveOptions) error +} + +type Client interface { + ProfileName() string + AccountID() string + SelectSpace(string) error + SelectedSpaceID() string + Context() context.Context + List(string) ([]webdav.Item, error) + Stat(string) (webdav.Item, error) + EnsureCollection(string) error + DiscoverUploadCapabilities(context.Context) webdav.TUSCapabilities + DAV() DAVClient +} + +type ClientFactory func(context.Context, string) (Client, error) + +type Options struct { + OutputMode appoutput.Mode + Out io.Writer + Space string + Logger logging.Logger + NewClient ClientFactory + SyncStates StateRepository + SyncJobs JobRepository + SyncRecoveries RecoveryRepository +} diff --git a/internal/app/sync_adapter.go b/internal/app/sync_adapter.go new file mode 100644 index 0000000..e2cc637 --- /dev/null +++ b/internal/app/sync_adapter.go @@ -0,0 +1,50 @@ +package app + +import ( + "context" + + syncapp "github.com/mzner/ocis-cli/internal/app/sync" + "github.com/mzner/ocis-cli/internal/webdav" +) + +type syncClientAdapter struct{ client *client } + +func (adapter syncClientAdapter) ProfileName() string { return adapter.client.name } +func (adapter syncClientAdapter) AccountID() string { return profileIdentity(adapter.client.profile) } +func (adapter syncClientAdapter) SelectSpace(value string) error { + return adapter.client.selectSpace(value) +} +func (adapter syncClientAdapter) SelectedSpaceID() string { return adapter.client.selectedSpaceID() } +func (adapter syncClientAdapter) Context() context.Context { return adapter.client.context() } +func (adapter syncClientAdapter) List(remote string) ([]webdav.Item, error) { + return adapter.client.list(remote) +} +func (adapter syncClientAdapter) Stat(remote string) (webdav.Item, error) { + return adapter.client.stat(remote) +} +func (adapter syncClientAdapter) EnsureCollection(remote string) error { + return adapter.client.ensureCollection(remote) +} +func (adapter syncClientAdapter) DiscoverUploadCapabilities(ctx context.Context) webdav.TUSCapabilities { + return discoverUploadCapabilities(ctx, adapter.client) +} +func (adapter syncClientAdapter) DAV() syncapp.DAVClient { return adapter.client.davClient() } + +func syncOptions(options RunOptions) syncapp.Options { + return syncapp.Options{ + OutputMode: options.OutputMode, + Out: options.Out, + Space: options.Space, + Logger: options.Logger, + NewClient: func(ctx context.Context, selectedProfile string) (syncapp.Client, error) { + selected, err := newClientWithOptions(ctx, selectedProfile, options) + if err != nil { + return nil, err + } + return syncClientAdapter{client: selected}, nil + }, + SyncStates: options.Dependencies.SyncStates, + SyncJobs: options.Dependencies.SyncJobs, + SyncRecoveries: options.Dependencies.SyncRecoveries, + } +} diff --git a/internal/app/sync_job_service_test.go b/internal/app/sync_job_service_test.go index 5469b29..0ae2ace 100644 --- a/internal/app/sync_job_service_test.go +++ b/internal/app/sync_job_service_test.go @@ -182,15 +182,6 @@ func TestSyncJobLifecycleAndExecution(t *testing.T) { } } -func TestSyncJobPullRootDisplay(t *testing.T) { - job := syncjob.Job{ - Direction: syncmodel.Pull, LocalRoot: "/local", RemoteRoot: "/remote", - } - if got := syncJobRoots(job); got != "/remote -> /local" { - t.Fatalf("roots=%q", got) - } -} - func TestSyncBidirectionalJobExecution(t *testing.T) { dav := newSyncDAV() dav.nodes["/job"] = &syncDAVNode{directory: true} @@ -216,8 +207,9 @@ func TestSyncBidirectionalJobExecution(t *testing.T) { ); err != nil { t.Fatal(err) } - if jobs.store.Jobs["two-way"].Direction != syncmodel.Bidirectional || - syncJobRoots(jobs.store.Jobs["two-way"]) != local+" <-> /job" { + saved := jobs.store.Jobs["two-way"] + if saved.Direction != syncmodel.Bidirectional || + saved.LocalRoot != local || saved.RemoteRoot != "/job" { t.Fatalf("saved bidirectional job=%#v", jobs.store.Jobs["two-way"]) } if err := RunSyncJobWithOptions( diff --git a/internal/app/sync_service_test.go b/internal/app/sync_service_test.go index 2186924..9d30608 100644 --- a/internal/app/sync_service_test.go +++ b/internal/app/sync_service_test.go @@ -556,52 +556,6 @@ func assertFileContent(t *testing.T, name string, expected string) { } } -func TestSyncDeletionFiltersAndValidation(t *testing.T) { - root := t.TempDir() - if err := os.WriteFile( - filepath.Join(root, "report.md"), []byte("report"), 0600, - ); err != nil { - t.Fatal(err) - } - if err := os.Symlink( - filepath.Join(root, "report.md"), filepath.Join(root, "link"), - ); err != nil { - t.Fatal(err) - } - if _, err := snapshotLocal( - context.Background(), root, false, 100, - ); err == nil || !strings.Contains(err.Error(), "unsupported local file type") { - t.Fatalf("symlink error: %v", err) - } - if _, err := snapshotLocal( - context.Background(), root, false, 1, - ); err == nil || !strings.Contains(err.Error(), "--max-entries") { - t.Fatalf("entry-limit error: %v", err) - } - for _, request := range []SyncRequest{ - {Direction: SyncDirection("sideways"), LocalRoot: root, RemoteRoot: "/", MaxEntries: 1}, - {Direction: SyncPush, RemoteRoot: "/", MaxEntries: 1}, - {Direction: SyncPull, LocalRoot: root, MaxEntries: 1}, - {Direction: SyncPull, LocalRoot: root, RemoteRoot: "/", MaxEntries: 0}, - } { - if _, err := validateSyncRequest(request); err == nil { - t.Fatalf("request accepted: %#v", request) - } - } - if direction, err := validateSyncRequest(SyncRequest{ - Direction: SyncBidirectional, LocalRoot: root, - RemoteRoot: "/", MaxEntries: 1, - }); err != nil || direction != syncmodel.Bidirectional { - t.Fatalf("bidirectional request: direction=%q err=%v", direction, err) - } - if _, err := syncLocalPath(root, "../../escape"); err == nil { - t.Fatal("escaping local sync path accepted") - } - if err := validateSyncRemoteName("../escape"); err == nil { - t.Fatal("unsafe remote name accepted") - } -} - func syncTestDependencies( server string, states *memorySyncStates, diff --git a/internal/app/sync_state_service_test.go b/internal/app/sync_state_service_test.go index 391c472..34803ac 100644 --- a/internal/app/sync_state_service_test.go +++ b/internal/app/sync_state_service_test.go @@ -14,6 +14,12 @@ import ( syncmodel "github.com/mzner/ocis-cli/internal/sync" ) +type syncStateExportDocument struct { + SchemaVersion string `json:"schemaVersion"` + ID string `json:"id"` + State syncmodel.State `json:"state"` +} + func TestSyncStateListShowAndExport(t *testing.T) { state := syncStateFixture("work") key := state.Binding.Key() @@ -239,17 +245,6 @@ func TestSyncStateInvalidAmbiguousAndCanceled(t *testing.T) { } } -func TestUniqueSyncStateID(t *testing.T) { - first := "123456789012a" + strings.Repeat("0", 51) - second := "123456789012b" + strings.Repeat("0", 51) - if got := uniqueSyncStateID(first, []string{first, second}); got != first[:13] { - t.Fatalf("unique ID=%q", got) - } - if got := uniqueSyncStateID(first, []string{first}); got != first[:12] { - t.Fatalf("single ID=%q", got) - } -} - func syncStateFixture(profile string) syncmodel.State { binding := syncmodel.Binding{ Profile: profile, AccountID: "v1:account", SpaceID: "space", diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index 7d33ea6..d1671dc 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -1,11 +1,13 @@ package integration_test import ( + "archive/zip" "bytes" "context" "encoding/json" "errors" "fmt" + "io" "os" "path/filepath" "slices" @@ -205,6 +207,15 @@ type version struct { Size int64 `json:"size"` } +type archiveResult struct { + Entries int64 `json:"entries"` + Files int64 `json:"files"` + Directories int64 `json:"directories"` + LogicalBytes int64 `json:"logicalBytes"` + ArchiveBytes int64 `json:"archiveBytes"` + DryRun bool `json:"dryRun"` +} + type errorData struct { Code int `json:"code"` Kind string `json:"kind"` @@ -1548,6 +1559,40 @@ func (current *fixture) testSpaces(t *testing.T) { current.success( t, nil, "--profile", current.admin, "mkdir", "/admin-folder", ) + archiveContent := []byte("project Space archive integration\n") + archiveSource := filepath.Join(current.local, "space-archive.txt") + writeFile(t, archiveSource, archiveContent) + current.success( + t, nil, "--profile", current.admin, "upload", + archiveSource, "/admin-folder/archive.txt", + ) + archiveDestination := filepath.Join(current.local, "project-space.zip") + archivePlan := decodeData[archiveResult]( + t, current.json( + t, current.admin, "archive", "download", "/admin-folder", + "--output", archiveDestination, "--dry-run", + ), + ) + if !archivePlan.DryRun || archivePlan.Entries != 2 || + archivePlan.Files != 1 || archivePlan.Directories != 1 || + archivePlan.LogicalBytes != int64(len(archiveContent)) { + t.Fatalf("archive dry-run = %#v", archivePlan) + } + if _, err := os.Stat(archiveDestination); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("archive dry-run created destination: %v", err) + } + archived := decodeData[archiveResult]( + t, current.json( + t, current.admin, "archive", "download", "/admin-folder", + "--output", archiveDestination, + ), + ) + if archived.DryRun || archived.Entries != 2 || archived.ArchiveBytes <= 0 { + t.Fatalf("archive result = %#v", archived) + } + assertZIPFile( + t, archiveDestination, "admin-folder/archive.txt", archiveContent, + ) current.success(t, nil, "--profile", current.admin, "space", "unset") added := decodeData[member]( @@ -1715,6 +1760,39 @@ func assertFile(t *testing.T, path string, expected []byte) { } } +func assertZIPFile( + t *testing.T, archivePath, entryName string, expected []byte, +) { + t.Helper() + archive, err := zip.OpenReader(archivePath) + if err != nil { + t.Fatal(err) + } + defer func() { _ = archive.Close() }() + for _, entry := range archive.File { + if entry.Name != entryName { + continue + } + reader, err := entry.Open() + if err != nil { + t.Fatal(err) + } + actual, readErr := io.ReadAll(reader) + closeErr := reader.Close() + if readErr != nil { + t.Fatal(readErr) + } + if closeErr != nil { + t.Fatal(closeErr) + } + if !bytes.Equal(actual, expected) { + t.Fatalf("archive entry %s = %q, want %q", entryName, actual, expected) + } + return + } + t.Fatalf("archive %s does not contain %s", archivePath, entryName) +} + func hasItem(items []item, name string) bool { for _, value := range items { if value.Name == name { diff --git a/tools/covercheck/main.go b/tools/covercheck/main.go index fdfa061..ddcefe1 100644 --- a/tools/covercheck/main.go +++ b/tools/covercheck/main.go @@ -12,6 +12,9 @@ import ( ) var coveragePattern = regexp.MustCompile(`coverage:\s+([0-9]+(?:\.[0-9]+)?)%`) +var totalCoveragePattern = regexp.MustCompile( + `total:\s+\(statements\)\s+([0-9]+(?:\.[0-9]+)?)%`, +) func main() { minimum := flag.Float64("min", 70, "minimum statement coverage percentage") @@ -23,6 +26,23 @@ func main() { failed := false for _, packageName := range flag.Args() { target := "./internal/" + packageName + if packageName == "app" { + value, err := applicationCoverage() + if err != nil { + fmt.Fprintln(os.Stderr, err) + failed = true + continue + } + fmt.Printf("%s/...: merged coverage: %.1f%% of statements\n", target, value) + if value < *minimum { + fmt.Fprintf( + os.Stderr, "%s/...: coverage %.1f%% is below %.1f%%\n", + target, value, *minimum, + ) + failed = true + } + continue + } command := exec.Command("go", "test", "-cover", target) //nolint:gosec // fixed executable, no shell, and package arguments are developer input var output bytes.Buffer command.Stdout, command.Stderr = &output, &output @@ -48,3 +68,44 @@ func main() { os.Exit(1) } } + +func applicationCoverage() (float64, error) { + profile, err := os.CreateTemp("", "ocis-cli-app-coverage-*.out") + if err != nil { + return 0, fmt.Errorf("create application coverage profile: %w", err) + } + name := profile.Name() + if err := profile.Close(); err != nil { + _ = os.Remove(name) + return 0, fmt.Errorf("close application coverage profile: %w", err) + } + defer func() { _ = os.Remove(name) }() + command := exec.Command( //nolint:gosec // fixed Go executable and repository-owned package patterns + "go", "test", "-coverpkg=./internal/app/...", + "-coverprofile="+name, "./internal/app/...", + ) + var testOutput bytes.Buffer + command.Stdout, command.Stderr = &testOutput, &testOutput + if err := command.Run(); err != nil { + fmt.Print(testOutput.String()) + return 0, fmt.Errorf("application coverage tests failed: %w", err) + } + command = exec.Command("go", "tool", "cover", "-func="+name) //nolint:gosec // fixed Go executable and generated temporary profile + var coverageOutput bytes.Buffer + command.Stdout, command.Stderr = &coverageOutput, &coverageOutput + if err := command.Run(); err != nil { + return 0, fmt.Errorf( + "summarize application coverage: %w: %s", + err, coverageOutput.String(), + ) + } + match := totalCoveragePattern.FindStringSubmatch(coverageOutput.String()) + if len(match) != 2 { + return 0, fmt.Errorf("application coverage total not found") + } + value, err := strconv.ParseFloat(match[1], 64) + if err != nil { + return 0, fmt.Errorf("parse application coverage: %w", err) + } + return value, nil +} diff --git a/tools/covercheck/main_test.go b/tools/covercheck/main_test.go new file mode 100644 index 0000000..6d40737 --- /dev/null +++ b/tools/covercheck/main_test.go @@ -0,0 +1,18 @@ +package main + +import "testing" + +func TestCoveragePatterns(t *testing.T) { + packageMatch := coveragePattern.FindStringSubmatch( + "coverage: 81.2% of statements", + ) + if len(packageMatch) != 2 || packageMatch[1] != "81.2" { + t.Fatalf("package match: %v", packageMatch) + } + totalMatch := totalCoveragePattern.FindStringSubmatch( + "total:\t(statements)\t\t76.4%", + ) + if len(totalMatch) != 2 || totalMatch[1] != "76.4" { + t.Fatalf("total match: %v", totalMatch) + } +} From a04af1fb202acf2205d52388ac21dc5cfcf326fe Mon Sep 17 00:00:00 2001 From: Matteo Date: Fri, 14 Aug 2026 10:00:21 +0200 Subject: [PATCH 2/3] refactor(app): extract filesystem and spaces - move filesystem, batch, and metadata policy behind a narrow client port - isolate project Space lifecycle, details, and membership policy - move focused tests into their owning domain packages - enforce the new dependency boundaries and preserve output contracts --- ARCHITECTURE.md | 12 +- CONTRIBUTING.md | 3 +- internal/app/admin_adapter.go | 5 +- internal/app/api.go | 20 ++- internal/app/app_test.go | 37 ----- internal/app/architecture_test.go | 4 +- internal/app/batch_service_test.go | 26 --- .../{batch_service.go => filesystem/batch.go} | 30 ++-- .../du.go} | 6 +- internal/app/filesystem/helpers.go | 45 ++++++ .../metadata.go} | 76 ++++----- .../mkdir.go} | 14 +- .../service.go} | 102 ++++++------ internal/app/filesystem/service_test.go | 63 ++++++++ .../touch.go} | 18 +-- .../tree.go} | 10 +- internal/app/filesystem/types.go | 151 ++++++++++++++++++ .../walk.go} | 17 +- internal/app/filesystem_adapter.go | 75 +++++++++ internal/app/path.go | 24 +++ internal/app/space_admin_test.go | 13 -- internal/app/space_service.go | 6 +- .../create.go} | 12 +- .../{space_details.go => spaces/details.go} | 22 +-- internal/app/spaces/helpers.go | 41 +++++ .../lifecycle.go} | 52 +++--- .../member.go} | 54 +++---- internal/app/spaces/member_test.go | 15 ++ .../recipient.go} | 10 +- .../recipient_test.go} | 2 +- internal/app/spaces/types.go | 78 +++++++++ .../update.go} | 28 ++-- internal/app/spaces_adapter.go | 52 ++++++ internal/app/sync_adapter.go | 7 +- 34 files changed, 805 insertions(+), 325 deletions(-) rename internal/app/{batch_service.go => filesystem/batch.go} (94%) rename internal/app/{filesystem_du_service.go => filesystem/du.go} (92%) create mode 100644 internal/app/filesystem/helpers.go rename internal/app/{metadata_service.go => filesystem/metadata.go} (81%) rename internal/app/{filesystem_mkdir_service.go => filesystem/mkdir.go} (82%) rename internal/app/{filesystem_service.go => filesystem/service.go} (86%) create mode 100644 internal/app/filesystem/service_test.go rename internal/app/{filesystem_touch_service.go => filesystem/touch.go} (81%) rename internal/app/{filesystem_tree_service.go => filesystem/tree.go} (86%) create mode 100644 internal/app/filesystem/types.go rename internal/app/{filesystem_walk.go => filesystem/walk.go} (86%) create mode 100644 internal/app/filesystem_adapter.go create mode 100644 internal/app/path.go rename internal/app/{space_create_service.go => spaces/create.go} (86%) rename internal/app/{space_details.go => spaces/details.go} (93%) create mode 100644 internal/app/spaces/helpers.go rename internal/app/{space_lifecycle_service.go => spaces/lifecycle.go} (73%) rename internal/app/{space_member_service.go => spaces/member.go} (90%) create mode 100644 internal/app/spaces/member_test.go rename internal/app/{space_recipient.go => spaces/recipient.go} (94%) rename internal/app/{space_recipient_test.go => spaces/recipient_test.go} (98%) create mode 100644 internal/app/spaces/types.go rename internal/app/{space_update_service.go => spaces/update.go} (80%) create mode 100644 internal/app/spaces_adapter.go diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index cf8aa10..5ce6e2b 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -14,7 +14,9 @@ internal/ app/ public application facade and runtime composition admin/ account and global Space administration policy archive/ archive-download application policy + filesystem/ remote filesystem, batch, and metadata policy share/ direct, received, and public-link share policy + spaces/ project Space lifecycle and membership policy sync/ sync execution, state, jobs, and recovery policy apperror/ stable error categories and exit-code mapping archiver/ authenticated archive-download protocol client @@ -46,8 +48,8 @@ test/ `cmd/ocis` depends on `internal/command`, which depends on the public `internal/app` facade. Large application domains live in subpackages such as -`internal/app/admin`, `internal/app/archive`, `internal/app/share`, and -`internal/app/sync`. The facade composes runtime +`internal/app/admin`, `internal/app/archive`, `internal/app/filesystem`, +`internal/app/share`, `internal/app/spaces`, and `internal/app/sync`. The facade composes runtime clients into their narrow ports and preserves the public request and result types used by Cobra. Domain subpackages never import the parent `internal/app` package, which makes the dependency boundary compiler-enforced and prevents @@ -80,6 +82,9 @@ without starting a subprocess. output, and safe local installation through a narrow client factory. It cannot access unrelated authentication, administration, sync, or sharing helpers in the parent package. +- `internal/app/filesystem`: own remote file operations, bounded traversal, + batch execution, transfer presentation, and resource metadata policy behind + a narrow authenticated WebDAV/Graph port. - `internal/app/admin`: own account inventory and mutation, advertised role assignment, MFA-gated administration policy, and global Space inventory through narrow Graph and OCS capability ports. @@ -87,6 +92,9 @@ without starting a subprocess. public-link application policy through a narrow authenticated client port. It cannot access unrelated archive, administration, sync, or configuration helpers in the parent package. +- `internal/app/spaces`: own project Space creation, updates, lifecycle, + details, recipient resolution, and membership policy through a narrow Graph + port. Profile persistence remains in the parent adapter. - `internal/app/sync`: own one-way and bidirectional execution, conflict policy, named jobs, local state, and interrupted-run recovery through narrow WebDAV and persistence ports. It cannot access authentication secrets, diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c4f1d25..90e9e29 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,7 +28,8 @@ debugging targets. - Put small cross-domain orchestration in the `internal/app` facade. Put large domain policy in `internal/app/` behind narrow injected ports; domain packages must not import the parent `internal/app` package. Current domain - boundaries are `admin`, `archive`, `share`, and `sync`. + boundaries are `admin`, `archive`, `filesystem`, `share`, `spaces`, and + `sync`. - Keep authentication and WebDAV protocol details out of commands. - Pass contexts, dependencies, and output streams explicitly. - Add tests at the narrowest package boundary. diff --git a/internal/app/admin_adapter.go b/internal/app/admin_adapter.go index a6fcc55..c9a6013 100644 --- a/internal/app/admin_adapter.go +++ b/internal/app/admin_adapter.go @@ -4,6 +4,7 @@ import ( "context" adminapp "github.com/mzner/ocis-cli/internal/app/admin" + spacesapp "github.com/mzner/ocis-cli/internal/app/spaces" "github.com/mzner/ocis-cli/internal/graph" ) @@ -40,14 +41,14 @@ func adminOptions(options RunOptions) adminapp.Options { if !ok { return writeAdminSpaceDetailsThroughPort(ctx, selected, drive, options) } - details, err := loadSpaceDetails(ctx, adapter.client, drive) + details, err := loadSpaceDetailsThroughDomain(ctx, adapter.client, drive, options) if err != nil { return err } if options.OutputMode != "human" { return writeOutput(options, "admin-space", details) } - return writeSpaceDetails(options, details) + return spacesapp.WriteDetails(spacesOptions(options), details) }, } } diff --git a/internal/app/api.go b/internal/app/api.go index 9fab257..e046bea 100644 --- a/internal/app/api.go +++ b/internal/app/api.go @@ -9,7 +9,9 @@ import ( "time" adminapp "github.com/mzner/ocis-cli/internal/app/admin" + filesystemapp "github.com/mzner/ocis-cli/internal/app/filesystem" shareapp "github.com/mzner/ocis-cli/internal/app/share" + spacesapp "github.com/mzner/ocis-cli/internal/app/spaces" syncapp "github.com/mzner/ocis-cli/internal/app/sync" "github.com/mzner/ocis-cli/internal/apperror" "github.com/mzner/ocis-cli/internal/graph" @@ -551,7 +553,7 @@ func RunAuthWithOptions(ctx context.Context, request AuthRequest, selectedProfil func RunFilesystemWithOptions(ctx context.Context, request FilesystemRequest, selectedProfile string, options RunOptions) error { return classifyProtocolError( string(request.Operation), - runFilesystem(ctx, request, selectedProfile, options.normalized()), + filesystemapp.Run(ctx, toFilesystemRequest(request), selectedProfile, filesystemOptions(options.normalized())), ) } @@ -563,7 +565,7 @@ func RunBatchWithOptions( options RunOptions, ) error { return classifyProtocolError( - "batch", runBatch(ctx, request, selectedProfile, options.normalized()), + "batch", filesystemapp.RunBatch(ctx, toBatchRequest(request), selectedProfile, filesystemOptions(options.normalized())), ) } @@ -625,7 +627,7 @@ func RunMetadataWithOptions( ) error { return classifyProtocolError( string(request.Operation), - runMetadata(ctx, request, selectedProfile, options.normalized()), + filesystemapp.RunMetadata(ctx, toMetadataRequest(request), selectedProfile, filesystemOptions(options.normalized())), ) } @@ -790,9 +792,7 @@ func RunSpaceCreateWithOptions( options RunOptions, ) error { return classifyProtocolError( - "create", runSpaceCreate( - ctx, request, selectedProfile, options.normalized(), - ), + "create", spacesapp.RunCreate(ctx, toSpaceCreateRequest(request), selectedProfile, spacesOptions(options.normalized())), ) } @@ -804,9 +804,7 @@ func RunSpaceUpdateWithOptions( options RunOptions, ) error { return classifyProtocolError( - "update", runSpaceUpdate( - ctx, request, selectedProfile, options.normalized(), - ), + "update", spacesapp.RunUpdate(ctx, toSpaceUpdateRequest(request), selectedProfile, spacesOptions(options.normalized())), ) } @@ -819,7 +817,7 @@ func RunSpaceLifecycleWithOptions( ) error { return classifyProtocolError( string(request.Operation), - runSpaceLifecycle(ctx, request, selectedProfile, options.normalized()), + spacesapp.RunLifecycle(ctx, toSpaceLifecycleRequest(request), selectedProfile, spacesOptions(options.normalized())), ) } @@ -832,7 +830,7 @@ func RunSpaceMemberWithOptions( ) error { return classifyProtocolError( "member "+string(request.Operation), - runSpaceMember(ctx, request, selectedProfile, options.normalized()), + spacesapp.RunMember(ctx, toSpaceMemberRequest(request), selectedProfile, spacesOptions(options.normalized())), ) } diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 9f0a521..6572779 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -24,7 +24,6 @@ import ( "github.com/mzner/ocis-cli/internal/auth" "github.com/mzner/ocis-cli/internal/credentials" appoutput "github.com/mzner/ocis-cli/internal/output" - "github.com/mzner/ocis-cli/internal/transfer" "github.com/zalando/go-keyring" ) @@ -129,29 +128,6 @@ func TestFilesystemDryRunPlansAllTransferKinds(t *testing.T) { } } -func TestProgressReporterWritesAggregateProgress(t *testing.T) { - var output bytes.Buffer - report := progressReporter(RunOptions{ - Err: &output, OutputMode: appoutput.Human, - }.normalized()) - if report == nil { - t.Fatal("progress reporter is nil") - } - report(transfer.Progress{ - Operation: "upload", Destination: "/report.txt", - CompletedBytes: 50, TotalBytes: 100, CompletedFiles: 1, TotalFiles: 2, - StartedAt: time.Now().Add(-time.Second), - }) - for _, expected := range []string{"upload", "1/2 files", "50/100 bytes", "50%", "/report.txt"} { - if !strings.Contains(output.String(), expected) { - t.Fatalf("progress missing %q: %s", expected, output.String()) - } - } - if progressReporter(RunOptions{Quiet: true}.normalized()) != nil { - t.Fatal("quiet mode returned a progress reporter") - } -} - func TestFilesystemMapsDAVStatusToStableKind(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { switch request.Method { @@ -1680,21 +1656,8 @@ func TestProfileSelectionErrorsAndAccessTokenOverride(t *testing.T) { } func TestStreamHelperErrors(t *testing.T) { - if _, cleanup, err := spoolInput(failingReader{}); err == nil { - cleanup() - t.Fatal("spooling a failing reader succeeded") - } - if err := writeFileTo(io.Discard, filepath.Join(t.TempDir(), "missing")); err == nil { - t.Fatal("writing a missing file succeeded") - } t.Setenv("OCIS_PASSWORD", "") if _, err := obtainPassword(RunOptions{Err: io.Discard}.normalized()); err == nil { t.Fatal("non-interactive password acquisition succeeded") } } - -type failingReader struct{} - -func (failingReader) Read([]byte) (int, error) { - return 0, errors.New("read failed") -} diff --git a/internal/app/architecture_test.go b/internal/app/architecture_test.go index 9228a7a..23dd27d 100644 --- a/internal/app/architecture_test.go +++ b/internal/app/architecture_test.go @@ -16,7 +16,9 @@ func TestDomainPackagesDoNotReachOuterLayers(t *testing.T) { if err != nil { t.Fatal(err) } - for _, domain := range []string{"admin", "archive", "share", "sync"} { + for _, domain := range []string{ + "admin", "archive", "filesystem", "share", "spaces", "sync", + } { directory := filepath.Join(root, domain) entries, err := os.ReadDir(directory) if err != nil { diff --git a/internal/app/batch_service_test.go b/internal/app/batch_service_test.go index ec64639..64fc5c6 100644 --- a/internal/app/batch_service_test.go +++ b/internal/app/batch_service_test.go @@ -330,30 +330,4 @@ func TestBatchInputGuards(t *testing.T) { t.Fatalf("confirmation error: %v", err) } - tests := []string{ - `{"operation":"upload","source":"-","destination":"/x"}`, - `{"operation":"download","source":"/x","destination":"-"}`, - `{"operation":"touch","path":"/x","parents":true}`, - `{"operation":"mkdir","path":"/x","unknown":true}`, - } - for _, input := range tests { - _, err := parseBatchOperations(strings.NewReader(input), 10) - if !apperror.IsKind(err, apperror.KindUsage) { - t.Errorf("input %s: %v", input, err) - } - } - _, err := parseBatchOperations(strings.NewReader( - "{\"operation\":\"mkdir\",\"path\":\"/a\"}\n"+ - "{\"operation\":\"mkdir\",\"path\":\"/b\"}\n", - ), 1) - if !apperror.IsKind(err, apperror.KindUsage) || - !strings.Contains(err.Error(), "--max-operations 1") { - t.Fatalf("limit error: %v", err) - } - for _, input := range []string{"", `{"operation":"mkdir","path":"/x"} {}`} { - _, err := parseBatchOperations(strings.NewReader(input), 10) - if !apperror.IsKind(err, apperror.KindUsage) { - t.Errorf("input %q: %v", input, err) - } - } } diff --git a/internal/app/batch_service.go b/internal/app/filesystem/batch.go similarity index 94% rename from internal/app/batch_service.go rename to internal/app/filesystem/batch.go index ebc5b9d..d0318ca 100644 --- a/internal/app/batch_service.go +++ b/internal/app/filesystem/batch.go @@ -1,4 +1,4 @@ -package app +package filesystem import ( "bufio" @@ -38,9 +38,9 @@ func (err *batchRunError) Unwrap() error { return err.first } -func runBatch( +func RunBatch( ctx context.Context, request BatchRequest, selected string, - options RunOptions, + options Options, ) error { if request.MaxOperations < 1 { return apperror.Wrap( @@ -62,11 +62,11 @@ func runBatch( if err != nil { return err } - client, err := newClientWithOptions(ctx, selected, options) + client, err := options.NewClient(ctx, selected) if err != nil { return err } - if err := client.selectSpace(options.Space); err != nil { + if err := client.SelectSpace(options.Space); err != nil { return err } @@ -300,39 +300,39 @@ func normalizeBatchOperation(value string) string { } } -func batchFilesystemRequest(operation BatchOperation) FilesystemRequest { +func batchFilesystemRequest(operation BatchOperation) Request { verify := true if operation.Verify != nil { verify = *operation.Verify } - request := FilesystemRequest{ + request := Request{ Recursive: operation.Recursive, Overwrite: operation.Overwrite, NoClobber: operation.NoClobber, Verify: verify, Parents: operation.Parents, } switch operation.Operation { case "mkdir": - request.Operation = FilesystemMkdir + request.Operation = Mkdir request.Source = operation.Path case "touch": - request.Operation = FilesystemTouch + request.Operation = Touch request.Source = operation.Path case "remove": - request.Operation = FilesystemRemove + request.Operation = Remove request.Source = operation.Path case "copy": - request.Operation = FilesystemCopy + request.Operation = Copy request.Source = operation.Source request.Destination = operation.Destination case "move": - request.Operation = FilesystemMove + request.Operation = Move request.Source = operation.Source request.Destination = operation.Destination case "upload": - request.Operation = FilesystemUpload + request.Operation = Upload request.Source = operation.Source request.Destination = operation.Destination case "download": - request.Operation = FilesystemDownload + request.Operation = Download request.Source = operation.Source request.Destination = operation.Destination } @@ -363,7 +363,7 @@ func newBatchResult(index int, operation parsedBatchOperation) BatchResult { return result } -func writeBatchSummary(options RunOptions, summary BatchSummary) error { +func writeBatchSummary(options Options, summary BatchSummary) error { if options.OutputMode == appoutput.JSONL { return writeOutput(options, "batch-result", summary.Results) } diff --git a/internal/app/filesystem_du_service.go b/internal/app/filesystem/du.go similarity index 92% rename from internal/app/filesystem_du_service.go rename to internal/app/filesystem/du.go index 5565c43..bbeaca6 100644 --- a/internal/app/filesystem_du_service.go +++ b/internal/app/filesystem/du.go @@ -1,4 +1,4 @@ -package app +package filesystem import ( "errors" @@ -8,7 +8,7 @@ import ( ) func duFilesystem( - client *client, request FilesystemRequest, options RunOptions, + client Client, request Request, options Options, ) error { if request.MaxDepth < 0 { return apperror.Wrap( @@ -29,7 +29,7 @@ func duFilesystem( if err != nil { return err } - usage := FilesystemUsage{ + usage := Usage{ Path: remote, Entries: len(walk.entries), MaxDepth: request.MaxDepth, MaxEntries: request.MaxEntries, Complete: !walk.depthLimited, } diff --git a/internal/app/filesystem/helpers.go b/internal/app/filesystem/helpers.go new file mode 100644 index 0000000..fbf9d86 --- /dev/null +++ b/internal/app/filesystem/helpers.go @@ -0,0 +1,45 @@ +package filesystem + +import ( + "context" + "errors" + "net/http" + + "github.com/mzner/ocis-cli/internal/apperror" + appoutput "github.com/mzner/ocis-cli/internal/output" +) + +func output(options Options, kind string, value any, format string, args ...any) error { + return (appoutput.Renderer{Writer: options.Out, Mode: options.OutputMode, Type: kind}).Write(value, format, args...) +} +func writeOutput(options Options, kind string, value any) error { + return output(options, kind, value, "") +} +func protocolStatus(err error) int { + var statusErr interface{ HTTPStatusCode() int } + if errors.As(err, &statusErr) { + return statusErr.HTTPStatusCode() + } + return http.StatusOK +} + +func classifyProtocolError(operation string, err error) error { + if err == nil { + return nil + } + if errors.Is(err, context.Canceled) { + return apperror.Wrap(apperror.KindCanceled, operation, err) + } + switch protocolStatus(err) { + case http.StatusBadRequest, http.StatusUnprocessableEntity: + return apperror.Wrap(apperror.KindUsage, operation, err) + case http.StatusUnauthorized, http.StatusForbidden: + return apperror.Wrap(apperror.KindAuthentication, operation, err) + case http.StatusNotFound: + return apperror.Wrap(apperror.KindNotFound, operation, err) + case http.StatusConflict, http.StatusPreconditionFailed: + return apperror.Wrap(apperror.KindConflict, operation, err) + default: + return err + } +} diff --git a/internal/app/metadata_service.go b/internal/app/filesystem/metadata.go similarity index 81% rename from internal/app/metadata_service.go rename to internal/app/filesystem/metadata.go index 2c756f0..c01dd59 100644 --- a/internal/app/metadata_service.go +++ b/internal/app/filesystem/metadata.go @@ -1,4 +1,4 @@ -package app +package filesystem import ( "context" @@ -43,33 +43,33 @@ type propertyResult struct { DryRun bool `json:"dryRun,omitempty"` } -func runMetadata( +func RunMetadata( ctx context.Context, request MetadataRequest, selected string, - options RunOptions, + options Options, ) error { if err := validateMetadataRequest(request); err != nil { return apperror.Wrap(apperror.KindUsage, "metadata", err) } - client, err := newClientWithOptions(ctx, selected, options) + client, err := options.NewClient(ctx, selected) if err != nil { return err } - if err := client.selectSpace(options.Space); err != nil { + if err := client.SelectSpace(options.Space); err != nil { return err } switch request.Operation { - case MetadataTagList: + case TagList: return listResourceTags(client, request.Path, options) - case MetadataTagAdd, MetadataTagRemove: + case TagAdd, TagRemove: return mutateResourceTags(ctx, client, request, options) - case MetadataFavoriteSet, MetadataFavoriteUnset: + case FavoriteSet, FavoriteUnset: return mutateFavorite(client, request, options) - case MetadataPropertyGet: + case PropertyGet: return getCustomProperty(client, request, options) - case MetadataPropertySet, MetadataPropertyRemove: + case PropertySet, PropertyRemove: return mutateCustomProperty(client, request, options) default: return apperror.Wrap( @@ -84,14 +84,14 @@ func validateMetadataRequest(request MetadataRequest) error { return errors.New("remote path is required") } switch request.Operation { - case MetadataTagList, MetadataFavoriteSet, MetadataFavoriteUnset: + case TagList, FavoriteSet, FavoriteUnset: return nil - case MetadataTagAdd, MetadataTagRemove: + case TagAdd, TagRemove: if len(normalizeTags(request.Tags)) == 0 { return errors.New("at least one non-empty tag is required") } return nil - case MetadataPropertyGet, MetadataPropertySet, MetadataPropertyRemove: + case PropertyGet, PropertySet, PropertyRemove: return validateCustomProperty(request.Namespace, request.Name) default: return fmt.Errorf("unknown metadata operation %q", request.Operation) @@ -144,9 +144,9 @@ func normalizeTags(values []string) []string { } func listResourceTags( - client *client, remote string, options RunOptions, + client Client, remote string, options Options, ) error { - item, err := client.stat(remote) + item, err := client.Stat(remote) if err != nil { return err } @@ -171,11 +171,11 @@ func listResourceTags( func mutateResourceTags( ctx context.Context, - client *client, + client Client, request MetadataRequest, - options RunOptions, + options Options, ) error { - item, err := client.stat(request.Path) + item, err := client.Stat(request.Path) if err != nil { return err } @@ -186,7 +186,7 @@ func mutateResourceTags( } tags := normalizeTags(request.Tags) operation := "add" - if request.Operation == MetadataTagRemove { + if request.Operation == TagRemove { operation = "remove" } if request.DryRun { @@ -200,10 +200,10 @@ func mutateResourceTags( operation, strings.Join(tags, ", "), item.Path, ) } - if request.Operation == MetadataTagAdd { - err = client.graphClient().AddTags(ctx, item.ResourceID, tags) + if request.Operation == TagAdd { + err = client.AddTags(ctx, item.ResourceID, tags) } else { - err = client.graphClient().RemoveTags(ctx, item.ResourceID, tags) + err = client.RemoveTags(ctx, item.ResourceID, tags) } if err != nil { if status := protocolStatus(err); status == http.StatusNotFound || @@ -214,7 +214,7 @@ func mutateResourceTags( } return err } - updated, err := client.stat(request.Path) + updated, err := client.Stat(request.Path) if err != nil { return fmt.Errorf("tags changed but refreshed metadata failed: %w", err) } @@ -229,13 +229,13 @@ func mutateResourceTags( } func mutateFavorite( - client *client, request MetadataRequest, options RunOptions, + client Client, request MetadataRequest, options Options, ) error { - item, err := client.stat(request.Path) + item, err := client.Stat(request.Path) if err != nil { return err } - selected := request.Operation == MetadataFavoriteSet + selected := request.Operation == FavoriteSet operation := "set" if !selected { operation = "unset" @@ -254,7 +254,7 @@ func mutateFavorite( ) } if selected { - err = client.setProperty( + err = client.SetProperty( request.Path, webdav.PropertyName{ Namespace: ownCloudNamespace, Name: "favorite", @@ -262,7 +262,7 @@ func mutateFavorite( "1", ) } else { - err = client.removeProperty( + err = client.RemoveProperty( request.Path, webdav.PropertyName{ Namespace: ownCloudNamespace, Name: "favorite", @@ -282,12 +282,12 @@ func mutateFavorite( } func getCustomProperty( - client *client, request MetadataRequest, options RunOptions, + client Client, request MetadataRequest, options Options, ) error { property := webdav.PropertyName{ Namespace: strings.TrimSpace(request.Namespace), Name: request.Name, } - value, err := client.getProperty(request.Path, property) + value, err := client.GetProperty(request.Path, property) if err != nil { if errors.Is(err, webdav.ErrPropertyNotFound) { return fmt.Errorf( @@ -307,9 +307,9 @@ func getCustomProperty( } func mutateCustomProperty( - client *client, request MetadataRequest, options RunOptions, + client Client, request MetadataRequest, options Options, ) error { - item, err := client.stat(request.Path) + item, err := client.Stat(request.Path) if err != nil { return err } @@ -317,7 +317,7 @@ func mutateCustomProperty( Namespace: strings.TrimSpace(request.Namespace), Name: request.Name, } operation := "set" - if request.Operation == MetadataPropertyRemove { + if request.Operation == PropertyRemove { operation = "remove" } result := propertyResult{ @@ -334,10 +334,10 @@ func mutateCustomProperty( operation, property.Namespace, property.Name, item.Path, ) } - if request.Operation == MetadataPropertySet { - err = client.setProperty(request.Path, property, request.Value) + if request.Operation == PropertySet { + err = client.SetProperty(request.Path, property, request.Value) } else { - err = client.removeProperty(request.Path, property) + err = client.RemoveProperty(request.Path, property) } if err != nil { return err @@ -350,8 +350,8 @@ func mutateCustomProperty( ) } -func requirePropertyWrites(client *client) error { - capabilities, err := client.capabilities() +func requirePropertyWrites(client Client) error { + capabilities, err := client.Capabilities() if err != nil { return fmt.Errorf("discover WebDAV property support: %w", err) } diff --git a/internal/app/filesystem_mkdir_service.go b/internal/app/filesystem/mkdir.go similarity index 82% rename from internal/app/filesystem_mkdir_service.go rename to internal/app/filesystem/mkdir.go index d1bc3f8..fcccba6 100644 --- a/internal/app/filesystem_mkdir_service.go +++ b/internal/app/filesystem/mkdir.go @@ -1,4 +1,4 @@ -package app +package filesystem import ( "fmt" @@ -10,11 +10,11 @@ import ( ) func mkdirFilesystem( - client *client, request FilesystemRequest, options RunOptions, + client Client, request Request, options Options, ) error { target := cleanRemote(request.Source) if !request.Parents { - if err := client.ensureCollection(target); err != nil { + if err := client.EnsureCollection(target); err != nil { return err } return output( @@ -40,7 +40,7 @@ func mkdirFilesystem( ) } -func ensureDirectoryPath(client *client, target string) ([]string, error) { +func ensureDirectoryPath(client Client, target string) ([]string, error) { if target == "/" { return nil, nil } @@ -49,7 +49,7 @@ func ensureDirectoryPath(client *client, target string) ([]string, error) { current := "/" for _, part := range parts { current = path.Join(current, part) - meta, err := client.stat(current) + meta, err := client.Stat(current) switch { case err == nil && meta.Type == "directory": continue @@ -61,10 +61,10 @@ func ensureDirectoryPath(client *client, target string) ([]string, error) { case webdav.StatusCode(err) != 404: return nil, err } - if err := client.ensureCollection(current); err != nil { + if err := client.EnsureCollection(current); err != nil { return nil, err } - meta, err = client.stat(current) + meta, err = client.Stat(current) if err != nil { return nil, fmt.Errorf("verify created directory %s: %w", current, err) } diff --git a/internal/app/filesystem_service.go b/internal/app/filesystem/service.go similarity index 86% rename from internal/app/filesystem_service.go rename to internal/app/filesystem/service.go index 65fafc3..d3111ef 100644 --- a/internal/app/filesystem_service.go +++ b/internal/app/filesystem/service.go @@ -1,4 +1,4 @@ -package app +package filesystem import ( "context" @@ -18,31 +18,31 @@ import ( "golang.org/x/term" ) -func runFilesystem( - ctx context.Context, request FilesystemRequest, selected string, options RunOptions, +func Run( + ctx context.Context, request Request, selected string, options Options, ) error { options.Logger.Debug("run filesystem operation", "operation", request.Operation) - client, err := newClientWithOptions(ctx, selected, options) + client, err := options.NewClient(ctx, selected) if err != nil { return err } - if err := client.selectSpace(options.Space); err != nil { + if err := client.SelectSpace(options.Space); err != nil { return err } return runFilesystemWithClient(ctx, client, request, options) } func runFilesystemWithClient( - ctx context.Context, client *client, - request FilesystemRequest, options RunOptions, + ctx context.Context, client Client, + request Request, options Options, ) error { switch request.Operation { - case FilesystemList: + case List: remote := request.Source if remote == "" { remote = "/" } - items, err := client.list(remote) + items, err := client.List(remote) if err != nil { return err } @@ -59,8 +59,8 @@ func runFilesystemWithClient( ) } return nil - case FilesystemStat: - meta, err := client.stat(request.Source) + case Stat: + meta, err := client.Stat(request.Source) if err != nil { return addSpaceStatHint(ctx, client, request.Source, err) } @@ -68,8 +68,8 @@ func runFilesystemWithClient( return writeOutput(options, "item", meta) } return writeHumanStat(options.Out, meta) - case FilesystemCat: - meta, err := client.stat(request.Source) + case Cat: + meta, err := client.Stat(request.Source) if err != nil { return err } @@ -79,22 +79,22 @@ func runFilesystemWithClient( fmt.Errorf("%s is a directory", cleanRemote(request.Source)), ) } - return client.stream(request.Source, options.Out) - case FilesystemTree: + return client.Stream(request.Source, options.Out) + case Tree: return treeFilesystem(client, request, options) - case FilesystemDU: + case DU: return duFilesystem(client, request, options) - case FilesystemUpload: + case Upload: return uploadFilesystem(ctx, client, request, options) - case FilesystemDownload: + case Download: return downloadFilesystem(ctx, client, request, options) - case FilesystemMkdir: + case Mkdir: return mkdirFilesystem(client, request, options) - case FilesystemTouch: + case Touch: return touchFilesystem(client, request, options) - case FilesystemMove, FilesystemCopy: + case Move, Copy: return copyOrMoveFilesystem(client, request, options) - case FilesystemRemove: + case Remove: if request.DryRun { return output( options, "resource", @@ -105,7 +105,7 @@ func runFilesystemWithClient( "Would delete %s\n", cleanRemote(request.Source), ) } - if err := client.remove(request.Source, request.Recursive); err != nil { + if err := client.Remove(request.Source, request.Recursive); err != nil { return err } return output( @@ -123,7 +123,7 @@ func runFilesystemWithClient( } } -func writeHumanStat(writer io.Writer, meta item) error { +func writeHumanStat(writer io.Writer, meta webdav.Item) error { fields := [][2]string{ {"Name", meta.Name}, {"Path", meta.Path}, @@ -170,12 +170,12 @@ func writeHumanStat(writer io.Writer, meta item) error { } func addSpaceStatHint( - ctx context.Context, client *client, remote string, statErr error, + ctx context.Context, client Client, remote string, statErr error, ) error { if webdav.StatusCode(statErr) != 404 { return statErr } - spaces, err := client.graphClient().ListMyDrives(ctx) + spaces, err := client.ListMyDrives(ctx) if err != nil { return statErr } @@ -205,7 +205,7 @@ func shellQuote(value string) string { } func uploadFilesystem( - ctx context.Context, client *client, request FilesystemRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { if request.DryRun { return output( @@ -246,7 +246,7 @@ func uploadFilesystem( } uploadCapabilities := webdav.TUSCapabilities{} if !request.NoClobber { - uploadCapabilities = discoverUploadCapabilities(ctx, client) + uploadCapabilities = discoverUploadCapabilities(ctx, client, options) } if err := transfer.UploadWithOptions( ctx, transferRemote(client, request, uploadCapabilities), @@ -268,7 +268,7 @@ func uploadFilesystem( } func downloadFilesystem( - ctx context.Context, client *client, request FilesystemRequest, options RunOptions, + ctx context.Context, client Client, request Request, options Options, ) error { if request.DryRun { return output( @@ -281,7 +281,7 @@ func downloadFilesystem( cleanRemote(request.Source), request.Destination, ) } - meta, err := client.stat(request.Source) + meta, err := client.Stat(request.Source) if err != nil { return err } @@ -331,7 +331,7 @@ func downloadFilesystem( } func copyOrMoveFilesystem( - client *client, request FilesystemRequest, options RunOptions, + client Client, request Request, options Options, ) error { resolvedDestination, err := resolveCopyMoveDestination( client, request.Source, request.Destination, @@ -344,10 +344,10 @@ func copyOrMoveFilesystem( } func copyOrMoveFilesystemResolved( - client *client, request FilesystemRequest, options RunOptions, + client Client, request Request, options Options, ) error { action, verb := "Moved", "move" - if request.Operation == FilesystemCopy { + if request.Operation == Copy { action, verb = "Copied", "copy" } if request.DryRun { @@ -363,10 +363,10 @@ func copyOrMoveFilesystemResolved( ) } var err error - if request.Operation == FilesystemCopy { - err = client.copy(request.Source, request.Destination, request.Overwrite) + if request.Operation == Copy { + err = client.Copy(request.Source, request.Destination, request.Overwrite) } else { - err = client.move(request.Source, request.Destination, request.Overwrite) + err = client.Move(request.Source, request.Destination, request.Overwrite) } if err != nil { return err @@ -384,11 +384,11 @@ func copyOrMoveFilesystemResolved( } func resolveCopyMoveDestination( - client *client, source, destination string, + client Client, source, destination string, ) (string, error) { cleanedDestination := cleanRemote(destination) requiresDirectory := strings.HasSuffix(destination, "/") - meta, err := client.stat(cleanedDestination) + meta, err := client.Stat(cleanedDestination) switch { case err == nil && meta.Type == "directory": name := path.Base(cleanRemote(source)) @@ -419,19 +419,19 @@ func resolveCopyMoveDestination( } func transferRemote( - client *client, - request FilesystemRequest, + client Client, + request Request, uploadCapabilities webdav.TUSCapabilities, ) transfer.Remote { return transfer.Remote{ Stat: func(_ context.Context, remote string) (transfer.Entry, error) { - value, err := client.stat(remote) + value, err := client.Stat(remote) return transfer.Entry{ Name: value.Name, Path: value.Path, Type: value.Type, Size: value.Size, }, err }, List: func(_ context.Context, remote string) ([]transfer.Entry, error) { - values, err := client.list(remote) + values, err := client.List(remote) entries := make([]transfer.Entry, len(values)) for index, value := range values { entries[index] = transfer.Entry{ @@ -444,8 +444,8 @@ func transferRemote( Upload: func( _ context.Context, local, remote string, progress func(int64), ) error { - return client.davClient().UploadWithOptions( - client.context(), local, remote, + return client.Upload( + client.Context(), local, remote, webdav.TransferOptions{ NoClobber: request.NoClobber, Verify: request.Verify, Progress: progress, @@ -456,8 +456,8 @@ func transferRemote( Download: func( _ context.Context, remote, local string, progress func(int64), ) error { - return client.davClient().DownloadWithOptions( - client.context(), remote, local, + return client.Download( + client.Context(), remote, local, webdav.TransferOptions{ NoClobber: request.NoClobber, Verify: request.Verify, Resume: true, Progress: progress, @@ -465,17 +465,17 @@ func transferRemote( ) }, Mkdir: func(_ context.Context, remote string) error { - return client.ensureCollection(remote) + return client.EnsureCollection(remote) }, } } func discoverUploadCapabilities( - ctx context.Context, client *client, + ctx context.Context, client Client, options Options, ) webdav.TUSCapabilities { - capabilities, err := client.sharingClient().Capabilities(ctx) + capabilities, err := client.SharingCapabilities(ctx) if err != nil { - client.logger.Debug( + options.Logger.Debug( "TUS capability discovery failed; using WebDAV PUT", "reason", err.Error(), ) @@ -494,7 +494,7 @@ func cleanRemote(remote string) string { return "/" + strings.TrimPrefix(path.Clean("/"+remote), "/") } -func progressReporter(options RunOptions) func(transfer.Progress) { +func progressReporter(options Options) func(transfer.Progress) { if options.Quiet || options.OutputMode != appoutput.Human { return nil } diff --git a/internal/app/filesystem/service_test.go b/internal/app/filesystem/service_test.go new file mode 100644 index 0000000..3b59aab --- /dev/null +++ b/internal/app/filesystem/service_test.go @@ -0,0 +1,63 @@ +package filesystem + +import ( + "bytes" + "errors" + "io" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/mzner/ocis-cli/internal/apperror" + appoutput "github.com/mzner/ocis-cli/internal/output" + "github.com/mzner/ocis-cli/internal/transfer" +) + +func TestProgressReporterWritesAggregateProgress(t *testing.T) { + var output bytes.Buffer + report := progressReporter(Options{Err: &output, OutputMode: appoutput.Human}) + if report == nil { + t.Fatal("progress reporter is nil") + } + report(transfer.Progress{Operation: "upload", Destination: "/report.txt", CompletedBytes: 50, TotalBytes: 100, CompletedFiles: 1, TotalFiles: 2, StartedAt: time.Now().Add(-time.Second)}) + for _, expected := range []string{"upload", "1/2 files", "50/100 bytes", "50%", "/report.txt"} { + if !strings.Contains(output.String(), expected) { + t.Fatalf("progress missing %q: %s", expected, output.String()) + } + } + if progressReporter(Options{Quiet: true}) != nil { + t.Fatal("quiet mode returned a progress reporter") + } +} + +func TestStreamHelperErrors(t *testing.T) { + if _, cleanup, err := spoolInput(failingReader{}); err == nil { + cleanup() + t.Fatal("spooling a failing reader succeeded") + } + if err := writeFileTo(io.Discard, filepath.Join(t.TempDir(), "missing")); err == nil { + t.Fatal("writing a missing file succeeded") + } +} + +type failingReader struct{} + +func (failingReader) Read([]byte) (int, error) { return 0, errors.New("read failed") } + +func TestBatchInputGuards(t *testing.T) { + for _, input := range []string{`{"operation":"upload","source":"-","destination":"/x"}`, `{"operation":"download","source":"/x","destination":"-"}`, `{"operation":"touch","path":"/x","parents":true}`, `{"operation":"mkdir","path":"/x","unknown":true}`} { + if _, err := parseBatchOperations(strings.NewReader(input), 10); !apperror.IsKind(err, apperror.KindUsage) { + t.Errorf("input %s: %v", input, err) + } + } + _, err := parseBatchOperations(strings.NewReader("{\"operation\":\"mkdir\",\"path\":\"/a\"}\n{\"operation\":\"mkdir\",\"path\":\"/b\"}\n"), 1) + if !apperror.IsKind(err, apperror.KindUsage) || !strings.Contains(err.Error(), "--max-operations 1") { + t.Fatalf("limit error: %v", err) + } + for _, input := range []string{"", `{"operation":"mkdir","path":"/x"} {}`} { + if _, err := parseBatchOperations(strings.NewReader(input), 10); !apperror.IsKind(err, apperror.KindUsage) { + t.Errorf("input %q: %v", input, err) + } + } +} diff --git a/internal/app/filesystem_touch_service.go b/internal/app/filesystem/touch.go similarity index 81% rename from internal/app/filesystem_touch_service.go rename to internal/app/filesystem/touch.go index 116ad1a..48a9a53 100644 --- a/internal/app/filesystem_touch_service.go +++ b/internal/app/filesystem/touch.go @@ -1,4 +1,4 @@ -package app +package filesystem import ( "fmt" @@ -10,10 +10,10 @@ import ( ) func touchFilesystem( - client *client, request FilesystemRequest, options RunOptions, + client Client, request Request, options Options, ) error { target := cleanRemote(request.Source) - meta, err := client.stat(target) + meta, err := client.Stat(target) switch { case err == nil: return existingTouchResult(options, target, meta) @@ -27,8 +27,8 @@ func touchFilesystem( } defer func() { _ = os.Remove(temporary) }() - err = client.davClient().UploadWithOptions( - client.context(), temporary, target, + err = client.Upload( + client.Context(), temporary, target, webdav.TransferOptions{NoClobber: true}, ) if err != nil { @@ -36,14 +36,14 @@ func touchFilesystem( if status != http.StatusConflict && status != http.StatusPreconditionFailed { return err } - meta, statErr := client.stat(target) + meta, statErr := client.Stat(target) if statErr != nil { return err } return existingTouchResult(options, target, meta) } - meta, err = client.stat(target) + meta, err = client.Stat(target) if err != nil { return fmt.Errorf("verify touched file %s: %w", target, err) } @@ -69,7 +69,7 @@ func createEmptyTemporaryFile() (string, error) { return name, nil } -func existingTouchResult(options RunOptions, target string, meta item) error { +func existingTouchResult(options Options, target string, meta webdav.Item) error { if meta.Type != "file" { return apperror.Wrap( apperror.KindConflict, "touch", @@ -79,7 +79,7 @@ func existingTouchResult(options RunOptions, target string, meta item) error { return touchResult(options, target, false) } -func touchResult(options RunOptions, target string, created bool) error { +func touchResult(options Options, target string, created bool) error { value := map[string]any{ "path": target, "created": created, "unchanged": !created, } diff --git a/internal/app/filesystem_tree_service.go b/internal/app/filesystem/tree.go similarity index 86% rename from internal/app/filesystem_tree_service.go rename to internal/app/filesystem/tree.go index 81b742a..47ca2fe 100644 --- a/internal/app/filesystem_tree_service.go +++ b/internal/app/filesystem/tree.go @@ -1,4 +1,4 @@ -package app +package filesystem import ( "errors" @@ -9,7 +9,7 @@ import ( ) func treeFilesystem( - client *client, request FilesystemRequest, options RunOptions, + client Client, request Request, options Options, ) error { if request.MaxDepth < 0 { return apperror.Wrap( @@ -30,15 +30,15 @@ func treeFilesystem( if err != nil { return err } - entries := make([]FilesystemTreeEntry, len(walk.entries)) + entries := make([]TreeEntry, len(walk.entries)) for index, node := range walk.entries { - entries[index] = FilesystemTreeEntry{ + entries[index] = TreeEntry{ Name: node.item.Name, Path: node.item.Path, Type: node.item.Type, Size: node.item.Size, Depth: node.depth, } } if options.OutputMode != appoutput.Human { - return writeOutput(options, "item", entries) + return writeOutput(options, "webdav.Item", entries) } rootLabel := remote if walk.entries[0].item.Type == "directory" && remote != "/" { diff --git a/internal/app/filesystem/types.go b/internal/app/filesystem/types.go new file mode 100644 index 0000000..8af4d1f --- /dev/null +++ b/internal/app/filesystem/types.go @@ -0,0 +1,151 @@ +// Package filesystem owns remote filesystem, batch, and metadata policy. +package filesystem + +import ( + "context" + "io" + + "github.com/mzner/ocis-cli/internal/graph" + "github.com/mzner/ocis-cli/internal/logging" + appoutput "github.com/mzner/ocis-cli/internal/output" + "github.com/mzner/ocis-cli/internal/sharing" + "github.com/mzner/ocis-cli/internal/webdav" +) + +type Operation string + +const ( + List Operation = "list" + Stat Operation = "stat" + Cat Operation = "cat" + Tree Operation = "tree" + DU Operation = "du" + Upload Operation = "upload" + Download Operation = "download" + Mkdir Operation = "mkdir" + Touch Operation = "touch" + Move Operation = "mv" + Copy Operation = "cp" + Remove Operation = "remove" +) + +type Request struct { + Operation Operation + Source, Destination string + Recursive, Overwrite, NoClobber, DryRun, Verify, Parents bool + MaxDepth, MaxEntries int +} +type TreeEntry struct { + Name string `json:"name"` + Path string `json:"path"` + Type string `json:"type"` + Size int64 `json:"size,omitempty"` + Depth int `json:"depth"` +} +type Usage struct { + Path string `json:"path"` + LogicalBytes int64 `json:"logicalBytes"` + Files int `json:"files"` + Directories int `json:"directories"` + Entries int `json:"entries"` + MaxDepth int `json:"maxDepth"` + MaxEntries int `json:"maxEntries"` + Complete bool `json:"complete"` +} + +type BatchRequest struct { + Input io.Reader + DryRun, Confirmed, ContinueOnError bool + MaxOperations int +} +type BatchOperation struct { + Operation string `json:"operation"` + Path string `json:"path,omitempty"` + Source string `json:"source,omitempty"` + Destination string `json:"destination,omitempty"` + Recursive bool `json:"recursive,omitempty"` + Overwrite bool `json:"overwrite,omitempty"` + NoClobber bool `json:"noClobber,omitempty"` + Parents bool `json:"parents,omitempty"` + Verify *bool `json:"verify,omitempty"` +} +type BatchOperationError struct { + Code int `json:"code"` + Kind string `json:"kind"` + Message string `json:"message"` +} +type BatchResult struct { + Index int `json:"index"` + Line int `json:"line"` + Operation string `json:"operation"` + Path string `json:"path,omitempty"` + Source string `json:"source,omitempty"` + Destination string `json:"destination,omitempty"` + Parents bool `json:"parents,omitempty"` + Status string `json:"status"` + Error *BatchOperationError `json:"error,omitempty"` +} +type BatchSummary struct { + Total int `json:"total"` + Succeeded int `json:"succeeded"` + Failed int `json:"failed"` + Planned int `json:"planned"` + Skipped int `json:"skipped"` + Stopped bool `json:"stopped"` + DryRun bool `json:"dryRun"` + Results []BatchResult `json:"results"` +} + +type MetadataOperation string + +const ( + TagList MetadataOperation = "tag-list" + TagAdd MetadataOperation = "tag-add" + TagRemove MetadataOperation = "tag-remove" + FavoriteSet MetadataOperation = "favorite-set" + FavoriteUnset MetadataOperation = "favorite-unset" + PropertyGet MetadataOperation = "property-get" + PropertySet MetadataOperation = "property-set" + PropertyRemove MetadataOperation = "property-remove" +) + +type MetadataRequest struct { + Operation MetadataOperation + Path string + Tags []string + Namespace, Name, Value string + DryRun bool +} + +type Client interface { + SelectSpace(string) error + Context() context.Context + List(string) ([]webdav.Item, error) + Stat(string) (webdav.Item, error) + Stream(string, io.Writer) error + Capabilities() (webdav.Capabilities, error) + GetProperty(string, webdav.PropertyName) (webdav.PropertyValue, error) + SetProperty(string, webdav.PropertyName, string) error + RemoveProperty(string, webdav.PropertyName) error + EnsureCollection(string) error + Move(string, string, bool) error + Copy(string, string, bool) error + Remove(string, bool) error + Upload(context.Context, string, string, webdav.TransferOptions) error + Download(context.Context, string, string, webdav.TransferOptions) error + ListMyDrives(context.Context) ([]graph.Drive, error) + SharingCapabilities(context.Context) (sharing.Capabilities, error) + AddTags(context.Context, string, []string) error + RemoveTags(context.Context, string, []string) error +} +type ClientFactory func(context.Context, string) (Client, error) +type Options struct { + OutputMode appoutput.Mode + In io.Reader + Out, Err io.Writer + Concurrency int + Quiet bool + Space string + Logger logging.Logger + NewClient ClientFactory +} diff --git a/internal/app/filesystem_walk.go b/internal/app/filesystem/walk.go similarity index 86% rename from internal/app/filesystem_walk.go rename to internal/app/filesystem/walk.go index c869d19..1e86101 100644 --- a/internal/app/filesystem_walk.go +++ b/internal/app/filesystem/walk.go @@ -1,4 +1,4 @@ -package app +package filesystem import ( "fmt" @@ -6,10 +6,11 @@ import ( "strings" "github.com/mzner/ocis-cli/internal/apperror" + "github.com/mzner/ocis-cli/internal/webdav" ) type filesystemWalkEntry struct { - item item + item webdav.Item depth int last bool parentsLast []bool @@ -21,10 +22,10 @@ type filesystemWalk struct { } func walkFilesystem( - client *client, remote string, maxDepth, maxEntries int, + client Client, remote string, maxDepth, maxEntries int, detectDepthLimit bool, operation string, ) (filesystemWalk, error) { - root, err := client.stat(remote) + root, err := client.Stat(remote) if err != nil { return filesystemWalk{}, err } @@ -36,7 +37,7 @@ func walkFilesystem( } if maxDepth == 0 { if detectDepthLimit { - children, listErr := client.list(remote) + children, listErr := client.List(remote) if listErr != nil { return filesystemWalk{}, listErr } @@ -54,11 +55,11 @@ func walkFilesystem( } func appendFilesystemWalk( - client *client, remote string, depth int, parentsLast []bool, + client Client, remote string, depth int, parentsLast []bool, maxDepth, maxEntries int, detectDepthLimit bool, operation string, result *filesystemWalk, ) error { - children, err := client.list(remote) + children, err := client.List(remote) if err != nil { return err } @@ -100,7 +101,7 @@ func appendFilesystemWalk( continue } if detectDepthLimit && childDepth == maxDepth { - grandchildren, listErr := client.list(child.Path) + grandchildren, listErr := client.List(child.Path) if listErr != nil { return listErr } diff --git a/internal/app/filesystem_adapter.go b/internal/app/filesystem_adapter.go new file mode 100644 index 0000000..6c77124 --- /dev/null +++ b/internal/app/filesystem_adapter.go @@ -0,0 +1,75 @@ +package app + +import ( + "context" + "io" + + filesystemapp "github.com/mzner/ocis-cli/internal/app/filesystem" + "github.com/mzner/ocis-cli/internal/graph" + "github.com/mzner/ocis-cli/internal/sharing" + "github.com/mzner/ocis-cli/internal/webdav" +) + +type filesystemClientAdapter struct{ client *client } + +func (a filesystemClientAdapter) SelectSpace(v string) error { return a.client.selectSpace(v) } +func (a filesystemClientAdapter) Context() context.Context { return a.client.context() } +func (a filesystemClientAdapter) List(v string) ([]webdav.Item, error) { return a.client.list(v) } +func (a filesystemClientAdapter) Stat(v string) (webdav.Item, error) { return a.client.stat(v) } +func (a filesystemClientAdapter) Stream(v string, w io.Writer) error { return a.client.stream(v, w) } +func (a filesystemClientAdapter) Capabilities() (webdav.Capabilities, error) { + return a.client.capabilities() +} +func (a filesystemClientAdapter) GetProperty(v string, p webdav.PropertyName) (webdav.PropertyValue, error) { + return a.client.getProperty(v, p) +} +func (a filesystemClientAdapter) SetProperty(v string, p webdav.PropertyName, s string) error { + return a.client.setProperty(v, p, s) +} +func (a filesystemClientAdapter) RemoveProperty(v string, p webdav.PropertyName) error { + return a.client.removeProperty(v, p) +} +func (a filesystemClientAdapter) EnsureCollection(v string) error { + return a.client.ensureCollection(v) +} +func (a filesystemClientAdapter) Move(s, d string, o bool) error { return a.client.move(s, d, o) } +func (a filesystemClientAdapter) Copy(s, d string, o bool) error { return a.client.copy(s, d, o) } +func (a filesystemClientAdapter) Remove(v string, r bool) error { return a.client.remove(v, r) } +func (a filesystemClientAdapter) Upload(ctx context.Context, l, r string, o webdav.TransferOptions) error { + return a.client.davClient().UploadWithOptions(ctx, l, r, o) +} +func (a filesystemClientAdapter) Download(ctx context.Context, r, l string, o webdav.TransferOptions) error { + return a.client.davClient().DownloadWithOptions(ctx, r, l, o) +} +func (a filesystemClientAdapter) ListMyDrives(ctx context.Context) ([]graph.Drive, error) { + return a.client.graphClient().ListMyDrives(ctx) +} +func (a filesystemClientAdapter) SharingCapabilities(ctx context.Context) (sharing.Capabilities, error) { + return a.client.sharingClient().Capabilities(ctx) +} +func (a filesystemClientAdapter) AddTags(ctx context.Context, id string, t []string) error { + return a.client.graphClient().AddTags(ctx, id, t) +} +func (a filesystemClientAdapter) RemoveTags(ctx context.Context, id string, t []string) error { + return a.client.graphClient().RemoveTags(ctx, id, t) +} + +func filesystemOptions(options RunOptions) filesystemapp.Options { + return filesystemapp.Options{OutputMode: options.OutputMode, In: options.In, Out: options.Out, Err: options.Err, Concurrency: options.Concurrency, Quiet: options.Quiet, Space: options.Space, Logger: options.Logger, NewClient: func(ctx context.Context, p string) (filesystemapp.Client, error) { + c, err := newClientWithOptions(ctx, p, options) + if err != nil { + return nil, err + } + return filesystemClientAdapter{c}, nil + }} +} + +func toFilesystemRequest(r FilesystemRequest) filesystemapp.Request { + return filesystemapp.Request{Operation: filesystemapp.Operation(r.Operation), Source: r.Source, Destination: r.Destination, Recursive: r.Recursive, Overwrite: r.Overwrite, NoClobber: r.NoClobber, DryRun: r.DryRun, Verify: r.Verify, Parents: r.Parents, MaxDepth: r.MaxDepth, MaxEntries: r.MaxEntries} +} +func toBatchRequest(r BatchRequest) filesystemapp.BatchRequest { + return filesystemapp.BatchRequest{Input: r.Input, DryRun: r.DryRun, Confirmed: r.Confirmed, ContinueOnError: r.ContinueOnError, MaxOperations: r.MaxOperations} +} +func toMetadataRequest(r MetadataRequest) filesystemapp.MetadataRequest { + return filesystemapp.MetadataRequest{Operation: filesystemapp.MetadataOperation(r.Operation), Path: r.Path, Tags: r.Tags, Namespace: r.Namespace, Name: r.Name, Value: r.Value, DryRun: r.DryRun} +} diff --git a/internal/app/path.go b/internal/app/path.go new file mode 100644 index 0000000..1133a63 --- /dev/null +++ b/internal/app/path.go @@ -0,0 +1,24 @@ +package app + +import ( + "path" + "strings" +) + +// cleanRemote normalizes a user-facing remote path at the application facade. +func cleanRemote(remote string) string { + remote = strings.TrimSpace(remote) + if remote == "" || remote == "/" { + return "/" + } + return path.Clean("/" + strings.Trim(remote, "/")) +} + +func fallback(values ...string) string { + for _, value := range values { + if strings.TrimSpace(value) != "" { + return value + } + } + return "" +} diff --git a/internal/app/space_admin_test.go b/internal/app/space_admin_test.go index 8ac96df..2913e6e 100644 --- a/internal/app/space_admin_test.go +++ b/internal/app/space_admin_test.go @@ -159,19 +159,6 @@ func TestSpaceMemberDryRunAndValidation(t *testing.T) { } } -func TestRoleSemanticAliases(t *testing.T) { - for value, expected := range map[string]string{ - "Can view": "viewer", "viewer": "viewer", "read": "viewer", - "Can edit with versions and trashbin": "editor", "write": "editor", - "Can manage": "manager", "manager": "manager", - "custom role": "", - } { - if actual := roleSemantic(value); actual != expected { - t.Errorf("%q: got %q, want %q", value, actual, expected) - } - } -} - func TestSpaceUpdateUseCase(t *testing.T) { state := &spaceAdminServerState{} server := newSpaceAdminServer(t, state) diff --git a/internal/app/space_service.go b/internal/app/space_service.go index a0ce790..524d69f 100644 --- a/internal/app/space_service.go +++ b/internal/app/space_service.go @@ -5,6 +5,8 @@ import ( "fmt" "strings" + spacesapp "github.com/mzner/ocis-cli/internal/app/spaces" + "github.com/mzner/ocis-cli/internal/apperror" appoutput "github.com/mzner/ocis-cli/internal/output" ) @@ -53,14 +55,14 @@ func runSpace( if err != nil { return err } - details, err := loadSpaceDetails(ctx, client, value) + details, err := loadSpaceDetailsThroughDomain(ctx, client, value, options) if err != nil { return err } if options.OutputMode != appoutput.Human { return writeOutput(options, "space", details) } - return writeSpaceDetails(options, details) + return spacesapp.WriteDetails(spacesOptions(options), details) case SpaceUse: spaces, err := client.graphClient().ListMyDrives(ctx) if err != nil { diff --git a/internal/app/space_create_service.go b/internal/app/spaces/create.go similarity index 86% rename from internal/app/space_create_service.go rename to internal/app/spaces/create.go index d4c4a0c..ce59242 100644 --- a/internal/app/space_create_service.go +++ b/internal/app/spaces/create.go @@ -1,4 +1,4 @@ -package app +package spaces import ( "context" @@ -9,11 +9,11 @@ import ( "github.com/mzner/ocis-cli/internal/graph" ) -func runSpaceCreate( +func RunCreate( ctx context.Context, - request SpaceCreateRequest, + request CreateRequest, selectedProfile string, - options RunOptions, + options Options, ) error { options.Logger.Debug("run space create") request.Name = strings.TrimSpace(request.Name) @@ -50,11 +50,11 @@ func runSpaceCreate( ) } - client, err := newClientWithOptions(ctx, selectedProfile, options) + client, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } - created, err := client.graphClient().CreateDrive(ctx, createRequest) + created, err := client.Graph().CreateDrive(ctx, createRequest) if err != nil { return err } diff --git a/internal/app/space_details.go b/internal/app/spaces/details.go similarity index 93% rename from internal/app/space_details.go rename to internal/app/spaces/details.go index 178c98c..c83288f 100644 --- a/internal/app/space_details.go +++ b/internal/app/spaces/details.go @@ -1,4 +1,4 @@ -package app +package spaces import ( "context" @@ -40,8 +40,8 @@ type spaceAdministration struct { CanManageMembers bool `json:"canManageMembers"` } -type spaceDetails struct { - Space space `json:"space"` +type Details struct { + Space graph.Drive `json:"space"` Members []SpaceMember `json:"members"` AvailableRoles []spaceRole `json:"availableRoles"` AllowedActions []string `json:"allowedActions"` @@ -53,10 +53,10 @@ type spaceDetails struct { QuotaUnlimited bool `json:"quotaUnlimited"` } -func loadSpaceDetails( - ctx context.Context, client *client, selected space, -) (spaceDetails, error) { - details := spaceDetails{ +func LoadDetails( + ctx context.Context, client Client, selected graph.Drive, +) (Details, error) { + details := Details{ Space: selected, Members: []SpaceMember{}, AvailableRoles: []spaceRole{}, QuotaUnlimited: selected.Quota.Total == 0, } @@ -64,13 +64,13 @@ func loadSpaceDetails( details.QuotaUsagePercent = float64(selected.Quota.Used) / float64(selected.Quota.Total) * 100 } - permissions, err := client.graphClient().ListSpacePermissions(ctx, selected.ID) + permissions, err := client.Graph().ListSpacePermissions(ctx, selected.ID) if err != nil { switch protocolStatus(err) { case 401, 403: return details, nil default: - return spaceDetails{}, err + return Details{}, err } } details.PermissionsAvailable = true @@ -80,7 +80,7 @@ func loadSpaceDetails( details.Administration = administrationFromActions(permissions.AllowedActions) details.Administration.CanListMembers = true - current, err := client.graphClient().GetMe(ctx) + current, err := client.Graph().GetMe(ctx) if err == nil { details.CurrentUser = ¤t for _, member := range details.Members { @@ -93,7 +93,7 @@ func loadSpaceDetails( return details, nil } -func writeSpaceDetails(options RunOptions, details spaceDetails) error { +func WriteDetails(options Options, details Details) error { value := details.Space quota := fmt.Sprintf("%d / %d bytes", value.Quota.Used, value.Quota.Total) if details.QuotaUnlimited { diff --git a/internal/app/spaces/helpers.go b/internal/app/spaces/helpers.go new file mode 100644 index 0000000..2ee9fe9 --- /dev/null +++ b/internal/app/spaces/helpers.go @@ -0,0 +1,41 @@ +package spaces + +import ( + "errors" + "fmt" + "net/http" + "strings" + + "github.com/mzner/ocis-cli/internal/apperror" + "github.com/mzner/ocis-cli/internal/graph" + appoutput "github.com/mzner/ocis-cli/internal/output" +) + +func output(options Options, kind string, value any, format string, args ...any) error { + return (appoutput.Renderer{Writer: options.Out, Mode: options.OutputMode, Type: kind}).Write(value, format, args...) +} +func writeOutput(options Options, kind string, value any) error { + return output(options, kind, value, "") +} +func protocolStatus(err error) int { + var statusErr interface{ HTTPStatusCode() int } + if errors.As(err, &statusErr) { + return statusErr.HTTPStatusCode() + } + return http.StatusOK +} +func Resolve(spaces []graph.Drive, identifier string) (graph.Drive, error) { + var matches []graph.Drive + for _, v := range spaces { + if v.ID == identifier || strings.EqualFold(v.Name, identifier) || strings.EqualFold(v.DriveAlias, identifier) { + matches = append(matches, v) + } + } + if len(matches) == 1 { + return matches[0], nil + } + if len(matches) == 0 { + return graph.Drive{}, apperror.Wrap(apperror.KindUsage, "space", fmt.Errorf("unknown space %q; run ocis space list", identifier)) + } + return graph.Drive{}, apperror.Wrap(apperror.KindUsage, "space", fmt.Errorf("space name %q is ambiguous; use its ID", identifier)) +} diff --git a/internal/app/space_lifecycle_service.go b/internal/app/spaces/lifecycle.go similarity index 73% rename from internal/app/space_lifecycle_service.go rename to internal/app/spaces/lifecycle.go index dab3524..a35b6b6 100644 --- a/internal/app/space_lifecycle_service.go +++ b/internal/app/spaces/lifecycle.go @@ -1,4 +1,4 @@ -package app +package spaces import ( "context" @@ -8,11 +8,11 @@ import ( "github.com/mzner/ocis-cli/internal/apperror" ) -func runSpaceLifecycle( +func RunLifecycle( ctx context.Context, - request SpaceLifecycleRequest, + request LifecycleRequest, selectedProfile string, - options RunOptions, + options Options, ) error { options.Logger.Debug( "run space lifecycle operation", "operation", request.Operation, @@ -25,11 +25,11 @@ func runSpaceLifecycle( ) } switch request.Operation { - case SpaceDisable: + case Disable: return disableSpace(ctx, request, selectedProfile, options) - case SpaceRestore: + case Restore: return restoreSpace(ctx, request, selectedProfile, options) - case SpaceDelete: + case Delete: if !request.Permanent { return apperror.Wrap( apperror.KindUsage, "space delete", @@ -49,9 +49,9 @@ func runSpaceLifecycle( func disableSpace( ctx context.Context, - request SpaceLifecycleRequest, + request LifecycleRequest, selectedProfile string, - options RunOptions, + options Options, ) error { client, selected, err := resolveProjectSpace( ctx, request.Identifier, selectedProfile, options, @@ -69,20 +69,14 @@ func disableSpace( "Would disable project space %s (%s)\n", selected.Name, selected.ID, ) } - if err := client.graphClient().DeleteDrive(ctx, selected.ID, false); err != nil { + if err := client.Graph().DeleteDrive(ctx, selected.ID, false); err != nil { return err } - profile := client.store.Profiles[client.name] - if profile.DefaultSpace == selected.ID { - profile.DefaultSpace = "" - profile.DefaultSpaceOwner = "" - client.store.Profiles[client.name] = profile - if err := saveStore(options.Dependencies, client.store); err != nil { - return fmt.Errorf( - "space was disabled but default-space configuration could not be cleared: %w", - err, - ) - } + if err := client.ClearDefaultSpace(selected.ID); err != nil { + return fmt.Errorf( + "space was disabled but default-space configuration could not be cleared: %w", + err, + ) } return output( options, "space", @@ -97,9 +91,9 @@ func disableSpace( func restoreSpace( ctx context.Context, - request SpaceLifecycleRequest, + request LifecycleRequest, selectedProfile string, - options RunOptions, + options Options, ) error { if request.DryRun { return output( @@ -110,11 +104,11 @@ func restoreSpace( "Would restore project space %s\n", request.Identifier, ) } - client, err := newClientWithOptions(ctx, selectedProfile, options) + client, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } - restored, err := client.graphClient().RestoreDrive(ctx, request.Identifier) + restored, err := client.Graph().RestoreDrive(ctx, request.Identifier) if err != nil { return err } @@ -126,9 +120,9 @@ func restoreSpace( func permanentlyDeleteSpace( ctx context.Context, - request SpaceLifecycleRequest, + request LifecycleRequest, selectedProfile string, - options RunOptions, + options Options, ) error { if request.DryRun { return output( @@ -141,11 +135,11 @@ func permanentlyDeleteSpace( request.Identifier, ) } - client, err := newClientWithOptions(ctx, selectedProfile, options) + client, err := options.NewClient(ctx, selectedProfile) if err != nil { return err } - if err := client.graphClient().DeleteDrive( + if err := client.Graph().DeleteDrive( ctx, request.Identifier, true, ); err != nil { return err diff --git a/internal/app/space_member_service.go b/internal/app/spaces/member.go similarity index 90% rename from internal/app/space_member_service.go rename to internal/app/spaces/member.go index d1bbb82..2af8d92 100644 --- a/internal/app/space_member_service.go +++ b/internal/app/spaces/member.go @@ -1,4 +1,4 @@ -package app +package spaces import ( "context" @@ -11,11 +11,11 @@ import ( appoutput "github.com/mzner/ocis-cli/internal/output" ) -func runSpaceMember( +func RunMember( ctx context.Context, - request SpaceMemberRequest, + request MemberRequest, selectedProfile string, - options RunOptions, + options Options, ) error { options.Logger.Debug( "run space member operation", "operation", request.Operation, @@ -27,13 +27,13 @@ func runSpaceMember( return err } switch request.Operation { - case SpaceMemberList: + case MemberList: return listSpaceMembers(ctx, client, selected, options) - case SpaceMemberAdd: + case MemberAdd: return addSpaceMember(ctx, client, selected, request, options) - case SpaceMemberUpdate: + case MemberUpdate: return updateSpaceMember(ctx, client, selected, request, options) - case SpaceMemberRemove: + case MemberRemove: return removeSpaceMember(ctx, client, selected, request, options) default: return apperror.Wrap( @@ -44,9 +44,9 @@ func runSpaceMember( } func listSpaceMembers( - ctx context.Context, client *client, selected space, options RunOptions, + ctx context.Context, client Client, selected graph.Drive, options Options, ) error { - permissions, err := client.graphClient().ListSpacePermissions(ctx, selected.ID) + permissions, err := client.Graph().ListSpacePermissions(ctx, selected.ID) if err != nil { return err } @@ -68,10 +68,10 @@ func listSpaceMembers( func addSpaceMember( ctx context.Context, - client *client, - selected space, - request SpaceMemberRequest, - options RunOptions, + client Client, + selected graph.Drive, + request MemberRequest, + options Options, ) error { request.RecipientID = strings.TrimSpace(request.RecipientID) request.RecipientType = strings.ToLower(strings.TrimSpace(request.RecipientType)) @@ -117,7 +117,7 @@ func addSpaceMember( strings.ToLower(role.DisplayName), ) } - permission, err := client.graphClient().AddSpaceMember( + permission, err := client.Graph().AddSpaceMember( ctx, selected.ID, graph.InviteRequest{ Recipients: []graph.Recipient{{ @@ -139,10 +139,10 @@ func addSpaceMember( func updateSpaceMember( ctx context.Context, - client *client, - selected space, - request SpaceMemberRequest, - options RunOptions, + client Client, + selected graph.Drive, + request MemberRequest, + options Options, ) error { request.PermissionID = strings.TrimSpace(request.PermissionID) if request.PermissionID == "" { @@ -168,7 +168,7 @@ func updateSpaceMember( strings.ToLower(role.DisplayName), ) } - permission, err := client.graphClient().UpdateSpaceMember( + permission, err := client.Graph().UpdateSpaceMember( ctx, selected.ID, request.PermissionID, graph.PermissionUpdateRequest{Roles: []string{role.ID}}, ) @@ -186,10 +186,10 @@ func updateSpaceMember( func removeSpaceMember( ctx context.Context, - client *client, - selected space, - request SpaceMemberRequest, - options RunOptions, + client Client, + selected graph.Drive, + request MemberRequest, + options Options, ) error { request.PermissionID = strings.TrimSpace(request.PermissionID) if request.PermissionID == "" { @@ -206,7 +206,7 @@ func removeSpaceMember( request.PermissionID, selected.Name, ) } - if err := client.graphClient().RemoveSpaceMember( + if err := client.Graph().RemoveSpaceMember( ctx, selected.ID, request.PermissionID, ); err != nil { return err @@ -223,7 +223,7 @@ func removeSpaceMember( func resolveSpaceRole( ctx context.Context, - client *client, + client Client, spaceID string, requested string, ) (graph.Permissions, graph.RoleDefinition, error) { @@ -232,7 +232,7 @@ func resolveSpaceRole( return graph.Permissions{}, graph.RoleDefinition{}, usageSpaceMember("role must not be empty") } - permissions, err := client.graphClient().ListSpacePermissions(ctx, spaceID) + permissions, err := client.Graph().ListSpacePermissions(ctx, spaceID) if err != nil { return graph.Permissions{}, graph.RoleDefinition{}, err } diff --git a/internal/app/spaces/member_test.go b/internal/app/spaces/member_test.go new file mode 100644 index 0000000..5c71b79 --- /dev/null +++ b/internal/app/spaces/member_test.go @@ -0,0 +1,15 @@ +package spaces + +import "testing" + +func TestRoleSemanticAliases(t *testing.T) { + for value, expected := range map[string]string{ + "Can view": "viewer", "viewer": "viewer", "read": "viewer", + "Can edit with versions and trashbin": "editor", "write": "editor", + "Can manage": "manager", "manager": "manager", "custom role": "", + } { + if actual := roleSemantic(value); actual != expected { + t.Errorf("%q: got %q, want %q", value, actual, expected) + } + } +} diff --git a/internal/app/space_recipient.go b/internal/app/spaces/recipient.go similarity index 94% rename from internal/app/space_recipient.go rename to internal/app/spaces/recipient.go index 87bb7ef..348f08c 100644 --- a/internal/app/space_recipient.go +++ b/internal/app/spaces/recipient.go @@ -1,4 +1,4 @@ -package app +package spaces import ( "context" @@ -18,7 +18,7 @@ type spaceRecipient struct { func resolveSpaceRecipient( ctx context.Context, - client *client, + client Client, recipientType string, identifier string, isID bool, @@ -30,7 +30,7 @@ func resolveSpaceRecipient( func resolveRecipient( ctx context.Context, - client *client, + client Client, recipientType string, identifier string, isID bool, @@ -42,7 +42,7 @@ func resolveRecipient( var candidates []spaceRecipient switch recipientType { case "user": - users, err := client.graphClient().SearchUsers(ctx, identifier) + users, err := client.Graph().SearchUsers(ctx, identifier) if err != nil { return spaceRecipient{}, err } @@ -51,7 +51,7 @@ func resolveRecipient( candidates = append(candidates, recipientFromUser(user)) } case "group": - groups, err := client.graphClient().SearchGroups(ctx, identifier) + groups, err := client.Graph().SearchGroups(ctx, identifier) if err != nil { return spaceRecipient{}, err } diff --git a/internal/app/space_recipient_test.go b/internal/app/spaces/recipient_test.go similarity index 98% rename from internal/app/space_recipient_test.go rename to internal/app/spaces/recipient_test.go index 52a1128..f3622ae 100644 --- a/internal/app/space_recipient_test.go +++ b/internal/app/spaces/recipient_test.go @@ -1,4 +1,4 @@ -package app +package spaces import ( "strings" diff --git a/internal/app/spaces/types.go b/internal/app/spaces/types.go new file mode 100644 index 0000000..9fb8f9f --- /dev/null +++ b/internal/app/spaces/types.go @@ -0,0 +1,78 @@ +// Package spaces owns project Space lifecycle, details, and membership policy. +package spaces + +import ( + "context" + "io" + + "github.com/mzner/ocis-cli/internal/graph" + "github.com/mzner/ocis-cli/internal/logging" + appoutput "github.com/mzner/ocis-cli/internal/output" +) + +type CreateRequest struct { + Name, Description string + Quota *int64 + DryRun bool +} +type UpdateRequest struct { + Identifier string + Name, Description, Alias *string + Quota *int64 + DryRun bool +} +type LifecycleOperation string + +const ( + Disable LifecycleOperation = "disable" + Restore LifecycleOperation = "restore" + Delete LifecycleOperation = "delete" +) + +type LifecycleRequest struct { + Operation LifecycleOperation + Identifier string + Permanent, DryRun bool +} +type MemberOperation string + +const ( + MemberList MemberOperation = "list" + MemberAdd MemberOperation = "add" + MemberUpdate MemberOperation = "update" + MemberRemove MemberOperation = "remove" +) + +type MemberRequest struct { + Operation MemberOperation + Space, PermissionID, RecipientID string + RecipientIsID bool + RecipientType, Role string + DryRun bool +} + +type GraphClient interface { + ListDrives(context.Context) ([]graph.Drive, error) + CreateDrive(context.Context, graph.CreateDriveRequest) (graph.Drive, error) + UpdateDrive(context.Context, string, graph.UpdateDriveRequest) (graph.Drive, error) + DeleteDrive(context.Context, string, bool) error + RestoreDrive(context.Context, string) (graph.Drive, error) + ListSpacePermissions(context.Context, string) (graph.Permissions, error) + GetMe(context.Context) (graph.Me, error) + SearchUsers(context.Context, string) ([]graph.DirectoryUser, error) + SearchGroups(context.Context, string) ([]graph.DirectoryGroup, error) + AddSpaceMember(context.Context, string, graph.InviteRequest) (graph.Permission, error) + UpdateSpaceMember(context.Context, string, string, graph.PermissionUpdateRequest) (graph.Permission, error) + RemoveSpaceMember(context.Context, string, string) error +} +type Client interface { + Graph() GraphClient + ClearDefaultSpace(string) error +} +type ClientFactory func(context.Context, string) (Client, error) +type Options struct { + OutputMode appoutput.Mode + Out io.Writer + Logger logging.Logger + NewClient ClientFactory +} diff --git a/internal/app/space_update_service.go b/internal/app/spaces/update.go similarity index 80% rename from internal/app/space_update_service.go rename to internal/app/spaces/update.go index 1165965..53df9f9 100644 --- a/internal/app/space_update_service.go +++ b/internal/app/spaces/update.go @@ -1,4 +1,4 @@ -package app +package spaces import ( "context" @@ -9,11 +9,11 @@ import ( "github.com/mzner/ocis-cli/internal/graph" ) -func runSpaceUpdate( +func RunUpdate( ctx context.Context, - request SpaceUpdateRequest, + request UpdateRequest, selectedProfile string, - options RunOptions, + options Options, ) error { options.Logger.Debug("run space update") if request.Name == nil && request.Description == nil && @@ -63,7 +63,7 @@ func runSpaceUpdate( "Would update project space %s (%s)\n", selected.Name, selected.ID, ) } - updated, err := client.graphClient().UpdateDrive(ctx, selected.ID, update) + updated, err := client.Graph().UpdateDrive(ctx, selected.ID, update) if err != nil { return err } @@ -77,22 +77,22 @@ func resolveProjectSpace( ctx context.Context, identifier string, selectedProfile string, - options RunOptions, -) (*client, space, error) { - client, err := newClientWithOptions(ctx, selectedProfile, options) + options Options, +) (Client, graph.Drive, error) { + client, err := options.NewClient(ctx, selectedProfile) if err != nil { - return nil, space{}, err + return nil, graph.Drive{}, err } - spaces, err := client.graphClient().ListDrives(ctx) + spaces, err := client.Graph().ListDrives(ctx) if err != nil { - return nil, space{}, err + return nil, graph.Drive{}, err } - selected, err := resolveSpace(spaces, identifier) + selected, err := Resolve(spaces, identifier) if err != nil { - return nil, space{}, err + return nil, graph.Drive{}, err } if selected.DriveType != "project" { - return nil, space{}, apperror.Wrap( + return nil, graph.Drive{}, apperror.Wrap( apperror.KindUsage, "space", fmt.Errorf( "space %q has type %q; this operation requires a project space", diff --git a/internal/app/spaces_adapter.go b/internal/app/spaces_adapter.go new file mode 100644 index 0000000..a71ed53 --- /dev/null +++ b/internal/app/spaces_adapter.go @@ -0,0 +1,52 @@ +package app + +import ( + "context" + "fmt" + + spacesapp "github.com/mzner/ocis-cli/internal/app/spaces" + "github.com/mzner/ocis-cli/internal/graph" +) + +type spacesClientAdapter struct { + client *client + dependencies Dependencies +} + +func (a spacesClientAdapter) Graph() spacesapp.GraphClient { return a.client.graphClient() } +func (a spacesClientAdapter) ClearDefaultSpace(id string) error { + p := a.client.store.Profiles[a.client.name] + if p.DefaultSpace != id { + return nil + } + p.DefaultSpace, p.DefaultSpaceOwner = "", "" + a.client.store.Profiles[a.client.name] = p + if err := saveStore(a.dependencies, a.client.store); err != nil { + return fmt.Errorf("save profile: %w", err) + } + return nil +} +func spacesOptions(options RunOptions) spacesapp.Options { + return spacesapp.Options{OutputMode: options.OutputMode, Out: options.Out, Logger: options.Logger, NewClient: func(ctx context.Context, p string) (spacesapp.Client, error) { + c, err := newClientWithOptions(ctx, p, options) + if err != nil { + return nil, err + } + return spacesClientAdapter{client: c, dependencies: options.Dependencies}, nil + }} +} +func toSpaceCreateRequest(r SpaceCreateRequest) spacesapp.CreateRequest { + return spacesapp.CreateRequest{Name: r.Name, Description: r.Description, Quota: r.Quota, DryRun: r.DryRun} +} +func toSpaceUpdateRequest(r SpaceUpdateRequest) spacesapp.UpdateRequest { + return spacesapp.UpdateRequest{Identifier: r.Identifier, Name: r.Name, Description: r.Description, Alias: r.Alias, Quota: r.Quota, DryRun: r.DryRun} +} +func toSpaceLifecycleRequest(r SpaceLifecycleRequest) spacesapp.LifecycleRequest { + return spacesapp.LifecycleRequest{Operation: spacesapp.LifecycleOperation(r.Operation), Identifier: r.Identifier, Permanent: r.Permanent, DryRun: r.DryRun} +} +func toSpaceMemberRequest(r SpaceMemberRequest) spacesapp.MemberRequest { + return spacesapp.MemberRequest{Operation: spacesapp.MemberOperation(r.Operation), Space: r.Space, PermissionID: r.PermissionID, RecipientID: r.RecipientID, RecipientIsID: r.RecipientIsID, RecipientType: r.RecipientType, Role: r.Role, DryRun: r.DryRun} +} +func loadSpaceDetailsThroughDomain(ctx context.Context, c *client, d graph.Drive, o RunOptions) (spacesapp.Details, error) { + return spacesapp.LoadDetails(ctx, spacesClientAdapter{client: c, dependencies: o.Dependencies}, d) +} diff --git a/internal/app/sync_adapter.go b/internal/app/sync_adapter.go index e2cc637..2c6c503 100644 --- a/internal/app/sync_adapter.go +++ b/internal/app/sync_adapter.go @@ -26,7 +26,12 @@ func (adapter syncClientAdapter) EnsureCollection(remote string) error { return adapter.client.ensureCollection(remote) } func (adapter syncClientAdapter) DiscoverUploadCapabilities(ctx context.Context) webdav.TUSCapabilities { - return discoverUploadCapabilities(ctx, adapter.client) + capabilities, err := adapter.client.sharingClient().Capabilities(ctx) + if err != nil { + adapter.client.logger.Debug("TUS capability discovery failed; using WebDAV PUT", "reason", err.Error()) + return webdav.TUSCapabilities{} + } + return webdav.TUSCapabilities{Version: capabilities.Files.TUS.Version, Resumable: capabilities.Files.TUS.Resumable, Extensions: capabilities.Files.TUS.Extensions, MaxChunkSize: capabilities.Files.TUS.MaxChunkSize, HTTPMethodOverride: capabilities.Files.TUS.HTTPMethodOverride} } func (adapter syncClientAdapter) DAV() syncapp.DAVClient { return adapter.client.davClient() } From 705bc31264343597b720cd95fc908296878d26d8 Mon Sep 17 00:00:00 2001 From: Matteo Date: Fri, 14 Aug 2026 10:14:15 +0200 Subject: [PATCH 3/3] fix(ci): update Go to 1.26.6 - use the patched standard library in builds and scans\n- resolve the reachable govulncheck findings Signed-off-by: Matteo --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 20dcba1..4a7c2d2 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/mzner/ocis-cli -go 1.26.5 +go 1.26.6 require ( github.com/bdragon300/tusgo v0.2.0