Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[maintenance] scd improve readability of OIR upsertion logic and OIR subscription handling #1088

Open
8 of 13 tasks
Shastick opened this issue Aug 30, 2024 · 0 comments
Open
8 of 13 tasks
Assignees
Labels
dss Relating to one of the DSS implementations feature Issue would improve software P2 Normal priority

Comments

@Shastick
Copy link
Contributor

Shastick commented Aug 30, 2024

The logic for creating and updating Operational Intent References (OIR) is central to the DSS's role.

The current work to address some open issues (#1056, #1074) revealed that the logic within this method is rather difficult to grasp, hindering its maintenance.

This issue's goals are:

  • provide a high level description of what this method needs to do
  • document how and where this method is tested
  • provide a place to discuss approaches to simplify and improve that method's maintainability

Logic

The description below covers the logic of upsertOperationalIntentReference with respect to the handling of subscriptions and their cleanup when they are implicit.

Below, cipasicr() is a short-hand for check if previously attached subscription is implicit and can be removed

Subscription Create OIR Update OIR
None 1. check state,
2. create OIR
1. check state
2. update OIR
3. cipasicr()
Provided 1. check sub covers OIR. If not and is implicit: adapt, otherwise fail.
2 .create OIR
1. check sub covers OIR. If not and is implicit: adapt, otherwise fail
2. update OIR
3. cipasicr()
Request Implicit 1. create implicit sub
2. create OIR
1 .create implicit sub
2. update OIR
3. cipasicr()

See also @mickmis's comment with a table that focuses purely on the subscription handling:

implicit old sub explicit old sub no old sub
implicit sub in params delete old if no dependent OI - -
explicit sub in params delete old if no dependent OI - -
no sub in params delete old if no dependent OI - -
request new implicit sub in params create new implicit sub,
delete old if no dependent OI
create new implicit sub create new implicit sub

Approaches

In a first step, we'll want to factor pieces of upsertOperationalIntentReference to into submethods. This should make the upsertion logic's behavior more tractable.

We should probably do so on the main branch, then incorporate the reorganisation into #1057

Testing

Subscription Create OIR Update OIR
None OIRImplicitSubHandling OIRImplicitSubHandling
Provided OIRImplicitSubHandling, OIRSimple OIRImplicitSubHandling
Request Implicit OIRImplicitSubHandling OIRImplicitSubHandling
implicit old sub explicit old sub no old sub
existing implicit sub in params OIRImplicitSubHandling OIRImplicitSubHandling ❌ (2)
existing explicit sub in params OIRImplicitSubHandling OIRSimple OIRSimple
no sub in params OIRImplicitSubHandling OIRSimple ❌ (6)
request new implicit sub in params OIRImplicitSubHandling ❌ (7) ❌ (8)

The missing test scenarios (marked with a ❌) can likely be added to the OIRSimple scenario introduced by #1082

Considerations

While working on this, we should keep in mind the following:

The above are likely to impact the implementation: we should avoid spending too much efforts on parts that we know may disappear in the near future.

Tasks

@Shastick Shastick added P2 Normal priority feature Issue would improve software dss Relating to one of the DSS implementations labels Aug 30, 2024
@Shastick Shastick self-assigned this Sep 3, 2024
Shastick added a commit to Orbitalize/dss that referenced this issue Sep 9, 2024
This is a rework of interuss#1057 after most changes for interuss#1088 have landed.

The DSS built from this PR successfully passed the new qualifier's implicit subscription handling scenario, although that scenario still needs to be extended to cover all corner cases.

The PR is open for review, as it addresses interuss#1056
mickmis pushed a commit that referenced this issue Sep 9, 2024
This is a rework of #1057 after most changes for #1088 have landed.

The DSS built from this PR successfully passed the new qualifier's implicit subscription handling scenario, although that scenario still needs to be extended to cover all corner cases.

The PR is open for review, as it addresses #1056
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dss Relating to one of the DSS implementations feature Issue would improve software P2 Normal priority
Projects
None yet
Development

No branches or pull requests

1 participant