NMS-19486: Support deletion of mibgroups,resourcetypes and systemdefs by aurang-zeb313 · Pull Request #8280 · OpenNMS/opennms

aurang-zeb313 · 2026-02-06T14:23:46Z

External References

Jira (Issue Tracker): https://opennms.atlassian.net/browse/NMS-19486

…rces | Expose services to delete list of mibgroups,resourcetypes and systemdefs

Copilot

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.

cgorantla · 2026-02-11T02:28:08Z

+        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();


Do we need to load all mibgroups to delete ?

cgorantla

Tests are failing

cgorantla · 2026-02-17T00:24:16Z

+    public void deleteByMibGroupIds(Integer snmpDataCollectionSourceId, List<Integer> snmpCollectionMibGroupIds) {
+        int deletedCount = getHibernateTemplate().execute(session ->
+                session.createQuery("delete from SnmpCollectionMibGroup g where g.collectionSource.id  = :snmpCollectionSourceId and  g.id in (:Ids)")
+                        .setParameter("snmpCollectionSourceId", snmpDataCollectionSourceId)
+                        .setParameterList("Ids", snmpCollectionMibGroupIds)
+                        .executeUpdate()
+        );
+        LOG.info("Deleted {} SnmpCollectionMibGroup(g) with IDs: {} for snmpCollectionSourceId: {}", deletedCount, snmpCollectionMibGroupIds, snmpDataCollectionSourceId);
    }


I think there is already code to delete and deleteAll in AbstractDaoHibernate, no need to re-implement these custom HQL

cgorantla · 2026-02-17T02:42:51Z

+        }
+
+        try {
+            snmpCollectionSourceDao.deleteByIds(snmpDataCollectionSourceDeletePayload.getSnmpCollectionSourceIds());


why are we calling dao here directly, can use service.

cgorantla · 2026-02-17T02:44:16Z

    }
+
+    @Override
+    public void deleteByIds(List<Integer> snmpDataCollectionIds) {


I think it would be good to have cascade relations defined for child entities. That way the deletion of children happen in Hibernate session.

cgorantla · 2026-02-23T04:09:39Z

+                @ApiResponse(responseCode = "500", description = "Internal server error")
+        })
+        Response deleteSnmpDataCollectionSources(
+                SnmpDataCollectionSourceDeletePayload snmpDataCollectionSourceDeletePayload,


Prefer to use simple query params with a list of ids here instead of using Payload

cgorantla · 2026-02-23T04:20:03Z

+        int deletedCount = getHibernateTemplate().execute(session ->
+                session.createQuery("delete from SnmpCollectionSource s where s.id in (:ids)")
+                        .setParameterList("ids", snmpDataCollectionIds)
+                        .executeUpdate()
+        );


don't need to write an actual SQL query or HQL. You can use entities,

something like this :
for (Integer id : ids) {
SnmpCollectionSource entity = get(id);
if (entity != null) {
delete(entity);
}
}

cgorantla · 2026-02-23T04:24:10Z


    PageResponse<SnmpCollectionResourceType> findByDataCollectionGroupId(Integer snmpCollectionSourceId, String resourceTypeFilter, String sortBy, String order, Integer totalRecords, Integer offset, Integer limit);

+    void deleteByResourceTypeIds(Integer snmpDataCollectionSourceId,List<Integer> snmpCollectionResourceTypeIds);


some of these apis are unused now

# Conflicts: # opennms-dao-api/src/main/java/org/opennms/netmgt/dao/api/SnmpCollectionMibGroupDao.java

cgorantla · 2026-02-24T02:22:51Z

    PageResponse<SnmpCollectionSource> filterDataCollectionSource(final String filter, final String sortBy, final String order, Integer totalRecords,
                                            Integer offset, Integer limit);

+    void deleteByIds(List<Integer> snmpDataCollectionIds);


only used in tests ?

cgorantla · 2026-02-24T02:26:15Z

+
+            final T entity = finder.apply(sourceId, id);
+            if (entity == null) {
+                throw new EntityNotFoundException(entityLabel + " not found for sourceId=" + sourceId + ", id=" + id);


may be continue would be better here ?

cgorantla · 2026-02-24T02:29:21Z

+        return handleDelete(
+                () -> { dataCollectionConfPersistenceService.deleteSnmpDataCollectionSystemDefs(snmpDataCollectionSourceId, ids); return null; },
+                "Snmp Data Collection System Def deleted successfully"
+        );
+    }
+
+    private Response handleDelete(final Supplier<Void> action, final String successMessage) {
+        try {
+            action.get();
+


Can you revisit this code and avoid using Supplier kind of overhead ?

cgorantla

LGTM!

NMS-19486: Expose rest end points to delete list of snmpCollectionSou…

598ec92

…rces | Expose services to delete list of mibgroups,resourcetypes and systemdefs

aurang-zeb313 changed the base branch from develop to features/snmp-datacollection-db February 6, 2026 14:36

aurang-zeb313 marked this pull request as draft February 6, 2026 14:37

aurang-zeb313 requested a review from Copilot February 6, 2026 14:37

Copilot AI reviewed Feb 6, 2026

View reviewed changes

NMS-19486: integration test cases.

e8e610c

aurang-zeb313 marked this pull request as ready for review February 9, 2026 16:53

aurang-zeb313 requested a review from cgorantla February 10, 2026 14:14

cgorantla changed the title ~~NMS-19486: Expose rest end points to delete list of snmpCollectionSources | Expose services to delete list of mibgroups,resourcetypes and systemdefs~~ NMS-19486: Support deletion of mibgroups,resourcetypes and systemdefs Feb 10, 2026

cgorantla requested changes Feb 11, 2026

View reviewed changes

aurang-zeb313 added 2 commits February 11, 2026 22:20

NMS-19486: fixed reviews

d99870f

NMS-19486: fixed reviews

a313e49

aurang-zeb313 requested a review from cgorantla February 11, 2026 17:22

cgorantla reviewed Feb 11, 2026

View reviewed changes

NMS-19486: fixed integration test cases issues

3a28969

aurang-zeb313 requested a review from cgorantla February 13, 2026 14:07

cgorantla reviewed Feb 17, 2026

View reviewed changes

NMS-19486: Fixed reviews.

932d8ad

aurang-zeb313 requested a review from cgorantla February 18, 2026 17:25

cgorantla reviewed Feb 23, 2026

View reviewed changes

NMS-19486: fixed reviews and test cases

96bf612

aurang-zeb313 requested a review from cgorantla February 23, 2026 17:40

Merge branch 'features/snmp-datacollection-db' into akhan/jira/NMS-19486

c2f563e

# Conflicts: # opennms-dao-api/src/main/java/org/opennms/netmgt/dao/api/SnmpCollectionMibGroupDao.java

cgorantla reviewed Feb 24, 2026

View reviewed changes

NMS-19486: fixed reviews and test cases

a7d6670

aurang-zeb313 requested a review from cgorantla February 24, 2026 14:30

NMS-19486: fixed reviews and test cases

1fcef93

cgorantla approved these changes Feb 24, 2026

View reviewed changes

delete service param fix

aed1a7c

aurang-zeb313 merged commit afd4990 into features/snmp-datacollection-db Feb 26, 2026
3 of 5 checks passed

aurang-zeb313 deleted the akhan/jira/NMS-19486 branch February 26, 2026 14:28


		PageResponse<SnmpCollectionResourceType> findByDataCollectionGroupId(Integer snmpCollectionSourceId, String resourceTypeFilter, String sortBy, String order, Integer totalRecords, Integer offset, Integer limit);

		void deleteByResourceTypeIds(Integer snmpDataCollectionSourceId,List<Integer> snmpCollectionResourceTypeIds);

Conversation

aurang-zeb313 commented Feb 6, 2026

External References

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgorantla left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgorantla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants