-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: data validation handler #20
Conversation
Co-authored-by: Youngjoon Lee <yjlee@medibloc.org>
# Conflicts: # cmd/oracled/cmd/root.go # service/handler.go
# Conflicts: # go.mod # panacea/query_client.go
I have not tested full cycle of this handler because some relevant functions of Panacea are in developing. |
I can make test codes for this handler as youngjoon did for |
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.
Thanks.
I left a few comments.
service/handler.go
Outdated
ProviderAddress string `json:"provider_address"` | ||
EncryptedDataBase64 string `json:"encrypted_data_base64"` | ||
DataHash string `json:"data_hash"` | ||
} | ||
|
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.
How about added validation function like func (m ValidateDataReq) ValidateBasic() {...}
.
This is a validation of request parameters.
service/handler.go
Outdated
http.Error(w, "failed to get deal", http.StatusBadRequest) | ||
return | ||
} | ||
|
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.
How about added validation a deal status?
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.
I'm approving merging this PR in general, but it seems there are some duplicated codes with PR #21, in regard to the AES encryption.
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.
Thanks, lgtm
I tried to write test for this handler, but it requires to mock service, and many components inside service (query client, ipfs, etc.). |
Yup. No worries :) |
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.
LGTM!
) | ||
|
||
// Encrypt combines secretKey and secondKey to encrypt with AES256-GCM method. | ||
func Encrypt(secretKey, additional, data []byte) ([]byte, error) { |
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.
Ah, If these functions are merged into main branch, I'll change it when executing approveOracleRegistration
tx.
# Conflicts: # go.sum # panacea/query_client.go # server/middleware/auth_test.go
close #6