mirror of https://github.com/usememos/memos.git
refactor(api): standardize API structure per Google AIP
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
This commit is contained in:
parent
5f57f48673
commit
133695da24
|
|
@ -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"}
|
||||
|
|
|
|||
|
|
@ -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"}
|
||||
];
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ const (
|
|||
AttachmentNamePrefix = "attachments/"
|
||||
ReactionNamePrefix = "reactions/"
|
||||
InboxNamePrefix = "inboxes/"
|
||||
IdentityProviderNamePrefix = "identityProviders/"
|
||||
IdentityProviderNamePrefix = "identity-providers/"
|
||||
ActivityNamePrefix = "activities/"
|
||||
WebhookNamePrefix = "webhooks/"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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) => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue