Support for summarized and scheduled notifications

[MM-msr]

Description

Currently, Dependency-Track Alerts send a single notification for every single event happening that an alert is subscribed to (e.g. NEW_VULNERABILITY). This can lead to a lot of emails for users and creates the risk of overlooking important notifications.

To improve the user experience of the Alerts, scheduled notifications are implemented in Dependency-Track to send summarized notifications of new events between the last and current scheduled notification. The schedule is defined as cron expression.

Scheduled notification management is available via API and Frontend under Administration -> Notifications -> Scheduled Alerts, which is introduced in the Frontend Pull Request [reference will be added after creation].

The PR includes default templates for console and email publishing. The needed UI-Changes are available in the Frontend-PR at DependencyTrack/frontend#903.

Addressed Issue

#322

Additional Details

• DEFAULT_SCHEDULED_CRON_EXPRESSION as environment variable for the default cron expression of new rules
• Error: "The type javax.validation.Payload cannot be resolved. It is indirectly referenced from required type com.github.packageurl.validator.PackageURL" was introduced in my local development environment after syncing the main project and the transition of some javax packages to jakarta
  • was fixed by manually adding javax.validation-api to pom.xml as depencency
• While evaluating and debugging the consideration of child projects in the notification process, it has turned out that projects delivered by QueryManager.getAllProjects() and some other methods didn't consistently include the correct child references. To workaround this behavior, the notification task "SendScheduledNotificationTask" uses a project-by-project-retrieval approach in "evaluateAffectedProjects(qm, rule)". This worked out empirically to deliver consistent child references.
• DefaultPublisher-Retrieval was changed from by-class to a combination of name + class (or by-enum on a higher layer) to support multiple default publishers with same publisher class. This change is needed to use the existing publisher classes without defining new ones for scheduled default publishers.

---

• There might be an open bug currently in the toJson-Process for PolicyViolation, where the policy is not fully included in the PolicyCondition and thus the Policy.getUUID().toString() call fails with NullRefException. Often it worked for me just fine, sometimes not. Sometimes fixed with creating a new policy, sometimes with reboot of the application. With some additional random exceptions with the H2 database on ORM-Queries, i don't know for sure, if my code is the reason for this behavior or the instability of H2 with the ORM-mapper in my development environment.

Getting violations for scheduled mail:

PolicyQueryManager, L. 340, getPolicyViolations(Project, boolean, ZonedDateTime)

JSONify the data:

NotificationUtil, L. 550ff., toJson(PolicyViolation) -> toJson(pv.getPolicyCondition) -> toJson(pc.getPolicy) -> policy.getUUID().toString() -> [EXCEPTION]

Please test the expected behavior in a more robust environment (docker, other infrastructures, etc.) to validate

Screenshots

Checklist

• I have read and understand the contributing guidelines
• This PR fixes a defect, and I have provided tests to verify that the fix is effective
• This PR implements an enhancement, and I have provided tests to verify that it works as intended
• This PR introduces changes to the database model, and I have added corresponding [update logic](https://github.com/DependencyTrack/dependency-track/tree/master/src/main/java/org/dependencytrack/upgrade)
• This PR introduces new or alters existing behavior, and I have updated the documentation accordingly

[MM-msr]

@nscuro After i finally found some time this week to restart the PR process, i cherry-picked all necessary commits from my previous branch, corrected all sign-offs and so on. From the coding side it seems to be fine for now, although additional testing from different perspectives should be done.

Please note in particular the missing javax.validation reference in "Additional details", that seems to be ignored in the current master and seems to be a personal problem of my workspace or an unseen error until now from the transition to jakarta. There may be some more topics to discuss, but the main implementation should be completed by now.

The branch for the PR is based on the same commit my other branches started, but merged with a master newer than this feature branch was created on. Because of this and the recent cherry-picks, the commit history is a bit mixed up with other commits, but that may be gone when the feature branch gets updated with the missing commits from the current master branch.

Please provide me with additional information for the errors of the Static Code Analysis as i cannot see any useful information in the details link. If there is anything else i need to do or you need/want to know, please get in touch with me. I will assist to the best of my abilities.

[valentijnscholten]

This looks very interesting. But so does the commit history ;-) Could you rebase it onto current master to make the code changes in the PR "reviewable"?

[MM-msr]

@valentijnscholten Yeah, i know :D In my git graph, all looked fine, after the PR i had this 😅 I'll try to rebase it and see, if it's possible without making it worse. After the first PR-attempt went terribly wrong, i'm a bit scared tbh 😆 But i will give it a try and see where i land. Maybe it helps already in the mean time to update the feature branch with the commits it is behind to resolve some "false-included" commits in this PR?

[nscuro]

Because of this and the recent cherry-picks, the commit history is a bit mixed up with other commits, but that may be gone when the feature branch gets updated with the missing commits from the current master branch.

Let me push the latest changes from master to that branch.

Please provide me with additional information for the errors of the Static Code Analysis as i cannot see any useful information in the details link.

Unfortunately Codacy requires us to manually enable each "base" branch for analysis. Unless that is done, analysis will fail immediately. I have now enabled the feature branch, so the check should work for the next push.

[nscuro]

Updated the branches for dependency-track and frontend with the latest master. You may need to close and reopen the PR since GitHub doesn't update PRs automatically when the base branch gets updated.

[MM-msr]

@valentijnscholten Should be "fixed" by now, thanks to @nscuro :) The update did thankfully what i hoped for.

The current CI test exceptions in BomUploadProcessingTaskTest.java seems to be caused by other stuff, that this PR does not affect, but if i can provide any support, please let me know.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
❌ -1.44% (target: -1.00%)	❌ 37.63% (target: 70.00%)

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (a4d5a7b)	22074	16811	76.16%
Head commit (7ea910c)	22993 (+919)	17180 (+369)	74.72% (-1.44%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#3925)	885	333	37.63%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

---

_{🚀 Don’t miss a bit, follow what’s new on Codacy.}

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}

[nscuro]

The current CI test exceptions in BomUploadProcessingTaskTest.java seems to be caused by other stuff, that this PR does not affect, but if i can provide any support, please let me know.

You got lucky and ran into a flaky test 😅 Re-ran the workflow and it completed successfully this time.

[nscuro]

Thanks for the PR @MM-msr! Some initial feedback, I figured it would be easier to give it before merging the PR. I can still merge it now if you'd like to get feedback from users sooner.

Please have a look at my comments and let me know how you'd like to proceed.

[nscuro]

Can we achieve the desired functionality without introducing a new table? Looking at this class, most of the fields and behaviors between it and NotificationRule are the same. notificationLevel is there but not used.

[MM-msr]

Actually, that was my first approach. I changed it to an extra table, because i don't know ORMs very well and it didn't work as i was expecting it with adding/editing the ORM-related attributes. Especially when loading all items depending on their class-type (such as fetching all ScheduledNotificationRule items from the database), i had too many problems to invest more time to get it to work.

I may investigate it again, maybe we can do sth. like filters to fetch differently when requesting NotificationRule vs ScheduledNotificationRule (like "isScheduled == true"). But if you can give me an advice how to maybe do it automatically with ORM-magic, i appreciate it. Depending on how much it affects the existing lines of code for NotificationRule, we can definitely merge it.

[nscuro]

While this works, requiring a restart to for changing cron expressions is a bit harsh I think.

Something you could investigate:

1. Create a single, global ScheduledExecutorService. Have it execute every X seconds.
2. Upon every execution of the scheduler, fetch all scheduled NotificationRules
3. Parse the rules's cron expression
4. Check if the rule is due for triggering based on the cron

This way, changing the schedule should not require a restart. Also, the solution would scale a bit better, since you won't need a timer for each rule, but only one.

[MM-msr]

The TaskInitializer class is only existing to register the existing cron expressions (CE) of enabled rules at startup. Every change of a CE will be handled by the TaskManager class, such as cancelling the current cron countdown and initiate the new one. So if you change a CE in the ui, the manager will cancel the old notification task and reschedule a new one. If a notification was performed, it calculates the new time until the next execution and schedules the next notification. And so on. So there is no need to restart to get new expressions to work.

If you mean sth different here that i don't get, please tell me, else mark it as resolved.

[nscuro]

Any way we can avoid loading all projects in one go? Also, do we need all fields of the project, or only a small subset?

Loading them all will perform very badly for large portfolios, potentially even causing OutOfMemoryExceptions.

[MM-msr]

What we really need in the end is the project id, uuid and the basic information for toJson(Project) for the template. In the basic mail template, we need the project name and version (if i didn't forget anything). The last thing we need to have, is the proper children-connection to find all projects with their children, if the setting in the scheduled notification rule is active.

A different approach here might be the following: instead of getting new findings since date x for each of the affected projects, we could fetch all findings since date x and then filter them similar to NotificationRouter.resolveRules(...):
if(rule has projects)
-> get only findings, which have a project id of the rule projects
else
-> use all findings to get the affected project ids, load them and use this project list for the scheduled notification.

This way, we only load what is needed. But what is important nonetheless: we need a proper children-connection in rule.getProjects(). If that's working fine, this approach should work well.

What do you think about that?

[MM-msr]

Thanks a lot @nscuro for the initial feedback! I'm currently in the process of writing additional test cases to meet the Code Coverage Quota. I may consider to add some of your suggested changes after that as well or reply to them later this week to discuss them. I think by the end of the week, we know if it's a good time to merge it :) I'll try to get the most things done until then.

[MM-msr]

Short update from my side: i added some tests and applied your suggested changes for records. Therefore, i created a factory to create these notification subjects with the logic which was previously located in the specific classes.
I want to test it a bit more and implement some additional suggested changes before i push it, but i will try to finish my local changes in the next days.

[MM-msr]

@nscuro Hey there :) Could you trigger a test execution? As i can't really test most of the net-related test cases i want to know if i have to fix sth prior to releasing the PRs from draft state. Thanks in advance!