Skip to content

[scd] Validate Volume4D when parsing#1380

Open
RustedBones wants to merge 1 commit intointeruss:masterfrom
skyguide-ansp:volume-validation
Open

[scd] Validate Volume4D when parsing#1380
RustedBones wants to merge 1 commit intointeruss:masterfrom
skyguide-ansp:volume-validation

Conversation

@RustedBones
Copy link
Contributor

@RustedBones RustedBones commented Feb 25, 2026

Factorize validation for Volume3D and Volume4D during parsing:

  • Ensure startTime < endTime when defined
  • Ensure altLo < altHi when defined

Introduce parsing options to tune validation:

  • Ensure altLo and altHi set when RequireAltitudeBounds enabled
  • Ensure startTime ans endTime set when RequireTimeBounds enabled

Configuration options for SCD entities mutation:

  • op-intent: RequireTimeBounds: true, RequireAltitudeBounds: true
  • constraints: RequireTimeBounds: true, RequireAltitudeBounds: false
  • subscriptions: RequireTimeBounds: false, RequireAltitudeBounds: false

For SCD queries: RequireTimeBounds: false, RequireAltitudeBounds: false

@RustedBones RustedBones force-pushed the volume-validation branch 5 times, most recently from 51e5d77 to 9a11fcb Compare February 26, 2026 16:23
@RustedBones RustedBones marked this pull request as ready for review March 4, 2026 09:34
Copy link
Contributor

@mickmis mickmis left a comment

Choose a reason for hiding this comment

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

Intent SGTM, I do have suggestions regarding implementation, please LMK what you think.

// Volume4DFromSCDRest converts vol4 SCD v1 REST model to a Volume4D
func Volume4DFromSCDRest(vol4 *restapi.Volume4D) (*Volume4D, error) {
vol3, err := Volume3DFromSCDRest(&vol4.Volume)
func Volume4DFromSCDRest(vol4 *restapi.Volume4D, opts Volume4DOpts) (*Volume4D, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Using something inspired from the options/builder patterns could provide a cleaner and extensible interface here, i.e. sth like:

type Volume4DValidator func(*Volume4D) error

func WithRequireTimeBounds() Volume4DValidator {
    return func(v *Volume4D) {
        if v.StartTime == nil {
            return fmt.Errorf("no start time")
        }
        if v.EndTime == nil {
            return fmt.Errorf("no end time")
        }
    }
}

func Volume4DFromSCDRest(vol4 *restapi.Volume4D, validators ...Volume4DValidator) (*Volume4D, error) {
    ...
    for _, validator := range validators {
        if err := validator(svc); err != nil {
            ...
        }
    }
    ...
}

}, nil
}

type Volume3DOpts struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually ever need to validate those directly on a Volume3D? If not consider doing the validation only from Volume4DFromSCDRest.

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.

2 participants