-
Notifications
You must be signed in to change notification settings - Fork 610
NMS-19486: Support deletion of mibgroups,resourcetypes and systemdefs #8280
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: features/snmp-datacollection-db
Are you sure you want to change the base?
NMS-19486: Support deletion of mibgroups,resourcetypes and systemdefs #8280
Conversation
…rces | Expose services to delete list of mibgroups,resourcetypes and systemdefs
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.
Pull request overview
Adds REST v2 delete endpoints and DAO support for bulk deletion of SNMP data collection sources and related entities (MIB groups, resource types, system defs).
Changes:
- Introduces new REST payload models for bulk delete requests.
- Adds REST API interface methods + REST service implementations for delete operations.
- Extends DAOs with bulk-delete methods (HQL
delete ... where id in (...)) for sources and child entities.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/model/SnmpDataCollectionSystemDefDeletePayload.java | Adds request payload for deleting system defs by ID list |
| opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/model/SnmpDataCollectionSourceDeletePayload.java | Adds request payload for deleting collection sources by ID list |
| opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/model/SnmpDataCollectionResourceTypeDeletePayload.java | Adds request payload for deleting resource types by ID list |
| opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/model/SnmpDataCollectionMibGroupDeletePayload.java | Adds request payload for deleting MIB groups by ID list |
| opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/api/DataCollectionConfRestApi.java | Exposes new DELETE endpoints for the above bulk deletions |
| opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/DataCollectionConfRestService.java | Implements REST endpoints and delegates deletions to DAO/service layer |
| opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/DataCollectionConfPersistenceService.java | Adds transactional delete operations with validation against DB state |
| opennms-dao/src/main/java/org/opennms/netmgt/dao/hibernate/SnmpCollectionSystemDefDaoHibernate.java | Adds bulk delete by source + systemDef IDs |
| opennms-dao/src/main/java/org/opennms/netmgt/dao/hibernate/SnmpCollectionSourceDaoHibernate.java | Adds bulk delete by source IDs |
| opennms-dao/src/main/java/org/opennms/netmgt/dao/hibernate/SnmpCollectionResourceTypeDaoHibernate.java | Adds bulk delete by source + resourceType IDs |
| opennms-dao/src/main/java/org/opennms/netmgt/dao/hibernate/SnmpCollectionMibGroupDaoHibernate.java | Adds bulk delete by source + mibGroup IDs |
| opennms-dao-api/src/main/java/org/opennms/netmgt/dao/api/SnmpCollectionSystemDefDao.java | Adds DAO API for bulk systemDef deletion |
| opennms-dao-api/src/main/java/org/opennms/netmgt/dao/api/SnmpCollectionSourceDao.java | Adds DAO API for bulk source deletion |
| opennms-dao-api/src/main/java/org/opennms/netmgt/dao/api/SnmpCollectionResourceTypeDao.java | Adds DAO API for bulk resourceType deletion |
| opennms-dao-api/src/main/java/org/opennms/netmgt/dao/api/SnmpCollectionMibGroupDao.java | Adds DAO API for bulk mibGroup deletion |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/DataCollectionConfRestService.java
Show resolved
Hide resolved
opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/DataCollectionConfRestService.java
Show resolved
Hide resolved
opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/DataCollectionConfRestService.java
Show resolved
Hide resolved
opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/DataCollectionConfRestService.java
Outdated
Show resolved
Hide resolved
opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/DataCollectionConfRestService.java
Show resolved
Hide resolved
opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/api/DataCollectionConfRestApi.java
Show resolved
Hide resolved
...src/main/java/org/opennms/web/rest/v2/model/SnmpDataCollectionResourceTypeDeletePayload.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/opennms/web/rest/v2/model/SnmpDataCollectionResourceTypeDeletePayload.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/opennms/web/rest/v2/model/SnmpDataCollectionResourceTypeDeletePayload.java
Outdated
Show resolved
Hide resolved
opennms-webapp-rest/src/main/java/org/opennms/web/rest/v2/DataCollectionConfRestService.java
Show resolved
Hide resolved
|
|
||
| @Transactional | ||
| public void deleteSnmpDataCollectionSources(final SnmpDataCollectionSourceDeletePayload request) throws Exception { | ||
| snmpCollectionSourceDao.deleteByIds(request.getSnmpCollectionSourceIds()); |
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 not use Service and what happens to it's child entities ?
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.
if we delete source its child entities will also be deleted
| final var mibGroupsIds = snmpCollectionMibGroupDao.findAllBySource(snmpDataCollectionSourceId); | ||
|
|
||
| final Set<Integer> databaseMibGroupIds = mibGroupsIds | ||
| .stream() | ||
| .map(SnmpCollectionMibGroup::getId) | ||
| .collect(Collectors.toSet()); | ||
|
|
||
| final var requestMibGroupIds = payload.getMibGroupsIds(); | ||
| final var existingMibGroupIds = requestMibGroupIds.stream() | ||
| .filter(databaseMibGroupIds::contains) | ||
| .toList(); |
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.
Do we need to load all mibgroups to delete ?
cgorantla
left a comment
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.
Tests are failing
External References