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(UUID): add IRI validation that allows only to create IRIs using UUID version 4 and 5 (DEV-402) #1990
Conversation
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.
looks good, apart from a few minor naming things
...a/org/knora/webapi/messages/admin/responder/permissionsmessages/PermissionsMessagesADM.scala
Outdated
Show resolved
Hide resolved
@@ -22,8 +23,12 @@ object GroupIRI { self => | |||
if (value.isEmpty) { | |||
Validation.fail(BadRequestException(GROUP_IRI_MISSING_ERROR)) | |||
} else { | |||
val isUUID: Boolean = sf.couldBeUuid(value.split("/").last) |
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 find the function name couldBeUuid()
confusing. Either it is or it's not, but could seems weird in this context
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.
You're partially right, because it only checks the length of string, so there is possibility checked string isn't the UUID, but I think to rename it.
.../main/scala/org/knora/webapi/messages/v2/responder/resourcemessages/ResourceMessagesV2.scala
Outdated
Show resolved
Hide resolved
Kudos, SonarCloud Quality Gate passed!
|
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, thanks
resolves DEV-402