ENG-11298 generate protos for managed catalog api#16
Conversation
| ProductPriceCurrency currency = 1 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "Currency code (e.g., USD)"}]; | ||
| ProductPricePlatform platform = 2 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "Platform (e.g., UNIVERSAL)"}]; | ||
| ProductPriceRegion region = 3 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "Region (e.g., US)"}]; | ||
| double price = 4 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { |
There was a problem hiding this comment.
hey @stephensxu can you please make this consistent with the uint32 cents in PriceAmount in /egress APIs? here and anywhere else?
I would consider this a High priority since using floating points to represent money is a BAD practice. please see: https://spin.atomicobject.com/floating-point-numbers
I know they're int64 in Spanner, that's not a problem; we never expect prices above uint32 range anyway, and they can never be negative
please LMK if you have any concerns or want to jump on GVC to discuss!
server/ingress/studio/v1/enums.proto
Outdated
| PRODUCT_STATUS_PUBLISHED = 2; | ||
| } | ||
|
|
||
| enum ProductPriceRegion { |
There was a problem hiding this comment.
I'd strongly suggest switching currency and region to strings here to match egress and avoid the maintenance burden.
the egress APIs use string currency (ISO-4217) and string region (ISO-3166-1 alpha-2), which means new currencies/regions don't require a proto change + redeploy.
protobuf's own docs cover the evolution challenges: https://protobuf.dev/programming-guides/enum/
| string name = 2 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "Internal product name"}]; | ||
| optional string external_id = 3 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "Partner-defined external identifier (e.g., SKU). Always non-empty for published products"}]; | ||
| optional string description = 4 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "Product description. Always non-empty for published products"}]; | ||
| optional google.protobuf.Struct images = 5 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { |
There was a problem hiding this comment.
please PLEASE don't mix JSON with protos.
please use explicit typed fields instead of google.protobuf.Struct
the shape (mainImage, backgroundImage, etc) is only documented in a description string with no schema enforcement.
egress uses typed fields for images (string image, Background message, etc.) which gives us codegen + validation for free.
the protobuf AEP guidance recommends Struct only as a last resort when the schema is truly unknown: https://aep.dev/146
| } | ||
| }; | ||
| string image = 1 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "URL to the item image. Always non-empty for published products"}]; | ||
| int64 quantity = 2 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "Quantity of this item. Encoded as a string in JSON (protobuf int64 convention). Always > 0 for published products"}]; |
There was a problem hiding this comment.
please consider using string for quantity to match egress, which uses string quantity with regex validation ^[0-9]+$ for arbitrarily large in-game item quantities.
since protobuf serializes int64 as a JSON string anyway (https://protobuf.dev/programming-guides/proto3/#json), the wire format is the same - so string gives us consistency for free.
not a blocker!
| } | ||
| }; | ||
| string product_id = 1 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "Unique product identifier (UUID)"}]; | ||
| string name = 2 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "Internal product name"}]; |
There was a problem hiding this comment.
name, description, and display_name are plain string here
is there any reason you can't copy the LocalizableText pattern from egress API for these?
I hate to be pedantic/annoying - but this is really important to get right from the start...
b/c retrofitting string -> LocalizableText later would be a breaking change!
so worth deciding now whether we want to support localization in v1 or intentionally skip this for the partner even though they're a big enough studio I'm 99% sure they're going to want localizations at some point soon...
I know Studio doesn't support localizations today, but I strongly recommend making the schema reflect the ability to support that. we can always just tell the partner the localizations aren't supported in this API yet :)
| description: "A published product from the managed catalog" | ||
| } | ||
| }; | ||
| string product_id = 1 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = {description: "Unique product identifier (UUID)"}]; |
There was a problem hiding this comment.
product_id here is described as a UUID, but in egress product_id is a free-form SKU with a separate guid field for the UUID.
this could confuse partners working with both APIs...
- could we change the field names to match? I feel this would be the simplest way to help avoid lots of confusion down the road
product_id->guidexternal_id->product_id
- alternatively, seems at least worth clarifying in the description that this is the Stash-internal UUID, not the partner's SKU (which looks like it maps to
external_idfield 3)...
| enum ProductStatus { | ||
| option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_enum) = {description: "Product publication status. This API only returns published products."}; | ||
| PRODUCT_STATUS_UNSPECIFIED = 0; | ||
| PRODUCT_STATUS_PUBLISHED = 2; |
There was a problem hiding this comment.
nit: looks like ProductStatus jumps from UNSPECIFIED=0 to PUBLISHED=2, skipping 1
is that intentional?
if your intent was to reserve 1 for DRAFT in case it's needed at some point, consider doing:
enum ProductStatus {
reserved 1;
reserved "PRODUCT_STATUS_DRAFT"; // DRAFT not supported in this API
PRODUCT_STATUS_UNSPECIFIED = 0;
PRODUCT_STATUS_PUBLISHED = 2;
}| enum ProductPricePlatform { | ||
| option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_enum) = {description: "Platform for a price entry"}; | ||
| PRODUCT_PRICE_PLATFORM_UNSPECIFIED = 0; | ||
| PRODUCT_PRICE_PLATFORM_UNIVERSAL = 1; |
There was a problem hiding this comment.
nit: looks like ANDROID and IOS were omitted.
that's fine for now, you can always add them later.
just checking that was intentional :)
dinosoeren
left a comment
There was a problem hiding this comment.
looks great! thank you for incorporating the feedback!

Summary
Adds the proto definitions for the new Managed Catalog API under
server/ingress/, following the ingress/egress split Soeren set up. Also generates a separate Redoc page (redoc.ingress.v1.html) for sharing with Rovio without publishing to the main public-facing docs.What's included
server/ingress/server.proto: OpenAPI metadata and HMAC security definition for the ingress APIserver/ingress/studio/v1/enums.proto:ProductStatus,ProductPriceRegion,ProductPricePlatform,ProductPriceCurrencyenums, all compliant withbuf lint ENUM_VALUE_PREFIXrulesserver/ingress/studio/v1/service.proto:ManagedCatalogServicewithGetProductandListProductsRPCs. Message types (Product,Price,Item) mirror the Studio schema but omit internal-only fields (publishable,freeGiftConfig,offerStartTime,offerEndTime)gen.sh)swagger-mergerstep with a separate ingress config (swagger-merger-ingress-config.json) to produceswagger.ingress.v1.jsondocs/gen/redoc.ingress.v1.html.github/workflows/check.yml)redoc.ingress.v1.htmlfrom the uncommitted-changes check (platform-specific HTML differences)Isolation from egress docs
The existing
swagger-merger-config.jsononly referencesserver/egress/paths, so the ingress protos do not appear in the main public Redoc page. The ingress docs are only accessible via the separateredoc.ingress.v1.htmlpage on GitHub Pages.