Skip to content

Commit

Permalink
use ErrorResponse instead of AccountsErrorResponse (#3737)
Browse files Browse the repository at this point in the history
## Summary

Complementing algorand/indexer#916 this merges the AccountsErrorResponse fields into the "data" object in ErrorResponse, so SDKs do not to have to distinguish between different error response types.

## Test Plan

Update existing tests checking the old AccountsErrorResponse fields.
  • Loading branch information
cce authored Mar 10, 2022
1 parent 9b13756 commit b92d526
Show file tree
Hide file tree
Showing 8 changed files with 378 additions and 447 deletions.
34 changes: 2 additions & 32 deletions daemon/algod/api/algod.oas2.json
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@
"400": {
"description": "Bad request",
"schema": {
"$ref": "#/definitions/AccountsErrorResponse"
"$ref": "#/definitions/ErrorResponse"
}
},
"401": {
Expand Down Expand Up @@ -2234,40 +2234,10 @@
],
"properties": {
"data": {
"type": "string"
},
"message": {
"type": "string"
}
}
},
"AccountsErrorResponse": {
"description": "An error response for the AccountInformation endpoint, with optional information about limits that were exceeded.",
"type": "object",
"required": [
"message"
],
"properties": {
"data": {
"type": "string"
"type": "object"
},
"message": {
"type": "string"
},
"max-results": {
"type": "integer"
},
"total-apps-opted-in": {
"type": "integer"
},
"total-assets-opted-in": {
"type": "integer"
},
"total-created-apps": {
"type": "integer"
},
"total-created-assets": {
"type": "integer"
}
}
},
Expand Down
37 changes: 4 additions & 33 deletions daemon/algod/api/algod.oas3.yml
Original file line number Diff line number Diff line change
Expand Up @@ -915,36 +915,6 @@
],
"type": "object"
},
"AccountsErrorResponse": {
"description": "An error response for the AccountInformation endpoint, with optional information about limits that were exceeded.",
"properties": {
"data": {
"type": "string"
},
"max-results": {
"type": "integer"
},
"message": {
"type": "string"
},
"total-apps-opted-in": {
"type": "integer"
},
"total-assets-opted-in": {
"type": "integer"
},
"total-created-apps": {
"type": "integer"
},
"total-created-assets": {
"type": "integer"
}
},
"required": [
"message"
],
"type": "object"
},
"Application": {
"description": "Application index and its parameters",
"properties": {
Expand Down Expand Up @@ -1391,7 +1361,8 @@
"description": "An error response with optional data field.",
"properties": {
"data": {
"type": "string"
"properties": {},
"type": "object"
},
"message": {
"type": "string"
Expand Down Expand Up @@ -1819,12 +1790,12 @@
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/AccountsErrorResponse"
"$ref": "#/components/schemas/ErrorResponse"
}
},
"application/msgpack": {
"schema": {
"$ref": "#/components/schemas/AccountsErrorResponse"
"$ref": "#/components/schemas/ErrorResponse"
}
}
},
Expand Down
292 changes: 145 additions & 147 deletions daemon/algod/api/server/v2/generated/private/routes.go

Large diffs are not rendered by default.

15 changes: 2 additions & 13 deletions daemon/algod/api/server/v2/generated/private/types.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

390 changes: 194 additions & 196 deletions daemon/algod/api/server/v2/generated/routes.go

Large diffs are not rendered by default.

15 changes: 2 additions & 13 deletions daemon/algod/api/server/v2/generated/types.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 10 additions & 7 deletions daemon/algod/api/server/v2/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,13 +305,16 @@ func (v2 *Handlers) AccountInformation(ctx echo.Context, address string, params
totalResults := record.TotalAssets + record.TotalAssetParams + record.TotalAppLocalStates + record.TotalAppParams
if totalResults > maxResults {
v2.Log.Info("MaxAccountAPIResults limit %d exceeded, total results %d", maxResults, totalResults)
return ctx.JSON(http.StatusBadRequest, generated.AccountsErrorResponse{
Message: "Result limit exceeded",
MaxResults: &maxResults,
TotalAssetsOptedIn: &record.TotalAssets,
TotalCreatedAssets: &record.TotalAssetParams,
TotalAppsOptedIn: &record.TotalAppLocalStates,
TotalCreatedApps: &record.TotalAppParams,
extraData := map[string]interface{}{
"max-results": maxResults,
"total-assets-opted-in": record.TotalAssets,
"total-created-assets": record.TotalAssetParams,
"total-apps-opted-in": record.TotalAppLocalStates,
"total-created-apps": record.TotalAppParams,
}
return ctx.JSON(http.StatusBadRequest, generated.ErrorResponse{
Message: "Result limit exceeded",
Data: &extraData,
})
}
}
Expand Down
25 changes: 19 additions & 6 deletions daemon/algod/api/server/v2/test/handlers_resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,17 +221,30 @@ func accountInformationResourceLimitsTest(t *testing.T, accountMaker func(int) b
CreatedAssets []interface{} `json:"createdAssets"`
}

err = json.Unmarshal(rec.Body.Bytes(), &ret)
require.NoError(t, err)

// totals should be present in both 200 and 400
require.Equal(t, acctSize, ret.TotalApps+ret.TotalAssets+ret.TotalCreatedApps+ret.TotalCreatedAssets, "totals incorrect: %+v", ret)
var errRet struct {
Data struct {
TotalApps int `json:"total-apps-opted-in"`
TotalAssets int `json:"total-assets-opted-in"`
TotalCreatedApps int `json:"total-created-apps"`
TotalCreatedAssets int `json:"total-created-assets"`
MaxResults int `json:"max-results"`
} `json:"data,omitempty"`
}

// check totals
switch rec.Code {
case 400:
require.Equal(t, maxResults, ret.MaxResults)
err = json.Unmarshal(rec.Body.Bytes(), &errRet)
require.NoError(t, err)
require.Equal(t, maxResults, errRet.Data.MaxResults)
// totals should be present in both 200 and 400
require.Equal(t, acctSize, errRet.Data.TotalApps+errRet.Data.TotalAssets+errRet.Data.TotalCreatedApps+errRet.Data.TotalCreatedAssets, "totals incorrect: %+v", ret)
case 200:
err = json.Unmarshal(rec.Body.Bytes(), &ret)
require.NoError(t, err)
// totals should be present in both 200 and 400
require.Equal(t, acctSize, ret.TotalApps+ret.TotalAssets+ret.TotalCreatedApps+ret.TotalCreatedAssets, "totals incorrect: %+v", ret)

if exclude == "all" {
require.Nil(t, ret.Apps)
require.Nil(t, ret.Assets)
Expand Down

0 comments on commit b92d526

Please sign in to comment.