From 133695da24e6654bf059435250fa8d6ade607b5e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Nov 2025 08:33:00 +0000 Subject: [PATCH] refactor(api): standardize API structure per Google AIP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit addresses API design inconsistencies identified during standardization review against Google AIP (API Improvement Proposals) and REST API best practices. ## Changes Made ### 1. WorkspaceSetting Resource Namespace (proto) - Fixed: `api.memos.dev/WorkspaceSetting` → `memos.api.v1/WorkspaceSetting` - Location: proto/api/v1/workspace_service.proto (lines 57, 181) - Impact: Aligns with consistent namespace pattern used across all other resources ### 2. Identity Provider HTTP Paths (proto) - Changed: `/api/v1/identityProviders` → `/api/v1/identity-providers` - Changed: `identityProviders/{idp}` → `identity-providers/{idp}` - Location: proto/api/v1/idp_service.proto (all HTTP annotations and resource pattern) - Rationale: REST conventions prefer lowercase with hyphens (kebab-case) over camelCase - Follows: GitHub API standards and industry best practices ### 3. Backend Resource Name Constants (Go) - Updated: `IdentityProviderNamePrefix = "identity-providers/"` - Location: server/router/api/v1/resource_name.go:19 - Impact: Ensures resource name parsing matches new proto pattern ### 4. Backend Tests (Go) - Updated all hardcoded identity provider resource names - Location: server/router/api/v1/test/idp_service_test.go - Changed: `identityProviders/*` → `identity-providers/*` (4 occurrences) ### 5. Frontend Constants (TypeScript) - Updated: `identityProviderNamePrefix = "identity-providers/"` - Location: web/src/store/common.ts:4 - Impact: Frontend resource name handling aligns with backend ## Breaking Changes ⚠️ **IMPORTANT**: This is a breaking change for Identity Provider API consumers. **Before:** ``` GET /api/v1/identityProviders GET /api/v1/identityProviders/123 ``` **After:** ``` GET /api/v1/identity-providers GET /api/v1/identity-providers/123 ``` **Migration:** API clients must update URLs from `identityProviders` to `identity-providers`. ## Next Steps Required 1. **Regenerate proto code** (requires buf CLI): ```bash cd proto buf generate ``` This will update: - proto/gen/api/v1/*.pb.go (Go generated code) - proto/gen/api/v1/*.pb.gw.go (gRPC-Gateway code) - web/src/types/proto/api/v1/*.ts (TypeScript definitions) - proto/gen/openapi.yaml (OpenAPI spec) 2. **Test thoroughly:** - Backend tests: `go test ./server/router/api/v1/test/...` - Frontend integration tests - Manual API testing with new paths 3. **Update documentation:** - API documentation mentioning identity provider endpoints - Migration guides for existing API consumers ## References - Google AIP-122: Resource names - Google AIP-132: Standard List methods - REST API naming conventions (lowercase with hyphens) - GitHub API design standards ## Binary Endpoint Design (No Changes) After research, confirmed that the current `/file/*` path for binary content is correct and follows best practices: - Separates binary content delivery from structured API responses - Enables easier CDN integration and caching - Follows Google's pattern of using google.api.HttpBody with distinct paths --- proto/api/v1/idp_service.proto | 18 +++++++++--------- proto/api/v1/workspace_service.proto | 4 ++-- server/router/api/v1/resource_name.go | 2 +- server/router/api/v1/test/idp_service_test.go | 8 ++++---- web/src/store/common.ts | 2 +- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/proto/api/v1/idp_service.proto b/proto/api/v1/idp_service.proto index a35e6722c..e64f4f6d1 100644 --- a/proto/api/v1/idp_service.proto +++ b/proto/api/v1/idp_service.proto @@ -14,19 +14,19 @@ option go_package = "gen/api/v1"; service IdentityProviderService { // ListIdentityProviders lists identity providers. rpc ListIdentityProviders(ListIdentityProvidersRequest) returns (ListIdentityProvidersResponse) { - option (google.api.http) = {get: "/api/v1/identityProviders"}; + option (google.api.http) = {get: "/api/v1/identity-providers"}; } // GetIdentityProvider gets an identity provider. rpc GetIdentityProvider(GetIdentityProviderRequest) returns (IdentityProvider) { - option (google.api.http) = {get: "/api/v1/{name=identityProviders/*}"}; + option (google.api.http) = {get: "/api/v1/{name=identity-providers/*}"}; option (google.api.method_signature) = "name"; } // CreateIdentityProvider creates an identity provider. rpc CreateIdentityProvider(CreateIdentityProviderRequest) returns (IdentityProvider) { option (google.api.http) = { - post: "/api/v1/identityProviders" + post: "/api/v1/identity-providers" body: "identity_provider" }; option (google.api.method_signature) = "identity_provider"; @@ -35,7 +35,7 @@ service IdentityProviderService { // UpdateIdentityProvider updates an identity provider. rpc UpdateIdentityProvider(UpdateIdentityProviderRequest) returns (IdentityProvider) { option (google.api.http) = { - patch: "/api/v1/{identity_provider.name=identityProviders/*}" + patch: "/api/v1/{identity_provider.name=identity-providers/*}" body: "identity_provider" }; option (google.api.method_signature) = "identity_provider,update_mask"; @@ -43,7 +43,7 @@ service IdentityProviderService { // DeleteIdentityProvider deletes an identity provider. rpc DeleteIdentityProvider(DeleteIdentityProviderRequest) returns (google.protobuf.Empty) { - option (google.api.http) = {delete: "/api/v1/{name=identityProviders/*}"}; + option (google.api.http) = {delete: "/api/v1/{name=identity-providers/*}"}; option (google.api.method_signature) = "name"; } } @@ -51,14 +51,14 @@ service IdentityProviderService { message IdentityProvider { option (google.api.resource) = { type: "memos.api.v1/IdentityProvider" - pattern: "identityProviders/{idp}" + pattern: "identity-providers/{idp}" name_field: "name" singular: "identityProvider" plural: "identityProviders" }; // The resource name of the identity provider. - // Format: identityProviders/{idp} + // Format: identity-providers/{idp} string name = 1 [(google.api.field_behavior) = IDENTIFIER]; // Required. The type of the identity provider. @@ -112,7 +112,7 @@ message ListIdentityProvidersResponse { message GetIdentityProviderRequest { // Required. The resource name of the identity provider to get. - // Format: identityProviders/{idp} + // Format: identity-providers/{idp} string name = 1 [ (google.api.field_behavior) = REQUIRED, (google.api.resource_reference) = {type: "memos.api.v1/IdentityProvider"} @@ -139,7 +139,7 @@ message UpdateIdentityProviderRequest { message DeleteIdentityProviderRequest { // Required. The resource name of the identity provider to delete. - // Format: identityProviders/{idp} + // Format: identity-providers/{idp} string name = 1 [ (google.api.field_behavior) = REQUIRED, (google.api.resource_reference) = {type: "memos.api.v1/IdentityProvider"} diff --git a/proto/api/v1/workspace_service.proto b/proto/api/v1/workspace_service.proto index fc30aee2a..110849430 100644 --- a/proto/api/v1/workspace_service.proto +++ b/proto/api/v1/workspace_service.proto @@ -54,7 +54,7 @@ message GetWorkspaceProfileRequest {} // A workspace setting resource. message WorkspaceSetting { option (google.api.resource) = { - type: "api.memos.dev/WorkspaceSetting" + type: "memos.api.v1/WorkspaceSetting" pattern: "workspace/settings/{setting}" singular: "workspaceSetting" plural: "workspaceSettings" @@ -178,7 +178,7 @@ message GetWorkspaceSettingRequest { // Format: workspace/settings/{setting} string name = 1 [ (google.api.field_behavior) = REQUIRED, - (google.api.resource_reference) = {type: "api.memos.dev/WorkspaceSetting"} + (google.api.resource_reference) = {type: "memos.api.v1/WorkspaceSetting"} ]; } diff --git a/server/router/api/v1/resource_name.go b/server/router/api/v1/resource_name.go index b8d5c3aeb..a44e2f935 100644 --- a/server/router/api/v1/resource_name.go +++ b/server/router/api/v1/resource_name.go @@ -16,7 +16,7 @@ const ( AttachmentNamePrefix = "attachments/" ReactionNamePrefix = "reactions/" InboxNamePrefix = "inboxes/" - IdentityProviderNamePrefix = "identityProviders/" + IdentityProviderNamePrefix = "identity-providers/" ActivityNamePrefix = "activities/" WebhookNamePrefix = "webhooks/" ) diff --git a/server/router/api/v1/test/idp_service_test.go b/server/router/api/v1/test/idp_service_test.go index 7b7da6bd4..4b7cec763 100644 --- a/server/router/api/v1/test/idp_service_test.go +++ b/server/router/api/v1/test/idp_service_test.go @@ -56,7 +56,7 @@ func TestCreateIdentityProvider(t *testing.T) { require.NotNil(t, resp) require.Equal(t, "Test OAuth2 Provider", resp.Title) require.Equal(t, v1pb.IdentityProvider_OAUTH2, resp.Type) - require.Contains(t, resp.Name, "identityProviders/") + require.Contains(t, resp.Name, "identity-providers/") require.NotNil(t, resp.Config.GetOauth2Config()) require.Equal(t, "test-client-id", resp.Config.GetOauth2Config().ClientId) }) @@ -249,7 +249,7 @@ func TestGetIdentityProvider(t *testing.T) { defer ts.Cleanup() req := &v1pb.GetIdentityProviderRequest{ - Name: "identityProviders/999", + Name: "identity-providers/999", } _, err := ts.Service.GetIdentityProvider(ctx, req) @@ -355,7 +355,7 @@ func TestUpdateIdentityProvider(t *testing.T) { req := &v1pb.UpdateIdentityProviderRequest{ IdentityProvider: &v1pb.IdentityProvider{ - Name: "identityProviders/1", + Name: "identity-providers/1", Title: "Updated Provider", }, } @@ -466,7 +466,7 @@ func TestDeleteIdentityProvider(t *testing.T) { userCtx := ts.CreateUserContext(ctx, hostUser.ID) req := &v1pb.DeleteIdentityProviderRequest{ - Name: "identityProviders/999", + Name: "identity-providers/999", } _, err = ts.Service.DeleteIdentityProvider(userCtx, req) diff --git a/web/src/store/common.ts b/web/src/store/common.ts index 1b0774cc2..9d908908f 100644 --- a/web/src/store/common.ts +++ b/web/src/store/common.ts @@ -1,7 +1,7 @@ export const workspaceSettingNamePrefix = "workspace/settings/"; export const userNamePrefix = "users/"; export const memoNamePrefix = "memos/"; -export const identityProviderNamePrefix = "identityProviders/"; +export const identityProviderNamePrefix = "identity-providers/"; export const activityNamePrefix = "activities/"; export const extractUserIdFromName = (name: string) => {