Skip to content

Conversation

@Strum355
Copy link
Member

@Strum355 Strum355 commented Nov 4, 2025

PR for #2056

Summary by Sourcery

Add support for version ranges in PurlStatus by introducing a VersionRange type, updating the vulnerability analysis flow and API schema, and adjusting tests to validate the new field.

New Features:

  • Include version_range in PurlStatus and return it in analysis responses
  • Introduce VersionRange enum and schema to represent package version bounds

Enhancements:

  • Refactor vulnerability analysis to replace status map with a flat list of PurlStatus entries
  • Simplify CVSS3 score handling by using Cvss3Base and streamline advisories aggregation

Documentation:

  • Extend OpenAPI schema with PurlStatus changes and VersionRange definition

Tests:

  • Update tests to verify version_range and new purl_statuses structure

Summary by Sourcery

Add support for version ranges in PurlStatus and refactor vulnerability analysis to return a flat list of statuses with version_range, updating models, API schema, and tests accordingly.

New Features:

  • Include version_range in PurlStatus and return it in analysis responses
  • Introduce VersionRange enum and schema to represent package version bounds

Enhancements:

  • Refactor vulnerability analysis to produce a flat list of PurlStatus entries instead of a status-to-advisory map
  • Simplify CVSS3 score processing by consolidating on Cvss3Base and using Score.from_iter

Documentation:

  • Extend OpenAPI schema to replace status map with purl_statuses array and define VersionRange

Tests:

  • Update tests to validate version_range field and new purl_statuses structure in vulnerability analysis and advisory reingestion

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Nov 4, 2025

Reviewer's Guide

This PR introduces support for version ranges in PurlStatus by defining a new VersionRange enum, refactoring the vulnerability analysis service to emit a flat list of PurlStatus entries (including version_range) instead of a status map, streamlining CVSS3 score handling with Cvss3Base, updating the OpenAPI schema to replace the old status structure with purl_statuses and define VersionRange, and adjusting tests to validate the new field.

ER diagram for new VersionRange schema in OpenAPI

erDiagram
    PurlStatus {
        string status
        string vulnerability
        string context
        VersionRange version_range
    }
    VersionRange {
        string version_scheme_id
        string left
        boolean left_inclusive
        string right
        boolean right_inclusive
    }
    PurlStatus ||--o| VersionRange : has
Loading

Class diagram for new VersionRange and updated PurlStatus

classDiagram
    class VersionRange {
        <<enum>>
        +Full(version_scheme_id: String, left: String, left_inclusive: bool, right: String, right_inclusive: bool)
        +Left(version_scheme_id: String, left: String, left_inclusive: bool)
        +Right(version_scheme_id: String, right: String, right_inclusive: bool)
        +from_entity(value: version_range::Model) Result<VersionRange, Error>
    }
    class PurlStatus {
        +vulnerability: VulnerabilityHead
        +average_severity: Option<f64>
        +average_score: Option<f64>
        +status: String
        +context: Option<StatusContext>
        +version_range: Option<VersionRange>
        +new(vuln: &vulnerability::Model, status: String, version_range: Option<VersionRange>, cpe: Option<String>, tx: &C) -> Result<PurlStatus, Error>
        +from_head(vuln_head: VulnerabilityHead, status: String, version_range: Option<VersionRange>, cpe: Option<String>, score: Score) -> Result<PurlStatus, Error>
    }
    VersionRange <.. PurlStatus : uses
Loading

Class diagram for refactored AnalysisDetails

classDiagram
    class AnalysisDetails {
        +head: VulnerabilityHead
        +purl_statuses: Vec<PurlStatus>
    }
    class PurlStatus {
        +vulnerability: VulnerabilityHead
        +average_severity: Option<f64>
        +average_score: Option<f64>
        +status: String
        +context: Option<StatusContext>
        +version_range: Option<VersionRange>
    }
    AnalysisDetails "1" o-- "*" PurlStatus : contains
Loading

File-Level Changes

Change Details Files
Introduce VersionRange enum and integrate it into PurlStatus model
  • Define VersionRange enum with Full/Left/Right variants
  • Add version_range field to PurlStatus struct and constructors
  • Wire VersionRange::from_entity where statuses are built
modules/fundamental/src/purl/model/details/version_range.rs
modules/fundamental/src/purl/model/details/purl.rs
modules/fundamental/src/purl/model/details/mod.rs
Refactor vulnerability analysis to include version_range and emit flat purl_statuses list
  • Extend SQL JSONB aggregation to include version_range object
  • Introduce RowEntry struct with version_range deserialization
  • Replace nested status map and advisories_map with grouping into Vec
modules/fundamental/src/vulnerability/service/mod.rs
Streamline CVSS3 score handling using Cvss3Base
  • Change cvss3_map values from Vec to Vec
  • Replace Score::try_from conversion with Cvss3Base::from
  • Update service tests to assert average_score instead of checking scores vector
modules/fundamental/src/vulnerability/service/mod.rs
modules/fundamental/src/vulnerability/service/test.rs
Update OpenAPI schema to replace status map with purl_statuses and define VersionRange
  • Remove AnalysisAdvisory and status properties in AnalysisDetails
  • Add purl_statuses property referencing PurlStatus schema
  • Add VersionRange schema and reference under PurlStatus
openapi.yaml
modules/fundamental/src/vulnerability/model/analyze.rs
Adapt existing tests for new purl_statuses structure and version_range field
  • Filter and assert on vuln_entry.purl_statuses in analysis tests
  • Include version_range in advisory reingest (csaf/osv) and delete tests
  • Adjust vulnerability service tests to locate status via purl_statuses
modules/fundamental/tests/vuln/mod.rs
modules/fundamental/tests/advisory/csaf/reingest.rs
modules/fundamental/tests/advisory/osv/reingest.rs
modules/fundamental/tests/advisory/csaf/delete.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@Strum355 Strum355 force-pushed the nsc/purl-status-version-range branch 2 times, most recently from 4a1a75e to 53c656b Compare November 5, 2025 15:49
@Strum355 Strum355 force-pushed the nsc/purl-status-version-range branch from cca7783 to 02df256 Compare November 10, 2025 14:52
@Strum355 Strum355 marked this pull request as ready for review November 10, 2025 14:52
@Strum355 Strum355 requested a review from dejanb November 10, 2025 14:52
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes - here's some feedback:

  • The row_to_vuln method is quite large and mixes SQL deserialization with PurlStatus construction—consider extracting the status‐aggregation and PurlStatus creation into smaller helper functions to improve readability and maintainability.
  • Using an untagged enum for VersionRange may lead to ambiguous deserialization if fields overlap—consider switching to a tagged enum or adding explicit serde annotations to make each variant unambiguous.
  • Hand-editing openapi.yaml alongside code changes can easily get out of sync—consider using derive-based OpenAPI generation or a single source of truth to avoid manual schema drift.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The row_to_vuln method is quite large and mixes SQL deserialization with PurlStatus construction—consider extracting the status‐aggregation and PurlStatus creation into smaller helper functions to improve readability and maintainability.
- Using an untagged enum for VersionRange may lead to ambiguous deserialization if fields overlap—consider switching to a tagged enum or adding explicit serde annotations to make each variant unambiguous.
- Hand-editing openapi.yaml alongside code changes can easily get out of sync—consider using derive-based OpenAPI generation or a single source of truth to avoid manual schema drift.

## Individual Comments

### Comment 1
<location> `openapi.yaml:5113-5133` </location>
<code_context>
         oneOf:
         - type: 'null'
         - type: string
+    VersionRange:
+      oneOf:
+      - type: object
+        required:
+        - version_scheme_id
+        - left
+        - left_inclusive
+        - right
+        - right_inclusive
+        properties:
+          left:
+            type: string
+          left_inclusive:
+            type: boolean
+          right:
+            type: string
+          right_inclusive:
+            type: boolean
+          version_scheme_id:
+            type: string
+      - type: object
</code_context>

<issue_to_address>
**suggestion:** Consider clarifying the VersionRange schema for consumers.

Document the three object variants in VersionRange and provide guidance on how to identify each, or refactor to a tagged union for improved clarity.

```suggestion
    VersionRange:
      description: |
        VersionRange is a tagged union with three object variants:
        1. Bounded range: includes left, right, left_inclusive, right_inclusive, and version_scheme_id.
        2. Unbounded range: only version_scheme_id is present.
        3. Single version: only left and version_scheme_id are present.
        Use the 'type' discriminator to identify the variant.
      discriminator:
        propertyName: type
        mapping:
          bounded: '#/components/schemas/VersionRangeBounded'
          unbounded: '#/components/schemas/VersionRangeUnbounded'
          single: '#/components/schemas/VersionRangeSingle'
      oneOf:
        - $ref: '#/components/schemas/VersionRangeBounded'
        - $ref: '#/components/schemas/VersionRangeUnbounded'
        - $ref: '#/components/schemas/VersionRangeSingle'

    VersionRangeBounded:
      type: object
      required:
        - type
        - version_scheme_id
        - left
        - left_inclusive
        - right
        - right_inclusive
      properties:
        type:
          type: string
          enum: [bounded]
        left:
          type: string
        left_inclusive:
          type: boolean
        right:
          type: string
        right_inclusive:
          type: boolean
        version_scheme_id:
          type: string

    VersionRangeUnbounded:
      type: object
      required:
        - type
        - version_scheme_id
      properties:
        type:
          type: string
          enum: [unbounded]
        version_scheme_id:
          type: string

    VersionRangeSingle:
      type: object
      required:
        - type
        - left
        - version_scheme_id
      properties:
        type:
          type: string
          enum: [single]
        left:
          type: string
        version_scheme_id:
          type: string
```
</issue_to_address>

### Comment 2
<location> `modules/fundamental/tests/vuln/mod.rs:60` </location>
<code_context>

-    assert_eq!(status_entry.len(), 1);
-    let json = serde_json::to_value(status_entry).expect("must serialize");
+    assert_eq!(status_entries.len(), 2);
+    let json = serde_json::to_value(status_entries).expect("must serialize");
     assert!(
</code_context>

<issue_to_address>
**suggestion (testing):** Consider adding tests for edge cases in version_range, such as left-only, right-only, and invalid ranges.

Please add tests for cases with only a left or right bound, and for missing or invalid version_range values, to ensure the enum handles all scenarios correctly.

Suggested implementation:

```rust
    assert_eq!(status_entries.len(), 2);
    let json = serde_json::to_value(status_entries).expect("must serialize");
    assert!(
        json.contains_subset(json!([{
    }));

    // Edge case: version_range with only left bound
    let left_only_status = StatusEntry {
        status: "affected".to_string(),
        version_range: Some(VersionRange::Left { left: "1.0.0".to_string() }),
        ..Default::default()
    };
    let json_left = serde_json::to_value(&left_only_status).expect("must serialize left-only");
    assert!(json_left["version_range"]["left"] == "1.0.0");

    // Edge case: version_range with only right bound
    let right_only_status = StatusEntry {
        status: "affected".to_string(),
        version_range: Some(VersionRange::Right { right: "2.0.0".to_string() }),
        ..Default::default()
    };
    let json_right = serde_json::to_value(&right_only_status).expect("must serialize right-only");
    assert!(json_right["version_range"]["right"] == "2.0.0");

    // Edge case: version_range missing
    let missing_range_status = StatusEntry {
        status: "affected".to_string(),
        version_range: None,
        ..Default::default()
    };
    let json_missing = serde_json::to_value(&missing_range_status).expect("must serialize missing");
    assert!(json_missing.get("version_range").is_none() || json_missing["version_range"].is_null());

    // Edge case: invalid version_range value (simulate with manual JSON)
    let invalid_json = json!({
        "status": "affected",
        "version_range": "not_a_valid_range"
    });
    let result: Result<StatusEntry, _> = serde_json::from_value(invalid_json);
    assert!(result.is_err(), "Invalid version_range should fail to deserialize");

```

- Ensure that `StatusEntry`, `VersionRange`, and their serialization/deserialization are available in scope.
- You may need to adjust the construction of `StatusEntry` and `VersionRange` to match your actual struct/enum definitions.
- If `Default::default()` is not implemented for `StatusEntry`, manually provide required fields.
</issue_to_address>

### Comment 3
<location> `modules/fundamental/tests/advisory/csaf/reingest.rs:166` </location>
<code_context>
             context: Some(StatusContext::Cpe(
                 "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
             )),
+            version_range: Some(VersionRange::Full {
+                version_scheme_id: "rpm".into(),
+                left: "1.76.0-4.redhat_00001.1.el9eap".into(),
</code_context>

<issue_to_address>
**suggestion (testing):** Test only covers the 'Full' variant of VersionRange; consider adding cases for 'Left' and 'Right' variants.

Please add tests for advisories with only a left or right bound to confirm correct serialization and deserialization of all VersionRange variants.

Suggested implementation:

```rust
            context: Some(StatusContext::Cpe(
                "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
            )),
            version_range: Some(VersionRange::Full {
                version_scheme_id: "rpm".into(),
                left: "1.76.0-4.redhat_00001.1.el9eap".into(),
                left_inclusive: true,
                right: "1.76.0-4.redhat_00001.1.el9eap".into(),
                right_inclusive: true
            }),
        },
        // Test case for VersionRange::Left
        {
            context: Some(StatusContext::Cpe(
                "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
            )),
            version_range: Some(VersionRange::Left {
                version_scheme_id: "rpm".into(),
                left: "1.76.0-4.redhat_00001.1.el9eap".into(),
                left_inclusive: true,
            }),
        },
        // Test case for VersionRange::Right
        {
            context: Some(StatusContext::Cpe(
                "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
            )),
            version_range: Some(VersionRange::Right {
                version_scheme_id: "rpm".into(),
                right: "1.76.0-4.redhat_00001.1.el9eap".into(),
                right_inclusive: true,
            }),
        }]
    );

```

If your test structure expects a specific type or struct for each advisory, ensure the new test cases match the expected format. You may also need to add assertions for serialization/deserialization of these new variants, similar to how you test the `Full` variant.
</issue_to_address>

### Comment 4
<location> `modules/fundamental/tests/advisory/osv/reingest.rs:134` </location>
<code_context>
             context: Some(StatusContext::Cpe(
                 "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
             )),
+            version_range: Some(VersionRange::Full {
+                version_scheme_id: "rpm".into(),
+                left: "1.76.0-4.redhat_00001.1.el9eap".into(),
</code_context>

<issue_to_address>
**suggestion (testing):** Missing tests for advisories with only left or right version bounds.

Please add tests for advisories using VersionRange::Left and VersionRange::Right to cover these scenarios.
</issue_to_address>

### Comment 5
<location> `modules/fundamental/src/vulnerability/service/test.rs:636` </location>
<code_context>

     assert_eq!(result.len(), 1);
     let advisory = &result[PURL].details[0];
-    let status = &advisory.status["affected"][0];
+    let status = &advisory
+        .purl_statuses
</code_context>

<issue_to_address>
**suggestion (testing):** Test does not verify the version_range field for affected status.

Please add assertions to check that version_range is correctly set for affected status, including cases where it should be None or a specific value.

Suggested implementation:

```rust
    let status = &advisory
        .purl_statuses
        .iter()
        .find(|status| status.status == "affected")
        .unwrap();

    // the scores must be empty
    assert_eq!(0.0, status.average_score);

    // verify version_range for affected status
    // If you expect a specific value, replace Some("expected_value") with the expected value.
    match &status.version_range {
        Some(range) => {
            // Replace "expected_value" with the actual expected version range string
            assert_eq!(range, "expected_value");
        }
        None => {
            // If you expect None, assert it
            assert!(status.version_range.is_none());
        }
    }

    Ok(())

```

You may need to adjust the `"expected_value"` string in the assertion to match the actual expected version range for your test case. If you have multiple test cases, consider parameterizing the test or adding more assertions for each scenario.
</issue_to_address>

### Comment 6
<location> `modules/fundamental/tests/advisory/csaf/delete.rs:148` </location>
<code_context>
             context: Some(StatusContext::Cpe(
                 "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
             )),
+            version_range: Some(VersionRange::Full {
+                version_scheme_id: "rpm".into(),
+                left: "1.76.0-4.redhat_00001.1.el9eap".into(),
</code_context>

<issue_to_address>
**suggestion (testing):** Test only covers deletion with 'Full' version range; consider edge cases for other variants.

Please add tests for advisories with only left or right bounds to cover all VersionRange variants.

Suggested implementation:

```rust
            version_range: Some(VersionRange::Full {
                version_scheme_id: "rpm".into(),
                left: "1.76.0-4.redhat_00001.1.el9eap".into(),
                left_inclusive: true,
                right: "1.76.0-4.redhat_00001.1.el9eap".into(),
                right_inclusive: true
            }),
        },
        // Edge case: Only left bound
        {
            context: Some(StatusContext::Cpe(
                "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
            )),
            version_range: Some(VersionRange::Left {
                version_scheme_id: "rpm".into(),
                left: "1.76.0-4.redhat_00001.1.el9eap".into(),
                left_inclusive: true,
            }),
        },
        // Edge case: Only right bound
        {
            context: Some(StatusContext::Cpe(
                "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
            )),
            version_range: Some(VersionRange::Right {
                version_scheme_id: "rpm".into(),
                right: "1.76.0-4.redhat_00001.1.el9eap".into(),
                right_inclusive: true,
            }),
        }
    ]
    );

```

If your test logic explicitly checks deletion or behavior for each advisory, ensure you iterate over all advisories (including the new ones) and assert correct deletion/results for each. You may need to update any loops or assertions to handle the expanded set.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +5113 to +5133
VersionRange:
oneOf:
- type: object
required:
- version_scheme_id
- left
- left_inclusive
- right
- right_inclusive
properties:
left:
type: string
left_inclusive:
type: boolean
right:
type: string
right_inclusive:
type: boolean
version_scheme_id:
type: string
- type: object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Consider clarifying the VersionRange schema for consumers.

Document the three object variants in VersionRange and provide guidance on how to identify each, or refactor to a tagged union for improved clarity.

Suggested change
VersionRange:
oneOf:
- type: object
required:
- version_scheme_id
- left
- left_inclusive
- right
- right_inclusive
properties:
left:
type: string
left_inclusive:
type: boolean
right:
type: string
right_inclusive:
type: boolean
version_scheme_id:
type: string
- type: object
VersionRange:
description: |
VersionRange is a tagged union with three object variants:
1. Bounded range: includes left, right, left_inclusive, right_inclusive, and version_scheme_id.
2. Unbounded range: only version_scheme_id is present.
3. Single version: only left and version_scheme_id are present.
Use the 'type' discriminator to identify the variant.
discriminator:
propertyName: type
mapping:
bounded: '#/components/schemas/VersionRangeBounded'
unbounded: '#/components/schemas/VersionRangeUnbounded'
single: '#/components/schemas/VersionRangeSingle'
oneOf:
- $ref: '#/components/schemas/VersionRangeBounded'
- $ref: '#/components/schemas/VersionRangeUnbounded'
- $ref: '#/components/schemas/VersionRangeSingle'
VersionRangeBounded:
type: object
required:
- type
- version_scheme_id
- left
- left_inclusive
- right
- right_inclusive
properties:
type:
type: string
enum: [bounded]
left:
type: string
left_inclusive:
type: boolean
right:
type: string
right_inclusive:
type: boolean
version_scheme_id:
type: string
VersionRangeUnbounded:
type: object
required:
- type
- version_scheme_id
properties:
type:
type: string
enum: [unbounded]
version_scheme_id:
type: string
VersionRangeSingle:
type: object
required:
- type
- left
- version_scheme_id
properties:
type:
type: string
enum: [single]
left:
type: string
version_scheme_id:
type: string


assert_eq!(status_entry.len(), 1);
let json = serde_json::to_value(status_entry).expect("must serialize");
assert_eq!(status_entries.len(), 2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Consider adding tests for edge cases in version_range, such as left-only, right-only, and invalid ranges.

Please add tests for cases with only a left or right bound, and for missing or invalid version_range values, to ensure the enum handles all scenarios correctly.

Suggested implementation:

    assert_eq!(status_entries.len(), 2);
    let json = serde_json::to_value(status_entries).expect("must serialize");
    assert!(
        json.contains_subset(json!([{
    }));

    // Edge case: version_range with only left bound
    let left_only_status = StatusEntry {
        status: "affected".to_string(),
        version_range: Some(VersionRange::Left { left: "1.0.0".to_string() }),
        ..Default::default()
    };
    let json_left = serde_json::to_value(&left_only_status).expect("must serialize left-only");
    assert!(json_left["version_range"]["left"] == "1.0.0");

    // Edge case: version_range with only right bound
    let right_only_status = StatusEntry {
        status: "affected".to_string(),
        version_range: Some(VersionRange::Right { right: "2.0.0".to_string() }),
        ..Default::default()
    };
    let json_right = serde_json::to_value(&right_only_status).expect("must serialize right-only");
    assert!(json_right["version_range"]["right"] == "2.0.0");

    // Edge case: version_range missing
    let missing_range_status = StatusEntry {
        status: "affected".to_string(),
        version_range: None,
        ..Default::default()
    };
    let json_missing = serde_json::to_value(&missing_range_status).expect("must serialize missing");
    assert!(json_missing.get("version_range").is_none() || json_missing["version_range"].is_null());

    // Edge case: invalid version_range value (simulate with manual JSON)
    let invalid_json = json!({
        "status": "affected",
        "version_range": "not_a_valid_range"
    });
    let result: Result<StatusEntry, _> = serde_json::from_value(invalid_json);
    assert!(result.is_err(), "Invalid version_range should fail to deserialize");
  • Ensure that StatusEntry, VersionRange, and their serialization/deserialization are available in scope.
  • You may need to adjust the construction of StatusEntry and VersionRange to match your actual struct/enum definitions.
  • If Default::default() is not implemented for StatusEntry, manually provide required fields.

context: Some(StatusContext::Cpe(
"cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
)),
version_range: Some(VersionRange::Full {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Test only covers the 'Full' variant of VersionRange; consider adding cases for 'Left' and 'Right' variants.

Please add tests for advisories with only a left or right bound to confirm correct serialization and deserialization of all VersionRange variants.

Suggested implementation:

            context: Some(StatusContext::Cpe(
                "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
            )),
            version_range: Some(VersionRange::Full {
                version_scheme_id: "rpm".into(),
                left: "1.76.0-4.redhat_00001.1.el9eap".into(),
                left_inclusive: true,
                right: "1.76.0-4.redhat_00001.1.el9eap".into(),
                right_inclusive: true
            }),
        },
        // Test case for VersionRange::Left
        {
            context: Some(StatusContext::Cpe(
                "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
            )),
            version_range: Some(VersionRange::Left {
                version_scheme_id: "rpm".into(),
                left: "1.76.0-4.redhat_00001.1.el9eap".into(),
                left_inclusive: true,
            }),
        },
        // Test case for VersionRange::Right
        {
            context: Some(StatusContext::Cpe(
                "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
            )),
            version_range: Some(VersionRange::Right {
                version_scheme_id: "rpm".into(),
                right: "1.76.0-4.redhat_00001.1.el9eap".into(),
                right_inclusive: true,
            }),
        }]
    );

If your test structure expects a specific type or struct for each advisory, ensure the new test cases match the expected format. You may also need to add assertions for serialization/deserialization of these new variants, similar to how you test the Full variant.


assert_eq!(result.len(), 1);
let advisory = &result[PURL].details[0];
let status = &advisory.status["affected"][0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Test does not verify the version_range field for affected status.

Please add assertions to check that version_range is correctly set for affected status, including cases where it should be None or a specific value.

Suggested implementation:

    let status = &advisory
        .purl_statuses
        .iter()
        .find(|status| status.status == "affected")
        .unwrap();

    // the scores must be empty
    assert_eq!(0.0, status.average_score);

    // verify version_range for affected status
    // If you expect a specific value, replace Some("expected_value") with the expected value.
    match &status.version_range {
        Some(range) => {
            // Replace "expected_value" with the actual expected version range string
            assert_eq!(range, "expected_value");
        }
        None => {
            // If you expect None, assert it
            assert!(status.version_range.is_none());
        }
    }

    Ok(())

You may need to adjust the "expected_value" string in the assertion to match the actual expected version range for your test case. If you have multiple test cases, consider parameterizing the test or adding more assertions for each scenario.

context: Some(StatusContext::Cpe(
"cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
)),
version_range: Some(VersionRange::Full {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Test only covers deletion with 'Full' version range; consider edge cases for other variants.

Please add tests for advisories with only left or right bounds to cover all VersionRange variants.

Suggested implementation:

            version_range: Some(VersionRange::Full {
                version_scheme_id: "rpm".into(),
                left: "1.76.0-4.redhat_00001.1.el9eap".into(),
                left_inclusive: true,
                right: "1.76.0-4.redhat_00001.1.el9eap".into(),
                right_inclusive: true
            }),
        },
        // Edge case: Only left bound
        {
            context: Some(StatusContext::Cpe(
                "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
            )),
            version_range: Some(VersionRange::Left {
                version_scheme_id: "rpm".into(),
                left: "1.76.0-4.redhat_00001.1.el9eap".into(),
                left_inclusive: true,
            }),
        },
        // Edge case: Only right bound
        {
            context: Some(StatusContext::Cpe(
                "cpe:/a:redhat:jboss_enterprise_application_platform:7.4:*:el9:*".to_string()
            )),
            version_range: Some(VersionRange::Right {
                version_scheme_id: "rpm".into(),
                right: "1.76.0-4.redhat_00001.1.el9eap".into(),
                right_inclusive: true,
            }),
        }
    ]
    );

If your test logic explicitly checks deletion or behavior for each advisory, ensure you iterate over all advisories (including the new ones) and assert correct deletion/results for each. You may need to update any loops or assertions to handle the expanded set.

@codecov
Copy link

codecov bot commented Nov 10, 2025

Codecov Report

❌ Patch coverage is 77.33333% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.19%. Comparing base (80947f5) to head (02df256).

Files with missing lines Patch % Lines
...undamental/src/purl/model/details/version_range.rs 56.00% 11 Missing ⚠️
modules/fundamental/src/purl/model/details/purl.rs 82.14% 0 Missing and 5 partials ⚠️
...ules/fundamental/src/vulnerability/service/test.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2079      +/-   ##
==========================================
+ Coverage   68.16%   68.19%   +0.03%     
==========================================
  Files         368      369       +1     
  Lines       20682    20747      +65     
  Branches    20682    20747      +65     
==========================================
+ Hits        14097    14148      +51     
- Misses       5747     5753       +6     
- Partials      838      846       +8     

☔ 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant