[backend] refactor: refactor processors (#4302)

[savacano28]

Proposed changes

• Refactor generateFinding functions

Testing Instructions

You should execute injects capables to generate findings: nuclei, nmap, text, numbr, portscan...

Related issues

• Closes Shodan Injector add asset-creation option #4302
• Closes Findings fail to be persisted when a duplicate is found in structured output #5107

Checklist

• I consider the submitted work as finished
• I tested the code for its functionality
• I wrote test cases for the relevant uses case
• I added/update the relevant documentation (either on github or on notion)
• Where necessary I refactored code to improve the overall quality
• For bug fix -> I implemented a test that covers the bug

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

[codecov]

Codecov Report

❌ Patch coverage is 79.68750% with 78 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.07%. Comparing base (0b26799) to head (2fc9bc4).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...n/java/io/openaev/rest/finding/FindingService.java	73.83%	14 Missing and 14 partials ⚠️
.../main/java/io/openaev/service/EndpointService.java	23.52%	21 Missing and 5 partials ⚠️
...a/io/openaev/service/InjectExpectationService.java	81.81%	1 Missing and 5 partials ⚠️
...openaev/output_processor/AssetOutputProcessor.java	92.53%	2 Missing and 3 partials ⚠️
...enaev/output_processor/OutputProcessorFactory.java	42.85%	3 Missing and 1 partial ⚠️
...ev/rest/inject/service/InjectExecutionService.java	66.66%	4 Missing ⚠️
...ct/service/InjectorExecutionProcessingHandler.java	92.85%	1 Missing and 1 partial ⚠️
...va/io/openaev/rest/asset/endpoint/EndpointApi.java	0.00%	1 Missing ⚠️
...aev/rest/inject/service/ContractOutputContext.java	92.85%	0 Missing and 1 partial ⚠️
...enaev/rest/inject/service/InjectStatusService.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #4975      +/-   ##
============================================
+ Coverage     56.76%   57.07%   +0.31%     
- Complexity     4598     4676      +78     
============================================
  Files          1008     1014       +6     
  Lines         29938    30103     +165     
  Branches       2174     2192      +18     
============================================
+ Hits          16994    17182     +188     
+ Misses        11976    11942      -34     
- Partials        968      979      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 48 out of 48 changed files in this pull request and generated 5 comments.

[antoinemzs]

On the design:

Please reconsider the public interface for AbstractOutputProcessor to avoid exposing internals.

Consider calling specialised InjectExecutionService service methods instead of resolving the correct handler. While the latter is not a bad design, it does not reflect the intention that a given inject execution context can only be handled by a single handler.

On the testing:

Individual output processors should be tested through their respective process() implementation (missing in most cases).

Since most tests are mocked, we are missing integration testing coverage which means we don't know for sure that a given structured output will trigger side effects as intended. Please consider adding end-to-end tests at least covering InjectExecutionService with various structured outputs.

Consider enforcing that test titles and implementation are in sync. As a reader, I found multiple examples (I've highlighted one for example) where the titles is a bit misleading as it describes not what is tested but how, which can lead to some misunderstanding.

[savacano28]

@antoinemzs Hi! I’ve made the requested changes. The PR is ready for review again. Ty! :)

[savacano28]

Hi @antoinemzs! I have made the changes for this issue and #5107 (found when nmap sends outputs type port), ty :)!

[antoinemzs]

looks good, dedupe tested on ports. let's have this tested in the wider arena.