From 33fa2bcf06a6a912063830f00f43ab60dd7244b1 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Wed, 20 Nov 2024 16:18:44 -0800 Subject: [PATCH 01/22] Add deletion to EditablePlan interface --- .../scheduling/plan/DeletedAnchorStrategy.kt | 8 +++++++ .../aerie/procedural/scheduling/plan/Edit.kt | 5 +++++ .../scheduling/plan/EditablePlan.kt | 22 +++++++++++++++++++ 3 files changed, 35 insertions(+) create mode 100644 procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt new file mode 100644 index 0000000000..7acce1ba00 --- /dev/null +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt @@ -0,0 +1,8 @@ +package gov.nasa.ammos.aerie.procedural.scheduling.plan + +enum class DeletedAnchorStrategy { + Error, + Cascade, + AnchorToParent, + AnchorToPlan +} diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/Edit.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/Edit.kt index 2edcca9fb5..2898caad8a 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/Edit.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/Edit.kt @@ -2,6 +2,7 @@ package gov.nasa.ammos.aerie.procedural.scheduling.plan import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.AnyDirective import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.Directive +import gov.nasa.jpl.aerie.types.ActivityDirectiveId /** * Edits that can be made to the plan. @@ -11,4 +12,8 @@ import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.Directive sealed interface Edit { /** Create a new activity from a given directive. */ data class Create(/***/ val directive: Directive): Edit + + /** Delete an activity, specified by directive id. */ + data class Delete(val directive: Directive): Edit { + } } diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/EditablePlan.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/EditablePlan.kt index 5a6e87b638..c86b78f19d 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/EditablePlan.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/EditablePlan.kt @@ -3,6 +3,7 @@ package gov.nasa.ammos.aerie.procedural.scheduling.plan import gov.nasa.jpl.aerie.merlin.protocol.types.SerializedValue import gov.nasa.ammos.aerie.procedural.scheduling.simulation.SimulateOptions import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.AnyDirective +import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.Directive import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.DirectiveStart import gov.nasa.ammos.aerie.procedural.timeline.plan.Plan import gov.nasa.ammos.aerie.procedural.timeline.plan.SimulationResults @@ -33,6 +34,27 @@ interface EditablePlan: Plan { start )) + /** Delete an activity specified by directive id, and throw an error if any activities are anchored to it. */ + fun delete(id: ActivityDirectiveId) = delete(id, DeletedAnchorStrategy.Error) + + /** + * Delete an activity specified by directive id, with a strategy to handle activities that are anchored to it. + * + * If other anchored activities are affected, extra addition and deletion edits may be created. + */ + fun delete(id: ActivityDirectiveId, strategy: DeletedAnchorStrategy) + + /** Delete an activity and throw an error if any activities are anchored to it. */ + fun delete(directive: Directive) = delete(directive, DeletedAnchorStrategy.Error) + + /** + * Delete an activity with a strategy to handle activities that are anchored to it. + * + * If other anchored activities are affected, extra addition and deletion edits may be created. + */ + fun delete(directive: Directive, strategy: DeletedAnchorStrategy) + + /** Commit plan edits, making them final. */ fun commit() From 6c2a0e025fb47ad97e13c67938e4d0bba82649e5 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Wed, 20 Nov 2024 16:19:39 -0800 Subject: [PATCH 02/22] Implement deletion in EasyEditablePlanDriver --- .../scheduling/plan/DeletedAnchorStrategy.kt | 3 +- .../aerie/procedural/scheduling/plan/Edit.kt | 9 +- .../utils/EasyEditablePlanDriver.kt | 298 ++++++++++++++++++ .../utils/PerishableSimulationResults.kt | 7 + .../payloads/activities/DirectiveStart.kt | 14 + .../timeline/plan/SimulationResults.kt | 5 + .../scheduler/plan/InMemoryEditablePlan.kt | 150 ++------- ...rlinToProcedureSimulationResultsAdapter.kt | 14 +- .../plan/SchedulerToProcedurePlanAdapter.kt | 2 +- 9 files changed, 365 insertions(+), 137 deletions(-) create mode 100644 procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt create mode 100644 procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/PerishableSimulationResults.kt diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt index 7acce1ba00..a385ef88ce 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt @@ -3,6 +3,5 @@ package gov.nasa.ammos.aerie.procedural.scheduling.plan enum class DeletedAnchorStrategy { Error, Cascade, - AnchorToParent, - AnchorToPlan + ReAnchor, } diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/Edit.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/Edit.kt index 2898caad8a..1b2e324b27 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/Edit.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/Edit.kt @@ -10,10 +10,15 @@ import gov.nasa.jpl.aerie.types.ActivityDirectiveId * Currently only creating new activities is supported. */ sealed interface Edit { + fun inverse(): Edit + /** Create a new activity from a given directive. */ - data class Create(/***/ val directive: Directive): Edit + data class Create(/***/ val directive: Directive): Edit { + override fun inverse() = Delete(directive) + } /** Delete an activity, specified by directive id. */ - data class Delete(val directive: Directive): Edit { + data class Delete(/***/ val directive: Directive): Edit { + override fun inverse() = Create(directive) } } diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt new file mode 100644 index 0000000000..347e9996bd --- /dev/null +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt @@ -0,0 +1,298 @@ +package gov.nasa.ammos.aerie.procedural.scheduling.utils + +import gov.nasa.ammos.aerie.procedural.scheduling.plan.* +import gov.nasa.ammos.aerie.procedural.scheduling.simulation.SimulateOptions +import gov.nasa.ammos.aerie.procedural.timeline.collections.Directives +import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.AnyDirective +import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.Directive +import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.DirectiveStart +import gov.nasa.ammos.aerie.procedural.timeline.plan.Plan +import gov.nasa.ammos.aerie.procedural.timeline.plan.SimulationResults +import gov.nasa.jpl.aerie.types.ActivityDirectiveId +import java.lang.ref.WeakReference + +/** + * A default (but optional) driver for [EditablePlan] implementations that handles + * commits/rollbacks, staleness checking, and anchor deletion automatically. + * + * The [EditablePlan] interface requires the implementor to perform some fairly complex + * stateful operations, with a tangle of interdependent algorithmic guarantees. + * Most of those operations are standard among all implementations though, so this driver + * captures most of it in a reusable form. Just inherit from this class to make a valid + * [EditablePlan]. + * + * The subclass is still responsible for simulation and the basic context-free creation + * and deletion operations. See the *Contracts* section of each abstract method's doc comment. + */ +/* + * ## Staleness checking + * + * The editable plan instance keeps track of sim results that it has produced using weak references, and can dynamically + * update their staleness if the plan is changed after it was simulated. The process is this: + * + * 1. [InMemoryEditablePlan] has a set of weak references to simulation results objects that are currently up-to-date. + * I used weak references because if the user can't access it anymore, staleness doesn't matter and we might as well + * let it get gc'ed. + * 2. When the user gets simulation results, either through simulation or by getting the latest, it always checks for + * plan equality between the returned results and the current plan, even if we just simulated. If it is up-to-date, a + * weak ref is added to the set. + * 3. When an edit is made, the sim results in the current set are marked stale; then the set is reset to new reference + * to an empty set. + * 4. When a commit is made, the commit object takes *shared ownership* of the set. If a new simulation is run (step 2) + * the plan can still add to the set while it is still jointly owned by the commit. Then when an edit is made (step 3) + * the commit will become the sole owner of the set. + * 5. When changes are rolled back, any sim results currently in the plan's set are marked stale, the previous commit's + * sim results are marked not stale, then the plan will resume joint ownership of the previous commit's set. + * + * The joint ownership freaks me out a wee bit, but I think it's safe because the commits are only used to keep the + * previous sets from getting gc'ed in the event of a rollback. Only the plan object actually mutates the set. + */ +abstract class EasyEditablePlanDriver( + private val plan: Plan +): EditablePlan, Plan by plan { + /** + * Create a unique directive ID. + * + * *Contract:* + * - the implementor must return an ID that is distinct from any activity ID that was in the initial plan + * or that has been returned from this method before during the implementor's lifetime. + */ + protected abstract fun generateDirectiveId(): ActivityDirectiveId + + /** + * Create a directive in the plan. + * + * *Contracts*: + * - the driver will guarantee that the directive ID does not collide with any other directive currently in the plan. + * - the implementor must return the new directive in future calls to [Plan.directives], unless it is later deleted. + * - the implementor must include the directive in future input plans for simulation, unless it is later deleted. + */ + protected abstract fun createInternal(directive: Directive) + + /** + * Remove a directive from the plan, specified by ID. + */ + protected abstract fun deleteInternal(id: ActivityDirectiveId) + + /** + * Get the latest simulation results. + * + * *Contract:* + * - the implementor must return equivalent results objects if this method is called multiple times without + * updates. + * + * The implementor doesn't have to return the exact same *instance* each time if no updates are made (i.e. referential + * equality isn't required, only structural equality). + */ + protected abstract fun latestResultsInternal(): PerishableSimulationResults? + + /** + * Simulate the current plan. + * + * *Contracts:* + * - all prior creations and deletions must be reflected in the simulation run. + * - the results corresponding to this run must be returned from future calls to [latestResultsInternal] + * until the next time [simulateInternal] is called. + */ + protected abstract fun simulateInternal(options: SimulateOptions) + + /** + * Optional validation hook for new activities. The default implementation does nothing. + * + * Implementor should throw if the arguments are invalid. + */ + protected open fun validateArguments(directive: Directive) {} + + private data class Commit( + val diff: Set, + + /** + * A record of the simulation results objects that were up-to-date when the commit + * was created. + * + * This has SHARED OWNERSHIP with [EasyEditablePlanDriver]; the editable plan may add more to + * this list AFTER the commit is created. + */ + val upToDateSimResultsSet: MutableSet> + ) + + private var committedChanges = Commit(setOf(), mutableSetOf()) + var uncommittedChanges = mutableListOf() + + val totalDiff: Set + get() = committedChanges.diff + + // Jointly owned set of up-to-date simulation results. See class-level comment for algorithm explanation. + private var upToDateSimResultsSet: MutableSet> = mutableSetOf() + + override fun latestResults(): SimulationResults? { + val internalResults = latestResultsInternal() + + // kotlin checks structural equality by default, not referential equality. + val isStale = internalResults?.inputDirectives()?.toSet() != directives().toSet() + + internalResults?.setStale(isStale) + + if (!isStale) upToDateSimResultsSet.add(WeakReference(internalResults)) + return internalResults + } + + override fun create(directive: NewDirective): ActivityDirectiveId { + class ParentSearchException(id: ActivityDirectiveId, size: Int): Exception("Expected one parent activity with id $id, found $size") + val id = generateDirectiveId() + val parent = when (val s = directive.start) { + is DirectiveStart.Anchor -> { + val parentList = directives() + .filter { it.id == s.parentId } + .collect(totalBounds()) + if (parentList.size != 1) throw ParentSearchException(s.parentId, parentList.size) + parentList.first() + } + is DirectiveStart.Absolute -> null + } + val resolved = directive.resolve(id, parent) + uncommittedChanges.add(Edit.Create(resolved)) + + validateArguments(resolved) + + createInternal(resolved) + + for (simResults in upToDateSimResultsSet) { + simResults.get()?.setStale(true) + } + // create a new list instead of `.clear` because commit objects have the same reference + upToDateSimResultsSet = mutableSetOf() + + return id + } + + override fun delete(directive: Directive, strategy: DeletedAnchorStrategy) { + val directives = directives().cache() + + + val directivesToDelete: Set> + val directivesToCreate: Set> + + if (strategy == DeletedAnchorStrategy.Cascade) { + directivesToDelete = deleteCascadeRecursive(directive, directives).toSet() + directivesToCreate = mutableSetOf() + } else { + directivesToDelete = mutableSetOf(directive) + directivesToCreate = mutableSetOf() + for (d in directives) { + when (val childStart = d.start) { + is DirectiveStart.Anchor -> { + if (childStart.parentId == directive.id) { + when (strategy) { + DeletedAnchorStrategy.Error -> throw Exception("Cannot delete an activity that has anchors pointing to it without a ${DeletedAnchorStrategy::class.java.simpleName}") + DeletedAnchorStrategy.ReAnchor -> { + directivesToDelete.add(d) + val start = when (val parentStart = directive.start) { + is DirectiveStart.Absolute -> DirectiveStart.Absolute(parentStart.time + childStart.offset) + is DirectiveStart.Anchor -> DirectiveStart.Anchor( + parentStart.parentId, + parentStart.offset + childStart.offset, + parentStart.anchorPoint, + childStart.estimatedStart + ) + } + directivesToCreate.add(d.copy(start = start)) + } + else -> throw Error("internal error; unreachable") + } + } + } + else -> {} + } + } + } + + for (d in directivesToDelete) { + uncommittedChanges.add(Edit.Delete(d)) + deleteInternal(d.id) + } + for (d in directivesToCreate) { + uncommittedChanges.add(Edit.Create(d)) + createInternal(d) + } + + for (simResults in upToDateSimResultsSet) { + simResults.get()?.setStale(true) + } + + upToDateSimResultsSet = mutableSetOf() + } + + private fun deleteCascadeRecursive(directive: Directive, allDirectives: Directives): List> { + val recurse = allDirectives.collect().flatMap { d -> + when (val s = d.start) { + is DirectiveStart.Anchor -> { + if (s.parentId == directive.id) deleteCascadeRecursive(d, allDirectives) + else listOf() + } + else -> listOf() + } + } + return recurse + listOf(directive) + } + + override fun delete(id: ActivityDirectiveId, strategy: DeletedAnchorStrategy) { + val matchingDirectives = plan.directives().filter { it.id == id }.collect() + if (matchingDirectives.isEmpty()) throw Exception("attempted to delete activity by ID that does not exist: $id") + if (matchingDirectives.size > 1) throw Exception("multiple activities with ID found: $id") + + delete(matchingDirectives.first(), strategy) + } + + override fun commit() { + // Early return if there are no changes. This prevents multiple commits from sharing ownership of the set, + // because new sets are only created when edits are made. + // Probably unnecessary, but shared ownership freaks me out enough already. + if (uncommittedChanges.isEmpty()) return + + val newCommittedChanges = uncommittedChanges + val newTotalDiff = committedChanges.diff.toMutableSet() + + for (newChange in newCommittedChanges) { + val inverse = newChange.inverse() + if (newTotalDiff.contains(inverse)) { + newTotalDiff.remove(inverse) + } else { + newTotalDiff.add(newChange) + } + } + + uncommittedChanges = mutableListOf() + + // Create a commit that shares ownership of the simResults set. + committedChanges = Commit(newTotalDiff, upToDateSimResultsSet) + } + + override fun rollback(): List { + // Early return if there are no changes, to keep staleness accuracy + if (uncommittedChanges.isEmpty()) return emptyList() + + val result = uncommittedChanges + uncommittedChanges = mutableListOf() + for (edit in result) { + when (edit) { + is Edit.Create -> deleteInternal(edit.directive.id) + is Edit.Delete -> createInternal(edit.directive) + } + } + for (simResult in upToDateSimResultsSet) { + simResult.get()?.setStale(true) + } + for (simResult in committedChanges.upToDateSimResultsSet) { + simResult.get()?.setStale(false) + } + upToDateSimResultsSet = committedChanges.upToDateSimResultsSet + return result + } + + override fun simulate(options: SimulateOptions): SimulationResults { + simulateInternal(options) + return latestResults()!! + } + +} diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/PerishableSimulationResults.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/PerishableSimulationResults.kt new file mode 100644 index 0000000000..bc86085e5c --- /dev/null +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/PerishableSimulationResults.kt @@ -0,0 +1,7 @@ +package gov.nasa.ammos.aerie.procedural.scheduling.utils + +import gov.nasa.ammos.aerie.procedural.timeline.plan.SimulationResults + +interface PerishableSimulationResults: SimulationResults { + fun setStale(stale: Boolean) +} diff --git a/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/payloads/activities/DirectiveStart.kt b/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/payloads/activities/DirectiveStart.kt index c799ab36a4..ef37385f51 100644 --- a/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/payloads/activities/DirectiveStart.kt +++ b/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/payloads/activities/DirectiveStart.kt @@ -44,5 +44,19 @@ sealed interface DirectiveStart { } override fun atNewTime(time: Duration) = Anchor(parentId, offset + time - estimatedStart, anchorPoint, time) + + // Override equality so that it doesn't check `estimatedStart`. Start estimate is not part of the source of truth. + override fun equals(other: Any?) = when (other) { + is Anchor -> parentId == other.parentId && offset == other.offset && anchorPoint == other.anchorPoint + else -> false + } + + // Override hashing so that it doesn't include `estimatedStart`. + override fun hashCode(): Int { + var result = parentId.hashCode() + result = 31 * result + offset.hashCode() + result = 31 * result + anchorPoint.hashCode() + return result + } } } diff --git a/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/plan/SimulationResults.kt b/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/plan/SimulationResults.kt index 58896179b1..7d8e49f7cd 100644 --- a/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/plan/SimulationResults.kt +++ b/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/plan/SimulationResults.kt @@ -2,10 +2,12 @@ package gov.nasa.ammos.aerie.procedural.timeline.plan import gov.nasa.jpl.aerie.merlin.protocol.types.SerializedValue import gov.nasa.ammos.aerie.procedural.timeline.Interval +import gov.nasa.ammos.aerie.procedural.timeline.collections.Directives import gov.nasa.ammos.aerie.procedural.timeline.payloads.Segment import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.AnyInstance import gov.nasa.ammos.aerie.procedural.timeline.collections.Instances import gov.nasa.ammos.aerie.procedural.timeline.ops.SerialSegmentOps +import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.AnyDirective /** An interface for querying plan information and simulation results. */ interface SimulationResults { @@ -34,4 +36,7 @@ interface SimulationResults { fun instances(type: String) = instances(type, AnyInstance.deserializer()) /** Queries all activity instances, deserializing them as [AnyInstance]. **/ fun instances() = instances(null, AnyInstance.deserializer()) + + fun inputDirectives(deserializer: (SerializedValue) -> A): Directives + fun inputDirectives() = inputDirectives(AnyDirective.deserializer()) } diff --git a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt index 049d7eb808..46539a4f96 100644 --- a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt +++ b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt @@ -2,158 +2,54 @@ package gov.nasa.jpl.aerie.scheduler.plan import gov.nasa.jpl.aerie.merlin.driver.MissionModel import gov.nasa.jpl.aerie.merlin.protocol.types.Duration -import gov.nasa.ammos.aerie.procedural.scheduling.plan.Edit -import gov.nasa.ammos.aerie.procedural.scheduling.plan.EditablePlan -import gov.nasa.ammos.aerie.procedural.scheduling.plan.NewDirective import gov.nasa.ammos.aerie.procedural.scheduling.simulation.SimulateOptions +import gov.nasa.ammos.aerie.procedural.scheduling.utils.EasyEditablePlanDriver +import gov.nasa.ammos.aerie.procedural.scheduling.utils.PerishableSimulationResults import gov.nasa.jpl.aerie.scheduler.simulation.SimulationFacade import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.AnyDirective import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.Directive import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.DirectiveStart -import gov.nasa.ammos.aerie.procedural.timeline.plan.Plan -import gov.nasa.ammos.aerie.procedural.timeline.plan.SimulationResults import gov.nasa.jpl.aerie.merlin.protocol.types.DurationType import gov.nasa.jpl.aerie.scheduler.DirectiveIdGenerator import gov.nasa.jpl.aerie.scheduler.model.* import gov.nasa.jpl.aerie.types.ActivityDirectiveId -import java.lang.ref.WeakReference import java.time.Instant import kotlin.jvm.optionals.getOrNull -import gov.nasa.ammos.aerie.procedural.timeline.plan.SimulationResults as TimelineSimResults /* * An implementation of [EditablePlan] that stores the plan in memory for use in the internal scheduler. * - * ## Staleness checking - * - * The editable plan instance keeps track of sim results that it has produced using weak references, and can dynamically - * update their staleness if the plan is changed after it was simulated. The process is this: - * - * 1. [InMemoryEditablePlan] has a set of weak references to simulation results objects that are currently up-to-date. - * I used weak references because if the user can't access it anymore, staleness doesn't matter and we might as well - * let it get gc'ed. - * 2. When the user gets simulation results, either through simulation or by getting the latest, it always checks for - * plan equality between the returned results and the current plan, even if we just simulated. If it is up-to-date, a - * weak ref is added to the set. - * 3. When an edit is made, the sim results in the current set are marked stale; then the set is reset to new reference - * to an empty set. - * 4. When a commit is made, the commit object takes *shared ownership* of the set. If a new simulation is run (step 2) - * the plan can still add to the set while it is still jointly owned by the commit. Then when an edit is made (step 3) - * the commit will become the sole owner of the set. - * 5. When changes are rolled back, any sim results currently in the plan's set are marked stale, the previous commit's - * sim results are marked not stale, then the plan will resume joint ownership of the previous commit's set. - * - * The joint ownership freaks me out a wee bit, but I think it's safe because the commits are only used to keep the - * previous sets from getting gc'ed in the event of a rollback. Only the plan object actually mutates the set. + */ data class InMemoryEditablePlan( - private val missionModel: MissionModel<*>, - private var idGenerator: DirectiveIdGenerator, - private val plan: SchedulerToProcedurePlanAdapter, - private val simulationFacade: SimulationFacade, - private val lookupActivityType: (String) -> ActivityType -) : EditablePlan, Plan by plan { - - private data class Commit( - val diff: List, - - /** - * A record of the simulation results objects that were up-to-date when the commit - * was created. - * - * This has SHARED OWNERSHIP with [InMemoryEditablePlan]; the editable plan may add more to - * this list AFTER the commit is created. - */ - val upToDateSimResultsSet: MutableSet> - ) - - private var committedChanges = Commit(listOf(), mutableSetOf()) - var uncommittedChanges = mutableListOf() - private set - - val totalDiff: List - get() = committedChanges.diff - - // Jointly owned set of up-to-date simulation results. See class-level comment for algorithm explanation. - private var upToDateSimResultsSet: MutableSet> = mutableSetOf() - - override fun latestResults(): SimulationResults? { + private val missionModel: MissionModel<*>, + private var idGenerator: DirectiveIdGenerator, + private val plan: SchedulerToProcedurePlanAdapter, + private val simulationFacade: SimulationFacade, + private val lookupActivityType: (String) -> ActivityType +) : EasyEditablePlanDriver(plan) { + + override fun generateDirectiveId(): ActivityDirectiveId = idGenerator.next() + override fun latestResultsInternal(): PerishableSimulationResults? { val merlinResults = simulationFacade.latestSimulationData.getOrNull() ?: return null - - // kotlin checks structural equality by default, not referential equality. - val isStale = merlinResults.plan.activities != plan.activities - - val results = MerlinToProcedureSimulationResultsAdapter(merlinResults.driverResults, isStale, plan) - if (!isStale) upToDateSimResultsSet.add(WeakReference(results)) - return results + return MerlinToProcedureSimulationResultsAdapter(merlinResults.driverResults, plan.copy(schedulerPlan = plan.duplicate())) } - override fun create(directive: NewDirective): ActivityDirectiveId { - class ParentSearchException(id: ActivityDirectiveId, size: Int): Exception("Expected one parent activity with id $id, found $size") - val id = idGenerator.next() - val parent = when (val s = directive.start) { - is DirectiveStart.Anchor -> { - val parentList = directives() - .filter { it.id == s.parentId } - .collect(totalBounds()) - if (parentList.size != 1) throw ParentSearchException(s.parentId, parentList.size) - parentList.first() - } - is DirectiveStart.Absolute -> null - } - val resolved = directive.resolve(id, parent) - uncommittedChanges.add(Edit.Create(resolved)) - resolved.validateArguments(lookupActivityType) - plan.add(resolved.toSchedulingActivity(lookupActivityType, true)) - - for (simResults in upToDateSimResultsSet) { - simResults.get()?.stale = true - } - // create a new list instead of `.clear` because commit objects have the same reference - upToDateSimResultsSet = mutableSetOf() - - return id + override fun createInternal(directive: Directive) { + plan.add(directive.toSchedulingActivity(lookupActivityType, true)) } - override fun commit() { - // Early return if there are no changes. This prevents multiple commits from sharing ownership of the set, - // because new sets are only created when edits are made. - // Probably unnecessary, but shared ownership freaks me out enough already. - if (uncommittedChanges.isEmpty()) return - - val newCommittedChanges = uncommittedChanges - uncommittedChanges = mutableListOf() - - // Create a commit that shares ownership of the simResults set. - committedChanges = Commit(committedChanges.diff + newCommittedChanges, upToDateSimResultsSet) + override fun deleteInternal(id: ActivityDirectiveId) { + plan.remove(plan.activitiesById[id]) } - override fun rollback(): List { - // Early return if there are no changes, to keep staleness accuracy - if (uncommittedChanges.isEmpty()) return emptyList() - val result = uncommittedChanges - uncommittedChanges = mutableListOf() - for (edit in result) { - when (edit) { - is Edit.Create -> { - plan.remove(edit.directive.toSchedulingActivity(lookupActivityType, true)) - } - } - } - for (simResult in upToDateSimResultsSet) { - simResult.get()?.stale = true - } - for (simResult in committedChanges.upToDateSimResultsSet) { - simResult.get()?.stale = false - } - upToDateSimResultsSet = committedChanges.upToDateSimResultsSet - return result + override fun simulateInternal(options: SimulateOptions) { + simulationFacade.simulateWithResults(plan, options.pause.resolve(this)) } - override fun simulate(options: SimulateOptions): TimelineSimResults { - simulationFacade.simulateWithResults(plan, options.pause.resolve(this)) - return latestResults()!! + override fun validateArguments(directive: Directive) { + lookupActivityType(directive.type).specType.inputType.validateArguments(directive.inner.arguments) } // These cannot be implemented with the by keyword, @@ -163,10 +59,6 @@ data class InMemoryEditablePlan( override fun toAbsolute(rel: Duration) = plan.toAbsolute(rel) companion object { - fun Directive.validateArguments(lookupActivityType: (String) -> ActivityType) { - lookupActivityType(type).specType.inputType.validateArguments(inner.arguments) - } - @JvmStatic fun Directive.toSchedulingActivity(lookupActivityType: (String) -> ActivityType, isNew: Boolean) = SchedulingActivity( id, lookupActivityType(type), diff --git a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/MerlinToProcedureSimulationResultsAdapter.kt b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/MerlinToProcedureSimulationResultsAdapter.kt index fecd52fcce..a1add89f01 100644 --- a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/MerlinToProcedureSimulationResultsAdapter.kt +++ b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/MerlinToProcedureSimulationResultsAdapter.kt @@ -1,12 +1,12 @@ package gov.nasa.jpl.aerie.scheduler.plan +import gov.nasa.ammos.aerie.procedural.scheduling.utils.PerishableSimulationResults import gov.nasa.ammos.aerie.procedural.timeline.Interval import gov.nasa.ammos.aerie.procedural.timeline.collections.Instances import gov.nasa.ammos.aerie.procedural.timeline.ops.SerialSegmentOps import gov.nasa.ammos.aerie.procedural.timeline.payloads.Segment import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.Instance import gov.nasa.ammos.aerie.procedural.timeline.plan.Plan -import gov.nasa.ammos.aerie.procedural.timeline.plan.SimulationResults import gov.nasa.ammos.aerie.procedural.timeline.util.duration.rangeTo import gov.nasa.jpl.aerie.merlin.driver.engine.ProfileSegment import gov.nasa.jpl.aerie.merlin.protocol.types.Duration @@ -18,9 +18,15 @@ import kotlin.jvm.optionals.getOrNull class MerlinToProcedureSimulationResultsAdapter( private val results: gov.nasa.jpl.aerie.merlin.driver.SimulationResults, - var stale: Boolean, + + /** A copy of the plan that will not be mutated after creation. */ private val plan: Plan -): SimulationResults { +): PerishableSimulationResults { + + private var stale = false; + override fun setStale(stale: Boolean) { + this.stale = stale; + } override fun isStale() = stale @@ -119,4 +125,6 @@ class MerlinToProcedureSimulationResultsAdapter( } return Instances(instances) } + + override fun inputDirectives(deserializer: (SerializedValue) -> A) = plan.directives(null, deserializer) } diff --git a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerToProcedurePlanAdapter.kt b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerToProcedurePlanAdapter.kt index c23b5cbdbc..4a53dea205 100644 --- a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerToProcedurePlanAdapter.kt +++ b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerToProcedurePlanAdapter.kt @@ -48,7 +48,7 @@ data class SchedulerToProcedurePlanAdapter( result.add( Directive( deserializer(SerializedValue.of(activity.arguments)), - "Name unavailable", + activity.name, activity.id, activity.type.name, if (activity.anchorId == null) DirectiveStart.Absolute(activity.startOffset) From f7caacfcfa201dbff59feda1d84eabbcf8c09569 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 21 Nov 2024 10:45:00 -0800 Subject: [PATCH 03/22] Add editable plan tests --- .../NotImplementedSimulationResults.kt | 2 + ...alenessTest.java => EditablePlanTest.java} | 78 ++++++++++++++++--- 2 files changed, 68 insertions(+), 12 deletions(-) rename scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/{EditablePlanStalenessTest.java => EditablePlanTest.java} (69%) diff --git a/procedural/constraints/src/test/kotlin/gov/nasa/ammos/aerie/procedural/constraints/NotImplementedSimulationResults.kt b/procedural/constraints/src/test/kotlin/gov/nasa/ammos/aerie/procedural/constraints/NotImplementedSimulationResults.kt index 4b783efd21..84b795c095 100644 --- a/procedural/constraints/src/test/kotlin/gov/nasa/ammos/aerie/procedural/constraints/NotImplementedSimulationResults.kt +++ b/procedural/constraints/src/test/kotlin/gov/nasa/ammos/aerie/procedural/constraints/NotImplementedSimulationResults.kt @@ -1,6 +1,7 @@ package gov.nasa.ammos.aerie.procedural.constraints import gov.nasa.ammos.aerie.procedural.timeline.Interval +import gov.nasa.ammos.aerie.procedural.timeline.collections.Directives import gov.nasa.ammos.aerie.procedural.timeline.collections.Instances import gov.nasa.ammos.aerie.procedural.timeline.ops.SerialSegmentOps import gov.nasa.ammos.aerie.procedural.timeline.payloads.Segment @@ -15,4 +16,5 @@ open class NotImplementedSimulationResults: SimulationResults { deserializer: (List>) -> TL ): TL = TODO() override fun instances(type: String?, deserializer: (SerializedValue) -> A): Instances = TODO() + override fun inputDirectives(deserializer: (SerializedValue) -> A) = TODO() } diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanStalenessTest.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanTest.java similarity index 69% rename from scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanStalenessTest.java rename to scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanTest.java index 200368d2ee..dbd7e4f991 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanStalenessTest.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanTest.java @@ -12,17 +12,20 @@ import gov.nasa.jpl.aerie.scheduler.plan.SchedulerToProcedurePlanAdapter; import gov.nasa.jpl.aerie.scheduler.simulation.CheckpointSimulationFacade; import gov.nasa.jpl.aerie.scheduler.simulation.SimulationFacade; +import gov.nasa.jpl.aerie.types.ActivityDirectiveId; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.time.Instant; import java.util.Map; +import java.util.Set; +import java.util.function.Supplier; +import java.util.stream.Collectors; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; -public class EditablePlanStalenessTest { +public class EditablePlanTest { MissionModel missionModel; Problem problem; @@ -63,6 +66,47 @@ public void tearDown() { plan = null; } + @Test + public void activityCreation() { + plan.create( + "BiteBanana", + new DirectiveStart.Absolute(Duration.MINUTE), + Map.of("biteSize", SerializedValue.of(1)) + ); + + assertEquals(1, plan.directives().collect().size()); + assertEquals(Map.of("biteSize", SerializedValue.of(1)), plan.directives().collect().getFirst().inner.arguments); + } + + @Test + public void activityDeletion() { + final var id1 = plan.create( + "BiteBanana", + new DirectiveStart.Absolute(Duration.MINUTE), + Map.of("biteSize", SerializedValue.of(1)) + ); + final var id2 = plan.create( + "GrowBanana", + new DirectiveStart.Absolute(Duration.HOUR), + Map.of("growingDuration", SerializedValue.of(1), "quantity", SerializedValue.of(1)) + ); + + final Supplier> idSet = + () -> plan.directives().collect().stream().map($ -> $.id).collect(Collectors.toSet()); + + plan.commit(); + + assertEquals(Set.of(id1, id2), idSet.get()); + + plan.delete(id1); + + assertEquals(Set.of(id2), idSet.get()); + + plan.delete(id2); + + assertEquals(Set.of(), idSet.get()); + } + @Test public void simResultMarkedStale() { plan.create( @@ -87,6 +131,23 @@ public void simResultMarkedStale() { assertTrue(simResults.isStale()); } + @Test + public void simResultMarkedStaleAfterDelete() { + final var id = plan.create( + "BiteBanana", + new DirectiveStart.Absolute(Duration.MINUTE), + Map.of("biteSize", SerializedValue.of(1)) + ); + + final var simResults = plan.simulate(); + + assertFalse(simResults.isStale()); + + plan.delete(id); + + assertTrue(simResults.isStale()); + } + @Test public void simResultMarkedNotStaleAfterRollback_CommitThenSimulate() { plan.create( @@ -118,7 +179,7 @@ public void simResultMarkedNotStaleAfterRollback_CommitThenSimulate() { @Test public void simResultMarkedNotStaleAfterRollback_SimulateThenCommit() { - plan.create( + final var id = plan.create( "BiteBanana", new DirectiveStart.Absolute(Duration.MINUTE), Map.of("biteSize", SerializedValue.of(1)) @@ -129,14 +190,7 @@ public void simResultMarkedNotStaleAfterRollback_SimulateThenCommit() { assertFalse(simResults.isStale()); - plan.create( - "GrowBanana", - new DirectiveStart.Absolute(Duration.HOUR), - Map.of( - "growingDuration", SerializedValue.of(10), - "quantity", SerializedValue.of(1) - ) - ); + plan.delete(id); assertTrue(simResults.isStale()); From bc6c0b1b59f4950ea988ac42366070a433865597 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 21 Nov 2024 10:54:44 -0800 Subject: [PATCH 04/22] Remove unnecessary overrides --- .../nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt | 7 ------- 1 file changed, 7 deletions(-) diff --git a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt index 46539a4f96..82cdbf5831 100644 --- a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt +++ b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt @@ -43,7 +43,6 @@ data class InMemoryEditablePlan( plan.remove(plan.activitiesById[id]) } - override fun simulateInternal(options: SimulateOptions) { simulationFacade.simulateWithResults(plan, options.pause.resolve(this)) } @@ -52,12 +51,6 @@ data class InMemoryEditablePlan( lookupActivityType(directive.type).specType.inputType.validateArguments(directive.inner.arguments) } - // These cannot be implemented with the by keyword, - // because directives() below needs a custom implementation. - override fun totalBounds() = plan.totalBounds() - override fun toRelative(abs: Instant) = plan.toRelative(abs) - override fun toAbsolute(rel: Duration) = plan.toAbsolute(rel) - companion object { @JvmStatic fun Directive.toSchedulingActivity(lookupActivityType: (String) -> ActivityType, isNew: Boolean) = SchedulingActivity( id, From 46656c765799d95a1a1e755131449f7f9e4d4ab0 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 21 Nov 2024 13:14:51 -0800 Subject: [PATCH 05/22] Send modifications and deletions to database --- .../gov/nasa/jpl/aerie/e2e/types/Plan.java | 4 +- .../jpl/aerie/scheduler/goals/Procedure.java | 2 - .../GraphQLMerlinDatabaseService.java | 72 +++++++++++++++++-- 3 files changed, 70 insertions(+), 8 deletions(-) diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/Plan.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/Plan.java index 4fb0d3ee80..d050a14496 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/Plan.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/Plan.java @@ -20,7 +20,9 @@ public static ActivityDirective fromJSON(JsonObject json){ json.getString("type"), json.getString("startOffset"), json.getJsonObject("arguments"), - json.getString("name") + json.getString("name"), + json.isNull("anchorId") ? null : json.getInt("anchorId"), + json.getBoolean("anchoredToStart") ); } } diff --git a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java index 306aff1468..7578d85a7c 100644 --- a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java +++ b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java @@ -80,8 +80,6 @@ public void run( for (final var edit : editablePlan.getTotalDiff()) { if (edit instanceof Edit.Create c) { newActivities.add(toSchedulingActivity(c.getDirective(), lookupActivityType::apply, true)); - } else { - throw new IllegalStateException("Unexpected value: " + edit); } } diff --git a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java index 347b013262..5c31c7eee9 100644 --- a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java +++ b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java @@ -421,6 +421,8 @@ public Map updatePlanActivityDirective final var ids = new HashMap(); //creation are done in batch as that's what the scheduler does the most final var toAdd = new ArrayList(); + final var toDelete = new ArrayList(); + final var toModify = new ArrayList(); for (final var activity : plan.getActivities()) { if(activity.getParentActivity().isPresent()) continue; // Skip generated activities if (!activity.isNew()) { @@ -440,8 +442,7 @@ public Map updatePlanActivityDirective activity.anchoredToStart() ); if (!activityDirectiveFromSchedulingDirective.equals(actFromInitialPlan.get())) { - throw new MerlinServiceException("The scheduler should not be updating activity instances"); - //updateActivityDirective(planId, schedulerActIntoMerlinAct, activityDirectiveId, activityToGoalId.get(activity)); + toModify.add(activity); } ids.put(activity.id(), activity.id()); } else { @@ -452,13 +453,14 @@ public Map updatePlanActivityDirective final var actsFromNewPlan = plan.getActivitiesById(); for (final var idInInitialPlan : initialPlan.getActivitiesById().keySet()) { if (!actsFromNewPlan.containsKey(idInInitialPlan)) { - throw new MerlinServiceException("The scheduler should not be deleting activity instances"); - //deleteActivityDirective(idActInInitialPlan.getValue()); + toDelete.add(idInInitialPlan); } } //Create ids.putAll(createActivityDirectives(planId, toAdd, activityToGoalId, schedulerModel)); + modifyActivityDirectives(planId, toModify); + deleteActivityDirectives(planId, toDelete); return ids; } @@ -554,7 +556,7 @@ public Map createAllPlanActivityDirect return createActivityDirectives(planId, plan.getActivitiesByTime(), activityToGoalId, schedulerModel); } - public Map createActivityDirectives( + private Map createActivityDirectives( final PlanId planId, final List orderedActivities, final Map activityToGoalId, @@ -636,6 +638,66 @@ mutation createAllPlanActivityDirectives($activities: [activity_directive_insert return activityToDirectiveId; } + private void modifyActivityDirectives( + final PlanId planId, + final List activities + ) + throws IOException, NoSuchPlanException, MerlinServiceException + { + if (activities.isEmpty()) return; + ensurePlanExists(planId); + final var request = new StringBuilder(); + request.append("mutation updatePlanActivityDirectives("); + request.append(String.join( + ",", + activities.stream().map($ -> "$activity_%d: activity_directive_set_input!".formatted($.id().id())).toList() + )); + request.append(") {"); + final var arguments = Json.createObjectBuilder(); + for (final var act : activities) { + final var id = act.id().id(); + request.append(""" + update_%d: update_activity_directive_by_pk(pk_columns: {id: %d, plan_id: %d}, _set: $activity_%d) { + affected_rows + } + """.formatted(id, id, planId.id(), id)); + + final var activityObject = Json + .createObjectBuilder() + .add("start_offset", act.startOffset().toString()) + .add("anchored_to_start", act.anchoredToStart()) + .add("name", act.name()); + + final var insertionObjectArguments = Json.createObjectBuilder(); + for (final var arg : act.arguments().entrySet()) { + insertionObjectArguments.add(arg.getKey(), serializedValueP.unparse(arg.getValue())); + } + activityObject.add("arguments", insertionObjectArguments.build()); + arguments.add("activity_%d".formatted(id), activityObject); + } + postRequest(request.toString(), arguments.build()).orElseThrow(() -> new NoSuchPlanException(planId)); + } + + private void deleteActivityDirectives( + final PlanId planId, + final List ids + ) + throws IOException, NoSuchPlanException, MerlinServiceException + { + if (ids.isEmpty()) return; + ensurePlanExists(planId); + final var request = new StringBuilder(); + request.append("mutation deletePlanActivityDirectives {"); + for (final var id : ids) { + request.append(""" + delete_%d: delete_activity_directive_by_pk(id: %d, plan_id: %d) {affected_rows} + """.formatted(id.id(), id.id(), planId.id())); + } + request.append("}"); + postRequest(request.toString()).orElseThrow(() -> new NoSuchPlanException(planId)); + } + + @Override public MerlinDatabaseService.MissionModelTypes getMissionModelTypes(final PlanId planId) throws IOException, MerlinServiceException From 500d59e4af4f38450b7a1a4e80ad560b7a93a2a7 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 21 Nov 2024 13:51:42 -0800 Subject: [PATCH 06/22] Validate that new directives are within the plan bounds --- .../scheduling/utils/EasyEditablePlanDriver.kt | 17 +++++++++++++---- .../scheduler/plan/InMemoryEditablePlan.kt | 3 ++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt index 347e9996bd..a7d897c5a9 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt @@ -97,11 +97,20 @@ abstract class EasyEditablePlanDriver( protected abstract fun simulateInternal(options: SimulateOptions) /** - * Optional validation hook for new activities. The default implementation does nothing. + * Optional validation hook for new activities. * - * Implementor should throw if the arguments are invalid. + * The default implementation checks if the activity is within the bounds of the plan. The implementor can + * add additional checks by overriding this method and calling `super.validate(directive)`. Implementor + * should throw if the directive is invalid. */ - protected open fun validateArguments(directive: Directive) {} + protected open fun validate(directive: Directive) { + if (directive.startTime > duration()) { + throw Exception("New activity with id ${directive.id.id()} would start after the end of the plan") + } + if (directive.start is DirectiveStart.Absolute && directive.startTime.isNegative) { + throw Exception("New activity with id ${directive.id.id()} would start before the beginning of the plan") + } + } private data class Commit( val diff: Set, @@ -153,7 +162,7 @@ abstract class EasyEditablePlanDriver( val resolved = directive.resolve(id, parent) uncommittedChanges.add(Edit.Create(resolved)) - validateArguments(resolved) + validate(resolved) createInternal(resolved) diff --git a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt index 82cdbf5831..49ca1dbaeb 100644 --- a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt +++ b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt @@ -47,7 +47,8 @@ data class InMemoryEditablePlan( simulationFacade.simulateWithResults(plan, options.pause.resolve(this)) } - override fun validateArguments(directive: Directive) { + override fun validate(directive: Directive) { + super.validate(directive) lookupActivityType(directive.type).specType.inputType.validateArguments(directive.inner.arguments) } From 55f64edc7ee119b9f0239b2b9718f71ba73cea1d Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 21 Nov 2024 18:04:25 -0800 Subject: [PATCH 07/22] Activity deletion e2e tests --- .../procedures/ActivityDeletionGoal.java | 47 ++++ .../e2e/procedural/scheduling/BasicTests.java | 8 + .../procedural/scheduling/DeletionTests.java | 215 ++++++++++++++++++ .../scheduling/ProceduralSchedulingSetup.java | 8 +- .../gov/nasa/jpl/aerie/e2e/types/Plan.java | 11 +- .../gov/nasa/jpl/aerie/e2e/utils/GQL.java | 2 + 6 files changed, 283 insertions(+), 8 deletions(-) create mode 100644 e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java create mode 100644 e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DeletionTests.java diff --git a/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java new file mode 100644 index 0000000000..874e6a5bc1 --- /dev/null +++ b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java @@ -0,0 +1,47 @@ +package gov.nasa.jpl.aerie.e2e.procedural.scheduling.procedures; + +import gov.nasa.ammos.aerie.procedural.scheduling.Goal; +import gov.nasa.ammos.aerie.procedural.scheduling.annotations.SchedulingProcedure; +import gov.nasa.ammos.aerie.procedural.scheduling.plan.DeletedAnchorStrategy; +import gov.nasa.ammos.aerie.procedural.scheduling.plan.EditablePlan; +import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.DirectiveStart; +import gov.nasa.jpl.aerie.merlin.protocol.types.Duration; +import gov.nasa.jpl.aerie.merlin.protocol.types.SerializedValue; +import gov.nasa.jpl.aerie.types.ActivityDirectiveId; +import org.jetbrains.annotations.NotNull; + +import java.util.Map; +import java.util.Optional; + +/** + * Creates three activities in a chain of anchors, then deletes one. + */ +@SchedulingProcedure +public record ActivityDeletionGoal(int whichToDelete, DeletedAnchorStrategy anchorStrategy) implements Goal { + @Override + public void run(@NotNull final EditablePlan plan) { + final var ids = new ActivityDirectiveId[3]; + + ids[0] = plan.create( + "BiteBanana", + new DirectiveStart.Absolute(Duration.HOUR), + Map.of("biteSize", SerializedValue.of(0)) + ); + ids[1] = plan.create( + "BiteBanana", + new DirectiveStart.Anchor(ids[0], Duration.HOUR, DirectiveStart.Anchor.AnchorPoint.End), + Map.of("biteSize", SerializedValue.of(1)) + ); + ids[2] = plan.create( + "BiteBanana", + new DirectiveStart.Anchor(ids[1], Duration.HOUR, DirectiveStart.Anchor.AnchorPoint.Start), + Map.of("biteSize", SerializedValue.of(2)) + ); + + if (whichToDelete >= 0) { + plan.delete(ids[whichToDelete], anchorStrategy); + } + + plan.commit(); + } +} diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/BasicTests.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/BasicTests.java index 71fb6aae76..6a8b8d7f04 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/BasicTests.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/BasicTests.java @@ -134,6 +134,14 @@ void executeEDSLAndProcedure() throws IOException { final var args = Json.createObjectBuilder().add("quantity", 4).build(); hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + final String recurrenceGoalDefinition = + """ + export default function myGoal() { + return Goal.ActivityRecurrenceGoal({ + activityTemplate: ActivityTemplates.PeelBanana({peelDirection: 'fromStem'}), + interval: Temporal.Duration.from({hours:1}) + })}"""; + hasura.createSchedulingSpecGoal( "Recurrence Scheduling Test Goal", recurrenceGoalDefinition, diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DeletionTests.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DeletionTests.java new file mode 100644 index 0000000000..5e38b57b19 --- /dev/null +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DeletionTests.java @@ -0,0 +1,215 @@ +package gov.nasa.jpl.aerie.e2e.procedural.scheduling; + +import gov.nasa.jpl.aerie.e2e.types.GoalInvocationId; +import gov.nasa.jpl.aerie.e2e.utils.GatewayRequests; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import javax.json.Json; +import java.io.IOException; +import java.util.Objects; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class DeletionTests extends ProceduralSchedulingSetup { + private GoalInvocationId procedureId; + + @BeforeEach + void localBeforeEach() throws IOException { + try (final var gateway = new GatewayRequests(playwright)) { + int procedureJarId = gateway.uploadJarFile("build/libs/ActivityDeletionGoal.jar"); + // Add Scheduling Procedure + procedureId = hasura.createSchedulingSpecProcedure( + "Test Scheduling Procedure", + procedureJarId, + specId, + 0 + ); + } + } + + @AfterEach + void localAfterEach() throws IOException { + hasura.deleteSchedulingGoal(procedureId.goalId()); + } + + @Test + void createsThreeActivities() throws IOException { + final var args = Json + .createObjectBuilder() + .add("whichToDelete", -1) + .add("anchorStrategy", "Error") + .build(); + + hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + + hasura.awaitScheduling(specId); + + final var plan = hasura.getPlan(planId); + final var activities = plan.activityDirectives(); + + assertEquals(3, activities.size()); + + final AtomicReference id1 = new AtomicReference<>(); + final AtomicReference id2 = new AtomicReference<>(); + assertTrue(activities.stream().anyMatch( + it -> { + final var result = Objects.equals(it.type(), "BiteBanana") && Objects.equals(it.anchorId(), null); + if (result) id1.set(it.id()); + return result; + } + )); + + assertTrue(activities.stream().anyMatch( + it -> { + final var result = Objects.equals(it.type(), "BiteBanana") && Objects.equals(it.anchorId(), id1.get()); + if (result) id2.set(it.id()); + return result; + } + )); + + assertTrue(activities.stream().anyMatch( + it -> Objects.equals(it.type(), "BiteBanana") && Objects.equals(it.anchorId(), id2.get()) + )); + } + + @Test + void deletesLast() throws IOException { + final var args = Json + .createObjectBuilder() + .add("whichToDelete", 2) + .add("anchorStrategy", "Error") + .build(); + + hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + + hasura.awaitScheduling(specId); + + final var plan = hasura.getPlan(planId); + final var activities = plan.activityDirectives(); + + assertEquals(2, activities.size()); + + final AtomicReference id1 = new AtomicReference<>(); + assertTrue(activities.stream().anyMatch( + it -> { + final var result = Objects.equals(it.type(), "BiteBanana") && Objects.equals(it.anchorId(), null); + if (result) id1.set(it.id()); + return result; + } + )); + + assertTrue(activities.stream().anyMatch( + it -> Objects.equals(it.type(), "BiteBanana") && Objects.equals(it.anchorId(), id1.get()) + )); + } + + @Test + void deletesMiddleCascade() throws IOException { + final var args = Json + .createObjectBuilder() + .add("whichToDelete", 1) + .add("anchorStrategy", "Cascade") + .build(); + + hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + + hasura.awaitScheduling(specId); + + final var plan = hasura.getPlan(planId); + final var activities = plan.activityDirectives(); + + assertEquals(1, activities.size()); + + assertTrue(activities.stream().anyMatch( + it -> Objects.equals(it.type(), "BiteBanana") && Objects.equals(it.anchorId(), null) + )); + } + + @Test + void deletesMiddleAnchorToParent() throws IOException { + final var args = Json + .createObjectBuilder() + .add("whichToDelete", 1) + .add("anchorStrategy", "ReAnchor") + .build(); + + hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + + hasura.awaitScheduling(specId); + + final var plan = hasura.getPlan(planId); + final var activities = plan.activityDirectives(); + + assertEquals(2, activities.size()); + + final AtomicReference id1 = new AtomicReference<>(); + assertTrue(activities.stream().anyMatch( + it -> { + final var result = Objects.equals(it.type(), "BiteBanana") && Objects.equals(it.anchorId(), null); + if (result) id1.set(it.id()); + return result; + } + )); + + assertTrue(activities.stream().anyMatch( + it -> Objects.equals(it.type(), "BiteBanana") + && Objects.equals(it.anchorId(), id1.get()) + && Objects.equals(it.startOffset(), "02:00:00") + )); + } + + @Test + void deletesFirstCascade() throws IOException { + final var args = Json + .createObjectBuilder() + .add("whichToDelete", 0) + .add("anchorStrategy", "Cascade") + .build(); + + hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + + hasura.awaitScheduling(specId); + + final var plan = hasura.getPlan(planId); + final var activities = plan.activityDirectives(); + + assertEquals(0, activities.size()); + } + + @Test + void deletesFirstReAnchorToPlan() throws IOException { + final var args = Json + .createObjectBuilder() + .add("whichToDelete", 0) + .add("anchorStrategy", "ReAnchor") + .build(); + + hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + + hasura.awaitScheduling(specId); + + final var plan = hasura.getPlan(planId); + final var activities = plan.activityDirectives(); + + assertEquals(2, activities.size()); + + final AtomicReference id2 = new AtomicReference<>(); + assertTrue(activities.stream().anyMatch( + it -> { + final var result = Objects.equals(it.type(), "BiteBanana") + && Objects.equals(it.anchorId(), null) + && Objects.equals(it.startOffset(), "02:00:00"); + if (result) id2.set(it.id()); + return result; + } + )); + + assertTrue(activities.stream().anyMatch( + it -> Objects.equals(it.type(), "BiteBanana") && Objects.equals(it.anchorId(), id2.get()) + )); + } +} diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/ProceduralSchedulingSetup.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/ProceduralSchedulingSetup.java index c1ef5853b1..5ab2db69d8 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/ProceduralSchedulingSetup.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/ProceduralSchedulingSetup.java @@ -26,13 +26,7 @@ public abstract class ProceduralSchedulingSetup { // Cross-Test Constants protected final String planStartTimestamp = "2023-01-01T00:00:00+00:00"; - protected final String recurrenceGoalDefinition = - """ - export default function myGoal() { - return Goal.ActivityRecurrenceGoal({ - activityTemplate: ActivityTemplates.PeelBanana({peelDirection: 'fromStem'}), - interval: Temporal.Duration.from({hours:1}) - })}"""; + @BeforeAll void beforeAll() { diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/Plan.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/Plan.java index d050a14496..5fb5d74501 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/Plan.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/types/Plan.java @@ -12,7 +12,16 @@ public record Plan( int revision, List activityDirectives ) { - public record ActivityDirective(int id, int planId, String type, String startOffset, JsonObject arguments, String name) { + public record ActivityDirective( + int id, + int planId, + String type, + String startOffset, + JsonObject arguments, + String name, + Integer anchorId, + boolean anchoredToStart + ) { public static ActivityDirective fromJSON(JsonObject json){ return new ActivityDirective( json.getInt("id"), diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/GQL.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/GQL.java index 8d07e3fa95..7c8c1ec9ce 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/GQL.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/GQL.java @@ -398,6 +398,8 @@ query GetPlan($id: Int!) { startOffset: start_offset type name + anchorId: anchor_id + anchoredToStart: anchored_to_start } constraint_specification { constraint_id From c55b03860f2367f810c6bede02d6422c5288f4df Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Fri, 22 Nov 2024 15:57:53 -0800 Subject: [PATCH 08/22] Add doc comments --- .../scheduling/plan/DeletedAnchorStrategy.kt | 24 +++++++++++++++++-- .../aerie/procedural/scheduling/plan/Edit.kt | 7 +++++- .../utils/EasyEditablePlanDriver.kt | 7 +++++- .../utils/PerishableSimulationResults.kt | 3 ++- .../timeline/plan/SimulationResults.kt | 2 ++ .../jpl/aerie/scheduler/goals/Procedure.java | 2 +- 6 files changed, 39 insertions(+), 6 deletions(-) diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt index a385ef88ce..173ed876dd 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt @@ -1,7 +1,27 @@ package gov.nasa.ammos.aerie.procedural.scheduling.plan +/** + * How to handle directives anchored to a deleted activity. + * + * If you intend to delete an activity that you believe has nothing anchored to it, + * using [Error] is recommended. This is the default. + */ enum class DeletedAnchorStrategy { - Error, - Cascade, + /** Throw an error. */ Error, + /** Recursively delete everything in the anchor chain. */ Cascade, + + /** + * Attempt to delete the activity in-place without changing the start times + * of any activities anchored to it. + * + * Consider the anchor chain `A <- B <- C`, where `A` starts at an absolute time and + * `B` and `C` are anchored. + * - If `A` is deleted with [ReAnchor], `B` will be set to start at the absolute time `A.startTime + B.offset`. + * `C` will be unchanged. + * - If `B` is deleted with [ReAnchor], `C` will be anchored to `A` with a new offset equal to `B.offset + C.offset`. + * + * If an activity is anchored to the end of the deleted activity, the delete activity's duration is assumed to be 0, + * which may change the ultimate start time of the anchored activity. + */ ReAnchor, } diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/Edit.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/Edit.kt index 1b2e324b27..9e1fac6fdf 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/Edit.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/Edit.kt @@ -7,9 +7,14 @@ import gov.nasa.jpl.aerie.types.ActivityDirectiveId /** * Edits that can be made to the plan. * - * Currently only creating new activities is supported. + * All edits are invertible. */ sealed interface Edit { + /** + * Returns the reverse operation. + * + * If both `E` and `E.inverse()` are applied, the plan is unchanged. + */ fun inverse(): Edit /** Create a new activity from a given directive. */ diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt index a7d897c5a9..fb03bf44a9 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt @@ -126,8 +126,13 @@ abstract class EasyEditablePlanDriver( ) private var committedChanges = Commit(setOf(), mutableSetOf()) - var uncommittedChanges = mutableListOf() + private var uncommittedChanges = mutableListOf() + /** Whether there are uncommitted changes. */ + val isDirty + get() = uncommittedChanges.isNotEmpty() + + /** The total reduced set of changes made to the plan. */ val totalDiff: Set get() = committedChanges.diff diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/PerishableSimulationResults.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/PerishableSimulationResults.kt index bc86085e5c..b60425270c 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/PerishableSimulationResults.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/PerishableSimulationResults.kt @@ -2,6 +2,7 @@ package gov.nasa.ammos.aerie.procedural.scheduling.utils import gov.nasa.ammos.aerie.procedural.timeline.plan.SimulationResults +/** Simulation results whose staleness can be changed after creation. */ interface PerishableSimulationResults: SimulationResults { - fun setStale(stale: Boolean) + /***/ fun setStale(stale: Boolean) } diff --git a/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/plan/SimulationResults.kt b/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/plan/SimulationResults.kt index 7d8e49f7cd..fdf86f5c7d 100644 --- a/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/plan/SimulationResults.kt +++ b/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/plan/SimulationResults.kt @@ -37,6 +37,8 @@ interface SimulationResults { /** Queries all activity instances, deserializing them as [AnyInstance]. **/ fun instances() = instances(null, AnyInstance.deserializer()) + /** The input directives that were used for this simulation. */ fun inputDirectives(deserializer: (SerializedValue) -> A): Directives + /** The input directives that were used for this simulation, deserialized as [AnyDirective]. */ fun inputDirectives() = inputDirectives(AnyDirective.deserializer()) } diff --git a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java index 7578d85a7c..91e68cffe6 100644 --- a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java +++ b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java @@ -74,7 +74,7 @@ public void run( procedureMapper.deserialize(SerializedValue.of(this.args)).run(editablePlan); - if (!editablePlan.getUncommittedChanges().isEmpty()) { + if (editablePlan.isDirty()) { throw new IllegalStateException("procedural goal %s had changes that were not committed or rolled back".formatted(jarPath.getFileName())); } for (final var edit : editablePlan.getTotalDiff()) { From 07dfaa5abc012dc1ad64b92366bd5661b2073818 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Fri, 22 Nov 2024 15:58:22 -0800 Subject: [PATCH 09/22] Test simulation input directives invariance --- .../activities/GrowBananaActivity.java | 2 +- .../jpl/aerie/scheduler/EditablePlanTest.java | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/GrowBananaActivity.java b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/GrowBananaActivity.java index 57489ccabb..5e6e2c8311 100644 --- a/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/GrowBananaActivity.java +++ b/examples/banananation/src/main/java/gov/nasa/jpl/aerie/banananation/activities/GrowBananaActivity.java @@ -40,7 +40,7 @@ public boolean validateGrowingDuration() { @EffectModel @ControllableDuration(parameterName = "growingDuration") public void run(final Mission mission) { - final var rate = this.quantity() / (double) this.growingDuration().in(Duration.SECONDS); + final var rate = this.quantity() / (double) this.growingDuration().ratioOver(Duration.SECOND); mission.fruit.rate.add(rate); delay(this.growingDuration()); mission.fruit.rate.add(-rate); diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanTest.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanTest.java index dbd7e4f991..d15ab6b1da 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanTest.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanTest.java @@ -198,4 +198,35 @@ public void simResultMarkedNotStaleAfterRollback_SimulateThenCommit() { assertFalse(simResults.isStale()); } + + @Test + void simulationInputDirectivesDontChange() { + plan.create( + "BiteBanana", + new DirectiveStart.Absolute(Duration.MINUTE), + Map.of("biteSize", SerializedValue.of(1)) + ); + + final var expectedDirectives = plan.directives(); + final var simResults = plan.simulate(); + + plan.create( + "GrowBanana", + new DirectiveStart.Absolute(Duration.HOUR), + Map.of( + "growingDuration", SerializedValue.of(10000), + "quantity", SerializedValue.of(1) + ) + ); + + assertIterableEquals(expectedDirectives, simResults.inputDirectives()); + assertNotEquals(plan.directives(), simResults.inputDirectives()); + + plan.commit(); + + assertIterableEquals(expectedDirectives, simResults.inputDirectives()); + assertNotEquals(plan.directives(), simResults.inputDirectives()); + + assertIterableEquals(plan.directives(), plan.simulate().inputDirectives()); + } } From 3b65b9c9f56a9f34c0476d78927a78a9ddfa9de2 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 23 Jan 2025 13:42:18 -0800 Subject: [PATCH 10/22] Resolve PR comments --- .../procedural/scheduling/procedures/ActivityDeletionGoal.java | 2 +- .../e2e/procedural/scheduling/ProceduralSchedulingSetup.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java index 874e6a5bc1..f9242bf14e 100644 --- a/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java +++ b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java @@ -11,10 +11,10 @@ import org.jetbrains.annotations.NotNull; import java.util.Map; -import java.util.Optional; /** * Creates three activities in a chain of anchors, then deletes one. + * If `whichToDelete` is negative, this leaves all three activities. */ @SchedulingProcedure public record ActivityDeletionGoal(int whichToDelete, DeletedAnchorStrategy anchorStrategy) implements Goal { diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/ProceduralSchedulingSetup.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/ProceduralSchedulingSetup.java index 5ab2db69d8..341b445299 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/ProceduralSchedulingSetup.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/ProceduralSchedulingSetup.java @@ -1,7 +1,6 @@ package gov.nasa.jpl.aerie.e2e.procedural.scheduling; import com.microsoft.playwright.Playwright; -import gov.nasa.jpl.aerie.e2e.types.GoalInvocationId; import gov.nasa.jpl.aerie.e2e.utils.GatewayRequests; import gov.nasa.jpl.aerie.e2e.utils.HasuraRequests; import org.junit.jupiter.api.AfterAll; From 664e38069f2b0d6fed5426242b81eb353f47121f Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 23 Jan 2025 15:25:44 -0800 Subject: [PATCH 11/22] Refactor editable plan driver to use composition --- .../scheduling/plan/DeletedAnchorStrategy.kt | 6 +- ...Driver.kt => DefaultEditablePlanDriver.kt} | 159 +++++++++--------- .../jpl/aerie/scheduler/goals/Procedure.java | 9 +- ...blePlan.kt => SchedulerPlanEditAdapter.kt} | 17 +- .../jpl/aerie/scheduler/EditablePlanTest.java | 6 +- 5 files changed, 103 insertions(+), 94 deletions(-) rename procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/{EasyEditablePlanDriver.kt => DefaultEditablePlanDriver.kt} (72%) rename scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/{InMemoryEditablePlan.kt => SchedulerPlanEditAdapter.kt} (85%) diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt index 173ed876dd..23e9da87ac 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/plan/DeletedAnchorStrategy.kt @@ -16,12 +16,12 @@ enum class DeletedAnchorStrategy { * * Consider the anchor chain `A <- B <- C`, where `A` starts at an absolute time and * `B` and `C` are anchored. - * - If `A` is deleted with [ReAnchor], `B` will be set to start at the absolute time `A.startTime + B.offset`. + * - If `A` is deleted with [PreserveTree], `B` will be set to start at the absolute time `A.startTime + B.offset`. * `C` will be unchanged. - * - If `B` is deleted with [ReAnchor], `C` will be anchored to `A` with a new offset equal to `B.offset + C.offset`. + * - If `B` is deleted with [PreserveTree], `C` will be anchored to `A` with a new offset equal to `B.offset + C.offset`. * * If an activity is anchored to the end of the deleted activity, the delete activity's duration is assumed to be 0, * which may change the ultimate start time of the anchored activity. */ - ReAnchor, + PreserveTree, } diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt similarity index 72% rename from procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt rename to procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt index fb03bf44a9..e8bf893aed 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/EasyEditablePlanDriver.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt @@ -18,11 +18,11 @@ import java.lang.ref.WeakReference * The [EditablePlan] interface requires the implementor to perform some fairly complex * stateful operations, with a tangle of interdependent algorithmic guarantees. * Most of those operations are standard among all implementations though, so this driver - * captures most of it in a reusable form. Just inherit from this class to make a valid - * [EditablePlan]. + * captures most of it in a reusable form. Just implement the must simpler [PlanEditAdapter] + * from this class to make a valid [EditablePlan]. * - * The subclass is still responsible for simulation and the basic context-free creation - * and deletion operations. See the *Contracts* section of each abstract method's doc comment. + * The implementor is still responsible for simulation and the basic context-free creation + * and deletion operations. See the *Contracts* section of the interface methods' doc comments. */ /* * ## Staleness checking @@ -47,68 +47,71 @@ import java.lang.ref.WeakReference * The joint ownership freaks me out a wee bit, but I think it's safe because the commits are only used to keep the * previous sets from getting gc'ed in the event of a rollback. Only the plan object actually mutates the set. */ -abstract class EasyEditablePlanDriver( - private val plan: Plan -): EditablePlan, Plan by plan { - /** - * Create a unique directive ID. - * - * *Contract:* - * - the implementor must return an ID that is distinct from any activity ID that was in the initial plan - * or that has been returned from this method before during the implementor's lifetime. - */ - protected abstract fun generateDirectiveId(): ActivityDirectiveId - - /** - * Create a directive in the plan. - * - * *Contracts*: - * - the driver will guarantee that the directive ID does not collide with any other directive currently in the plan. - * - the implementor must return the new directive in future calls to [Plan.directives], unless it is later deleted. - * - the implementor must include the directive in future input plans for simulation, unless it is later deleted. - */ - protected abstract fun createInternal(directive: Directive) - - /** - * Remove a directive from the plan, specified by ID. - */ - protected abstract fun deleteInternal(id: ActivityDirectiveId) - - /** - * Get the latest simulation results. - * - * *Contract:* - * - the implementor must return equivalent results objects if this method is called multiple times without - * updates. - * - * The implementor doesn't have to return the exact same *instance* each time if no updates are made (i.e. referential - * equality isn't required, only structural equality). - */ - protected abstract fun latestResultsInternal(): PerishableSimulationResults? - - /** - * Simulate the current plan. - * - * *Contracts:* - * - all prior creations and deletions must be reflected in the simulation run. - * - the results corresponding to this run must be returned from future calls to [latestResultsInternal] - * until the next time [simulateInternal] is called. - */ - protected abstract fun simulateInternal(options: SimulateOptions) - - /** - * Optional validation hook for new activities. - * - * The default implementation checks if the activity is within the bounds of the plan. The implementor can - * add additional checks by overriding this method and calling `super.validate(directive)`. Implementor - * should throw if the directive is invalid. - */ - protected open fun validate(directive: Directive) { - if (directive.startTime > duration()) { - throw Exception("New activity with id ${directive.id.id()} would start after the end of the plan") - } - if (directive.start is DirectiveStart.Absolute && directive.startTime.isNegative) { - throw Exception("New activity with id ${directive.id.id()} would start before the beginning of the plan") +class DefaultEditablePlanDriver( + private val adapter: PlanEditAdapter +): EditablePlan, Plan by adapter { + + interface PlanEditAdapter: Plan { + /** + * Create a unique directive ID. + * + * *Contract:* + * - the implementor must return an ID that is distinct from any activity ID that was in the initial plan + * or that has been returned from this method before during the implementor's lifetime. + */ + fun generateDirectiveId(): ActivityDirectiveId + + /** + * Create a directive in the plan. + * + * *Contracts*: + * - the driver will guarantee that the directive ID does not collide with any other directive currently in the plan. + * - the implementor must return the new directive in future calls to [Plan.directives], unless it is later deleted. + * - the implementor must include the directive in future input plans for simulation, unless it is later deleted. + */ + fun create(directive: Directive) + + /** + * Remove a directive from the plan, specified by ID. + */ + fun delete(id: ActivityDirectiveId) + + /** + * Get the latest simulation results. + * + * *Contract:* + * - the implementor must return equivalent results objects if this method is called multiple times without + * updates. + * + * The implementor doesn't have to return the exact same *instance* each time if no updates are made (i.e. referential + * equality isn't required, only structural equality). + */ + fun latestResults(): PerishableSimulationResults? + + /** + * Simulate the current plan. + * + * *Contracts:* + * - all prior creations and deletions must be reflected in the simulation run. + * - the results corresponding to this run must be returned from future calls to [latestResultsInternal] + * until the next time [simulateInternal] is called. + */ + fun simulate(options: SimulateOptions) + + /** + * Optional validation hook for new activities. + * + * The default implementation checks if the activity is within the bounds of the plan. The implementor can + * add additional checks by overriding this method and calling `super.validate(directive)`. Implementor + * should throw if the directive is invalid. + */ + fun validate(directive: Directive) { + if (directive.startTime > duration()) { + throw Exception("New activity with id ${directive.id.id()} would start after the end of the plan") + } + if (directive.start is DirectiveStart.Absolute && directive.startTime.isNegative) { + throw Exception("New activity with id ${directive.id.id()} would start before the beginning of the plan") + } } } @@ -119,7 +122,7 @@ abstract class EasyEditablePlanDriver( * A record of the simulation results objects that were up-to-date when the commit * was created. * - * This has SHARED OWNERSHIP with [EasyEditablePlanDriver]; the editable plan may add more to + * This has SHARED OWNERSHIP with [DefaultEditablePlanDriver]; the editable plan may add more to * this list AFTER the commit is created. */ val upToDateSimResultsSet: MutableSet> @@ -140,7 +143,7 @@ abstract class EasyEditablePlanDriver( private var upToDateSimResultsSet: MutableSet> = mutableSetOf() override fun latestResults(): SimulationResults? { - val internalResults = latestResultsInternal() + val internalResults = adapter.latestResults() // kotlin checks structural equality by default, not referential equality. val isStale = internalResults?.inputDirectives()?.toSet() != directives().toSet() @@ -153,7 +156,7 @@ abstract class EasyEditablePlanDriver( override fun create(directive: NewDirective): ActivityDirectiveId { class ParentSearchException(id: ActivityDirectiveId, size: Int): Exception("Expected one parent activity with id $id, found $size") - val id = generateDirectiveId() + val id = adapter.generateDirectiveId() val parent = when (val s = directive.start) { is DirectiveStart.Anchor -> { val parentList = directives() @@ -167,9 +170,9 @@ abstract class EasyEditablePlanDriver( val resolved = directive.resolve(id, parent) uncommittedChanges.add(Edit.Create(resolved)) - validate(resolved) + adapter.validate(resolved) - createInternal(resolved) + adapter.create(resolved) for (simResults in upToDateSimResultsSet) { simResults.get()?.setStale(true) @@ -199,7 +202,7 @@ abstract class EasyEditablePlanDriver( if (childStart.parentId == directive.id) { when (strategy) { DeletedAnchorStrategy.Error -> throw Exception("Cannot delete an activity that has anchors pointing to it without a ${DeletedAnchorStrategy::class.java.simpleName}") - DeletedAnchorStrategy.ReAnchor -> { + DeletedAnchorStrategy.PreserveTree -> { directivesToDelete.add(d) val start = when (val parentStart = directive.start) { is DirectiveStart.Absolute -> DirectiveStart.Absolute(parentStart.time + childStart.offset) @@ -223,11 +226,11 @@ abstract class EasyEditablePlanDriver( for (d in directivesToDelete) { uncommittedChanges.add(Edit.Delete(d)) - deleteInternal(d.id) + adapter.delete(d.id) } for (d in directivesToCreate) { uncommittedChanges.add(Edit.Create(d)) - createInternal(d) + adapter.create(d) } for (simResults in upToDateSimResultsSet) { @@ -251,7 +254,7 @@ abstract class EasyEditablePlanDriver( } override fun delete(id: ActivityDirectiveId, strategy: DeletedAnchorStrategy) { - val matchingDirectives = plan.directives().filter { it.id == id }.collect() + val matchingDirectives = directives().filter { it.id == id }.collect() if (matchingDirectives.isEmpty()) throw Exception("attempted to delete activity by ID that does not exist: $id") if (matchingDirectives.size > 1) throw Exception("multiple activities with ID found: $id") @@ -290,8 +293,8 @@ abstract class EasyEditablePlanDriver( uncommittedChanges = mutableListOf() for (edit in result) { when (edit) { - is Edit.Create -> deleteInternal(edit.directive.id) - is Edit.Delete -> createInternal(edit.directive) + is Edit.Create -> adapter.delete(edit.directive.id) + is Edit.Delete -> adapter.create(edit.directive) } } for (simResult in upToDateSimResultsSet) { @@ -305,7 +308,7 @@ abstract class EasyEditablePlanDriver( } override fun simulate(options: SimulateOptions): SimulationResults { - simulateInternal(options) + adapter.simulate(options) return latestResults()!! } diff --git a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java index 91e68cffe6..becd43554c 100644 --- a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java +++ b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java @@ -1,5 +1,6 @@ package gov.nasa.jpl.aerie.scheduler.goals; +import gov.nasa.ammos.aerie.procedural.scheduling.utils.DefaultEditablePlanDriver; import gov.nasa.ammos.aerie.procedural.timeline.payloads.ExternalEvent; import gov.nasa.jpl.aerie.merlin.driver.MissionModel; import gov.nasa.jpl.aerie.merlin.protocol.types.SerializedValue; @@ -12,7 +13,7 @@ import gov.nasa.jpl.aerie.scheduler.model.PlanningHorizon; import gov.nasa.jpl.aerie.scheduler.model.Problem; import gov.nasa.jpl.aerie.scheduler.model.SchedulingActivity; -import gov.nasa.jpl.aerie.scheduler.plan.InMemoryEditablePlan; +import gov.nasa.jpl.aerie.scheduler.plan.SchedulerPlanEditAdapter; import gov.nasa.jpl.aerie.scheduler.plan.SchedulerToProcedurePlanAdapter; import gov.nasa.jpl.aerie.scheduler.simulation.SimulationFacade; import gov.nasa.jpl.aerie.scheduler.solver.ConflictSatisfaction; @@ -24,7 +25,7 @@ import java.util.Map; import java.util.function.Function; -import static gov.nasa.jpl.aerie.scheduler.plan.InMemoryEditablePlan.toSchedulingActivity; +import static gov.nasa.jpl.aerie.scheduler.plan.SchedulerPlanEditAdapter.toSchedulingActivity; public class Procedure extends Goal { private final Path jarPath; @@ -64,7 +65,7 @@ public void run( problem.getRealExternalProfiles() ); - final var editablePlan = new InMemoryEditablePlan( + final var editAdapter = new SchedulerPlanEditAdapter( missionModel, idGenerator, planAdapter, @@ -72,6 +73,8 @@ public void run( lookupActivityType::apply ); + final var editablePlan = new DefaultEditablePlanDriver(editAdapter); + procedureMapper.deserialize(SerializedValue.of(this.args)).run(editablePlan); if (editablePlan.isDirty()) { diff --git a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerPlanEditAdapter.kt similarity index 85% rename from scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt rename to scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerPlanEditAdapter.kt index 49ca1dbaeb..313f8afa6a 100644 --- a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/InMemoryEditablePlan.kt +++ b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerPlanEditAdapter.kt @@ -3,7 +3,7 @@ package gov.nasa.jpl.aerie.scheduler.plan import gov.nasa.jpl.aerie.merlin.driver.MissionModel import gov.nasa.jpl.aerie.merlin.protocol.types.Duration import gov.nasa.ammos.aerie.procedural.scheduling.simulation.SimulateOptions -import gov.nasa.ammos.aerie.procedural.scheduling.utils.EasyEditablePlanDriver +import gov.nasa.ammos.aerie.procedural.scheduling.utils.DefaultEditablePlanDriver import gov.nasa.ammos.aerie.procedural.scheduling.utils.PerishableSimulationResults import gov.nasa.jpl.aerie.scheduler.simulation.SimulationFacade import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.AnyDirective @@ -15,35 +15,36 @@ import gov.nasa.jpl.aerie.scheduler.model.* import gov.nasa.jpl.aerie.types.ActivityDirectiveId import java.time.Instant import kotlin.jvm.optionals.getOrNull +import gov.nasa.ammos.aerie.procedural.timeline.plan.Plan; /* * An implementation of [EditablePlan] that stores the plan in memory for use in the internal scheduler. * */ -data class InMemoryEditablePlan( +data class SchedulerPlanEditAdapter( private val missionModel: MissionModel<*>, private var idGenerator: DirectiveIdGenerator, - private val plan: SchedulerToProcedurePlanAdapter, + val plan: SchedulerToProcedurePlanAdapter, private val simulationFacade: SimulationFacade, private val lookupActivityType: (String) -> ActivityType -) : EasyEditablePlanDriver(plan) { +): Plan by plan, DefaultEditablePlanDriver.PlanEditAdapter { override fun generateDirectiveId(): ActivityDirectiveId = idGenerator.next() - override fun latestResultsInternal(): PerishableSimulationResults? { + override fun latestResults(): PerishableSimulationResults? { val merlinResults = simulationFacade.latestSimulationData.getOrNull() ?: return null return MerlinToProcedureSimulationResultsAdapter(merlinResults.driverResults, plan.copy(schedulerPlan = plan.duplicate())) } - override fun createInternal(directive: Directive) { + override fun create(directive: Directive) { plan.add(directive.toSchedulingActivity(lookupActivityType, true)) } - override fun deleteInternal(id: ActivityDirectiveId) { + override fun delete(id: ActivityDirectiveId) { plan.remove(plan.activitiesById[id]) } - override fun simulateInternal(options: SimulateOptions) { + override fun simulate(options: SimulateOptions) { simulationFacade.simulateWithResults(plan, options.pause.resolve(this)) } diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanTest.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanTest.java index d15ab6b1da..47598bd865 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanTest.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/EditablePlanTest.java @@ -1,6 +1,7 @@ package gov.nasa.jpl.aerie.scheduler; import gov.nasa.ammos.aerie.procedural.scheduling.plan.EditablePlan; +import gov.nasa.ammos.aerie.procedural.scheduling.utils.DefaultEditablePlanDriver; import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.DirectiveStart; import gov.nasa.jpl.aerie.merlin.driver.MissionModel; import gov.nasa.jpl.aerie.merlin.protocol.types.Duration; @@ -8,7 +9,7 @@ import gov.nasa.jpl.aerie.scheduler.model.PlanInMemory; import gov.nasa.jpl.aerie.scheduler.model.PlanningHorizon; import gov.nasa.jpl.aerie.scheduler.model.Problem; -import gov.nasa.jpl.aerie.scheduler.plan.InMemoryEditablePlan; +import gov.nasa.jpl.aerie.scheduler.plan.SchedulerPlanEditAdapter; import gov.nasa.jpl.aerie.scheduler.plan.SchedulerToProcedurePlanAdapter; import gov.nasa.jpl.aerie.scheduler.simulation.CheckpointSimulationFacade; import gov.nasa.jpl.aerie.scheduler.simulation.SimulationFacade; @@ -43,7 +44,7 @@ public void setUp() { final var schedulerModel = SimulationUtility.getBananaSchedulerModel(); facade = new CheckpointSimulationFacade(horizon, missionModel, schedulerModel); problem = new Problem(missionModel, horizon, facade, schedulerModel); - plan = new InMemoryEditablePlan( + final var editAdapter = new SchedulerPlanEditAdapter( missionModel, new DirectiveIdGenerator(0), new SchedulerToProcedurePlanAdapter( @@ -56,6 +57,7 @@ public void setUp() { facade, problem::getActivityType ); + plan = new DefaultEditablePlanDriver(editAdapter); } @AfterEach From 22309a5a4d598c7cfb77705d5c60c32283c92481 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 23 Jan 2025 15:36:31 -0800 Subject: [PATCH 12/22] Add anchor rollback test --- .../procedures/ActivityDeletionGoal.java | 6 ++- .../procedural/scheduling/DeletionTests.java | 51 ++++++++++++++++++- .../GraphQLMerlinDatabaseService.java | 14 +++++ 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java index f9242bf14e..9c4f12a315 100644 --- a/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java +++ b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java @@ -15,9 +15,10 @@ /** * Creates three activities in a chain of anchors, then deletes one. * If `whichToDelete` is negative, this leaves all three activities. + * If `rollback` is true, this will roll the edit back before finishing. */ @SchedulingProcedure -public record ActivityDeletionGoal(int whichToDelete, DeletedAnchorStrategy anchorStrategy) implements Goal { +public record ActivityDeletionGoal(int whichToDelete, DeletedAnchorStrategy anchorStrategy, boolean rollback) implements Goal { @Override public void run(@NotNull final EditablePlan plan) { final var ids = new ActivityDirectiveId[3]; @@ -42,6 +43,7 @@ public void run(@NotNull final EditablePlan plan) { plan.delete(ids[whichToDelete], anchorStrategy); } - plan.commit(); + if (rollback) plan.rollback(); + else plan.commit(); } } diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DeletionTests.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DeletionTests.java index 5e38b57b19..9cd51f98f5 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DeletionTests.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DeletionTests.java @@ -42,6 +42,7 @@ void createsThreeActivities() throws IOException { .createObjectBuilder() .add("whichToDelete", -1) .add("anchorStrategy", "Error") + .add("rollback", false) .build(); hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); @@ -82,6 +83,7 @@ void deletesLast() throws IOException { .createObjectBuilder() .add("whichToDelete", 2) .add("anchorStrategy", "Error") + .add("rollback", false) .build(); hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); @@ -113,6 +115,7 @@ void deletesMiddleCascade() throws IOException { .createObjectBuilder() .add("whichToDelete", 1) .add("anchorStrategy", "Cascade") + .add("rollback", false) .build(); hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); @@ -134,7 +137,8 @@ void deletesMiddleAnchorToParent() throws IOException { final var args = Json .createObjectBuilder() .add("whichToDelete", 1) - .add("anchorStrategy", "ReAnchor") + .add("anchorStrategy", "PreserveTree") + .add("rollback", false) .build(); hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); @@ -168,6 +172,7 @@ void deletesFirstCascade() throws IOException { .createObjectBuilder() .add("whichToDelete", 0) .add("anchorStrategy", "Cascade") + .add("rollback", false) .build(); hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); @@ -185,7 +190,8 @@ void deletesFirstReAnchorToPlan() throws IOException { final var args = Json .createObjectBuilder() .add("whichToDelete", 0) - .add("anchorStrategy", "ReAnchor") + .add("anchorStrategy", "PreserveTree") + .add("rollback", false) .build(); hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); @@ -212,4 +218,45 @@ void deletesFirstReAnchorToPlan() throws IOException { it -> Objects.equals(it.type(), "BiteBanana") && Objects.equals(it.anchorId(), id2.get()) )); } + + @Test + void anchorResetOnRollback() throws IOException { + final var args = Json + .createObjectBuilder() + .add("whichToDelete", 1) + .add("anchorStrategy", "PreserveTree") + .add("rollback", true) + .build(); + + hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + + hasura.awaitScheduling(specId); + + final var plan = hasura.getPlan(planId); + final var activities = plan.activityDirectives(); + + assertEquals(3, activities.size()); + + final AtomicReference id1 = new AtomicReference<>(); + final AtomicReference id2 = new AtomicReference<>(); + assertTrue(activities.stream().anyMatch( + it -> { + final var result = Objects.equals(it.type(), "BiteBanana") && Objects.equals(it.anchorId(), null); + if (result) id1.set(it.id()); + return result; + } + )); + + assertTrue(activities.stream().anyMatch( + it -> { + final var result = Objects.equals(it.type(), "BiteBanana") && Objects.equals(it.anchorId(), id1.get()); + if (result) id2.set(it.id()); + return result; + } + )); + + assertTrue(activities.stream().anyMatch( + it -> Objects.equals(it.type(), "BiteBanana") && Objects.equals(it.anchorId(), id2.get()) + )); + } } diff --git a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java index 5c31c7eee9..43c1a8c9fb 100644 --- a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java +++ b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java @@ -442,6 +442,20 @@ public Map updatePlanActivityDirective activity.anchoredToStart() ); if (!activityDirectiveFromSchedulingDirective.equals(actFromInitialPlan.get())) { + final var newState = activityDirectiveFromSchedulingDirective.serializedActivity(); + final var oldState = actFromInitialPlan.get().serializedActivity(); + if (!Objects.equals(newState.getTypeName(), oldState.getTypeName())) { + throw new IllegalStateException( + "Modified activities cannot change type. Was " + oldState.getTypeName() + + ", now " + newState.getTypeName() + ); + } + if (!Objects.equals(newState.getArguments(), oldState.getArguments())) { + throw new IllegalStateException( + "Modified activities cannot change arguments. Was " + oldState.getArguments() + + ", now " + newState.getArguments() + ); + } toModify.add(activity); } ids.put(activity.id(), activity.id()); From c2b7d6613fcd7af884247c13cff5d74f3afa874d Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 23 Jan 2025 16:12:09 -0800 Subject: [PATCH 13/22] Rollback edits in reverse --- .../procedural/scheduling/procedures/ActivityDeletionGoal.java | 2 ++ .../procedural/scheduling/utils/DefaultEditablePlanDriver.kt | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java index 9c4f12a315..82f3b029a1 100644 --- a/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java +++ b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/ActivityDeletionGoal.java @@ -39,6 +39,8 @@ public void run(@NotNull final EditablePlan plan) { Map.of("biteSize", SerializedValue.of(2)) ); + plan.commit(); + if (whichToDelete >= 0) { plan.delete(ids[whichToDelete], anchorStrategy); } diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt index e8bf893aed..bb56862deb 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt @@ -291,7 +291,7 @@ class DefaultEditablePlanDriver( val result = uncommittedChanges uncommittedChanges = mutableListOf() - for (edit in result) { + for (edit in result.reversed()) { when (edit) { is Edit.Create -> adapter.delete(edit.directive.id) is Edit.Delete -> adapter.create(edit.directive) From 5f162fbc860402c442a3cbef0b746ddc5e489154 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 30 Jan 2025 15:40:17 -0800 Subject: [PATCH 14/22] Misc minor review comments --- .../procedural/scheduling/utils/DefaultEditablePlanDriver.kt | 4 +++- .../java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java | 2 ++ .../nasa/jpl/aerie/scheduler/plan/SchedulerPlanEditAdapter.kt | 1 - 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt index bb56862deb..5ce5a294e8 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt @@ -186,7 +186,6 @@ class DefaultEditablePlanDriver( override fun delete(directive: Directive, strategy: DeletedAnchorStrategy) { val directives = directives().cache() - val directivesToDelete: Set> val directivesToCreate: Set> @@ -197,6 +196,8 @@ class DefaultEditablePlanDriver( directivesToDelete = mutableSetOf(directive) directivesToCreate = mutableSetOf() for (d in directives) { + // the when block is used to smart-cast d.start to an Anchor. This is basically just an if statement. + // Basically we're just iterating through looking for activities anchored to the deleted one. when (val childStart = d.start) { is DirectiveStart.Anchor -> { if (childStart.parentId == directive.id) { @@ -219,6 +220,7 @@ class DefaultEditablePlanDriver( } } } + // if d.start wasn't an anchor, do nothing else -> {} } } diff --git a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java index becd43554c..79df3b8eb8 100644 --- a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java +++ b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java @@ -83,6 +83,8 @@ public void run( for (final var edit : editablePlan.getTotalDiff()) { if (edit instanceof Edit.Create c) { newActivities.add(toSchedulingActivity(c.getDirective(), lookupActivityType::apply, true)); + } else if (!(edit instanceof Edit.Delete)) { + throw new IllegalStateException("Unexpected edit type: " + edit); } } diff --git a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerPlanEditAdapter.kt b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerPlanEditAdapter.kt index 313f8afa6a..aa942396bf 100644 --- a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerPlanEditAdapter.kt +++ b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerPlanEditAdapter.kt @@ -20,7 +20,6 @@ import gov.nasa.ammos.aerie.procedural.timeline.plan.Plan; /* * An implementation of [EditablePlan] that stores the plan in memory for use in the internal scheduler. * - */ data class SchedulerPlanEditAdapter( private val missionModel: MissionModel<*>, From 2b66f65272d0e4d44f38e1567bdbff1d46a1ef23 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 30 Jan 2025 15:40:32 -0800 Subject: [PATCH 15/22] Remove unnecessary duration argument assignment --- .../server/services/GraphQLMerlinDatabaseService.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java index 43c1a8c9fb..49716468f3 100644 --- a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java +++ b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java @@ -426,12 +426,6 @@ public Map updatePlanActivityDirective for (final var activity : plan.getActivities()) { if(activity.getParentActivity().isPresent()) continue; // Skip generated activities if (!activity.isNew()) { - //add duration to parameters if controllable - if (activity.getType().getDurationType() instanceof DurationType.Controllable durationType){ - if (!activity.arguments().containsKey(durationType.parameterName())){ - activity.addArgument(durationType.parameterName(), schedulerModel.serializeDuration(activity.duration())); - } - } final var actFromInitialPlan = initialPlan.getActivityById(activity.id()); //if act was present in initial plan final var activityDirectiveFromSchedulingDirective = new ActivityDirective( From 539b861c4a8a364817b4a3049eff98bee4f85917 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 30 Jan 2025 16:22:32 -0800 Subject: [PATCH 16/22] Move anchor update into general activity modify mutation --- .../GraphQLMerlinDatabaseService.java | 152 ++++++++++-------- .../services/MerlinDatabaseService.java | 9 -- .../services/SynchronousSchedulerAgent.java | 1 - .../services/MockMerlinDatabaseService.java | 4 - 4 files changed, 88 insertions(+), 78 deletions(-) diff --git a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java index 49716468f3..4984f3dba5 100644 --- a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java +++ b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java @@ -46,6 +46,7 @@ import gov.nasa.jpl.aerie.types.ActivityDirective; import gov.nasa.jpl.aerie.types.ActivityDirectiveId; import gov.nasa.jpl.aerie.types.MissionModelId; +import gov.nasa.jpl.aerie.types.SerializedActivity; import org.apache.commons.lang3.tuple.Pair; import org.apache.commons.lang3.tuple.Triple; @@ -54,6 +55,7 @@ import javax.json.JsonArrayBuilder; import javax.json.JsonException; import javax.json.JsonObject; +import javax.json.JsonObjectBuilder; import javax.json.JsonValue; import java.io.IOException; import java.net.URI; @@ -70,6 +72,8 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -135,7 +139,7 @@ public record DatasetIds(DatasetId datasetId, SimulationDatasetId simulationData * @param gqlStr the graphQL query or mutation to send to aerie * @return the json response returned by aerie, or an empty optional in case of io errors */ - protected Optional postRequest(final String gqlStr) throws IOException, MerlinServiceException { + private Optional postRequest(final String gqlStr) throws IOException, MerlinServiceException { try { //TODO: (mem optimization) use streams here to avoid several copies of strings final var reqBody = Json.createObjectBuilder().add("query", gqlStr).build(); @@ -422,11 +426,11 @@ public Map updatePlanActivityDirective //creation are done in batch as that's what the scheduler does the most final var toAdd = new ArrayList(); final var toDelete = new ArrayList(); - final var toModify = new ArrayList(); + final var toModify = new ArrayList(); for (final var activity : plan.getActivities()) { if(activity.getParentActivity().isPresent()) continue; // Skip generated activities if (!activity.isNew()) { - final var actFromInitialPlan = initialPlan.getActivityById(activity.id()); + final var actFromInitialPlan = initialPlan.getActivityById(activity.id()).get(); //if act was present in initial plan final var activityDirectiveFromSchedulingDirective = new ActivityDirective( activity.startOffset(), @@ -435,22 +439,9 @@ public Map updatePlanActivityDirective activity.anchorId(), activity.anchoredToStart() ); - if (!activityDirectiveFromSchedulingDirective.equals(actFromInitialPlan.get())) { - final var newState = activityDirectiveFromSchedulingDirective.serializedActivity(); - final var oldState = actFromInitialPlan.get().serializedActivity(); - if (!Objects.equals(newState.getTypeName(), oldState.getTypeName())) { - throw new IllegalStateException( - "Modified activities cannot change type. Was " + oldState.getTypeName() - + ", now " + newState.getTypeName() - ); - } - if (!Objects.equals(newState.getArguments(), oldState.getArguments())) { - throw new IllegalStateException( - "Modified activities cannot change arguments. Was " + oldState.getArguments() - + ", now " + newState.getArguments() - ); - } - toModify.add(activity); + if (!activityDirectiveFromSchedulingDirective.equals(actFromInitialPlan)) { + final var ops = generateModification(actFromInitialPlan, activityDirectiveFromSchedulingDirective); + if (!ops.isEmpty()) toModify.add(new ActivityModification(activity.id(), ops)); } ids.put(activity.id(), activity.id()); } else { @@ -467,35 +458,65 @@ public Map updatePlanActivityDirective //Create ids.putAll(createActivityDirectives(planId, toAdd, activityToGoalId, schedulerModel)); + + // Create does not upload the anchor ids, because directive IDs can change during upload + // and it would cause a foreign key violation. So we map the anchor ids using the creation results + // and add an anchor-modification entry to the `toModify` list after the fact. + for (final var act: toAdd) { + if (act.anchorId() != null) { + toModify.add(new ActivityModification( + act.id(), + List.of($ -> $.add("anchor_id", ids.get(act.anchorId()).id())) + )); + } + } + modifyActivityDirectives(planId, toModify); deleteActivityDirectives(planId, toDelete); return ids; } - @Override - public void updatePlanActivityDirectiveAnchors(final PlanId planId, final Plan plan, final Map uploadIdMap) - throws MerlinServiceException, IOException - { - final var request = new StringBuilder(); - final var acts = plan.getActivities(); - request.append("mutation {"); - var hasUpdate = false; - for (final SchedulingActivity act: acts) { - if (act.isNew() && act.anchorId() != null) { - hasUpdate = true; - final var id = uploadIdMap.get(act.id()).id(); - request.append(""" - update_%d: update_activity_directive_by_pk(pk_columns: {id: %d, plan_id: %d}, _set: {anchor_id: %d}) { - id - } - """.formatted(id, id, planId.id(), uploadIdMap.get(act.anchorId()).id()) - ); - } + /** + * Generates the list of operations needed to change an activity in the database. + * + * @param oldState the old activity before modification + * @param newState the modified activity + */ + private List generateModification(final ActivityDirective oldState, final ActivityDirective newState) { + final var operations = new ArrayList(); + + if (!Objects.equals(newState.serializedActivity().getTypeName(), oldState.serializedActivity().getTypeName())) { + throw new IllegalStateException( + "Modified activities cannot change type. Was " + oldState.serializedActivity().getTypeName() + + ", now " + newState.serializedActivity().getTypeName() + ); + } + if (!Objects.equals(newState.serializedActivity().getArguments(), oldState.serializedActivity().getArguments())) { + throw new IllegalStateException( + "Modified activities cannot change arguments. Was " + oldState.serializedActivity().getArguments() + + ", now " + newState.serializedActivity().getArguments() + ); + } + + if (newState.startOffset() != oldState.startOffset()) { + operations.add( + $ -> $.add("start_offset", newState.startOffset().toString()) + ); + } + + if (newState.anchorId() != oldState.anchorId()) { + operations.add( + $ -> $.add("anchor_id", newState.anchorId().id()) + ); } - if (hasUpdate) { - request.append("}"); - postRequest(request.toString()); + + if (newState.anchoredToStart() != oldState.anchoredToStart()) { + operations.add( + $ -> $.add("anchor_id", newState.anchoredToStart()) + ); } + + return operations; } /** @@ -646,24 +667,33 @@ mutation createAllPlanActivityDirectives($activities: [activity_directive_insert return activityToDirectiveId; } + private record ActivityModification( + ActivityDirectiveId id, + List operations + ) {} + + interface ActivityOperation { + void apply(JsonObjectBuilder obj); + } + private void modifyActivityDirectives( final PlanId planId, - final List activities + final List modifications ) throws IOException, NoSuchPlanException, MerlinServiceException { - if (activities.isEmpty()) return; + if (modifications.isEmpty()) return; ensurePlanExists(planId); final var request = new StringBuilder(); request.append("mutation updatePlanActivityDirectives("); request.append(String.join( ",", - activities.stream().map($ -> "$activity_%d: activity_directive_set_input!".formatted($.id().id())).toList() + modifications.stream().map($ -> "$activity_%d: activity_directive_set_input!".formatted($.id().id())).toList() )); request.append(") {"); final var arguments = Json.createObjectBuilder(); - for (final var act : activities) { - final var id = act.id().id(); + for (final var mod : modifications) { + final var id = mod.id().id(); request.append(""" update_%d: update_activity_directive_by_pk(pk_columns: {id: %d, plan_id: %d}, _set: $activity_%d) { affected_rows @@ -671,18 +701,12 @@ private void modifyActivityDirectives( """.formatted(id, id, planId.id(), id)); final var activityObject = Json - .createObjectBuilder() - .add("start_offset", act.startOffset().toString()) - .add("anchored_to_start", act.anchoredToStart()) - .add("name", act.name()); + .createObjectBuilder(); + mod.operations.forEach($ -> $.apply(activityObject)); - final var insertionObjectArguments = Json.createObjectBuilder(); - for (final var arg : act.arguments().entrySet()) { - insertionObjectArguments.add(arg.getKey(), serializedValueP.unparse(arg.getValue())); - } - activityObject.add("arguments", insertionObjectArguments.build()); arguments.add("activity_%d".formatted(id), activityObject); } + request.append("}"); postRequest(request.toString(), arguments.build()).orElseThrow(() -> new NoSuchPlanException(planId)); } @@ -694,15 +718,15 @@ private void deleteActivityDirectives( { if (ids.isEmpty()) return; ensurePlanExists(planId); - final var request = new StringBuilder(); - request.append("mutation deletePlanActivityDirectives {"); - for (final var id : ids) { - request.append(""" - delete_%d: delete_activity_directive_by_pk(id: %d, plan_id: %d) {affected_rows} - """.formatted(id.id(), id.id(), planId.id())); - } - request.append("}"); - postRequest(request.toString()).orElseThrow(() -> new NoSuchPlanException(planId)); + final var idString = ids.stream().map(String::valueOf).collect(Collectors.joining(",")); + final var request = """ + mutation deletePlanActivityDirectives($planId: Int! = %d, $directiveIds: [Int!]! = [%s]) { + delete_activity_directive(where: {_and: {plan_id: {_eq: $planId}, id: {_in: $directiveIds}}}) { + affected_rows + } + } + """.formatted(planId.id(), idString); + postRequest(request).orElseThrow(() -> new NoSuchPlanException(planId)); } diff --git a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/MerlinDatabaseService.java b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/MerlinDatabaseService.java index 18939f8d1f..dba36cd5d4 100644 --- a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/MerlinDatabaseService.java +++ b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/MerlinDatabaseService.java @@ -169,15 +169,6 @@ Map updatePlanActivityDirectives( ) throws IOException, NoSuchPlanException, MerlinServiceException, NoSuchActivityInstanceException; - /** - * update the list of SchedulingActivityDirectives with anchors, replacing the anchorIds generated by the - * scheduler by the new ids generated by the database - * @throws MerlinServiceException - * @throws IOException - */ - void updatePlanActivityDirectiveAnchors(final PlanId planId, final Plan plan, final Map uploadIdMap) - throws MerlinServiceException, IOException; - /** * delete all the activity instances stored in the target plan container * diff --git a/scheduler-worker/src/main/java/gov/nasa/jpl/aerie/scheduler/worker/services/SynchronousSchedulerAgent.java b/scheduler-worker/src/main/java/gov/nasa/jpl/aerie/scheduler/worker/services/SynchronousSchedulerAgent.java index 1347e2afac..3cbfbc9571 100644 --- a/scheduler-worker/src/main/java/gov/nasa/jpl/aerie/scheduler/worker/services/SynchronousSchedulerAgent.java +++ b/scheduler-worker/src/main/java/gov/nasa/jpl/aerie/scheduler/worker/services/SynchronousSchedulerAgent.java @@ -279,7 +279,6 @@ public void schedule( ); } - merlinDatabaseService.updatePlanActivityDirectiveAnchors(specification.planId(), solutionPlan, uploadIdMap); //collect results and notify subscribers of success final var results = collectResults(solutionPlan, uploadIdMap, goals); diff --git a/scheduler-worker/src/test/java/gov/nasa/jpl/aerie/scheduler/worker/services/MockMerlinDatabaseService.java b/scheduler-worker/src/test/java/gov/nasa/jpl/aerie/scheduler/worker/services/MockMerlinDatabaseService.java index f79c1acf1d..c9ea480a5f 100644 --- a/scheduler-worker/src/test/java/gov/nasa/jpl/aerie/scheduler/worker/services/MockMerlinDatabaseService.java +++ b/scheduler-worker/src/test/java/gov/nasa/jpl/aerie/scheduler/worker/services/MockMerlinDatabaseService.java @@ -140,10 +140,6 @@ public Map updatePlanActivityDirective return res; } - @Override - public void updatePlanActivityDirectiveAnchors(final PlanId planId, final Plan plan, final Map uploadIdMap) - {} - @Override public void ensurePlanExists(final PlanId planId) { From 64bc3fabdcda2332099881826dcdbdce81952107 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Thu, 30 Jan 2025 16:53:22 -0800 Subject: [PATCH 17/22] Add database deletion e2e tests --- .../procedures/DeleteBiteBananasGoal.java | 26 ++ .../scheduling/DatabaseDeletionTests.java | 265 ++++++++++++++++++ .../jpl/aerie/e2e/utils/HasuraRequests.java | 6 +- .../utils/DefaultEditablePlanDriver.kt | 3 + .../GraphQLMerlinDatabaseService.java | 8 +- 5 files changed, 303 insertions(+), 5 deletions(-) create mode 100644 e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/DeleteBiteBananasGoal.java create mode 100644 e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DatabaseDeletionTests.java diff --git a/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/DeleteBiteBananasGoal.java b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/DeleteBiteBananasGoal.java new file mode 100644 index 0000000000..cb34f1ecfc --- /dev/null +++ b/e2e-tests/src/main/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/procedures/DeleteBiteBananasGoal.java @@ -0,0 +1,26 @@ +package gov.nasa.jpl.aerie.e2e.procedural.scheduling.procedures; + +import gov.nasa.ammos.aerie.procedural.scheduling.Goal; +import gov.nasa.ammos.aerie.procedural.scheduling.annotations.SchedulingProcedure; +import gov.nasa.ammos.aerie.procedural.scheduling.plan.DeletedAnchorStrategy; +import gov.nasa.ammos.aerie.procedural.scheduling.plan.EditablePlan; +import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.DirectiveStart; +import gov.nasa.jpl.aerie.merlin.protocol.types.Duration; +import gov.nasa.jpl.aerie.merlin.protocol.types.SerializedValue; +import gov.nasa.jpl.aerie.types.ActivityDirectiveId; +import org.jetbrains.annotations.NotNull; + +import java.util.Map; + +/** + * Deletes all Bite Bananas with extreme prejudice. Used to test that updated + * anchors are saved in the database properly. + */ +@SchedulingProcedure +public record DeleteBiteBananasGoal(DeletedAnchorStrategy anchorStrategy) implements Goal { + @Override + public void run(@NotNull final EditablePlan plan) { + plan.directives("BiteBanana").forEach($ -> plan.delete($, anchorStrategy)); + plan.commit(); + } +} diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DatabaseDeletionTests.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DatabaseDeletionTests.java new file mode 100644 index 0000000000..dbba0e0b4f --- /dev/null +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DatabaseDeletionTests.java @@ -0,0 +1,265 @@ +package gov.nasa.jpl.aerie.e2e.procedural.scheduling; + +import gov.nasa.jpl.aerie.e2e.types.GoalInvocationId; +import gov.nasa.jpl.aerie.e2e.utils.GatewayRequests; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.opentest4j.AssertionFailedError; + +import javax.json.Json; +import javax.json.JsonValue; +import java.io.IOException; +import java.util.Objects; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class DatabaseDeletionTests extends ProceduralSchedulingSetup { + private GoalInvocationId procedureId; + + @BeforeEach + void localBeforeEach() throws IOException { + try (final var gateway = new GatewayRequests(playwright)) { + int procedureJarId = gateway.uploadJarFile("build/libs/DeleteBiteBananasGoal.jar"); + // Add Scheduling Procedure + procedureId = hasura.createSchedulingSpecProcedure( + "Test Scheduling Procedure", + procedureJarId, + specId, + 0 + ); + } + } + + @AfterEach + void localAfterEach() throws IOException { + hasura.deleteSchedulingGoal(procedureId.goalId()); + } + + @Test + void deletesDirectiveAlreadyInDatabase() throws IOException { + final var args = Json + .createObjectBuilder() + .add("anchorStrategy", "PreserveTree") + .build(); + hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + + hasura.insertActivityDirective( + planId, + "BiteBanana", + "1h", + JsonValue.EMPTY_JSON_OBJECT + ); + + var plan = hasura.getPlan(planId); + assertEquals(1, plan.activityDirectives().size()); + + hasura.awaitScheduling(specId); + + plan = hasura.getPlan(planId); + assertEquals(0, plan.activityDirectives().size()); + } + + @Test + void deletesDirectiveInDatabaseWithAnchor() throws IOException { + final var args = Json + .createObjectBuilder() + .add("anchorStrategy", "PreserveTree") + .build(); + hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + + final var bite = hasura.insertActivityDirective( + planId, + "BiteBanana", + "1h", + JsonValue.EMPTY_JSON_OBJECT + ); + + final var grow = hasura.insertActivityDirective( + planId, + "GrowBanana", + "1h", + JsonValue.EMPTY_JSON_OBJECT, + Json.createObjectBuilder().add("anchor_id", bite) + ); + + var plan = hasura.getPlan(planId); + var activities = plan.activityDirectives(); + assertEquals(2, activities.size()); + assertTrue(activities.stream().anyMatch( + it -> Objects.equals(it.type(), "GrowBanana") + && Objects.equals(it.anchorId(), bite) + && Objects.equals(it.startOffset(), "01:00:00") + )); + + hasura.awaitScheduling(specId); + + plan = hasura.getPlan(planId); + + activities = plan.activityDirectives(); + assertEquals(1, activities.size()); + + assertTrue(activities.stream().anyMatch( + it -> Objects.equals(it.type(), "GrowBanana") + && Objects.equals(it.anchorId(), null) + && Objects.equals(it.startOffset(), "02:00:00") + )); + } + + @Test + void deletesDirectiveInDatabaseInMiddleOfChain() throws IOException { + + // Creates 5 activities, deletes "Bite". + // grow1 <- bite + // bite <- grow (id not assigned to a variable) + // bite <- grow2 + // grow2 <- grow3 + + // Bite has two children, a grandchild, and a parent. + + final var args = Json + .createObjectBuilder() + .add("anchorStrategy", "PreserveTree") + .build(); + hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + + final var grow1 = hasura.insertActivityDirective( + planId, + "GrowBanana", + "1h", + JsonValue.EMPTY_JSON_OBJECT + ); + + final var bite = hasura.insertActivityDirective( + planId, + "BiteBanana", + "1h", + JsonValue.EMPTY_JSON_OBJECT, + Json.createObjectBuilder().add("anchor_id", grow1) + ); + + int grow2 = -1; + for (int i = 0; i < 2; i++) { + grow2 = hasura.insertActivityDirective( + planId, + "GrowBanana", + i + "h", + JsonValue.EMPTY_JSON_OBJECT, + Json.createObjectBuilder().add("anchor_id", bite) + ); + } + + final var grow3 = hasura.insertActivityDirective( + planId, + "GrowBanana", + "0h", + JsonValue.EMPTY_JSON_OBJECT, + Json.createObjectBuilder().add("anchor_id", grow2) + ); + + var plan = hasura.getPlan(planId); + var activities = plan.activityDirectives(); + assertEquals(5, activities.size()); + + hasura.awaitScheduling(specId); + + plan = hasura.getPlan(planId); + + activities = plan.activityDirectives(); + assertEquals(4, activities.size()); + + assertTrue(activities.stream().anyMatch( + it -> Objects.equals(it.type(), "GrowBanana") + && Objects.equals(it.id(), grow1) + && Objects.equals(it.anchorId(), null) + )); + final int finalGrow2 = grow2; + assertTrue(activities.stream().anyMatch( + it -> Objects.equals(it.type(), "GrowBanana") + && Objects.equals(it.id(), finalGrow2) + && Objects.equals(it.anchorId(), grow1) + && Objects.equals(it.startOffset(), "02:00:00") + )); + assertTrue(activities.stream().anyMatch( + it -> Objects.equals(it.type(), "GrowBanana") + && Objects.equals(it.anchorId(), grow1) + && Objects.equals(it.startOffset(), "01:00:00") + )); + assertTrue(activities.stream().anyMatch( + it -> Objects.equals(it.type(), "GrowBanana") + && Objects.equals(it.id(), grow3) + && Objects.equals(it.anchorId(), finalGrow2) + && Objects.equals(it.startOffset(), "00:00:00") + )); + } + + @Test + void deleteCascadeInDatabase() throws IOException { + final var args = Json + .createObjectBuilder() + .add("anchorStrategy", "Cascade") + .build(); + hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + + final var bite = hasura.insertActivityDirective( + planId, + "BiteBanana", + "1h", + JsonValue.EMPTY_JSON_OBJECT + ); + + final var grow = hasura.insertActivityDirective( + planId, + "GrowBanana", + "1h", + JsonValue.EMPTY_JSON_OBJECT, + Json.createObjectBuilder().add("anchor_id", bite) + ); + + + var plan = hasura.getPlan(planId); + assertEquals(2, plan.activityDirectives().size()); + + hasura.awaitScheduling(specId); + + plan = hasura.getPlan(planId); + + assertEquals(0, plan.activityDirectives().size()); + } + + @Test + void deleteErrorInDatabase() throws IOException { + final var args = Json + .createObjectBuilder() + .add("anchorStrategy", "Error") + .build(); + hasura.updateSchedulingSpecGoalArguments(procedureId.invocationId(), args); + + final var bite = hasura.insertActivityDirective( + planId, + "BiteBanana", + "1h", + JsonValue.EMPTY_JSON_OBJECT + ); + + final var grow = hasura.insertActivityDirective( + planId, + "GrowBanana", + "1h", + JsonValue.EMPTY_JSON_OBJECT, + Json.createObjectBuilder().add("anchor_id", bite) + ); + + + var plan = hasura.getPlan(planId); + assertEquals(2, plan.activityDirectives().size()); + + assertThrows(AssertionFailedError.class, () -> hasura.awaitScheduling(specId)); + + plan = hasura.getPlan(planId); + + assertEquals(2, plan.activityDirectives().size()); + } +} diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/HasuraRequests.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/HasuraRequests.java index 71b5dff661..c0394a518e 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/HasuraRequests.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/utils/HasuraRequests.java @@ -11,6 +11,7 @@ import javax.json.Json; import javax.json.JsonArray; import javax.json.JsonArrayBuilder; +import javax.json.JsonObjectBuilder; import javax.json.JsonValue; import javax.json.JsonObject; import java.io.IOException; @@ -225,12 +226,15 @@ public void deletePlan(int planId) throws IOException { makeRequest(GQL.DELETE_PLAN, variables); } - public int insertActivityDirective(int planId, String type, String startOffset, JsonObject arguments) throws IOException { + public int insertActivityDirective(int planId, String type, String startOffset, JsonObject arguments, JsonObjectBuilder ...extraArgs) throws IOException { final var insertActivityBuilder = Json.createObjectBuilder() .add("plan_id", planId) .add("type", type) .add("start_offset", startOffset) .add("arguments", arguments); + for (final var extraArg : extraArgs) { + insertActivityBuilder.addAll(extraArg); + } final var variables = Json.createObjectBuilder().add("activityDirectiveInsertInput", insertActivityBuilder).build(); return makeRequest(GQL.CREATE_ACTIVITY_DIRECTIVE, variables).getJsonObject("createActivityDirective").getInt("id"); } diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt index 5ce5a294e8..5655280905 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt @@ -198,6 +198,9 @@ class DefaultEditablePlanDriver( for (d in directives) { // the when block is used to smart-cast d.start to an Anchor. This is basically just an if statement. // Basically we're just iterating through looking for activities anchored to the deleted one. + + // Kotlin doesn't smart cast objects whose origins it can't statically check for race conditions, + // which is why I have to bind `d.start` to a local variable. Then `childStart` can be smart cast. when (val childStart = d.start) { is DirectiveStart.Anchor -> { if (childStart.parentId == directive.id) { diff --git a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java index 4984f3dba5..1bd08b2313 100644 --- a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java +++ b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java @@ -622,9 +622,9 @@ mutation createAllPlanActivityDirectives($activities: [activity_directive_insert //add duration to parameters if controllable final var insertionObjectArguments = Json.createObjectBuilder(); - if(act.getType().getDurationType() instanceof DurationType.Controllable durationType){ - if(!act.arguments().containsKey(durationType.parameterName())){ - insertionObjectArguments.add(durationType.parameterName(), serializedValueP.unparse(schedulerModel.serializeDuration(act.duration()))); + if(act.getType().getDurationType() instanceof DurationType.Controllable(String parameterName)){ + if(!act.arguments().containsKey(parameterName)){ + insertionObjectArguments.add(parameterName, serializedValueP.unparse(schedulerModel.serializeDuration(act.duration()))); } } @@ -718,7 +718,7 @@ private void deleteActivityDirectives( { if (ids.isEmpty()) return; ensurePlanExists(planId); - final var idString = ids.stream().map(String::valueOf).collect(Collectors.joining(",")); + final var idString = ids.stream().map($ -> String.valueOf($.id())).collect(Collectors.joining(",")); final var request = """ mutation deletePlanActivityDirectives($planId: Int! = %d, $directiveIds: [Int!]! = [%s]) { delete_activity_directive(where: {_and: {plan_id: {_eq: $planId}, id: {_in: $directiveIds}}}) { From 21bacc599236f4832a4ea03fb489931fd8c1f937 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Fri, 31 Jan 2025 14:43:10 -0800 Subject: [PATCH 18/22] Reduce complexity of deletion method --- .../utils/DefaultEditablePlanDriver.kt | 53 ++++++++----------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt index 5655280905..9fdd3b5d9b 100644 --- a/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt +++ b/procedural/scheduling/src/main/kotlin/gov/nasa/ammos/aerie/procedural/scheduling/utils/DefaultEditablePlanDriver.kt @@ -6,6 +6,7 @@ import gov.nasa.ammos.aerie.procedural.timeline.collections.Directives import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.AnyDirective import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.Directive import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.DirectiveStart +import gov.nasa.ammos.aerie.procedural.timeline.payloads.activities.DirectiveStart.Anchor import gov.nasa.ammos.aerie.procedural.timeline.plan.Plan import gov.nasa.ammos.aerie.procedural.timeline.plan.SimulationResults import gov.nasa.jpl.aerie.types.ActivityDirectiveId @@ -158,7 +159,7 @@ class DefaultEditablePlanDriver( class ParentSearchException(id: ActivityDirectiveId, size: Int): Exception("Expected one parent activity with id $id, found $size") val id = adapter.generateDirectiveId() val parent = when (val s = directive.start) { - is DirectiveStart.Anchor -> { + is Anchor -> { val parentList = directives() .filter { it.id == s.parentId } .collect(totalBounds()) @@ -195,36 +196,28 @@ class DefaultEditablePlanDriver( } else { directivesToDelete = mutableSetOf(directive) directivesToCreate = mutableSetOf() - for (d in directives) { - // the when block is used to smart-cast d.start to an Anchor. This is basically just an if statement. - // Basically we're just iterating through looking for activities anchored to the deleted one. - - // Kotlin doesn't smart cast objects whose origins it can't statically check for race conditions, - // which is why I have to bind `d.start` to a local variable. Then `childStart` can be smart cast. - when (val childStart = d.start) { - is DirectiveStart.Anchor -> { - if (childStart.parentId == directive.id) { - when (strategy) { - DeletedAnchorStrategy.Error -> throw Exception("Cannot delete an activity that has anchors pointing to it without a ${DeletedAnchorStrategy::class.java.simpleName}") - DeletedAnchorStrategy.PreserveTree -> { - directivesToDelete.add(d) - val start = when (val parentStart = directive.start) { - is DirectiveStart.Absolute -> DirectiveStart.Absolute(parentStart.time + childStart.offset) - is DirectiveStart.Anchor -> DirectiveStart.Anchor( - parentStart.parentId, - parentStart.offset + childStart.offset, - parentStart.anchorPoint, - childStart.estimatedStart - ) - } - directivesToCreate.add(d.copy(start = start)) - } - else -> throw Error("internal error; unreachable") - } + val childActivities = directives.filter { + it.start is Anchor + && (it.start as Anchor).parentId == directive.id + } + for (c in childActivities) { + when (strategy) { + DeletedAnchorStrategy.Error -> throw Exception("Cannot delete an activity that has anchors pointing to it without a ${DeletedAnchorStrategy::class.java.simpleName}") + DeletedAnchorStrategy.PreserveTree -> { + directivesToDelete.add(c) + val cStart = c.start as Anchor // Cannot smart cast + val start = when (val parentStart = directive.start) { + is DirectiveStart.Absolute -> DirectiveStart.Absolute(parentStart.time + cStart.offset) + is Anchor -> Anchor( + parentStart.parentId, + parentStart.offset + cStart.offset, + parentStart.anchorPoint, + cStart.estimatedStart + ) } + directivesToCreate.add(c.copy(start = start)) } - // if d.start wasn't an anchor, do nothing - else -> {} + else -> throw Error("internal error; unreachable") } } } @@ -248,7 +241,7 @@ class DefaultEditablePlanDriver( private fun deleteCascadeRecursive(directive: Directive, allDirectives: Directives): List> { val recurse = allDirectives.collect().flatMap { d -> when (val s = d.start) { - is DirectiveStart.Anchor -> { + is Anchor -> { if (s.parentId == directive.id) deleteCascadeRecursive(d, allDirectives) else listOf() } From d1b6898dc7a382245ca4f41253860475e966368c Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Fri, 31 Jan 2025 17:07:01 -0800 Subject: [PATCH 19/22] Update plan revision in tests --- .../e2e/procedural/scheduling/DatabaseDeletionTests.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DatabaseDeletionTests.java b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DatabaseDeletionTests.java index dbba0e0b4f..7b8d464f7d 100644 --- a/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DatabaseDeletionTests.java +++ b/e2e-tests/src/test/java/gov/nasa/jpl/aerie/e2e/procedural/scheduling/DatabaseDeletionTests.java @@ -52,6 +52,7 @@ void deletesDirectiveAlreadyInDatabase() throws IOException { "1h", JsonValue.EMPTY_JSON_OBJECT ); + hasura.updatePlanRevisionSchedulingSpec(planId); var plan = hasura.getPlan(planId); assertEquals(1, plan.activityDirectives().size()); @@ -84,6 +85,7 @@ void deletesDirectiveInDatabaseWithAnchor() throws IOException { JsonValue.EMPTY_JSON_OBJECT, Json.createObjectBuilder().add("anchor_id", bite) ); + hasura.updatePlanRevisionSchedulingSpec(planId); var plan = hasura.getPlan(planId); var activities = plan.activityDirectives(); @@ -158,6 +160,7 @@ void deletesDirectiveInDatabaseInMiddleOfChain() throws IOException { JsonValue.EMPTY_JSON_OBJECT, Json.createObjectBuilder().add("anchor_id", grow2) ); + hasura.updatePlanRevisionSchedulingSpec(planId); var plan = hasura.getPlan(planId); var activities = plan.activityDirectives(); @@ -217,6 +220,7 @@ void deleteCascadeInDatabase() throws IOException { JsonValue.EMPTY_JSON_OBJECT, Json.createObjectBuilder().add("anchor_id", bite) ); + hasura.updatePlanRevisionSchedulingSpec(planId); var plan = hasura.getPlan(planId); @@ -251,6 +255,7 @@ void deleteErrorInDatabase() throws IOException { JsonValue.EMPTY_JSON_OBJECT, Json.createObjectBuilder().add("anchor_id", bite) ); + hasura.updatePlanRevisionSchedulingSpec(planId); var plan = hasura.getPlan(planId); From 15684cad3517a83bab49771f1692f90b432c458d Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Fri, 31 Jan 2025 17:09:02 -0800 Subject: [PATCH 20/22] Fix query for affected_rows --- .../scheduler/server/services/GraphQLMerlinDatabaseService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java index 1bd08b2313..33daa53cf9 100644 --- a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java +++ b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java @@ -696,7 +696,7 @@ private void modifyActivityDirectives( final var id = mod.id().id(); request.append(""" update_%d: update_activity_directive_by_pk(pk_columns: {id: %d, plan_id: %d}, _set: $activity_%d) { - affected_rows + id } """.formatted(id, id, planId.id(), id)); From 1829f670d289a4075669bb4966b98d837623c2fb Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Mon, 3 Feb 2025 10:55:59 -0800 Subject: [PATCH 21/22] Fix e2e tests --- .../aerie/procedural/timeline/payloads/activities/Directive.kt | 2 +- .../scheduler/server/services/GraphQLMerlinDatabaseService.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/payloads/activities/Directive.kt b/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/payloads/activities/Directive.kt index f0a3a9d01d..4c9732577a 100644 --- a/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/payloads/activities/Directive.kt +++ b/procedural/timeline/src/main/kotlin/gov/nasa/ammos/aerie/procedural/timeline/payloads/activities/Directive.kt @@ -10,7 +10,7 @@ data class Directive( @JvmField val inner: A, /** The name of this specific directive. */ - @JvmField val name: String, + @JvmField val name: String?, /** The directive id. */ @JvmField val id: ActivityDirectiveId, diff --git a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java index 33daa53cf9..7f0c30d2e0 100644 --- a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java +++ b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java @@ -465,7 +465,7 @@ public Map updatePlanActivityDirective for (final var act: toAdd) { if (act.anchorId() != null) { toModify.add(new ActivityModification( - act.id(), + ids.get(act.id()), List.of($ -> $.add("anchor_id", ids.get(act.anchorId()).id())) )); } From 81f23aa3ba9ba6fdb9be2e3987ef4ee6dd82b276 Mon Sep 17 00:00:00 2001 From: JoelCourtney Date: Mon, 3 Feb 2025 15:01:56 -0800 Subject: [PATCH 22/22] Remove schedulingActivity.isNew() --- .../jpl/aerie/scheduler/goals/Procedure.java | 2 +- .../scheduler/model/SchedulingActivity.java | 15 +----- .../simulation/SimulationFacadeUtils.java | 3 +- .../scheduler/solver/PrioritySolver.java | 4 -- .../plan/SchedulerPlanEditAdapter.kt | 5 +- .../aerie/scheduler/LongDurationPlanTest.java | 8 +-- .../aerie/scheduler/PrioritySolverTest.java | 16 +++--- .../scheduler/SchedulingGrounderTest.java | 18 +++---- .../aerie/scheduler/SimulationFacadeTest.java | 14 ++--- .../jpl/aerie/scheduler/TestApplyWhen.java | 54 +++++++++---------- .../aerie/scheduler/TestPersistentAnchor.java | 18 +++---- .../scheduler/TestRecurrenceGoalExtended.java | 16 +++--- .../TestUnsatisfiableCompositeGoals.java | 4 +- .../scheduler/UncontrollableDurationTest.java | 2 +- .../CheckpointSimulationFacadeTest.java | 8 +-- .../GraphQLMerlinDatabaseService.java | 17 ++++-- 16 files changed, 97 insertions(+), 107 deletions(-) diff --git a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java index 79df3b8eb8..b219c8053c 100644 --- a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java +++ b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/goals/Procedure.java @@ -82,7 +82,7 @@ public void run( } for (final var edit : editablePlan.getTotalDiff()) { if (edit instanceof Edit.Create c) { - newActivities.add(toSchedulingActivity(c.getDirective(), lookupActivityType::apply, true)); + newActivities.add(toSchedulingActivity(c.getDirective(), lookupActivityType::apply)); } else if (!(edit instanceof Edit.Delete)) { throw new IllegalStateException("Unexpected edit type: " + edit); } diff --git a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/model/SchedulingActivity.java b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/model/SchedulingActivity.java index b756022cac..364fb6be59 100644 --- a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/model/SchedulingActivity.java +++ b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/model/SchedulingActivity.java @@ -30,7 +30,6 @@ * @param duration the length of time this activity instances lasts for after its start * @param arguments arguments are stored in a String/SerializedValue hashmap. * @param topParent the parent activity if any - * @param isNew whether this activity was created in this scheduling run, or already existed in the plan */ public record SchedulingActivity( ActivityDirectiveId id, @@ -41,7 +40,6 @@ public record SchedulingActivity( ActivityDirectiveId topParent, ActivityDirectiveId anchorId, boolean anchoredToStart, - boolean isNew, String name ) { @@ -51,8 +49,7 @@ public static SchedulingActivity of( Duration startOffset, Duration duration, ActivityDirectiveId anchorId, - boolean anchoredToStart, - boolean isNew + boolean anchoredToStart ) { return new SchedulingActivity( id, @@ -63,7 +60,6 @@ public static SchedulingActivity of( null, anchorId, anchoredToStart, - isNew, null ); } @@ -76,8 +72,7 @@ public static SchedulingActivity of( Map parameters, ActivityDirectiveId topParent, ActivityDirectiveId anchorId, - boolean anchoredToStart, - boolean isNew + boolean anchoredToStart ) { return new SchedulingActivity( id, @@ -88,7 +83,6 @@ public static SchedulingActivity of( topParent, anchorId, anchoredToStart, - isNew, null ); } @@ -103,7 +97,6 @@ public SchedulingActivity withNewDuration(Duration duration){ this.topParent, this.anchorId, this.anchoredToStart, - this.isNew(), this.name ); } @@ -118,7 +111,6 @@ public SchedulingActivity withNewAnchor(ActivityDirectiveId anchorId, boolean an this.topParent, anchorId, anchoredToStart, - this.isNew, this.name ); } @@ -133,7 +125,6 @@ public SchedulingActivity withNewDirectiveId(ActivityDirectiveId id) { this.topParent, this.anchorId, this.anchoredToStart, - this.isNew, this.name ); } @@ -148,7 +139,6 @@ public SchedulingActivity withNewTopParent(ActivityDirectiveId topParent) { topParent, this.anchorId, this.anchoredToStart, - this.isNew, this.name ); } @@ -163,7 +153,6 @@ public static SchedulingActivity fromExistingActivityDirective(ActivityDirective null, activity.anchorId(), activity.anchoredToStart(), - false, null ); } diff --git a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/simulation/SimulationFacadeUtils.java b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/simulation/SimulationFacadeUtils.java index 377dd09300..d18134deff 100644 --- a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/simulation/SimulationFacadeUtils.java +++ b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/simulation/SimulationFacadeUtils.java @@ -98,8 +98,7 @@ public static void updatePlanWithChildActivities( activity.arguments(), rootParent.get(), null, - true, - false + true ); plan.add(activityInstance); } diff --git a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/solver/PrioritySolver.java b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/solver/PrioritySolver.java index a6629d3182..2f1eaee8ef 100644 --- a/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/solver/PrioritySolver.java +++ b/scheduler-driver/src/main/java/gov/nasa/jpl/aerie/scheduler/solver/PrioritySolver.java @@ -1238,7 +1238,6 @@ public Duration valueAt(Duration start, final EquationSolvingAlgorithms.History< null, null, true, - true, null ); Duration computedDuration = null; @@ -1299,7 +1298,6 @@ public Duration valueAt(Duration start, final EquationSolvingAlgorithms.History< instantiatedArguments, null, null, - true, true )); } else if (activityExpression.type().getDurationType() instanceof DurationType.Fixed dt) { @@ -1323,7 +1321,6 @@ public Duration valueAt(Duration start, final EquationSolvingAlgorithms.History< activityExpression.type()), null, null, - true, true )); } else if (activityExpression.type().getDurationType() instanceof DurationType.Parametric dt) { @@ -1349,7 +1346,6 @@ public Duration valueAt(final Duration start, final EquationSolvingAlgorithms.Hi instantiatedArgs, null, null, - true, true ); history.add(new EquationSolvingAlgorithms.FunctionCoordinate<>(start, start.plus(duration)), new ActivityMetadata(activity)); diff --git a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerPlanEditAdapter.kt b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerPlanEditAdapter.kt index aa942396bf..6528bcac1f 100644 --- a/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerPlanEditAdapter.kt +++ b/scheduler-driver/src/main/kotlin/gov/nasa/jpl/aerie/scheduler/plan/SchedulerPlanEditAdapter.kt @@ -36,7 +36,7 @@ data class SchedulerPlanEditAdapter( } override fun create(directive: Directive) { - plan.add(directive.toSchedulingActivity(lookupActivityType, true)) + plan.add(directive.toSchedulingActivity(lookupActivityType)) } override fun delete(id: ActivityDirectiveId) { @@ -53,7 +53,7 @@ data class SchedulerPlanEditAdapter( } companion object { - @JvmStatic fun Directive.toSchedulingActivity(lookupActivityType: (String) -> ActivityType, isNew: Boolean) = SchedulingActivity( + @JvmStatic fun Directive.toSchedulingActivity(lookupActivityType: (String) -> ActivityType) = SchedulingActivity( id, lookupActivityType(type), when (val s = start) { @@ -82,7 +82,6 @@ data class SchedulerPlanEditAdapter( is DirectiveStart.Absolute -> true is DirectiveStart.Anchor -> s.anchorPoint == DirectiveStart.Anchor.AnchorPoint.Start }, - isNew, name ) } diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/LongDurationPlanTest.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/LongDurationPlanTest.java index 13cb2f985c..4ed3375a56 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/LongDurationPlanTest.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/LongDurationPlanTest.java @@ -41,10 +41,10 @@ private static PlanInMemory makePlanA012(Problem problem) { final var plan = new PlanInMemory(); final var actTypeA = problem.getActivityType("GrowBanana"); final var idGenerator = new DirectiveIdGenerator(0); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t0, d1min, null, true, false)); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t1year, d1min, null, true, false)); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t2year, d1min, null, true, false)); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t3year, d1min, null, true, false)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t0, d1min, null, true)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t1year, d1min, null, true)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t2year, d1min, null, true)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t3year, d1min, null, true)); return plan; } diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/PrioritySolverTest.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/PrioritySolverTest.java index 6272b58c81..9b0f5308b9 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/PrioritySolverTest.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/PrioritySolverTest.java @@ -107,26 +107,26 @@ private static Problem makeTestMissionAB() { private static PlanInMemory makePlanA012(Problem problem) { final var plan = new PlanInMemory(); final var actTypeA = problem.getActivityType("ControllableDurationActivity"); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t0, d1min, null, true, false)); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t1hr, d1min, null, true, false)); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t2hr, d1min, null, true, false)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t0, d1min, null, true)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t1hr, d1min, null, true)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t2hr, d1min, null, true)); return plan; } private static PlanInMemory makePlanA12(Problem problem) { final var plan = new PlanInMemory(); final var actTypeA = problem.getActivityType("ControllableDurationActivity"); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t1hr, d1min, null, true, false)); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t2hr, d1min, null, true, false)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t1hr, d1min, null, true)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t2hr, d1min, null, true)); return plan; } private static PlanInMemory makePlanAB012(Problem problem) { final var plan = makePlanA012(problem); final var actTypeB = problem.getActivityType("OtherControllableDurationActivity"); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeB, t0, d1min, null, true, false)); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeB, t1hr, d1min, null, true, false)); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeB, t2hr, d1min, null, true, false)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeB, t0, d1min, null, true)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeB, t1hr, d1min, null, true)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeB, t2hr, d1min, null, true)); return plan; } diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/SchedulingGrounderTest.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/SchedulingGrounderTest.java index 6d5e425ab4..8e2915516f 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/SchedulingGrounderTest.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/SchedulingGrounderTest.java @@ -31,9 +31,9 @@ public void testChainAnchors(){ final var id1 = new ActivityDirectiveId(1); final var id2 = new ActivityDirectiveId(2); final var id3 = new ActivityDirectiveId(3); - final var act1 = SchedulingActivity.of(id1, at, t0, d1min, null, true, false); - final var act2 = SchedulingActivity.of(id2, at, t1hr, d1min, act1.id(), false, false); - final var act3 = SchedulingActivity.of(id3, at, t2hr, d1min, act2.id(), false, false); + final var act1 = SchedulingActivity.of(id1, at, t0, d1min, null, true); + final var act2 = SchedulingActivity.of(id2, at, t1hr, d1min, act1.id(), false); + final var act3 = SchedulingActivity.of(id3, at, t2hr, d1min, act2.id(), false); final var acts = List.of(act1, act3, act2); final var result = SchedulePlanGrounder.groundSchedule(acts, h.getEndAerie()); //act 1 should start at 0 min into the plan @@ -51,7 +51,7 @@ public void testChainAnchors(){ public void testEmptyDueToEmptyDuration(){ final var at = new ActivityType("at"); final var id1 = new ActivityDirectiveId(1); - final var act1 = SchedulingActivity.of(id1, at, t0, null, null, true, false); + final var act1 = SchedulingActivity.of(id1, at, t0, null, null, true); final var result = SchedulePlanGrounder.groundSchedule(List.of(act1), h.getEndAerie()); assertTrue(result.isEmpty()); } @@ -60,7 +60,7 @@ public void testEmptyDueToEmptyDuration(){ public void testAnchoredToPlanEnd(){ final var at = new ActivityType("at"); final var id1 = new ActivityDirectiveId(1); - final var act1 = SchedulingActivity.of(id1, at, Duration.negate(d1hr), d1min, null, false, false); + final var act1 = SchedulingActivity.of(id1, at, Duration.negate(d1hr), d1min, null, false); final var result = SchedulePlanGrounder.groundSchedule(List.of(act1), h.getEndAerie()); final var act1expt = new ActivityInstance(id1.id(), at.getName(), Map.of(), Interval.between(h.getEndAerie().minus(d1hr), h.getEndAerie().minus(d1hr).plus(d1min)), Optional.of(id1)); assertEquals(act1expt, result.get().get(0)); @@ -72,8 +72,8 @@ public void noAnchor(){ final var at = new ActivityType("at"); final var id1 = new ActivityDirectiveId(1); final var id2 = new ActivityDirectiveId(2); - final var act1 = SchedulingActivity.of(id1, at, t0, d1min, null, true, false); - final var act2 = SchedulingActivity.of(id2, at, t1hr, d1min, null, true, false); + final var act1 = SchedulingActivity.of(id1, at, t0, d1min, null, true); + final var act2 = SchedulingActivity.of(id2, at, t1hr, d1min, null, true); final var result = SchedulePlanGrounder.groundSchedule(List.of(act1, act2), h.getEndAerie()); final var act1expt = new ActivityInstance(id1.id(), at.getName(), Map.of(), Interval.between(t0, t0.plus(d1min)), Optional.of(id1)); final var act2expt = new ActivityInstance(id2.id(), at.getName(), Map.of(), Interval.between(t1hr, t1hr.plus(d1min)), Optional.of(id2)); @@ -86,8 +86,8 @@ public void startTimeAnchor(){ final var at = new ActivityType("at"); final var id1 = new ActivityDirectiveId(1); final var id2 = new ActivityDirectiveId(2); - final var act1 = SchedulingActivity.of(id1, at, t1hr, d1min, null, true, false); - final var act2 = SchedulingActivity.of(id2, at, t1hr, d1min, act1.id(), true, false); + final var act1 = SchedulingActivity.of(id1, at, t1hr, d1min, null, true); + final var act2 = SchedulingActivity.of(id2, at, t1hr, d1min, act1.id(), true); final var result = SchedulePlanGrounder.groundSchedule(List.of(act1, act2), h.getEndAerie()); final var act1expt = new ActivityInstance(id1.id(), at.getName(), Map.of(), Interval.between(t1hr, t1hr.plus(d1min)), Optional.of(id1)); final var act2expt = new ActivityInstance(id2.id(), at.getName(), Map.of(), Interval.between(t2hr, t2hr.plus(d1min)), Optional.of(id2)); diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/SimulationFacadeTest.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/SimulationFacadeTest.java index cccea1c0b2..211f27dbef 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/SimulationFacadeTest.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/SimulationFacadeTest.java @@ -116,10 +116,10 @@ private PlanInMemory makeTestPlanP0B1() { final var actTypeBite = problem.getActivityType("BiteBanana"); final var actTypePeel = problem.getActivityType("PeelBanana"); - var act1 = SchedulingActivity.of(idGenerator.next(), actTypePeel, t1, null, Map.of("peelDirection", SerializedValue.of("fromStem")), null, null, true, false); + var act1 = SchedulingActivity.of(idGenerator.next(), actTypePeel, t1, null, Map.of("peelDirection", SerializedValue.of("fromStem")), null, null, true); plan.add(act1); - var act2 = SchedulingActivity.of(idGenerator.next(), actTypeBite, t2, null, Map.of("biteSize", SerializedValue.of(0.1)), null, null, true, false); + var act2 = SchedulingActivity.of(idGenerator.next(), actTypeBite, t2, null, Map.of("biteSize", SerializedValue.of(0.1)), null, null, true); plan.add(act2); return plan; @@ -131,7 +131,7 @@ public void associationToExistingSatisfyingActivity() throws SchedulingInterrupt final var actTypePeel = problem.getActivityType("PeelBanana"); final var actTypeBite = problem.getActivityType("BiteBanana"); - var act1 = SchedulingActivity.of(idGenerator.next(), actTypePeel, t1, t2, Map.of("peelDirection", SerializedValue.of("fromStem")), null, null, true, false); + var act1 = SchedulingActivity.of(idGenerator.next(), actTypePeel, t1, t2, Map.of("peelDirection", SerializedValue.of("fromStem")), null, null, true); plan.add(act1); final var goal = new CoexistenceGoal.Builder() @@ -284,10 +284,10 @@ public void testProceduralGoalWithResourceConstraint() throws SchedulingInterrup final var actTypePeel = problem.getActivityType("PeelBanana"); SchedulingActivity act1 = SchedulingActivity.of(idGenerator.next(), actTypePeel, - t0, Duration.ZERO, Map.of(), null, null, true, false); + t0, Duration.ZERO, Map.of(), null, null, true); SchedulingActivity act2 = SchedulingActivity.of(idGenerator.next(), actTypePeel, - t2, Duration.ZERO, Map.of(), null, null, true, false); + t2, Duration.ZERO, Map.of(), null, null, true); //create an "external tool" that insists on a few fixed activities final var externalActs = java.util.List.of( @@ -326,10 +326,10 @@ public void testActivityTypeWithResourceConstraint() throws SchedulingInterrupte actTypePeel.setResourceConstraint(constraint); SchedulingActivity act1 = SchedulingActivity.of(idGenerator.next(), actTypePeel, - t0, Duration.ZERO, Map.of(), null, null, true, false); + t0, Duration.ZERO, Map.of(), null, null, true); SchedulingActivity act2 = SchedulingActivity.of(idGenerator.next(), actTypePeel, - t2, Duration.ZERO, Map.of(), null, null, true, false); + t2, Duration.ZERO, Map.of(), null, null, true); //create an "external tool" that insists on a few fixed activities final var externalActs = java.util.List.of( diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestApplyWhen.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestApplyWhen.java index a3e0f4721f..5b25280f19 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestApplyWhen.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestApplyWhen.java @@ -628,9 +628,9 @@ public void testCoexistenceWindowCutoff() throws SchedulingInterruptedException // create a PlanInMemory, add ActivityInstances PlanInMemory partialPlan = new PlanInMemory(); final var actTypeA = problem.getActivityType("ControllableDurationActivity"); - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(5, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, start at start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(11, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 11s after start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(16, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 16s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(5, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, start at start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(11, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 11s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(16, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 16s after start // pass this plan as initialPlan to Problem object problem.setInitialPlan(partialPlan); @@ -673,9 +673,9 @@ public void testCoexistenceJustFits() throws SchedulingInterruptedException { // create a PlanInMemory, add ActivityInstances PlanInMemory partialPlan = new PlanInMemory(); final var actTypeA = problem.getActivityType("ControllableDurationActivity"); - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(5, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, start at start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(11, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 11s after start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(16, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 16s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(5, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, start at start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(11, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 11s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(16, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 16s after start // pass this plan as initialPlan to Problem object problem.setInitialPlan(partialPlan); @@ -727,9 +727,9 @@ public void testCoexistenceUncontrollableCutoff() throws SchedulingInterruptedEx // create a PlanInMemory, add ActivityInstances PlanInMemory partialPlan = new PlanInMemory(); final var actTypeA = problem.getActivityType("ControllableDurationActivity"); - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(5, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, start at start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(11, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 11s after start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(16, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 16s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(5, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, start at start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(11, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 11s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(16, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 16s after start // pass this plan as initialPlan to Problem object problem.setInitialPlan(partialPlan); @@ -778,10 +778,10 @@ public void testCoexistenceWindows() throws SchedulingInterruptedException { // create a PlanInMemory, add ActivityInstances PlanInMemory partialPlan = new PlanInMemory(); final var actTypeA = problem.getActivityType("ControllableDurationActivity"); - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(1, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, start at start. NOTE: must start at time=1, not time=0, else test fails. - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(8, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 11s after start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(14, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 16s after start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(19, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 16s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(1, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, start at start. NOTE: must start at time=1, not time=0, else test fails. + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(8, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 11s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(14, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 16s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(19, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 16s after start // pass this plan as initialPlan to Problem object @@ -841,11 +841,11 @@ public void testCoexistenceWindowsCutoffMidActivity() throws SchedulingInterrupt // create a PlanInMemory, add ActivityInstances PlanInMemory partialPlan = new PlanInMemory(); final var actTypeA = problem.getActivityType("ControllableDurationActivity"); - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(1, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, start at start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(7, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 11s after start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(14, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 16s after start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(19, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 16s after start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(25, Duration.SECONDS)), Duration.of(2, Duration.SECONDS), null, true, false)); //create an activity that's 2 seconds long, 25s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(1, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, start at start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(7, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 11s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(14, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 16s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(19, Duration.SECONDS)), Duration.of(4, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 16s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(25, Duration.SECONDS)), Duration.of(2, Duration.SECONDS), null, true)); //create an activity that's 2 seconds long, 25s after start // pass this plan as initialPlan to Problem object @@ -916,8 +916,8 @@ public void testCoexistenceWindowsBisect() throws SchedulingInterruptedException // create a PlanInMemory, add ActivityInstances PlanInMemory partialPlan = new PlanInMemory(); final var actTypeA = problem.getActivityType("ControllableDurationActivity"); - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(4, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, start at start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(8, Duration.SECONDS)), Duration.of(3, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 11s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(4, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, start at start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(8, Duration.SECONDS)), Duration.of(3, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 11s after start // pass this plan as initialPlan to Problem object problem.setInitialPlan(partialPlan); @@ -980,7 +980,7 @@ public void testCoexistenceWindowsBisect2() throws SchedulingInterruptedExceptio // create a PlanInMemory, add ActivityInstances PlanInMemory partialPlan = new PlanInMemory(); final var actTypeA = problem.getActivityType("ControllableDurationActivity"); - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(13, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, start at start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(13, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, start at start // pass this plan as initialPlan to Problem object problem.setInitialPlan(partialPlan); @@ -1038,9 +1038,9 @@ public void testCoexistenceUncontrollableJustFits() throws SchedulingInterrupted // create a PlanInMemory, add ActivityInstances PlanInMemory partialPlan = new PlanInMemory(); final var actTypeA = problem.getActivityType("ControllableDurationActivity"); - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(5, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, start at start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(11, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 11s after start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(16, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true, false)); //create an activity that's 5 seconds long, 16s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(5, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, start at start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(11, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 11s after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(16, Duration.SECONDS)), Duration.of(5, Duration.SECONDS), null, true)); //create an activity that's 5 seconds long, 16s after start // pass this plan as initialPlan to Problem object problem.setInitialPlan(partialPlan); @@ -1153,9 +1153,9 @@ public void testCoexistenceWithAnchors() throws SchedulingInterruptedException { final var partialPlan = new PlanInMemory(); final var actTypeA = problem.getActivityType("GrowBanana"); final var actTypeB = problem.getActivityType("PickBanana"); - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(3, Duration.HOURS), null, true, false)); - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(5, Duration.HOURS)), Duration.of(3, Duration.HOURS), null, true, false)); //create an activity that's 5 hours long, start 5 hours after start - partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(10, Duration.HOURS)), Duration.of(3, Duration.HOURS), null, true, false)); //create an activity that's 5 seconds long, starts 10 hours after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(3, Duration.HOURS), null, true)); + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(5, Duration.HOURS)), Duration.of(3, Duration.HOURS), null, true)); //create an activity that's 5 hours long, start 5 hours after start + partialPlan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(10, Duration.HOURS)), Duration.of(3, Duration.HOURS), null, true)); //create an activity that's 5 seconds long, starts 10 hours after start // pass this plan as initialPlan to Problem object problem.setInitialPlan(partialPlan); diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestPersistentAnchor.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestPersistentAnchor.java index 2e022cfa57..1c984d04f8 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestPersistentAnchor.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestPersistentAnchor.java @@ -458,21 +458,21 @@ public TestData createTestCaseStartsAt(final PersistentTimeAnchor persistentAnch SchedulingActivity act1 = SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie(), Duration.of(activityDurationHours, Duration.HOURS), Map.of( "quantity", SerializedValue.of(1), "growingDuration", SerializedValue.of(Duration.HOUR.times(activityDurationHours).in(Duration.HOURS)) - ), null, null, true, false); + ), null, null, true); partialPlan.add(act1); actsToBeAnchored.add(act1); SchedulingActivity act2 = SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(5, Duration.HOURS)), Duration.of(activityDurationHours, Duration.HOURS), Map.of( "quantity", SerializedValue.of(1), "growingDuration", SerializedValue.of(Duration.HOUR.times(activityDurationHours).in(Duration.HOURS)) - ), null, null, true, false); + ), null, null, true); partialPlan.add(act2); actsToBeAnchored.add(act2); SchedulingActivity act3 = SchedulingActivity.of(idGenerator.next(), actTypeA, planningHorizon.getStartAerie().plus(Duration.of(10, Duration.HOURS)), Duration.of(activityDurationHours, Duration.HOURS), Map.of( "quantity", SerializedValue.of(1), "growingDuration", SerializedValue.of(Duration.HOUR.times(activityDurationHours).in(Duration.HOURS)) - ), null, null, true, false); + ), null, null, true); partialPlan.add(act3); actsToBeAnchored.add(act3); @@ -499,17 +499,17 @@ public TestData createTestCaseStartsAt(final PersistentTimeAnchor persistentAnch if(missingActAssociationsWithAnchor){ // Activities with anchors SchedulingActivity act4 = SchedulingActivity.of(idGenerator.next(), actTypeB, relativeOffset, Duration.of(activityDurationHours, Duration.HOURS), Map.of( - "quantity", SerializedValue.of(1)), null, act1.id(), anchoredToStart, false); + "quantity", SerializedValue.of(1)), null, act1.id(), anchoredToStart); partialPlan.add(act4); templateActsAlreadyAnchor.add(act4); SchedulingActivity act5 = SchedulingActivity.of(idGenerator.next(), actTypeB, relativeOffset, Duration.of(activityDurationHours, Duration.HOURS), Map.of( - "quantity", SerializedValue.of(1)), null, act2.id(), anchoredToStart, false); + "quantity", SerializedValue.of(1)), null, act2.id(), anchoredToStart); partialPlan.add(act5); templateActsAlreadyAnchor.add(act5); SchedulingActivity act6 = SchedulingActivity.of(idGenerator.next(), actTypeB, relativeOffset, Duration.of(activityDurationHours, Duration.HOURS), Map.of( - "quantity", SerializedValue.of(1)), null, act3.id(), anchoredToStart, false); + "quantity", SerializedValue.of(1)), null, act3.id(), anchoredToStart); partialPlan.add(act6); templateActsAlreadyAnchor.add(act6); } @@ -517,15 +517,15 @@ public TestData createTestCaseStartsAt(final PersistentTimeAnchor persistentAnch if(missingActAssociationsWithoutAnchor){ // Activities without anchors SchedulingActivity act7 = SchedulingActivity.of(idGenerator.next(), actTypeB, planningHorizon.getStartAerie().plus(offsetWithDuration), Duration.of(activityDurationHours, Duration.HOURS), Map.of( - "quantity", SerializedValue.of(1)), null, null, anchoredToStart, false); + "quantity", SerializedValue.of(1)), null, null, anchoredToStart); partialPlan.add(act7); SchedulingActivity act8 = SchedulingActivity.of(idGenerator.next(), actTypeB, planningHorizon.getStartAerie().plus(Duration.of(5, Duration.HOURS)).plus(offsetWithDuration), Duration.of(activityDurationHours, Duration.HOURS), Map.of( - "quantity", SerializedValue.of(1)), null, null, anchoredToStart, false); + "quantity", SerializedValue.of(1)), null, null, anchoredToStart); partialPlan.add(act8); SchedulingActivity act9 = SchedulingActivity.of(idGenerator.next(), actTypeB, planningHorizon.getStartAerie().plus(Duration.of(10, Duration.HOURS)).plus(offsetWithDuration), Duration.of(activityDurationHours, Duration.HOURS), Map.of( - "quantity", SerializedValue.of(1)), null, null, anchoredToStart, false); + "quantity", SerializedValue.of(1)), null, null, anchoredToStart); partialPlan.add(act9); if (!persistentAnchor.equals(PersistentTimeAnchor.DISABLED) && !missingActAssociationsWithAnchor) { diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestRecurrenceGoalExtended.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestRecurrenceGoalExtended.java index b64fbdea98..0fe839ffc8 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestRecurrenceGoalExtended.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestRecurrenceGoalExtended.java @@ -241,8 +241,8 @@ public void incompletePlan() throws SchedulingInterruptedException { .build(); problem.setGoals(List.of(goal)); final var initialPlan = new PlanInMemory(); - initialPlan.add(SchedulingActivity.of(idGenerator.next(), activityType, Duration.ZERO, Duration.of(2, SECONDS), null, true, false)); - initialPlan.add(SchedulingActivity.of(idGenerator.next(), activityType, SECONDS.times(10), Duration.of(2, SECONDS), null, true, false)); + initialPlan.add(SchedulingActivity.of(idGenerator.next(), activityType, Duration.ZERO, Duration.of(2, SECONDS), null, true)); + initialPlan.add(SchedulingActivity.of(idGenerator.next(), activityType, SECONDS.times(10), Duration.of(2, SECONDS), null, true)); problem.setInitialPlan(initialPlan); final var solver = new PrioritySolver(problem); var plan = solver.getNextSolution().orElseThrow(); @@ -271,9 +271,9 @@ public void incompletePlanWithMutex() throws SchedulingInterruptedException { .build(); problem.setGoals(List.of(goal)); final var initialPlan = new PlanInMemory(); - initialPlan.add(SchedulingActivity.of(idGenerator.next(), controllableActivity, Duration.ZERO, Duration.of(2, SECONDS), null, true, false)); - initialPlan.add(SchedulingActivity.of(idGenerator.next(), controllableActivity, SECONDS.times(10), Duration.of(2, SECONDS), null, true, false)); - initialPlan.add(SchedulingActivity.of(idGenerator.next(), basicActivity, SECONDS.times(5), Duration.of(2, SECONDS), null, true, false)); + initialPlan.add(SchedulingActivity.of(idGenerator.next(), controllableActivity, Duration.ZERO, Duration.of(2, SECONDS), null, true)); + initialPlan.add(SchedulingActivity.of(idGenerator.next(), controllableActivity, SECONDS.times(10), Duration.of(2, SECONDS), null, true)); + initialPlan.add(SchedulingActivity.of(idGenerator.next(), basicActivity, SECONDS.times(5), Duration.of(2, SECONDS), null, true)); problem.setInitialPlan(initialPlan); createMutex(controllableActivity, basicActivity).forEach(problem::add); final var solver = new PrioritySolver(problem); @@ -307,7 +307,7 @@ public void flexibilityWithMutex() throws SchedulingInterruptedException { .build(); problem.setGoals(List.of(goal)); final var initialPlan = new PlanInMemory(); - initialPlan.add(SchedulingActivity.of(idGenerator.next(), basicActivity, SECONDS.times(8), Duration.of(4, SECONDS), null, true, false)); + initialPlan.add(SchedulingActivity.of(idGenerator.next(), basicActivity, SECONDS.times(8), Duration.of(4, SECONDS), null, true)); problem.setInitialPlan(initialPlan); createMutex(controllableActivity, basicActivity).forEach(problem::add); final var solver = new PrioritySolver(problem); @@ -342,7 +342,7 @@ public void flexibilityWithMutex2() throws SchedulingInterruptedException { .build(); problem.setGoals(List.of(goal)); final var initialPlan = new PlanInMemory(); - initialPlan.add(SchedulingActivity.of(idGenerator.next(), basicActivity, SECONDS.times(13), Duration.of(3, SECONDS), null, true, false)); + initialPlan.add(SchedulingActivity.of(idGenerator.next(), basicActivity, SECONDS.times(13), Duration.of(3, SECONDS), null, true)); problem.setInitialPlan(initialPlan); createMutex(controllableActivity, basicActivity).forEach(problem::add); final var solver = new PrioritySolver(problem); @@ -377,7 +377,7 @@ public void unsolvableRecurrence() throws SchedulingInterruptedException { .build(); problem.setGoals(List.of(goal)); final var initialPlan = new PlanInMemory(); - initialPlan.add(SchedulingActivity.of(idGenerator.next(), basicActivity, SECONDS.times(7), Duration.of(7, SECONDS), null, true, false)); + initialPlan.add(SchedulingActivity.of(idGenerator.next(), basicActivity, SECONDS.times(7), Duration.of(7, SECONDS), null, true)); problem.setInitialPlan(initialPlan); createMutex(controllableActivity, basicActivity).forEach(problem::add); final var solver = new PrioritySolver(problem); diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestUnsatisfiableCompositeGoals.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestUnsatisfiableCompositeGoals.java index d2802cf614..eef3958101 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestUnsatisfiableCompositeGoals.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/TestUnsatisfiableCompositeGoals.java @@ -57,8 +57,8 @@ private static Problem makeTestMissionABWithCache() { private static PlanInMemory makePlanA12(Problem problem) { final var plan = new PlanInMemory(); final var actTypeA = problem.getActivityType("ControllableDurationActivity"); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t1hr, d1min, null, true, false)); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t2hr, d1min, null, true, false)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t1hr, d1min, null, true)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t2hr, d1min, null, true)); return plan; } diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/UncontrollableDurationTest.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/UncontrollableDurationTest.java index 9487ebefd2..2881a84aa0 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/UncontrollableDurationTest.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/UncontrollableDurationTest.java @@ -152,7 +152,7 @@ public void testTimeDependent() throws SchedulingInterruptedException { public void testBug() throws SchedulingInterruptedException { final var controllableDurationActivity = SchedulingActivity.of(idGenerator.next(), problem.getActivityType("ControllableDurationActivity"), Duration.of(1, Duration.MICROSECONDS), - Duration.of(3, Duration.MICROSECONDS), null, true, false); + Duration.of(3, Duration.MICROSECONDS), null, true); final var zeroDurationUncontrollableActivity = new ActivityExpression.Builder() .ofType(problem.getActivityType("ZeroDurationUncontrollableActivity")) diff --git a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/simulation/CheckpointSimulationFacadeTest.java b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/simulation/CheckpointSimulationFacadeTest.java index 92d213c63a..9a33e562e6 100644 --- a/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/simulation/CheckpointSimulationFacadeTest.java +++ b/scheduler-driver/src/test/java/gov/nasa/jpl/aerie/scheduler/simulation/CheckpointSimulationFacadeTest.java @@ -35,9 +35,9 @@ public class CheckpointSimulationFacadeTest { private static PlanInMemory makePlanA012(Map activityTypeMap) { final var plan = new PlanInMemory(); final var actTypeA = activityTypeMap.get("BasicActivity"); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t0, null, null, true, false)); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t1hr, null, null, true, false)); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t2hr, null, null, true, false)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t0, null, null, true)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t1hr, null, null, true)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t2hr, null, null, true)); return plan; } @BeforeEach @@ -103,7 +103,7 @@ public void testStopsAtEndOfPlanningHorizon() { final var plan = new PlanInMemory(); final var actTypeA = activityTypes.get("ControllableDurationActivity"); - plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t0, HOUR.times(200), null, true, false)); + plan.add(SchedulingActivity.of(idGenerator.next(), actTypeA, t0, HOUR.times(200), null, true)); final var results = newSimulationFacade.simulateNoResultsAllActivities(plan).computeResults(); assertEquals(H.getEndAerie(), results.duration); assert(results.unfinishedActivities.size() == 1); diff --git a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java index 7f0c30d2e0..230a308d6d 100644 --- a/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java +++ b/scheduler-server/src/main/java/gov/nasa/jpl/aerie/scheduler/server/services/GraphQLMerlinDatabaseService.java @@ -429,8 +429,9 @@ public Map updatePlanActivityDirective final var toModify = new ArrayList(); for (final var activity : plan.getActivities()) { if(activity.getParentActivity().isPresent()) continue; // Skip generated activities - if (!activity.isNew()) { - final var actFromInitialPlan = initialPlan.getActivityById(activity.id()).get(); + final var actFromInitialPlanOptional = initialPlan.getActivityById(activity.id()); + if (actFromInitialPlanOptional.isPresent()) { + final var actFromInitialPlan = actFromInitialPlanOptional.get(); //if act was present in initial plan final var activityDirectiveFromSchedulingDirective = new ActivityDirective( activity.startOffset(), @@ -505,9 +506,15 @@ private List generateModification(final ActivityDirective old } if (newState.anchorId() != oldState.anchorId()) { - operations.add( - $ -> $.add("anchor_id", newState.anchorId().id()) - ); + if (newState.anchorId() != null) { + operations.add( + $ -> $.add("anchor_id", newState.anchorId().id()) + ); + } else { + operations.add( + $ -> $.addNull("anchor_id") + ); + } } if (newState.anchoredToStart() != oldState.anchoredToStart()) {