Skip to content

Commit 1965eb4

Browse files
domdomeggdomdomegg
committed
Clean up publish and edit handler and service logic
Thanks Camila for being a great Go consultant and advising on how to tidy this up! (I'm still a lil grumpy about some parts, but it's in much better shape 😄) - Consolidated publisher extension validation into a single function. - Updated the `Publish` and `EditServer` methods to directly use `XPublisher` from the request. - Removed outdated tests for publisher extension extraction and validation. - Introduced new validation functions for server details and remote URL matching. - Enhanced error handling for server name format and remote URL validation. - Updated existing tests to reflect changes in validation logic and structure.
1 parent 9d04671 commit 1965eb4

File tree

12 files changed

+533
-778
lines changed

12 files changed

+533
-778
lines changed

‎internal/api/handlers/v0/edit.go‎

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package v0
22

33
import (
44
"context"
5-
"encoding/json"
65
"errors"
76
"net/http"
87
"strings"
@@ -18,9 +17,9 @@ import (
1817

1918
// EditServerInput represents the input for editing a server
2019
type EditServerInput struct {
21-
Authorization string `header:"Authorization" doc:"Registry JWT token with edit permissions" required:"true"`
22-
ID string `path:"id" doc:"Server ID (UUID)" format:"uuid"`
23-
RawBody []byte `body:"raw"`
20+
Authorization string `header:"Authorization" doc:"Registry JWT token with edit permissions" required:"true"`
21+
ID string `path:"id" doc:"Server ID (UUID)" format:"uuid"`
22+
Body model.PublishRequest `body:""`
2423
}
2524

2625
// RegisterEditEndpoints registers the edit endpoint
@@ -53,11 +52,6 @@ func RegisterEditEndpoints(api huma.API, registry service.RegistryService, cfg *
5352
return nil, huma.Error401Unauthorized("Invalid or expired Registry JWT token", err)
5453
}
5554

56-
// Validate that only allowed extension fields are present
57-
if err := model.ValidatePublishRequestExtensions(input.RawBody); err != nil {
58-
return nil, huma.Error400BadRequest("Invalid request format", err)
59-
}
60-
6155
// Get current server to check permissions against existing name
6256
currentServer, err := registry.GetByID(input.ID)
6357
if err != nil {
@@ -72,30 +66,23 @@ func RegisterEditEndpoints(api huma.API, registry service.RegistryService, cfg *
7266
return nil, huma.Error403Forbidden("You do not have edit permissions for this server")
7367
}
7468

75-
// Parse the validated request body
76-
var editRequest model.PublishRequest
77-
if err := json.Unmarshal(input.RawBody, &editRequest); err != nil {
78-
return nil, huma.Error400BadRequest("Invalid JSON format", err)
79-
}
80-
81-
// Validate the server detail
82-
validator := validators.NewObjectValidator()
83-
if err := validator.Validate(&editRequest.Server); err != nil {
69+
// Perform all schema validation
70+
if err := validators.ValidatePublishRequest(input.Body); err != nil {
8471
return nil, huma.Error400BadRequest(err.Error())
8572
}
8673

8774
// Prevent renaming servers
88-
if currentServer.Server.Name != editRequest.Server.Name {
75+
if currentServer.Server.Name != input.Body.Server.Name {
8976
return nil, huma.Error400BadRequest("Cannot rename server")
9077
}
9178

9279
// Prevent undeleting servers - once deleted, they stay deleted
93-
if currentServer.Server.Status == model.ServerStatusDeleted && editRequest.Server.Status != model.ServerStatusDeleted {
80+
if currentServer.Server.Status == model.ServerStatusDeleted && input.Body.Server.Status != model.ServerStatusDeleted {
9481
return nil, huma.Error400BadRequest("Cannot change status of deleted server. Deleted servers cannot be undeleted.")
9582
}
9683

9784
// Edit the server
98-
updatedServer, err := registry.EditServer(input.ID, editRequest)
85+
updatedServer, err := registry.EditServer(input.ID, input.Body)
9986
if err != nil {
10087
if errors.Is(err, database.ErrNotFound) {
10188
return nil, huma.Error404NotFound("Server not found")

‎internal/api/handlers/v0/edit_test.go‎

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,13 @@ func TestEditServerEndpoint(t *testing.T) {
9494
name: "invalid authorization header format",
9595
serverID: "550e8400-e29b-41d4-a716-446655440001",
9696
authHeader: "InvalidFormat token123",
97-
requestBody: model.PublishRequest{},
97+
requestBody: model.PublishRequest{
98+
Server: model.ServerDetail{
99+
Name: "io.github.domdomegg/test-server",
100+
Description: "Test server",
101+
VersionDetail: model.VersionDetail{Version: "1.0.0"},
102+
},
103+
},
98104
setupMocks: func(_ *MockRegistryService) {},
99105
expectedStatus: http.StatusUnauthorized,
100106
expectedError: "Unauthorized",
@@ -103,7 +109,13 @@ func TestEditServerEndpoint(t *testing.T) {
103109
name: "invalid token",
104110
serverID: "550e8400-e29b-41d4-a716-446655440001",
105111
authHeader: "Bearer invalid-token",
106-
requestBody: model.PublishRequest{},
112+
requestBody: model.PublishRequest{
113+
Server: model.ServerDetail{
114+
Name: "io.github.domdomegg/test-server",
115+
Description: "Test server",
116+
VersionDetail: model.VersionDetail{Version: "1.0.0"},
117+
},
118+
},
107119
setupMocks: func(_ *MockRegistryService) {},
108120
expectedStatus: http.StatusUnauthorized,
109121
expectedError: "Unauthorized",
@@ -239,7 +251,7 @@ func TestEditServerEndpoint(t *testing.T) {
239251
expectedError: "Bad Request",
240252
},
241253
{
242-
name: "invalid request body",
254+
name: "validation error - invalid server name",
243255
serverID: "550e8400-e29b-41d4-a716-446655440001",
244256
authHeader: func() string {
245257
cfg := &config.Config{JWTPrivateKey: "bb2c6b424005acd5df47a9e2c87f446def86dd740c888ea3efb825b23f7ef47c"}
@@ -252,8 +264,23 @@ func TestEditServerEndpoint(t *testing.T) {
252264
})
253265
return "Bearer " + token
254266
}(),
255-
requestBody: "invalid json",
256-
setupMocks: func(_ *MockRegistryService) {},
267+
requestBody: model.PublishRequest{
268+
Server: model.ServerDetail{
269+
Name: "invalid-name-format", // Missing namespace/name format
270+
Description: "Test server",
271+
VersionDetail: model.VersionDetail{Version: "1.0.0"},
272+
},
273+
},
274+
setupMocks: func(registry *MockRegistryService) {
275+
currentServer := &model.ServerResponse{
276+
Server: model.ServerDetail{
277+
Name: "io.github.domdomegg/test-server",
278+
Description: "Original server",
279+
Status: model.ServerStatusActive,
280+
},
281+
}
282+
registry.On("GetByID", "550e8400-e29b-41d4-a716-446655440001").Return(currentServer, nil)
283+
},
257284
expectedStatus: http.StatusBadRequest,
258285
expectedError: "Bad Request",
259286
},

‎internal/api/handlers/v0/publish.go‎

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package v0
22

33
import (
44
"context"
5-
"encoding/json"
65
"net/http"
76
"strings"
87

@@ -16,8 +15,8 @@ import (
1615

1716
// PublishServerInput represents the input for publishing a server
1817
type PublishServerInput struct {
19-
Authorization string `header:"Authorization" doc:"Registry JWT token (obtained from /v0/auth/token/github)" required:"true"`
20-
RawBody []byte `body:"raw"`
18+
Authorization string `header:"Authorization" doc:"Registry JWT token (obtained from /v0/auth/token/github)" required:"true"`
19+
Body model.PublishRequest `body:""`
2120
}
2221

2322
// RegisterPublishEndpoint registers the publish endpoint
@@ -50,33 +49,18 @@ func RegisterPublishEndpoint(api huma.API, registry service.RegistryService, cfg
5049
return nil, huma.Error401Unauthorized("Invalid or expired Registry JWT token", err)
5150
}
5251

53-
// Validate that only allowed extension fields are present
54-
if err := model.ValidatePublishRequestExtensions(input.RawBody); err != nil {
55-
return nil, huma.Error400BadRequest("Invalid request format", err)
56-
}
57-
58-
// Parse the validated request body
59-
var publishRequest model.PublishRequest
60-
if err := json.Unmarshal(input.RawBody, &publishRequest); err != nil {
61-
return nil, huma.Error400BadRequest("Invalid JSON format", err)
62-
}
63-
64-
// Get server details from request body
65-
serverDetail := publishRequest.Server
66-
67-
// Validate the server detail
68-
validator := validators.NewObjectValidator()
69-
if err := validator.Validate(&serverDetail); err != nil {
52+
// Perform all schema validation
53+
if err := validators.ValidatePublishRequest(input.Body); err != nil {
7054
return nil, huma.Error400BadRequest(err.Error())
7155
}
7256

73-
// Verify that the token's repository matches the server being published
74-
if !jwtManager.HasPermission(serverDetail.Name, auth.PermissionActionPublish, claims.Permissions) {
57+
// Verify that the token has permission to publish the server
58+
if !jwtManager.HasPermission(input.Body.Server.Name, auth.PermissionActionPublish, claims.Permissions) {
7559
return nil, huma.Error403Forbidden("You do not have permission to publish this server")
7660
}
7761

7862
// Publish the server with extensions
79-
publishedServer, err := registry.Publish(publishRequest)
63+
publishedServer, err := registry.Publish(input.Body)
8064
if err != nil {
8165
return nil, huma.Error400BadRequest("Failed to publish server", err)
8266
}

‎internal/api/handlers/v0/publish_integration_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,9 @@ func TestPublishIntegration(t *testing.T) {
168168
t.Run("publish fails with invalid token", func(t *testing.T) {
169169
publishReq := model.PublishRequest{
170170
Server: model.ServerDetail{
171-
Name: "test-server",
171+
Name: "io.github.domdomegg/test-server",
172+
Description: "Test server",
173+
VersionDetail: model.VersionDetail{Version: "1.0.0"},
172174
},
173175
}
174176

‎internal/api/handlers/v0/publish_test.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,14 @@ func TestPublishEndpoint(t *testing.T) {
149149
expectedError: "required header parameter is missing",
150150
},
151151
{
152-
name: "invalid authorization header format",
153-
requestBody: model.PublishRequest{},
152+
name: "invalid authorization header format",
153+
requestBody: model.PublishRequest{
154+
Server: model.ServerDetail{
155+
Name: "io.github.domdomegg/test-server",
156+
Description: "Test server",
157+
VersionDetail: model.VersionDetail{Version: "1.0.0"},
158+
},
159+
},
154160
authHeader: "InvalidFormat",
155161
setupMocks: func(_ *MockRegistryService) {},
156162
expectedStatus: http.StatusUnauthorized,

‎internal/database/import.go‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515

1616
// ReadSeedFile reads seed data from various sources:
1717
// 1. Local file paths (*.json files) - expects extension wrapper format
18-
// 2. Direct HTTP URLs to seed.json files - expects extension wrapper format
18+
// 2. Direct HTTP URLs to seed.json files - expects extension wrapper format
1919
// 3. Registry root URLs (automatically appends /v0/servers and paginates)
2020
// Only the extension wrapper format is supported (array of ServerResponse objects)
2121
func ReadSeedFile(ctx context.Context, path string) ([]*model.ServerRecord, error) {
@@ -127,7 +127,7 @@ func fetchFromRegistryAPI(ctx context.Context, baseURL string) ([]*model.ServerR
127127
func convertServerResponseToRecord(response model.ServerResponse) *model.ServerRecord {
128128
// Extract registry metadata from the extension
129129
registryExt := response.XIOModelContextProtocolRegistry
130-
130+
131131
// Parse timestamps
132132
publishedAt, _ := time.Parse(time.RFC3339, getStringFromInterface(registryExt, "published_at"))
133133
updatedAt, _ := time.Parse(time.RFC3339, getStringFromInterface(registryExt, "updated_at"))
@@ -141,11 +141,9 @@ func convertServerResponseToRecord(response model.ServerResponse) *model.ServerR
141141
}
142142

143143
// Publisher extensions
144-
publisherExtensions := make(map[string]interface{})
145-
if response.XPublisher != nil {
146-
if publisherMap, ok := response.XPublisher.(map[string]interface{}); ok {
147-
publisherExtensions = publisherMap
148-
}
144+
publisherExtensions := response.XPublisher
145+
if publisherExtensions == nil {
146+
publisherExtensions = make(map[string]interface{})
149147
}
150148

151149
return &model.ServerRecord{
@@ -175,4 +173,4 @@ func getBoolFromInterface(data interface{}, key string) bool {
175173
}
176174
}
177175
return false
178-
}
176+
}

0 commit comments

Comments
 (0)