-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support LIST datatype #79
base: master
Are you sure you want to change the base?
Support LIST datatype #79
Conversation
api/v1/condition.go
Outdated
continue | ||
} | ||
// Validate index bounds | ||
if index < 0 || index >= len(oldList) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be index > len(oldList) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
@@ -241,16 +241,6 @@ func Test_extractOperations(t *testing.T) { | |||
"DELETE": "Color :p", | |||
}, | |||
}, | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this test case removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a valid test case, all operations are supported by any data type
api/v1/middleware.go
Outdated
@@ -26,7 +25,6 @@ import ( | |||
func PanicHandler(c *gin.Context) { | |||
if e := recover(); e != nil { | |||
stack := string(debug.Stack()) | |||
fmt.Println(stack) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
integrationtest/api_test.go
Outdated
@@ -1484,6 +1609,7 @@ func testGetBatchAPI(t *testing.T) { | |||
createPostTestCase(TestGetBatch7Name, "/v1", "BatchGetItem", TestGetBatch7Output, TestGetBatch7), | |||
createPostTestCase(TestGetBatch8Name, "/v1", "BatchGetItem", TestGetBatch8Output, TestGetBatch8), | |||
createPostTestCase(TestGetBatch9Name, "/v1", "BatchGetItem", TestGetBatch9Output, TestGetBatch9), | |||
// createPostTestCase(TestGetBatchForListName, "/v1", "BatchGetItem", TestGetBatchForListOutput, TestGetBatchForList), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
service/services/services.go
Outdated
// Example: listAttribute[2] | ||
matches := listRemoveTargetRegex.FindStringSubmatch(target) | ||
if len(matches) == 3 { | ||
index, _ := strconv.Atoi(matches[2]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No error handling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
service/services/services.go
Outdated
for _, target := range colsToRemove { | ||
if strings.Contains(target, "[") && strings.Contains(target, "]") { | ||
// Handle list index removal | ||
listAttr, idx := parseListRemoveTarget(target) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle idx = -1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
service/services/services.go
Outdated
if idx < 0 || idx >= len(list) { | ||
return list // Invalid index; return original list | ||
} | ||
return append(list[:idx], list[idx+1:]...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test case for removing last element in the list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
Fixes #<issue_number_goes_here>