-
Notifications
You must be signed in to change notification settings - Fork 6
End point standardization #155
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
base: main
Are you sure you want to change the base?
Conversation
Nefarious46
commented
May 13, 2025
- Entity variables into right format
- Controllers and mappers in correct order of CREATE, GET, GET ALL, DELETE
- Naming convention same where possible. e.g. Storage and HostMeta controllers have some postfixes that separate paths and mappers
- Procedures take advantage of row counting instead of "Not null" and "Null" where possible. e.g. input variables have to still be checked using "if not null then" statement
- Responsibilities separated per endpoint. New endpoints were introduced to accommodate linking needs e.g. captures to groups and ip to hostmeta. (Hub and HostMeta do not follow this but maybe should?)
- More unit tests on controller side to accommodate new endpoints
- Removed deprecated procedure tests instead of rewriting and creating more
|
|
||
| CaptureGroup delete(int id); | ||
|
|
||
| CaptureGroup deleteCapture(int captureId, int id); |
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.
deleteCapture is unlink operation, not delete operation in api perspective, please rename
|
|
||
| List<CaptureGroup> getAll(Integer version); | ||
|
|
||
| CaptureGroup delete(int id); |
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.
delete should not return anything
| String capture_def_group_name, | ||
| Integer capture_definition_id | ||
| ); | ||
| CaptureGroup createLink(int captureId, int id); |
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.
just link, why return value is needed?
| CaptureGroup get(int id,Integer version); | ||
|
|
||
| CaptureGroup deleteCaptureGroup(String name); | ||
| List<CaptureGroup> getCaptures(int id,Integer version); |
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 do getCaptures and getAll differ?
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.
getCaptures return all captures in group
This is because groups can be inserted alone without captures. Select statement fails if capture is not linked to a group.
getCaptures in this situation is a bit more like getLinkedCaptures(captureGroupId)
getAll returns all groups
getAll contents
captureGroupId, groupname, groupType
| List<CaptureStorage> getAllCapture(Integer version); | ||
|
|
||
| FlowStorage deleteFlowStorage(String flow, int id); | ||
| Storage delete(int id); |
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.
delete should not return anything
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.
separate different types into different controllers
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.
separate different types into different controllers
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.
too many responsibilities for one controller
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.
so many responsibilities, too many responsibilities