Copilot commented on code in PR #95: URL: https://github.com/apache/incubator-seata-go-samples/pull/95#discussion_r3385155869
########## at/ecommerce/sql/mysql_ecommerce.sql: ########## @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +CREATE DATABASE IF NOT EXISTS seata_ecommerce_order DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci; +CREATE DATABASE IF NOT EXISTS seata_ecommerce_inventory DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci; +CREATE DATABASE IF NOT EXISTS seata_ecommerce_account DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci; + +USE seata_ecommerce_order; + +CREATE TABLE IF NOT EXISTS order_tbl ( + id INT NOT NULL AUTO_INCREMENT, + user_id VARCHAR(64) NOT NULL, + commodity_code VARCHAR(64) NOT NULL, + count INT NOT NULL, + money INT NOT NULL, + status VARCHAR(32) NOT NULL, + PRIMARY KEY (id) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +CREATE TABLE IF NOT EXISTS undo_log ( + id BIGINT NOT NULL AUTO_INCREMENT, + branch_id BIGINT NOT NULL, + xid VARCHAR(100) NOT NULL, + context VARCHAR(128) NOT NULL, + rollback_info LONGBLOB NOT NULL, + log_status INT NOT NULL, + log_created DATETIME NOT NULL, + log_modified DATETIME NOT NULL, + ext VARCHAR(100) DEFAULT NULL, + PRIMARY KEY (id), + UNIQUE KEY ux_undo_log (xid, branch_id) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +USE seata_ecommerce_inventory; + +CREATE TABLE IF NOT EXISTS inventory_tbl ( + id INT NOT NULL AUTO_INCREMENT, + commodity_code VARCHAR(64) NOT NULL, + stock INT NOT NULL, + PRIMARY KEY (id), + UNIQUE KEY uk_commodity_code (commodity_code) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +INSERT INTO inventory_tbl (commodity_code, stock) VALUES ('C100001', 100) +ON DUPLICATE KEY UPDATE stock = VALUES(stock); + +CREATE TABLE IF NOT EXISTS undo_log ( + id BIGINT NOT NULL AUTO_INCREMENT, + branch_id BIGINT NOT NULL, + xid VARCHAR(100) NOT NULL, + context VARCHAR(128) NOT NULL, + rollback_info LONGBLOB NOT NULL, + log_status INT NOT NULL, + log_created DATETIME NOT NULL, + log_modified DATETIME NOT NULL, + ext VARCHAR(100) DEFAULT NULL, + PRIMARY KEY (id), + UNIQUE KEY ux_undo_log (xid, branch_id) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +USE seata_ecommerce_account; + +CREATE TABLE IF NOT EXISTS account_tbl ( + id INT NOT NULL AUTO_INCREMENT, + user_id VARCHAR(64) NOT NULL, + balance INT NOT NULL, + PRIMARY KEY (id), + UNIQUE KEY uk_user_id (user_id) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +INSERT INTO account_tbl (user_id, balance) VALUES ('U100001', 50) +ON DUPLICATE KEY UPDATE balance = VALUES(balance); Review Comment: Same issue as above: `VALUES(balance)` is deprecated in MySQL 8.0 for `ON DUPLICATE KEY UPDATE`. Use the alias-based syntax to avoid deprecation warnings and future breakage. ########## at/ecommerce/order/create.go: ########## @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package main + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "time" + + "github.com/gin-gonic/gin" + "seata.apache.org/seata-go-samples/util" + "seata.apache.org/seata-go/pkg/constant" + "seata.apache.org/seata-go/pkg/tm" + "seata.apache.org/seata-go/pkg/util/log" +) + +type OrderRequest struct { + UserID string `json:"userId"` + CommodityCode string `json:"commodityCode"` + Count int `json:"count"` + Money int `json:"money"` +} + +type InventoryRequest struct { + CommodityCode string `json:"commodityCode"` + Count int `json:"count"` +} + +type AccountRequest struct { + UserID string `json:"userId"` + Money int `json:"money"` +} + +func createOrder(c *gin.Context) error { + var req OrderRequest + if err := c.ShouldBindJSON(&req); err != nil { + return util.NewValidationError(err.Error()) + } + if strings.TrimSpace(req.UserID) == "" { + return util.NewValidationError("userId is required") + } + if strings.TrimSpace(req.CommodityCode) == "" { + return util.NewValidationError("commodityCode is required") + } + if req.Count <= 0 { + return util.NewValidationError("count must be greater than 0") + } + if req.Money <= 0 { + return util.NewValidationError("money must be greater than 0") + } + + return tm.WithGlobalTx(c.Request.Context(), &tm.GtxConfig{ + Name: "ATSampleEcommerceCreateOrder", + Timeout: time.Second * 30, + }, func(ctx context.Context) error { + if err := insertOrder(ctx, req); err != nil { + return err + } + if err := deductInventory(ctx, req); err != nil { + return err + } + if err := deductAccount(ctx, req); err != nil { + return err + } + return nil + }) +} + +func insertOrder(ctx context.Context, req OrderRequest) error { + query := "insert into order_tbl(user_id, commodity_code, count, money, status) values (?, ?, ?, ?, ?)" + ret, err := db.ExecContext(ctx, query, req.UserID, req.CommodityCode, req.Count, req.Money, "CREATED") + if err != nil { + return err + } + + rows, err := ret.RowsAffected() + if err != nil { + return err + } + if rows != 1 { + return fmt.Errorf("create order affected unexpected rows: %d", rows) + } + return nil +} + +func deductInventory(ctx context.Context, req OrderRequest) error { + payload, err := json.Marshal(InventoryRequest{ + CommodityCode: req.CommodityCode, + Count: req.Count, + }) + if err != nil { + return err + } + + log.Infof("call inventory service, xid=%s", tm.GetXID(ctx)) + return postJSON(ctx, inventoryService+"/deductInventory", payload) +} + +func deductAccount(ctx context.Context, req OrderRequest) error { + payload, err := json.Marshal(AccountRequest{ + UserID: req.UserID, + Money: req.Money, + }) + if err != nil { + return err + } + + log.Infof("call account service, xid=%s", tm.GetXID(ctx)) + return postJSON(ctx, accountService+"/deductAccount", payload) +} + +func postJSON(ctx context.Context, url string, payload []byte) error { + requestCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + httpReq, err := http.NewRequestWithContext(requestCtx, http.MethodPost, url, bytes.NewReader(payload)) + if err != nil { + return err + } + httpReq.Header.Set(constant.XidKey, tm.GetXID(ctx)) + httpReq.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(httpReq) + if err != nil { + return util.NewDownstreamError(0, fmt.Sprintf("request %s failed: %v", url, err)) + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return err + } + if resp.StatusCode != http.StatusOK { + message := strings.TrimSpace(string(body)) + var response util.APIResponse + if err := json.Unmarshal(body, &response); err == nil { + if response.Error != "" { + message = response.Error + } else if response.Message != "" { + message = response.Message + } + } + return util.NewDownstreamError(resp.StatusCode, message) + } + return nil Review Comment: The response body is always fully read (via `io.ReadAll`) even for successful `200` responses, which can be wasteful and adds latency/memory pressure under load. Consider only reading/parsing the body on non-`200` responses (and for `200`, either skip reading entirely if the response is guaranteed empty, or read a bounded amount if you need diagnostics). ########## util/http.go: ########## @@ -0,0 +1,109 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package util + +import ( + "errors" + "net/http" +) + +type APIResponse struct { + Message string `json:"message,omitempty"` + Error string `json:"error,omitempty"` +} + +type ValidationError struct { + Message string +} + +func (e *ValidationError) Error() string { + return e.Message +} + +func NewValidationError(message string) error { + return &ValidationError{Message: message} +} + +type ConflictError struct { + Message string +} + +func (e *ConflictError) Error() string { + return e.Message +} + +func NewConflictError(message string) error { + return &ConflictError{Message: message} +} + +type DownstreamError struct { + StatusCode int + Message string +} + +func (e *DownstreamError) Error() string { + return e.Message +} Review Comment: `DownstreamError.Error()` drops the HTTP `StatusCode`, which makes logs/debugging less informative (e.g., a `400` vs `409` vs `503` will all look the same if messages match/are empty). Consider including `StatusCode` in the returned string (while still keeping `Message` available separately). ########## at/ecommerce/order/create.go: ########## @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package main + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "time" + + "github.com/gin-gonic/gin" + "seata.apache.org/seata-go-samples/util" + "seata.apache.org/seata-go/pkg/constant" + "seata.apache.org/seata-go/pkg/tm" + "seata.apache.org/seata-go/pkg/util/log" +) + +type OrderRequest struct { + UserID string `json:"userId"` + CommodityCode string `json:"commodityCode"` + Count int `json:"count"` + Money int `json:"money"` +} + +type InventoryRequest struct { + CommodityCode string `json:"commodityCode"` + Count int `json:"count"` +} + +type AccountRequest struct { + UserID string `json:"userId"` + Money int `json:"money"` +} + +func createOrder(c *gin.Context) error { + var req OrderRequest + if err := c.ShouldBindJSON(&req); err != nil { + return util.NewValidationError(err.Error()) + } + if strings.TrimSpace(req.UserID) == "" { + return util.NewValidationError("userId is required") + } + if strings.TrimSpace(req.CommodityCode) == "" { + return util.NewValidationError("commodityCode is required") + } + if req.Count <= 0 { + return util.NewValidationError("count must be greater than 0") + } + if req.Money <= 0 { + return util.NewValidationError("money must be greater than 0") + } + + return tm.WithGlobalTx(c.Request.Context(), &tm.GtxConfig{ + Name: "ATSampleEcommerceCreateOrder", + Timeout: time.Second * 30, + }, func(ctx context.Context) error { + if err := insertOrder(ctx, req); err != nil { + return err + } + if err := deductInventory(ctx, req); err != nil { + return err + } + if err := deductAccount(ctx, req); err != nil { + return err + } + return nil + }) +} + +func insertOrder(ctx context.Context, req OrderRequest) error { + query := "insert into order_tbl(user_id, commodity_code, count, money, status) values (?, ?, ?, ?, ?)" + ret, err := db.ExecContext(ctx, query, req.UserID, req.CommodityCode, req.Count, req.Money, "CREATED") + if err != nil { + return err + } + + rows, err := ret.RowsAffected() + if err != nil { + return err + } + if rows != 1 { + return fmt.Errorf("create order affected unexpected rows: %d", rows) + } + return nil +} + +func deductInventory(ctx context.Context, req OrderRequest) error { + payload, err := json.Marshal(InventoryRequest{ + CommodityCode: req.CommodityCode, + Count: req.Count, + }) + if err != nil { + return err + } + + log.Infof("call inventory service, xid=%s", tm.GetXID(ctx)) + return postJSON(ctx, inventoryService+"/deductInventory", payload) +} + +func deductAccount(ctx context.Context, req OrderRequest) error { + payload, err := json.Marshal(AccountRequest{ + UserID: req.UserID, + Money: req.Money, + }) + if err != nil { + return err + } + + log.Infof("call account service, xid=%s", tm.GetXID(ctx)) + return postJSON(ctx, accountService+"/deductAccount", payload) +} + +func postJSON(ctx context.Context, url string, payload []byte) error { + requestCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + httpReq, err := http.NewRequestWithContext(requestCtx, http.MethodPost, url, bytes.NewReader(payload)) + if err != nil { + return err + } + httpReq.Header.Set(constant.XidKey, tm.GetXID(ctx)) + httpReq.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(httpReq) + if err != nil { + return util.NewDownstreamError(0, fmt.Sprintf("request %s failed: %v", url, err)) + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return err + } + if resp.StatusCode != http.StatusOK { + message := strings.TrimSpace(string(body)) + var response util.APIResponse + if err := json.Unmarshal(body, &response); err == nil { + if response.Error != "" { + message = response.Error + } else if response.Message != "" { + message = response.Message + } + } + return util.NewDownstreamError(resp.StatusCode, message) + } Review Comment: `postJSON` extracts a specific downstream error message into `DownstreamError.Message`, but `util.PublicErrorMessage` intentionally replaces all downstream 4xx messages with a generic string. This makes order-service client errors less actionable while still doing the work to parse/retain the message. Consider mapping known “safe to expose” downstream statuses into first-class local errors (e.g., translate downstream `409` into `util.NewConflictError(message)`), so the order-service can return helpful messages without exposing arbitrary downstream error payloads. ########## at/ecommerce/sql/mysql_ecommerce.sql: ########## @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +CREATE DATABASE IF NOT EXISTS seata_ecommerce_order DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci; +CREATE DATABASE IF NOT EXISTS seata_ecommerce_inventory DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci; +CREATE DATABASE IF NOT EXISTS seata_ecommerce_account DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci; + +USE seata_ecommerce_order; + +CREATE TABLE IF NOT EXISTS order_tbl ( + id INT NOT NULL AUTO_INCREMENT, + user_id VARCHAR(64) NOT NULL, + commodity_code VARCHAR(64) NOT NULL, + count INT NOT NULL, + money INT NOT NULL, + status VARCHAR(32) NOT NULL, + PRIMARY KEY (id) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +CREATE TABLE IF NOT EXISTS undo_log ( + id BIGINT NOT NULL AUTO_INCREMENT, + branch_id BIGINT NOT NULL, + xid VARCHAR(100) NOT NULL, + context VARCHAR(128) NOT NULL, + rollback_info LONGBLOB NOT NULL, + log_status INT NOT NULL, + log_created DATETIME NOT NULL, + log_modified DATETIME NOT NULL, + ext VARCHAR(100) DEFAULT NULL, + PRIMARY KEY (id), + UNIQUE KEY ux_undo_log (xid, branch_id) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +USE seata_ecommerce_inventory; + +CREATE TABLE IF NOT EXISTS inventory_tbl ( + id INT NOT NULL AUTO_INCREMENT, + commodity_code VARCHAR(64) NOT NULL, + stock INT NOT NULL, + PRIMARY KEY (id), + UNIQUE KEY uk_commodity_code (commodity_code) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + +INSERT INTO inventory_tbl (commodity_code, stock) VALUES ('C100001', 100) +ON DUPLICATE KEY UPDATE stock = VALUES(stock); Review Comment: MySQL 8.0 has deprecated `VALUES(col)` in `ON DUPLICATE KEY UPDATE` (it emits warnings and may be removed in a future release). Consider rewriting using the newer alias form (e.g., `INSERT ... VALUES ... AS new ... UPDATE stock = new.stock`) for forward compatibility. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
