Copilot commented on code in PR #95: URL: https://github.com/apache/incubator-seata-go-samples/pull/95#discussion_r3385070623
########## at/ecommerce/order/main.go: ########## @@ -0,0 +1,68 @@ +/* + * 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 ( + "database/sql" + "net/http" + "os" + + "github.com/gin-gonic/gin" + "seata.apache.org/seata-go-samples/util" + "seata.apache.org/seata-go/pkg/client" + "seata.apache.org/seata-go/pkg/util/log" +) + +var ( + db *sql.DB + inventoryService = "http://127.0.0.1:18081"; + accountService = "http://127.0.0.1:18082"; +) + +type apiResponse struct { + Message string `json:"message,omitempty"` + Error string `json:"error,omitempty"` +} + +func main() { + client.InitPath("conf/seatago.yml") + util.SetDefaultEnv("MYSQL_DB", "seata_ecommerce_order") + if value := os.Getenv("INVENTORY_SERVICE_URL"); value != "" { + inventoryService = value + } + if value := os.Getenv("ACCOUNT_SERVICE_URL"); value != "" { + accountService = value + } + db = util.GetAtMySqlDb() + + r := gin.Default() + r.POST("/createOrder", createOrderHandler) + + if err := r.Run(":18080"); err != nil { + log.Fatalf("start order service fatal: %v", err) + } +} + +func createOrderHandler(c *gin.Context) { + log.Infof("receive create order request") + if err := createOrder(c); err != nil { + c.JSON(http.StatusBadRequest, apiResponse{Error: err.Error()}) + return + } Review Comment: The handler returns HTTP 400 for all errors, including internal failures (DB insert errors) and downstream/network failures (inventory/account calls). This makes failures look like client input issues. Consider mapping validation/binding errors to 400, and mapping internal/downstream errors to 500/502 (and apply the same pattern to inventory/account handlers in this PR). ########## at/ecommerce/account/deduct.go: ########## @@ -0,0 +1,58 @@ +/* + * 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 ( + "fmt" + "strings" + + "github.com/gin-gonic/gin" +) + +type AccountRequest struct { + UserID string `json:"userId"` + Money int `json:"money"` +} + +func deductAccount(c *gin.Context) error { + var req AccountRequest + if err := c.ShouldBindJSON(&req); err != nil { + return err + } + if strings.TrimSpace(req.UserID) == "" { + return fmt.Errorf("userId is required") + } + if req.Money <= 0 { + return fmt.Errorf("money must be greater than 0") + } + + sql := "update account_tbl set balance = balance - ? where user_id = ? and balance >= ?" + ret, err := db.ExecContext(c.Request.Context(), sql, req.Money, req.UserID, req.Money) + if err != nil { + return err + } + + rows, err := ret.RowsAffected() + if err != nil { + return err + } + if rows != 1 { + return fmt.Errorf("balance not enough") + } Review Comment: Error message is grammatically unclear and loses context. Consider a clearer message like “insufficient balance” and/or include the `userId` (and requested money) to aid debugging. ########## at/ecommerce/order/create.go: ########## @@ -0,0 +1,156 @@ +/* + * 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/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 err + } + if strings.TrimSpace(req.UserID) == "" { + return fmt.Errorf("userId is required") + } + if strings.TrimSpace(req.CommodityCode) == "" { + return fmt.Errorf("commodityCode is required") + } + if req.Count <= 0 { + return fmt.Errorf("count must be greater than 0") + } + if req.Money <= 0 { + return fmt.Errorf("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 { + sql := "insert into order_tbl(user_id, commodity_code, count, money, status) values (?, ?, ?, ?, ?)" + ret, err := db.ExecContext(ctx, sql, req.UserID, req.CommodityCode, req.Count, req.Money, "CREATED") Review Comment: Using `sql` as a variable name for a query string is ambiguous and can be confused with the `database/sql` package in Go codebases. Rename it to something like `query`/`stmt` (same applies to similar variables in inventory/account files in this PR). ########## util/env.go: ########## @@ -0,0 +1,26 @@ +/* + * 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 "os" + +func SetDefaultEnv(key string, value string) { + if os.Getenv(key) == "" { + _ = os.Setenv(key, value) + } +} Review Comment: `SetDefaultEnv` treats an explicitly-set empty string the same as an unset variable (because it relies on `os.Getenv(key) == \"\"`). If callers intentionally set `KEY=` to mean “empty”, this helper will override it unexpectedly. Prefer `os.LookupEnv` to distinguish unset vs empty; also consider returning/handling the `os.Setenv` error instead of discarding it. ########## at/ecommerce/README.md: ########## @@ -0,0 +1,133 @@ +<!-- + 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. +--> + +# AT E-commerce Sample + +This sample demonstrates an e-commerce order flow in Seata Go AT mode. + +The scenario contains three independent Go services: + +1. `order-service` creates the order record and starts the global transaction +2. `inventory-service` deducts stock +3. `account-service` deducts the user balance + +All three operations must succeed or fail together. If the account balance is insufficient, `account-service` rejects the request and Seata automatically rolls back the order creation and inventory deduction through `undo_log`. + +## Directory Layout + +- `order/`: `order-service` +- `inventory/`: `inventory-service` +- `account/`: `account-service` +- `sql/mysql_ecommerce.sql`: schema and seed data for the three MySQL databases +- `docker-compose.yml`: MySQL and Seata Server + +## Start the Infrastructure + +Optionally customize the MySQL root password first: + +```bash +cd at/ecommerce +cp .env.example .env +``` + +Edit `.env` if you want a different password. The sample defaults to `123456` when `MYSQL_ROOT_PASSWORD` is not set. + +If you change the password, export the same value for the local Go services before starting them: Review Comment: The new `defaultEnv()` logic in `util/db.go` already falls back from `MYSQL_ROOT_PASSWORD` to `MYSQL_PASSWORD` when `MYSQL_PASSWORD` is empty, so exporting `MYSQL_PASSWORD` explicitly is optional. Consider updating this section to clarify the precedence (e.g., “set either `MYSQL_PASSWORD` or `MYSQL_ROOT_PASSWORD`”) to avoid redundant steps. ########## at/ecommerce/docker-compose.yml: ########## @@ -0,0 +1,46 @@ +# 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. + +services: + mysql: + image: mysql:8.0.32 + container_name: ecommerce_at_mysql + environment: + MYSQL_ROOT_PASSWORD: ${MYSQL_ROOT_PASSWORD:-123456} Review Comment: docker-compose defaults the DB root password to a very weak value (`123456`) when no env is provided. Even for samples, this can lead to accidental exposure when run on shared networks. Consider requiring an explicit `MYSQL_ROOT_PASSWORD` (no default) or adding a prominent warning in the README and restricting MySQL port exposure by default. ########## at/ecommerce/inventory/deduct.go: ########## @@ -0,0 +1,58 @@ +/* + * 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 ( + "fmt" + "strings" + + "github.com/gin-gonic/gin" +) + +type InventoryRequest struct { + CommodityCode string `json:"commodityCode"` + Count int `json:"count"` +} + +func deductInventory(c *gin.Context) error { + var req InventoryRequest + if err := c.ShouldBindJSON(&req); err != nil { + return err + } + if strings.TrimSpace(req.CommodityCode) == "" { + return fmt.Errorf("commodityCode is required") + } + if req.Count <= 0 { + return fmt.Errorf("count must be greater than 0") + } + + sql := "update inventory_tbl set stock = stock - ? where commodity_code = ? and stock >= ?" + ret, err := db.ExecContext(c.Request.Context(), sql, req.Count, req.CommodityCode, req.Count) + if err != nil { + return err + } + + rows, err := ret.RowsAffected() + if err != nil { + return err + } + if rows != 1 { + return fmt.Errorf("inventory not enough") + } Review Comment: Error message is grammatically unclear and loses context. Consider a clearer message like “insufficient inventory” and/or include the `commodityCode` (and possibly requested count) to aid debugging. -- 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]
