fix: maven dverbose scope collisions failures

[gemaxim]

Pull Request Submission Checklist

• Follows CONTRIBUTING guidelines
• Includes detailed description of changes
• Contains risk assessment (Low | Medium | High)
• Highlights breaking API changes (if applicable)
• Links to automated tests covering new functionality
• Includes manual testing instructions (if necessary)
• Updates relevant GitBook documentation (PR link: ___)
• Includes product update to be announced in the next stable release notes

What does this PR do?

Fixes a bug in the maven Dverbose graph building algorithm for specific cases where there are collisions of scope for some packages from the '-Dverbose' maven output.

These 3 PRs come together:
snyk/snyk-mvn-plugin#188 - split of dverbose/not dverbose graph build functions. dverbose function includes the fix where we use scope in the package key for visited + correct queue push in the bfs.
snyk/snyk-mvn-plugin#186
snyk/snyk-mvn-plugin#187

Where should the reviewer start?

This PR explains the fix: snyk/snyk-mvn-plugin#188 and also includes a fixture to represent the use case for the bug.

How should this be manually tested?

Using this pom.xml. Run snyk test/monitor -- -Dverbose or snyk sbom.
Before the fix: parentNodeId does not exist error.
With fix: successful response.

What's the product update that needs to be communicated to CLI users?

Fix for maven sbom and test/monitor with -- -Dverbose affecting scope collisions from the maven Dverbose command output (parentNodeId does not exist error).

What are the relevant tickets?

Common issue: OSM-2676

[dotkas]

@gemaxim nice, thanks for this. Some questions:

1. Should there be an acceptance test added here as well? Even if the Maven plugin repo has a fixture to reproduce it, we're still exposed in the CLI to regression errors.
2. I might be misunderstanding, but does this change not constitute a breaking change? It creates a different dependency graph than before, no? Should it not be a major version change of the plugin?

[gemaxim]

@dotkas thanks for the comms.

1. I agree, I'll add an acceptance test, I'm not sure if there was a reason behind not adding any for Dverbose until now.
2. This only affects Dverbose and I treated this change as a bugfix, as we've done before for any fixes for this flow, even if they change the dependency graph - example1, example2. It does not change the graph in every scenario, only in specific cases where the maven resolves the same package with different scopes and different versions.