Skip to content

Commit 8fa515a

Browse files
committed
chore(milpacs): deprecate GetUserViaKeycloakId rpc + keycloak_id fields (#97)
Phase 1 of 2 — adds deprecation surface only, no behavior change. - `option deprecated = true;` on the RPC; `deprecated: true` in the openapiv2_operation so swagger UI flags the endpoint. - `[deprecated = true]` on `keycloak_id` in `KeycloakIdRequest`, `Profile`, `LiteProfile`. Propagates to .pb.go getters as `// Deprecated:` comments. - Enable protoc-gen-openapiv2 `enable_field_deprecation=true` so the `keycloakId` path parameter renders deprecated in swagger. - Pin the contract: descriptor-level deprecation test in proto/, swagger surface test in openapi/. Existing TestGetUserViaKeycloakId_* unchanged. Schema-property `deprecated` is OpenAPI 3.0 only — Swagger 2.0 has no slot for it, so the doc-side signal lives on the op + path param. Removal is tracked in #100.
1 parent 8747b58 commit 8fa515a

7 files changed

Lines changed: 153 additions & 53 deletions

File tree

buf.gen.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@ plugins:
1111
opt: paths=source_relative
1212
- local: protoc-gen-openapiv2
1313
out: openapi/assets
14+
opt: enable_field_deprecation=true

openapi/assets/milpacs.swagger.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,14 @@
107107
"name": "keycloakId",
108108
"in": "path",
109109
"required": true,
110+
"deprecated": true,
110111
"type": "string"
111112
}
112113
],
113114
"tags": [
114115
"Roster, Milpacs, Keycloak"
115-
]
116+
],
117+
"deprecated": true
116118
}
117119
},
118120
"/api/v1/milpacs/awol": {

openapi/openapi_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package openapi
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
// Phase 1 deprecation surface for GetUserViaKeycloakId (issue #97).
12+
// OpenAPI 2.0 supports `deprecated` on operations and parameters but NOT on
13+
// schema properties (added in OpenAPI 3.0). This test pins what the
14+
// swagger.json — i.e. the API doc surface — actually advertises to consumers.
15+
// Schema-property deprecation for Profile.keycloak_id / LiteProfile.keycloak_id
16+
// is asserted via the proto descriptor in proto_deprecation_test.go.
17+
func TestMilpacsSwagger_DeprecatesKeycloakIdSurface(t *testing.T) {
18+
raw, err := Files.ReadFile("assets/milpacs.swagger.json")
19+
require.NoError(t, err)
20+
21+
var spec struct {
22+
Paths map[string]map[string]struct {
23+
Deprecated bool `json:"deprecated"`
24+
Parameters []struct {
25+
Name string `json:"name"`
26+
Deprecated bool `json:"deprecated"`
27+
} `json:"parameters"`
28+
} `json:"paths"`
29+
}
30+
require.NoError(t, json.Unmarshal(raw, &spec))
31+
32+
op, ok := spec.Paths["/api/v1/milpac/keycloak/{keycloakId}"]["get"]
33+
require.True(t, ok, "GET /api/v1/milpac/keycloak/{keycloakId} missing from spec")
34+
assert.True(t, op.Deprecated, "GetUserViaKeycloakId op must be deprecated")
35+
36+
require.Len(t, op.Parameters, 1)
37+
assert.Equal(t, "keycloakId", op.Parameters[0].Name)
38+
assert.True(t, op.Parameters[0].Deprecated, "keycloakId path parameter must be deprecated")
39+
}

proto/deprecation_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package proto
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
"google.golang.org/protobuf/reflect/protoreflect"
9+
"google.golang.org/protobuf/types/descriptorpb"
10+
)
11+
12+
// Phase 1 deprecation contract for GetUserViaKeycloakId (issue #97).
13+
// Asserts the proto-level deprecation flags that downstream consumers
14+
// (generated code, descriptors, linters) actually observe. The OpenAPI
15+
// surface is asserted in openapi/openapi_test.go.
16+
func TestKeycloakIdDeprecationContract(t *testing.T) {
17+
svc := File_milpacs_proto.Services().ByName("MilpacService")
18+
require.NotNil(t, svc, "MilpacService missing from descriptor")
19+
20+
rpc := svc.Methods().ByName("GetUserViaKeycloakId")
21+
require.NotNil(t, rpc, "GetUserViaKeycloakId rpc missing")
22+
assert.True(t, methodDeprecated(rpc), "GetUserViaKeycloakId must be deprecated")
23+
24+
for _, c := range []struct {
25+
msg, field string
26+
}{
27+
{"KeycloakIdRequest", "keycloak_id"},
28+
{"Profile", "keycloak_id"},
29+
{"LiteProfile", "keycloak_id"},
30+
} {
31+
msg := File_milpacs_proto.Messages().ByName(protoreflect.Name(c.msg))
32+
require.NotNil(t, msg, "%s missing from descriptor", c.msg)
33+
f := msg.Fields().ByName(protoreflect.Name(c.field))
34+
require.NotNil(t, f, "%s.%s missing from descriptor", c.msg, c.field)
35+
assert.True(t, fieldDeprecated(f), "%s.%s must be deprecated", c.msg, c.field)
36+
}
37+
}
38+
39+
func methodDeprecated(m protoreflect.MethodDescriptor) bool {
40+
opts, ok := m.Options().(*descriptorpb.MethodOptions)
41+
return ok && opts.GetDeprecated()
42+
}
43+
44+
func fieldDeprecated(f protoreflect.FieldDescriptor) bool {
45+
opts, ok := f.Options().(*descriptorpb.FieldOptions)
46+
return ok && opts.GetDeprecated()
47+
}

proto/milpacs.pb.go

Lines changed: 55 additions & 49 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

proto/milpacs.proto

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,15 @@ service MilpacService {
8181
};
8282
};
8383
rpc GetUserViaKeycloakId(KeycloakIdRequest) returns (Profile){
84+
option deprecated = true;
8485
option(google.api.http) = {
8586
get: "/api/v1/milpac/keycloak/{keycloak_id}"
8687
};
8788
option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = {
8889
summary: "Get a Milpac profile via keycloak ID"
8990
description: "Get all milpac Profile data for a given user via their Keycloak ID"
9091
tags: "Roster, Milpacs, Keycloak"
92+
deprecated: true
9193
};
9294
};
9395
rpc GetUserViaDiscordId(DiscordIdRequest) returns (Profile){
@@ -239,7 +241,7 @@ message ProfileRequest {
239241
}
240242

241243
message KeycloakIdRequest {
242-
string keycloak_id = 1;
244+
string keycloak_id = 1 [deprecated = true];
243245
}
244246

245247
message DiscordIdRequest {
@@ -269,7 +271,7 @@ message Profile {
269271
repeated Award awards = 9;
270272
string join_date = 10;
271273
string promotion_date = 11;
272-
string keycloak_id = 12;
274+
string keycloak_id = 12 [deprecated = true];
273275
string discord_id = 13;
274276
string last_forum_post_date = 14;
275277
string mos = 15;
@@ -319,7 +321,7 @@ message LiteProfile {
319321
repeated Position secondaries = 7;
320322
string join_date = 8;
321323
string promotion_date = 9;
322-
string keycloak_id = 10;
324+
string keycloak_id = 10 [deprecated = true];
323325
string discord_id = 11;
324326
string award_date = 12;
325327
string record_date = 13;

0 commit comments

Comments
 (0)