From 8d6e68bc4c94d03c4042d467816496c65e2db8b4 Mon Sep 17 00:00:00 2001 From: Kristian Bendiksen Date: Mon, 26 Oct 2026 14:19:29 +0100 Subject: [PATCH 1/3] Workflow UI: Support conditional branches Support taskmaestro conditional branches in the workflow editor: - Read and write `when` guards and `first_of` alternatives in workflow definitions - Validate guards, switch branch coverage and alternative types - Draw switch tasks as diamonds, guards as control edges and skipped tasks greyed out - Edit guards with Run When..., add alternatives when dropping on a connected input - Report skipped tasks from the run helper --- .../Workflow/RimWorkflowDefinition.cpp | 18 +- .../Workflow/RimWorkflowDefinition.h | 44 +- .../Workflow/RimWorkflowDefinitionTools.cpp | 715 ++++++++++- .../Workflow/RimWorkflowDefinitionTools.h | 22 +- .../Workflow/RimWorkflowDescribeTools.cpp | 61 +- .../Workflow/RimWorkflowFieldBinding.cpp | 16 + .../Workflow/RimWorkflowFieldBinding.h | 2 + .../Workflow/RimWorkflowJob.cpp | 8 +- .../Workflow/RimWorkflowJob.h | 1 + .../Workflow/RimWorkflowPortCompatibility.cpp | 31 +- .../Workflow/RimWorkflowPortCompatibility.h | 1 + .../Workflow/RimWorkflowSchemaTools.cpp | 63 +- .../Workflow/RimWorkflowSchemaTools.h | 6 + .../Workflow/RimWorkflowStringBinding.cpp | 48 +- .../Workflow/RimWorkflowStringBinding.h | 2 + .../Workflow/RimWorkflowTaskInput.cpp | 18 +- .../UnitTests/RimWorkflowDefinition-Test.cpp | 340 ++++++ .../RimWorkflowDescribeTools-Test.cpp | 46 + .../UnitTests/RimWorkflowJobSync-Test.cpp | 54 + .../RimWorkflowDefinition/conditional.json | 1084 +++++++++++++++++ .../UserInterface/RiuWorkflowEditorWidget.cpp | 293 ++++- .../UserInterface/RiuWorkflowEditorWidget.h | 3 + .../UserInterface/RiuWorkflowGraphView.cpp | 308 ++++- .../UserInterface/RiuWorkflowGraphView.h | 4 +- .../UserInterface/RiuWorkflowJobRunner.cpp | 4 +- .../Python/rips/taskmaestro_helper/run.py | 13 +- .../conditional/input.yaml | 1 + .../conditional/workflow.yaml | 33 + .../tm_helper_conditional.py | 51 + .../rips/tests/test_taskmaestro_helper.py | 61 + .../test_taskmaestro_helper_definition.py | 70 ++ README-experimental-workflow-ui.md | 17 + docs/workflow-ui-design.md | 59 + 33 files changed, 3402 insertions(+), 95 deletions(-) create mode 100644 ApplicationLibCode/UnitTests/TestData/RimWorkflowDefinition/conditional.json create mode 100644 GrpcInterface/Python/rips/tests/test_data/taskmaestro_workflows/conditional/input.yaml create mode 100644 GrpcInterface/Python/rips/tests/test_data/taskmaestro_workflows/conditional/workflow.yaml create mode 100644 GrpcInterface/Python/rips/tests/test_data/taskmaestro_workflows/tm_helper_conditional.py diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinition.cpp b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinition.cpp index f29bf4ec6e..ad5f8c4635 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinition.cpp +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinition.cpp @@ -20,6 +20,14 @@ #include "RimWorkflowPortCompatibility.h" +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +bool RimWorkflowTaskGuard::isSwitchBranch() const +{ + return field.isEmpty() && !malformedRef; +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- @@ -41,7 +49,15 @@ bool RimWorkflowDefinitionEdge::isWholeOutput() const //-------------------------------------------------------------------------------------------------- bool RimWorkflowDefinitionEdge::isCollected() const { - return collect != Collect::None; + return collect == Collect::List || collect == Collect::Dict; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +bool RimWorkflowDefinitionEdge::isFirstOf() const +{ + return collect == Collect::FirstOf; } //-------------------------------------------------------------------------------------------------- diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinition.h b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinition.h index f182544bf3..acaa06932a 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinition.h +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinition.h @@ -18,6 +18,7 @@ #pragma once +#include #include #include #include @@ -41,6 +42,40 @@ struct RimWorkflowTaskMap bool operator==( const RimWorkflowTaskMap& other ) const = default; }; +//================================================================================================== +/// A run condition (`when:` in YAML). The guarded task runs only when the condition holds and is +/// skipped otherwise. An empty field refers to the `branch` output of the switch task `source`. +/// `Case` and `In` match any of `values`; `Equals` matches its single value; `Otherwise` matches +/// the switch branches no other guard claims. The flags keep the YAML form for a lossless save. +//================================================================================================== +struct RimWorkflowTaskGuard +{ + enum class Op + { + Case, + Otherwise, + Equals, + In, + Truthy, + Falsy + }; + + QString source{}; + QString field{}; + Op op = Op::Case; + QJsonArray values{}; + + // `case: [a, b]` rather than `case: a` + bool caseAsList = false; + + // A `ref` array that is not a [task, field] pair; reported by validation + bool malformedRef = false; + + bool isSwitchBranch() const; + + bool operator==( const RimWorkflowTaskGuard& other ) const = default; +}; + //================================================================================================== /// One task instance in a workflow //================================================================================================== @@ -55,6 +90,8 @@ struct RimWorkflowDefinitionNode std::optional map{}; + std::optional when{}; + // Keys of the YAML entry the editor does not model (nested workflow); kept for read-only display QJsonObject extra{}; @@ -64,7 +101,8 @@ struct RimWorkflowDefinitionNode //================================================================================================== /// A connection between two tasks. An empty output or input means the whole model. /// Several edges can collect outputs into one `list[T]` or `dict[str, T]` input (`collect:` in -/// YAML). List members are ordered like the edges; dict members have a key. +/// YAML). List members are ordered like the edges; dict members have a key. isCollected() is true +/// for list and dict members only. `FirstOf` edges are alternatives for one input: the first member whose task ran provides it. //================================================================================================== struct RimWorkflowDefinitionEdge { @@ -72,7 +110,8 @@ struct RimWorkflowDefinitionEdge { None, List, - Dict + Dict, + FirstOf }; QString from{}; @@ -85,6 +124,7 @@ struct RimWorkflowDefinitionEdge bool isWholeInput() const; bool isWholeOutput() const; bool isCollected() const; + bool isFirstOf() const; bool sameConnection( const RimWorkflowDefinitionEdge& other ) const; bool operator==( const RimWorkflowDefinitionEdge& other ) const = default; diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinitionTools.cpp b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinitionTools.cpp index 844dced665..641bb3826d 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinitionTools.cpp +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinitionTools.cpp @@ -23,11 +23,13 @@ #include "RimWorkflowTaskCatalog.h" #include +#include #include #include #include #include +#include namespace { @@ -48,6 +50,86 @@ QJsonValue nullIfEmpty( const QString& text ) return text.isEmpty() ? QJsonValue( QJsonValue::Null ) : QJsonValue( text ); } +QJsonArray asArray( const QJsonValue& value ) +{ + return value.isArray() ? value.toArray() : QJsonArray{ value }; +} + +std::optional guardFromJson( const QJsonValue& value ) +{ + if ( !value.isObject() ) return std::nullopt; + const QJsonObject json = value.toObject(); + + using Op = RimWorkflowTaskGuard::Op; + RimWorkflowTaskGuard guard; + if ( json.contains( "switch" ) ) + { + guard.source = json.value( "switch" ).toString(); + if ( json.value( "otherwise" ).toBool() ) + { + guard.op = Op::Otherwise; + return guard; + } + guard.op = Op::Case; + guard.caseAsList = json.value( "case" ).isArray(); + // Without a case there are no branches, which validation reports + if ( json.contains( "case" ) ) guard.values = asArray( json.value( "case" ) ); + return guard; + } + + const QJsonValue ref = json.value( "ref" ); + if ( ref.isArray() ) + { + const QJsonArray items = ref.toArray(); + guard.source = items.at( 0 ).toString(); + guard.field = items.at( 1 ).toString(); + guard.malformedRef = items.size() != 2; + } + else + { + guard.source = ref.toString(); + } + if ( json.contains( "truthy" ) ) + { + guard.op = json.value( "truthy" ).toBool() ? Op::Truthy : Op::Falsy; + } + else if ( json.contains( "in" ) ) + { + guard.op = Op::In; + guard.values = json.value( "in" ).toArray(); + } + else + { + guard.op = Op::Equals; + guard.values = QJsonArray{ json.value( "equals" ) }; + } + return guard; +} + +QJsonObject guardToJson( const RimWorkflowTaskGuard& guard ) +{ + using Op = RimWorkflowTaskGuard::Op; + switch ( guard.op ) + { + case Op::Otherwise: + return QJsonObject{ { "switch", guard.source }, { "otherwise", true } }; + case Op::Case: + return QJsonObject{ { "switch", guard.source }, + { "case", guard.caseAsList || guard.values.size() != 1 ? QJsonValue( guard.values ) : guard.values.at( 0 ) } }; + default: + break; + } + + QJsonObject json{ { "ref", guard.isSwitchBranch() ? QJsonValue( guard.source ) : QJsonValue( QJsonArray{ guard.source, guard.field } ) } }; + if ( guard.op == Op::Truthy || guard.op == Op::Falsy ) + json["truthy"] = guard.op == Op::Truthy; + else if ( guard.op == Op::In ) + json["in"] = guard.values; + else + json["equals"] = guard.values.isEmpty() ? QJsonValue( QJsonValue::Null ) : guard.values.at( 0 ); + return json; +} + RimWorkflowDefinitionNode nodeFromJson( const QJsonObject& json ) { RimWorkflowDefinitionNode node; @@ -63,10 +145,11 @@ RimWorkflowDefinitionNode nodeFromJson( const QJsonObject& json ) .valueAs = map.value( "value_as" ).toString(), .errorMode = map.value( "error_mode" ).toString( "fail_fast" ) }; } + node.when = guardFromJson( json.value( "when" ) ); for ( auto it = json.begin(); it != json.end(); ++it ) { - if ( it.key() != "name" && it.key() != "task" && it.key() != "config_fields" && it.key() != explicitConfigKey && it.key() != "map" ) - node.extra.insert( it.key(), it.value() ); + static const QStringList modeled{ "name", "task", "config_fields", explicitConfigKey, "map", "when" }; + if ( !modeled.contains( it.key() ) ) node.extra.insert( it.key(), it.value() ); } return node; } @@ -84,6 +167,8 @@ QString collectName( RimWorkflowDefinitionEdge::Collect collect ) return "list"; case RimWorkflowDefinitionEdge::Collect::Dict: return "dict"; + case RimWorkflowDefinitionEdge::Collect::FirstOf: + return "first_of"; default: return {}; } @@ -97,6 +182,7 @@ RimWorkflowDefinitionEdge edgeFromJson( const QJsonObject& json ) .input = json.value( "input" ).toString() }; const QString collect = json.value( "collect" ).toString(); if ( collect == "list" ) edge.collect = RimWorkflowDefinitionEdge::Collect::List; + if ( collect == "first_of" ) edge.collect = RimWorkflowDefinitionEdge::Collect::FirstOf; if ( collect == "dict" ) { edge.collect = RimWorkflowDefinitionEdge::Collect::Dict; @@ -113,13 +199,14 @@ QJsonObject nodeToJson( const RimWorkflowDefinitionNode& node ) json["config_fields"] = QJsonArray::fromStringList( node.configFields ); json[explicitConfigKey] = QJsonArray::fromStringList( node.explicitConfigFields ); if ( node.map ) json["map"] = mapToJson( *node.map ); + if ( node.when ) json["when"] = guardToJson( *node.when ); return json; } QJsonObject edgeToJson( const RimWorkflowDefinitionEdge& edge ) { QJsonObject json{ { "from", edge.from }, { "output", nullIfEmpty( edge.output ) }, { "to", edge.to }, { "input", nullIfEmpty( edge.input ) } }; - if ( edge.isCollected() ) + if ( edge.collect != RimWorkflowDefinitionEdge::Collect::None ) { json["collect"] = collectName( edge.collect ); json["key"] = edge.collect == RimWorkflowDefinitionEdge::Collect::Dict ? nullIfEmpty( edge.key ) : QJsonValue( QJsonValue::Null ); @@ -144,14 +231,28 @@ void addIssue( std::vector& issues, const QString& task, const if ( std::find( issues.begin(), issues.end(), issue ) == issues.end() ) issues.push_back( issue ); } +// Connections and run conditions, as (upstream, downstream) pairs +std::vector> dependencyLinks( const RimWorkflowDefinition& definition ) +{ + std::vector> links; + for ( const auto& edge : definition.edges ) + links.emplace_back( edge.from, edge.to ); + for ( const auto& node : definition.nodes ) + { + if ( node.when ) links.emplace_back( node.when->source, node.name ); + } + return links; +} + bool hasCycle( const RimWorkflowDefinition& definition ) { + const auto links = dependencyLinks( definition ); QMap inDegree; for ( const auto& node : definition.nodes ) inDegree[node.name] = 0; - for ( const auto& edge : definition.edges ) + for ( const auto& [from, to] : links ) { - if ( inDegree.contains( edge.to ) && inDegree.contains( edge.from ) ) inDegree[edge.to]++; + if ( inDegree.contains( to ) && inDegree.contains( from ) ) inDegree[to]++; } QStringList ready; @@ -164,15 +265,21 @@ bool hasCycle( const RimWorkflowDefinition& definition ) { const QString current = ready.takeLast(); ++visited; - for ( const auto& edge : definition.edges ) + for ( const auto& [from, to] : links ) { - if ( edge.from != current || !inDegree.contains( edge.to ) ) continue; - if ( --inDegree[edge.to] == 0 ) ready.append( edge.to ); + if ( from != current || !inDegree.contains( to ) ) continue; + if ( --inDegree[to] == 0 ) ready.append( to ); } } return visited != inDegree.size(); } +QString directAndMembersMessage( const QString& input, bool firstOf ) +{ + return firstOf ? QString( "Input '%1' has both a direct connection and alternatives (first of)" ).arg( input ) + : QString( "Input '%1' has both a direct connection and collected members" ).arg( input ); +} + void validateConnections( const RimWorkflowDefinition& definition, const RimWorkflowDefinitionNode& node, std::vector& issues ) { const auto incoming = definition.incomingEdges( node.name ); @@ -180,6 +287,7 @@ void validateConnections( const RimWorkflowDefinition& definition, const RimWork QSet fieldInputs; QMap collectKinds; QMap collectKeys; + QMap firstOfCounts; for ( const auto& edge : incoming ) { if ( edge.isWholeInput() ) @@ -187,17 +295,22 @@ void validateConnections( const RimWorkflowDefinition& definition, const RimWork ++wholeCount; continue; } - if ( edge.isCollected() ) + if ( edge.isCollected() || edge.isFirstOf() ) { const QString kind = collectName( edge.collect ); if ( fieldInputs.contains( edge.input ) && !collectKinds.contains( edge.input ) ) - addIssue( issues, - node.name, - edge.input, - QString( "Input '%1' has both a direct connection and collected members" ).arg( edge.input ) ); + addIssue( issues, node.name, edge.input, directAndMembersMessage( edge.input, edge.isFirstOf() ) ); if ( collectKinds.contains( edge.input ) && collectKinds.value( edge.input ) != kind ) - addIssue( issues, node.name, edge.input, QString( "Input '%1' mixes list and keyed members" ).arg( edge.input ) ); + { + const bool firstOf = edge.isFirstOf() || + collectKinds.value( edge.input ) == collectName( RimWorkflowDefinitionEdge::Collect::FirstOf ); + const QString message = firstOf + ? QString( "Input '%1' mixes alternatives (first of) with collected members" ).arg( edge.input ) + : QString( "Input '%1' mixes list and keyed members" ).arg( edge.input ); + addIssue( issues, node.name, edge.input, message ); + } collectKinds.insert( edge.input, kind ); + if ( edge.isFirstOf() ) firstOfCounts[edge.input]++; if ( edge.collect == RimWorkflowDefinitionEdge::Collect::Dict ) { if ( edge.key.isEmpty() ) @@ -210,11 +323,23 @@ void validateConnections( const RimWorkflowDefinition& definition, const RimWork continue; } if ( collectKinds.contains( edge.input ) ) - addIssue( issues, node.name, edge.input, QString( "Input '%1' has both a direct connection and collected members" ).arg( edge.input ) ); + addIssue( issues, + node.name, + edge.input, + directAndMembersMessage( edge.input, + collectKinds.value( edge.input ) == collectName( RimWorkflowDefinitionEdge::Collect::FirstOf ) ) ); else if ( fieldInputs.contains( edge.input ) ) addIssue( issues, node.name, edge.input, QString( "Input '%1' has more than one connection" ).arg( edge.input ) ); fieldInputs.insert( edge.input ); } + for ( auto it = firstOfCounts.begin(); it != firstOfCounts.end(); ++it ) + { + if ( it.value() < 2 ) + addIssue( issues, + node.name, + it.key(), + QString( "Input '%1' has a single alternative; first of needs at least two connections" ).arg( it.key() ) ); + } if ( wholeCount > 1 ) addIssue( issues, node.name, {}, "The whole input has more than one connection" ); if ( wholeCount > 0 && !fieldInputs.isEmpty() ) addIssue( issues, node.name, {}, "The task takes both its whole input and individual input fields from other tasks" ); @@ -223,9 +348,10 @@ void validateConnections( const RimWorkflowDefinition& definition, const RimWork for ( const auto& edge : incoming ) { if ( !definition.findNode( edge.from ) ) continue; - if ( edge.isCollected() ) + if ( edge.isCollected() || edge.isFirstOf() ) { - auto check = RimWorkflowPortCompatibility::checkCollectMember( definition, edge ); + auto check = edge.isFirstOf() ? RimWorkflowPortCompatibility::checkFirstOfMember( definition, edge ) + : RimWorkflowPortCompatibility::checkCollectMember( definition, edge ); if ( !check ) addIssue( issues, node.name, edge.input, check.error() ); continue; } @@ -242,6 +368,160 @@ void validateConnections( const RimWorkflowDefinition& definition, const RimWork } } +const QString branchField = "branch"; + +bool sameValue( const QJsonValue& a, const QJsonValue& b ) +{ + if ( a.isDouble() && b.isDouble() ) return a.toDouble() == b.toDouble(); + return a == b; +} + +bool containsValue( const QJsonArray& values, const QJsonValue& value ) +{ + return std::any_of( values.begin(), values.end(), [&value]( const QJsonValue& item ) { return sameValue( item, value ); } ); +} + +bool labelIsTruthy( const QJsonValue& label ) +{ + if ( label.isBool() ) return label.toBool(); + if ( label.isDouble() ) return label.toDouble() != 0.0; + if ( label.isString() ) return !label.toString().isEmpty(); + return !label.isNull(); +} + +QJsonArray schemaOptions( const QJsonObject& schema, const QJsonObject& rootSchema ) +{ + const QJsonObject resolved = RimWorkflowSchemaTools::resolveReferences( schema, rootSchema ); + QJsonArray options; + for ( const char* unionKey : { "anyOf", "oneOf" } ) + { + for ( const QJsonValue& option : resolved.value( unionKey ).toArray() ) + options.append( RimWorkflowSchemaTools::resolveReferences( option.toObject(), rootSchema ) ); + } + if ( options.isEmpty() ) options.append( resolved ); + return options; +} + +// Mirrors taskmaestro: only bool, boolean literals and optional fields can be truthy or falsy +bool acceptsTruthiness( const QJsonObject& schema, const QJsonObject& rootSchema ) +{ + const QJsonArray options = schemaOptions( schema, rootSchema ); + if ( options.size() > 1 ) + return std::any_of( options.begin(), + options.end(), + []( const QJsonValue& option ) { return option.toObject().value( "type" ) == "null"; } ); + + const QJsonObject option = options.at( 0 ).toObject(); + if ( option.contains( "enum" ) ) + { + const QJsonArray values = option.value( "enum" ).toArray(); + return std::all_of( values.begin(), values.end(), []( const QJsonValue& value ) { return value.isBool(); } ); + } + if ( option.value( "type" ).isArray() ) return option.value( "type" ).toArray().contains( QJsonValue( "null" ) ); + return !option.contains( "type" ) || option.value( "type" ) == "boolean" || option.value( "type" ) == "null"; +} + +bool valueMatchesSchema( const QJsonValue& value, const QJsonObject& schema, const QJsonObject& rootSchema ) +{ + for ( const QJsonValue& item : schemaOptions( schema, rootSchema ) ) + { + const QJsonObject option = item.toObject(); + if ( option.contains( "enum" ) ) + { + if ( containsValue( option.value( "enum" ).toArray(), value ) ) return true; + continue; + } + if ( option.contains( "const" ) ) + { + if ( sameValue( option.value( "const" ), value ) ) return true; + continue; + } + + QStringList types; + if ( option.value( "type" ).isArray() ) + types = stringList( option.value( "type" ) ); + else if ( option.contains( "type" ) ) + types.append( option.value( "type" ).toString() ); + if ( types.isEmpty() ) return true; + for ( const QString& type : types ) + { + const bool matches = ( type == "string" && value.isString() ) || ( type == "boolean" && value.isBool() ) || + ( type == "number" && value.isDouble() ) || + ( type == "integer" && value.isDouble() && value.toDouble() == std::floor( value.toDouble() ) ) || + ( type == "null" && value.isNull() ) || type == "object" || type == "array"; + if ( matches ) return true; + } + } + return false; +} + +// Every branch of a switch needs a task, and `otherwise` must be able to match +void validateSwitchCoverage( const RimWorkflowDefinition& definition, + const RimWorkflowDefinitionNode& switchNode, + std::vector& issues ) +{ + using Op = RimWorkflowTaskGuard::Op; + const QJsonArray labels = RimWorkflowDefinitionTools::switchBranches( definition.taskType( switchNode.taskId ) ); + if ( labels.isEmpty() || switchNode.map ) return; + + bool guarded = false; + bool hasOtherwise = false; + QJsonArray claimed; + for ( const auto& node : definition.nodes ) + { + if ( !node.when || node.when->source != switchNode.name ) continue; + if ( !node.when->isSwitchBranch() && node.when->field != branchField ) continue; + guarded = true; + switch ( node.when->op ) + { + case Op::Otherwise: + hasOtherwise = true; + break; + case Op::Truthy: + case Op::Falsy: + for ( const QJsonValue& label : labels ) + { + if ( labelIsTruthy( label ) == ( node.when->op == Op::Truthy ) ) claimed.append( label ); + } + break; + default: + for ( const QJsonValue& value : node.when->values ) + claimed.append( value ); + } + } + if ( !guarded ) return; + + QStringList unclaimed; + for ( const QJsonValue& label : labels ) + { + if ( !containsValue( claimed, label ) ) unclaimed.append( RimWorkflowDefinitionTools::guardValueText( label ) ); + } + if ( !hasOtherwise && !unclaimed.isEmpty() ) + { + addIssue( issues, + switchNode.name, + {}, + QString( "Switch '%1' has no task for branch %2; add a run condition with this case, or otherwise" ) + .arg( switchNode.name, unclaimed.join( ", " ) ) ); + } + if ( hasOtherwise && unclaimed.isEmpty() ) + { + addIssue( issues, + switchNode.name, + {}, + QString( "Otherwise on switch '%1' can never match: every branch already has a case" ).arg( switchNode.name ) ); + } +} + +void validateGuard( const RimWorkflowDefinition& definition, const RimWorkflowDefinitionNode& node, std::vector& issues ) +{ + if ( !node.when ) return; + RimWorkflowDefinition others = definition; + others.findNode( node.name )->when.reset(); + auto check = RimWorkflowDefinitionTools::checkTaskGuard( others, node.name, *node.when ); + if ( !check ) addIssue( issues, node.name, {}, check.error() ); +} + void validateMap( const RimWorkflowDefinition& definition, const RimWorkflowDefinitionNode& node, std::vector& issues ) { if ( !node.map ) return; @@ -290,6 +570,22 @@ RimWorkflowDefinition withoutInvalidReferences( RimWorkflowDefinition definition std::erase_if( definition.edges, [&names]( const RimWorkflowDefinitionEdge& edge ) { return !names.contains( edge.from ) || !names.contains( edge.to ); } ); if ( !definition.resultTask.isEmpty() && !names.contains( definition.resultTask ) ) definition.resultTask.clear(); + for ( auto& node : definition.nodes ) + { + if ( node.when && !names.contains( node.when->source ) ) node.when.reset(); + } + + // A single remaining alternative becomes a plain connection + QMap firstOfCounts; + for ( const auto& edge : definition.edges ) + { + if ( edge.isFirstOf() ) firstOfCounts[edge.to + "." + edge.input]++; + } + for ( auto& edge : definition.edges ) + { + if ( edge.isFirstOf() && firstOfCounts.value( edge.to + "." + edge.input ) == 1 ) + edge.collect = RimWorkflowDefinitionEdge::Collect::None; + } QJsonObject inputs; for ( const QString& name : names ) @@ -307,9 +603,13 @@ QJsonArray inputPortsJson( const RimWorkflowDefinition& definit QJsonArray result; for ( const auto& port : ports ) { - bool wired = false; + bool wired = false; + bool firstOf = false; for ( const auto& edge : incoming ) - wired = wired || edge.input == port.name; + { + wired = wired || edge.input == port.name; + firstOf = firstOf || ( edge.input == port.name && edge.isFirstOf() ); + } QJsonObject json{ { "name", port.name }, { "type", port.typeName }, { "icon", port.iconResource }, @@ -320,6 +620,7 @@ QJsonArray inputPortsJson( const RimWorkflowDefinition& definit { "covered", !port.isWhole() && covered.contains( port.name ) }, { "wired", wired } }; if ( const auto target = RimWorkflowPortCompatibility::collectTarget( port ) ) json["collect"] = collectName( target->first ); + if ( firstOf ) json["first_of"] = true; result.append( json ); } return result; @@ -417,6 +718,11 @@ QJsonObject graphTask( const RimWorkflowDefinition& definition, { "output_icons", RimWorkflowPortCompatibility::portIcons( outputPorts ) }, { "map", mapJson.isEmpty() ? QJsonValue( QJsonValue::Null ) : QJsonValue( mapJson ) }, { "is_result", node.name == resultTask }, + { "switch", RimWorkflowDefinitionTools::isSwitchTask( definition.taskType( node.taskId ) ) }, + { "branches", RimWorkflowDefinitionTools::switchBranches( definition.taskType( node.taskId ) ) }, + { "when", node.when ? QJsonValue( node.when->source ) : QJsonValue( QJsonValue::Null ) }, + { "when_label", + node.when ? QJsonValue( RimWorkflowDefinitionTools::guardLabel( *node.when ) ) : QJsonValue( QJsonValue::Null ) }, { "issues", taskIssues } }; } } // namespace @@ -588,14 +894,18 @@ std::expected } //-------------------------------------------------------------------------------------------------- -/// Tasks without downstream tasks, in definition order +/// Tasks without downstream tasks, in definition order. A task used by a run condition is not a sink. //-------------------------------------------------------------------------------------------------- QStringList RimWorkflowDefinitionTools::sinkTasks( const RimWorkflowDefinition& definition ) { QStringList sinks; for ( const auto& node : definition.nodes ) { - if ( definition.outgoingEdges( node.name ).empty() ) sinks.append( node.name ); + if ( !definition.outgoingEdges( node.name ).empty() ) continue; + const bool guardsOthers = std::any_of( definition.nodes.begin(), + definition.nodes.end(), + [&node]( const auto& other ) { return other.when && other.when->source == node.name; } ); + if ( !guardsOthers ) sinks.append( node.name ); } return sinks; } @@ -802,13 +1112,20 @@ RimWorkflowDefinitionTools::EditResult RimWorkflowDefinitionTools::setCollectKey } //-------------------------------------------------------------------------------------------------- -/// Move a member of a `list[T]` input `delta` positions within the list +/// Move a member of a `list[T]` input, or an alternative of a `first_of` input, `delta` positions //-------------------------------------------------------------------------------------------------- RimWorkflowDefinitionTools::EditResult RimWorkflowDefinitionTools::moveCollectMember( const RimWorkflowDefinition& definition, const RimWorkflowDefinitionEdge& edge, int delta ) { if ( !definition.editable ) return std::unexpected( "The workflow is read-only" ); + const auto current = std::find_if( definition.edges.begin(), + definition.edges.end(), + [&edge]( const auto& existing ) { return existing.sameConnection( edge ); } ); + if ( current == definition.edges.end() || current->collect == RimWorkflowDefinitionEdge::Collect::None ) + return std::unexpected( "The connection is not a collected member" ); + const auto kind = current->collect; + RimWorkflowDefinition result = definition; std::vector members; size_t position = 0; @@ -816,7 +1133,7 @@ RimWorkflowDefinitionTools::EditResult for ( size_t i = 0; i < result.edges.size(); ++i ) { const auto& existing = result.edges[i]; - if ( existing.to != edge.to || existing.input != edge.input || !existing.isCollected() ) continue; + if ( existing.to != edge.to || existing.input != edge.input || existing.collect != kind ) continue; if ( existing.sameConnection( edge ) ) { position = members.size(); @@ -949,7 +1266,7 @@ RimWorkflowDefinitionTools::EditResult RimWorkflowDefinitionTools::setTaskMap( c } //-------------------------------------------------------------------------------------------------- -/// Rename a task instance, updating connections, the result task and the input values +/// Rename a task instance, updating connections, run conditions, the result task and the input values //-------------------------------------------------------------------------------------------------- RimWorkflowDefinitionTools::EditResult RimWorkflowDefinitionTools::renameTask( const RimWorkflowDefinition& definition, const QString& oldName, const QString& newName ) @@ -966,6 +1283,10 @@ RimWorkflowDefinitionTools::EditResult if ( edge.from == oldName ) edge.from = newName; if ( edge.to == oldName ) edge.to = newName; } + for ( auto& node : result.nodes ) + { + if ( node.when && node.when->source == oldName ) node.when->source = newName; + } if ( result.resultTask == oldName ) result.resultTask = newName; const QJsonValue values = result.inputs.take( oldName ); @@ -1090,6 +1411,329 @@ RimWorkflowDefinition RimWorkflowDefinitionTools::autoWireResInsightInputs( cons return result; } +//-------------------------------------------------------------------------------------------------- +/// A switch or condition task: its output is a `taskmaestro.conditions.Branch` +//-------------------------------------------------------------------------------------------------- +bool RimWorkflowDefinitionTools::isSwitchTask( const QJsonObject& taskType ) +{ + const QJsonObject outputSchema = taskType.value( "output_schema" ).toObject(); + return outputSchema.value( "x-ri-python-bases" ).toArray().contains( QJsonValue( "taskmaestro.conditions.Branch" ) ) && + outputSchema.value( "properties" ).toObject().contains( branchField ); +} + +//-------------------------------------------------------------------------------------------------- +/// The branch labels of a switch task, from the type of its `branch` output field: the values of a +/// Literal or an enum, or true and false for a condition task. Empty for other tasks. +//-------------------------------------------------------------------------------------------------- +QJsonArray RimWorkflowDefinitionTools::switchBranches( const QJsonObject& taskType ) +{ + if ( !isSwitchTask( taskType ) ) return {}; + + const QJsonObject outputSchema = taskType.value( "output_schema" ).toObject(); + const QJsonObject branch = + RimWorkflowSchemaTools::resolveReferences( outputSchema.value( "properties" ).toObject().value( branchField ).toObject(), outputSchema ); + if ( branch.contains( "enum" ) ) return branch.value( "enum" ).toArray(); + if ( branch.contains( "const" ) ) return QJsonArray{ branch.value( "const" ) }; + if ( branch.value( "type" ) == "boolean" ) return QJsonArray{ true, false }; + return {}; +} + +//-------------------------------------------------------------------------------------------------- +/// Output fields of a task that a run condition can test +//-------------------------------------------------------------------------------------------------- +QStringList RimWorkflowDefinitionTools::guardFields( const RimWorkflowDefinition& definition, const QString& sourceName ) +{ + const auto* node = definition.findNode( sourceName ); + if ( !node ) return {}; + return RimWorkflowSchemaTools::propertyNames( definition.taskType( node->taskId ).value( "output_schema" ).toObject() ); +} + +//-------------------------------------------------------------------------------------------------- +/// Schema of the output field a run condition tests, with the `$defs` of the output schema. An +/// empty field is the branch of a switch task. +//-------------------------------------------------------------------------------------------------- +QJsonObject + RimWorkflowDefinitionTools::guardFieldSchema( const RimWorkflowDefinition& definition, const QString& sourceName, const QString& field ) +{ + const auto* node = definition.findNode( sourceName ); + if ( !node ) return {}; + const QJsonObject outputSchema = definition.taskType( node->taskId ).value( "output_schema" ).toObject(); + const QJsonObject property = outputSchema.value( "properties" ).toObject().value( field.isEmpty() ? branchField : field ).toObject(); + if ( property.isEmpty() ) return {}; + + QJsonObject result = RimWorkflowSchemaTools::resolveReferences( property, outputSchema ); + if ( outputSchema.contains( "$defs" ) ) result["$defs"] = outputSchema.value( "$defs" ); + return result; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +QString RimWorkflowDefinitionTools::guardValueText( const QJsonValue& value ) +{ + if ( value.isBool() ) return value.toBool() ? "true" : "false"; + if ( value.isDouble() ) return QString::number( value.toDouble() ); + if ( value.isNull() || value.isUndefined() ) return "None"; + return value.toString(); +} + +//-------------------------------------------------------------------------------------------------- +/// Values typed as `a, "b, c", 2`: JSON scalars where the field is not a string, strings otherwise. +/// Commas inside double quotes do not separate values, and a quoted value is always a string. +//-------------------------------------------------------------------------------------------------- +QJsonArray RimWorkflowDefinitionTools::guardValuesFromText( const QString& text, const QJsonObject& fieldSchema ) +{ + QStringList tokens; + QString token; + bool inQuotes = false; + for ( qsizetype i = 0; i < text.size(); ++i ) + { + const QChar c = text.at( i ); + if ( inQuotes && c == '\\' && i + 1 < text.size() ) + { + token += c; + token += text.at( ++i ); + continue; + } + if ( c == '"' ) inQuotes = !inQuotes; + if ( c == ',' && !inQuotes ) + { + tokens.append( token ); + token.clear(); + continue; + } + token += c; + } + tokens.append( token ); + + bool stringField = fieldSchema.value( "type" ).toString() == "string"; + if ( !fieldSchema.contains( "type" ) ) + { + // A string Literal has only enum/const; every value must be a string + QJsonArray allowed = fieldSchema.value( "enum" ).toArray(); + if ( fieldSchema.contains( "const" ) ) allowed.append( fieldSchema.value( "const" ) ); + stringField = !allowed.isEmpty() && std::ranges::all_of( allowed, []( const QJsonValue& v ) { return v.isString(); } ); + } + QJsonArray values; + for ( const QString& part : tokens ) + { + const QString trimmed = part.trimmed(); + if ( trimmed.isEmpty() ) continue; + + const QJsonDocument document = QJsonDocument::fromJson( ( "[" + trimmed + "]" ).toUtf8() ); + const bool parsed = document.isArray() && document.array().size() == 1; + const bool quoted = trimmed.size() > 1 && trimmed.startsWith( '"' ) && trimmed.endsWith( '"' ); + if ( quoted ) + values.append( parsed ? document.array().first() : QJsonValue( trimmed.mid( 1, trimmed.size() - 2 ) ) ); + else if ( !stringField && parsed ) + values.append( document.array().first() ); + else + values.append( trimmed ); + } + return values; +} + +//-------------------------------------------------------------------------------------------------- +/// The text that `guardValuesFromText` reads back as `values`. Strings are quoted only when needed. +//-------------------------------------------------------------------------------------------------- +QString RimWorkflowDefinitionTools::guardValuesToText( const QJsonArray& values, const QJsonObject& fieldSchema ) +{ + QStringList texts; + for ( const QJsonValue& value : values ) + { + if ( value.isString() && guardValuesFromText( value.toString(), fieldSchema ) == QJsonArray{ value } ) + { + texts.append( value.toString() ); + continue; + } + const QByteArray json = QJsonDocument( QJsonArray{ value } ).toJson( QJsonDocument::Compact ); + texts.append( QString::fromUtf8( json.mid( 1, json.size() - 2 ) ) ); + } + return texts.join( ", " ); +} + +//-------------------------------------------------------------------------------------------------- +/// Short text shown on the control edge of a run condition +//-------------------------------------------------------------------------------------------------- +QString RimWorkflowDefinitionTools::guardLabel( const RimWorkflowTaskGuard& guard ) +{ + using Op = RimWorkflowTaskGuard::Op; + QStringList values; + for ( const QJsonValue& value : guard.values ) + values.append( guardValueText( value ) ); + + const QString field = guard.isSwitchBranch() ? branchField : guard.field; + switch ( guard.op ) + { + case Op::Case: + return values.join( ", " ); + case Op::Otherwise: + return "otherwise"; + case Op::Equals: + return QString( "%1 = %2" ).arg( field, values.value( 0 ) ); + case Op::In: + return QString( "%1 in {%2}" ).arg( field, values.join( ", " ) ); + case Op::Truthy: + return field; + case Op::Falsy: + return "not " + field; + } + return {}; +} + +//-------------------------------------------------------------------------------------------------- +/// Label of the alternative at a 1-based position of a `first_of` input: 1st, 2nd, ... +//-------------------------------------------------------------------------------------------------- +QString RimWorkflowDefinitionTools::firstOfLabel( int position ) +{ + // Two-letter ordinal suffixes for 0 (and 4-9, 11-13), 1, 2 and 3 + const QString suffixes = "thstndrd"; + + const int lastTwo = position % 100; + const int index = ( lastTwo >= 11 && lastTwo <= 13 ) || position % 10 > 3 ? 0 : position % 10; + return QString::number( position ) + suffixes.mid( 2 * index, 2 ); +} + +//-------------------------------------------------------------------------------------------------- +/// Checks a run condition of a task, mirroring the checks of taskmaestro +//-------------------------------------------------------------------------------------------------- +std::expected RimWorkflowDefinitionTools::checkTaskGuard( const RimWorkflowDefinition& definition, + const QString& nodeName, + const RimWorkflowTaskGuard& guard ) +{ + using Op = RimWorkflowTaskGuard::Op; + + if ( !definition.findNode( nodeName ) ) return std::unexpected( QString( "Unknown task '%1'" ).arg( nodeName ) ); + const auto* source = definition.findNode( guard.source ); + if ( !source ) return std::unexpected( QString( "The run condition refers to unknown task '%1'" ).arg( guard.source ) ); + if ( guard.malformedRef ) return std::unexpected( "A field condition must refer to a task and an output field" ); + if ( guard.source == nodeName ) return std::unexpected( "A task cannot depend on its own output" ); + if ( source->map ) return std::unexpected( QString( "The run condition cannot refer to the mapped task '%1'" ).arg( guard.source ) ); + + RimWorkflowDefinition others = definition; + others.findNode( nodeName )->when.reset(); + if ( RimWorkflowPortCompatibility::wouldCreateCycle( others, guard.source, nodeName ) ) + return std::unexpected( "The run condition would create a cycle" ); + + const QJsonObject sourceType = definition.taskType( source->taskId ); + if ( sourceType.isEmpty() ) return {}; + + const bool isSwitch = isSwitchTask( sourceType ); + const bool isBranch = isSwitch && ( guard.isSwitchBranch() || guard.field == branchField ); + if ( guard.isSwitchBranch() && !isSwitch ) + return std::unexpected( QString( "'%1' is not a switch task; choose an output field to test" ).arg( guard.source ) ); + if ( guard.op == Op::Otherwise && !isBranch ) + return std::unexpected( QString( "Otherwise requires a switch task, but '%1' is not one" ).arg( guard.source ) ); + if ( guard.op == Op::Case && !guard.isSwitchBranch() ) return std::unexpected( "A case refers to the branch of a switch task" ); + + const QJsonObject outputSchema = sourceType.value( "output_schema" ).toObject(); + const QString field = guard.isSwitchBranch() ? branchField : guard.field; + if ( !outputSchema.value( "properties" ).toObject().contains( field ) ) + return std::unexpected( QString( "'%1' has no output field '%2'" ).arg( guard.source, field ) ); + const QJsonObject property = outputSchema.value( "properties" ).toObject().value( field ).toObject(); + const QString target = guard.isSwitchBranch() ? guard.source : guard.source + "." + field; + + switch ( guard.op ) + { + case Op::Otherwise: + return {}; + case Op::Truthy: + case Op::Falsy: + if ( !acceptsTruthiness( property, outputSchema ) ) + { + return std::unexpected( QString( "%1 is %2; true/false conditions need a bool or optional field" ) + .arg( target, RimWorkflowPortCompatibility::typeName( property, outputSchema ) ) ); + } + return {}; + case Op::Equals: + if ( guard.values.size() != 1 ) return std::unexpected( "Choose the value to compare with" ); + break; + default: + if ( guard.values.isEmpty() ) return std::unexpected( isBranch ? "Choose at least one branch" : "Choose at least one value" ); + } + + const QJsonArray labels = isBranch ? switchBranches( sourceType ) : QJsonArray(); + for ( const QJsonValue& value : guard.values ) + { + const bool valid = labels.isEmpty() ? valueMatchesSchema( value, property, outputSchema ) : containsValue( labels, value ); + if ( !valid ) + { + return std::unexpected( + QString( "%1 is not a valid value of %2 (%3)" ) + .arg( guardValueText( value ), target, RimWorkflowPortCompatibility::typeName( property, outputSchema ) ) ); + } + } + return {}; +} + +//-------------------------------------------------------------------------------------------------- +/// Run a task only when a condition holds, or always with std::nullopt +//-------------------------------------------------------------------------------------------------- +RimWorkflowDefinitionTools::EditResult RimWorkflowDefinitionTools::setTaskGuard( const RimWorkflowDefinition& definition, + const QString& nodeName, + const std::optional& guard ) +{ + if ( !definition.editable ) return std::unexpected( "The workflow is read-only" ); + if ( !definition.findNode( nodeName ) ) return std::unexpected( QString( "Unknown task '%1'" ).arg( nodeName ) ); + if ( guard ) + { + if ( auto check = checkTaskGuard( definition, nodeName, *guard ); !check ) return std::unexpected( check.error() ); + } + + RimWorkflowDefinition result = definition; + result.findNode( nodeName )->when = guard; + return normalize( result ); +} + +//-------------------------------------------------------------------------------------------------- +/// Add an alternative to a connected input (`first_of` in YAML): the input takes the output of the +/// first alternative whose task ran. An existing plain connection becomes the first alternative. +//-------------------------------------------------------------------------------------------------- +RimWorkflowDefinitionTools::EditResult RimWorkflowDefinitionTools::addFirstOfMember( const RimWorkflowDefinition& definition, + const RimWorkflowDefinitionEdge& edge ) +{ + if ( !definition.editable ) return std::unexpected( "The workflow is read-only" ); + if ( edge.input.isEmpty() ) return std::unexpected( "Alternatives (first of) need a named input" ); + if ( edge.from == edge.to ) return std::unexpected( "A task cannot be connected to itself" ); + for ( const QString& name : { edge.from, edge.to } ) + { + if ( !definition.findNode( name ) ) return std::unexpected( QString( "Unknown task '%1'" ).arg( name ) ); + } + + bool hasExisting = false; + for ( const auto& existing : definition.incomingEdges( edge.to ) ) + { + if ( existing.input != edge.input ) continue; + if ( existing.isCollected() ) + return std::unexpected( QString( "Input '%1' collects members and cannot take alternatives" ).arg( edge.input ) ); + if ( existing.sameConnection( edge ) ) + return std::unexpected( QString( "%1 is already connected to %2.%3" ).arg( edge.from, edge.to, edge.input ) ); + hasExisting = true; + } + if ( !hasExisting ) return std::unexpected( QString( "Input '%1' has no connection to add an alternative to" ).arg( edge.input ) ); + + RimWorkflowDefinitionEdge member{ .from = edge.from, .output = edge.output, .to = edge.to, .input = edge.input }; + member.collect = RimWorkflowDefinitionEdge::Collect::FirstOf; + if ( auto check = RimWorkflowPortCompatibility::checkFirstOfMember( definition, member ); !check ) + return std::unexpected( check.error() ); + if ( RimWorkflowPortCompatibility::wouldCreateCycle( definition, edge.from, edge.to ) ) + return std::unexpected( "The connection would create a cycle" ); + + RimWorkflowDefinition result = definition; + auto insertAt = result.edges.end(); + for ( auto it = result.edges.begin(); it != result.edges.end(); ++it ) + { + if ( it->to == edge.to && it->input == edge.input ) + { + it->collect = RimWorkflowDefinitionEdge::Collect::FirstOf; + insertAt = std::next( it ); + } + } + // Keep the alternatives of an input together, after the existing ones + result.edges.insert( insertAt, member ); + return normalize( result ); +} + //-------------------------------------------------------------------------------------------------- /// Static checks that do not need Python. `taskmaestro workflow describe` has the final say. //-------------------------------------------------------------------------------------------------- @@ -1117,6 +1761,11 @@ std::vector RimWorkflowDefinitionTools::validate( const RimWor if ( !names.contains( edge.from ) || !names.contains( edge.to ) ) addIssue( issues, edge.to, edge.input, QString( "Connection from unknown task '%1'" ).arg( edge.from ) ); } + for ( const auto& node : definition.nodes ) + { + if ( node.when && !names.contains( node.when->source ) ) + addIssue( issues, node.name, {}, QString( "The run condition refers to unknown task '%1'" ).arg( node.when->source ) ); + } if ( hasCycle( definition ) ) addIssue( issues, {}, {}, "The workflow contains a cycle" ); for ( const auto& node : definition.nodes ) @@ -1129,6 +1778,8 @@ std::vector RimWorkflowDefinitionTools::validate( const RimWor validateConnections( definition, node, issues ); validateInputs( definition, node, issues ); validateMap( definition, node, issues ); + validateGuard( definition, node, issues ); + validateSwitchCoverage( definition, node, issues ); } if ( !definition.resultTask.isEmpty() && !names.contains( definition.resultTask ) ) @@ -1171,7 +1822,8 @@ QJsonObject RimWorkflowDefinitionTools::graphFromDefinition( const RimWorkflowDe QMap listCount; for ( const auto& edge : definition.edges ) { - if ( edge.collect == RimWorkflowDefinitionEdge::Collect::List ) listIndex[&edge] = listCount[edge.to + "." + edge.input]++; + if ( edge.collect == RimWorkflowDefinitionEdge::Collect::List || edge.isFirstOf() ) + listIndex[&edge] = listCount[edge.to + "." + edge.input]++; } QJsonArray edges; @@ -1185,9 +1837,22 @@ QJsonObject RimWorkflowDefinitionTools::graphFromDefinition( const RimWorkflowDe json["collect"] = collectName( edge.collect ); json["label"] = edge.collect == RimWorkflowDefinitionEdge::Collect::Dict ? edge.key : QString( "[%1]" ).arg( listIndex[&edge] ); } + else if ( edge.isFirstOf() ) + { + json["collect"] = collectName( edge.collect ); + json["label"] = firstOfLabel( listIndex[&edge] + 1 ); + } edges.append( json ); } + // Run conditions are drawn as control edges from the source task + for ( const auto& node : definition.nodes ) + { + if ( !node.when ) continue; + edges.append( + QJsonObject{ { "from", node.when->source }, { "to", node.name }, { "kind", "when" }, { "label", guardLabel( *node.when ) } } ); + } + const QStringList nodeNames = definition.nodeNames(); QJsonArray workflowIssues; for ( const auto& issue : issues ) diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinitionTools.h b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinitionTools.h index b3c1f13cea..d05f672a47 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinitionTools.h +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinitionTools.h @@ -20,6 +20,7 @@ #include "RimWorkflowDefinition.h" +#include #include #include #include @@ -76,13 +77,28 @@ EditResult moveCollectMember( const RimWorkflowDefinition& definition, const Rim EditResult setTaskMap( const RimWorkflowDefinition& definition, const QString& nodeName, const std::optional& map ); std::expected checkTaskMap( const RimWorkflowDefinition& definition, const QString& nodeName, const RimWorkflowTaskMap& map ); QJsonObject mapOverSchema( const RimWorkflowDefinition& definition, const QString& nodeName ); -EditResult renameTask( const RimWorkflowDefinition& definition, const QString& oldName, const QString& newName ); -EditResult renameWorkflow( const RimWorkflowDefinition& definition, const QString& newName ); -EditResult setResultTask( const RimWorkflowDefinition& definition, const QString& nodeName ); +EditResult setTaskGuard( const RimWorkflowDefinition& definition, const QString& nodeName, const std::optional& guard ); +std::expected + checkTaskGuard( const RimWorkflowDefinition& definition, const QString& nodeName, const RimWorkflowTaskGuard& guard ); +EditResult addFirstOfMember( const RimWorkflowDefinition& definition, const RimWorkflowDefinitionEdge& edge ); +EditResult renameTask( const RimWorkflowDefinition& definition, const QString& oldName, const QString& newName ); +EditResult renameWorkflow( const RimWorkflowDefinition& definition, const QString& newName ); +EditResult setResultTask( const RimWorkflowDefinition& definition, const QString& nodeName ); EditResult setOptionalInputConfigured( const RimWorkflowDefinition& definition, const QString& nodeName, const QString& field, bool configured ); RimWorkflowDefinition autoWireResInsightInputs( const RimWorkflowDefinition& definition, const RimWorkflowTaskCatalog& catalog, const QString& nodeName ); +// Conditional branches +bool isSwitchTask( const QJsonObject& taskType ); +QJsonArray switchBranches( const QJsonObject& taskType ); +QStringList guardFields( const RimWorkflowDefinition& definition, const QString& sourceName ); +QJsonObject guardFieldSchema( const RimWorkflowDefinition& definition, const QString& sourceName, const QString& field ); +QString guardValueText( const QJsonValue& value ); +QJsonArray guardValuesFromText( const QString& text, const QJsonObject& fieldSchema ); +QString guardValuesToText( const QJsonArray& values, const QJsonObject& fieldSchema ); +QString guardLabel( const RimWorkflowTaskGuard& guard ); +QString firstOfLabel( int position ); + // Validation and display std::vector validate( const RimWorkflowDefinition& definition ); QJsonObject graphFromDefinition( const RimWorkflowDefinition& definition, bool editMode, const std::vector& extraIssues = {} ); diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDescribeTools.cpp b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDescribeTools.cpp index 495718df84..010f4242a7 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDescribeTools.cpp +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDescribeTools.cpp @@ -18,6 +18,7 @@ #include "RimWorkflowDescribeTools.h" +#include "RimWorkflowDefinitionTools.h" #include "RimWorkflowPortCompatibility.h" #include "RimWorkflowSchemaTools.h" @@ -57,6 +58,21 @@ void appendDependencyEdges( QJsonArray& edges, const QString& taskName, const QJ for ( auto it = deps.begin(); it != deps.end(); ++it ) { const QJsonObject ref = it.value().toObject(); + if ( ref.value( "first_of" ).isArray() ) + { + int position = 0; + for ( const QJsonValue& member : ref.value( "first_of" ).toArray() ) + { + const qsizetype count = edges.size(); + appendEdge( edges, member.toObject(), taskName, it.key() ); + if ( edges.size() == count ) continue; + QJsonObject edge = edges.last().toObject(); + edge["collect"] = "first_of"; + edge["label"] = RimWorkflowDefinitionTools::firstOfLabel( ++position ); + edges.replace( edges.size() - 1, edge ); + } + continue; + } if ( !ref.contains( "collect" ) ) { appendEdge( edges, ref, taskName, it.key() ); @@ -77,6 +93,40 @@ void appendDependencyEdges( QJsonArray& edges, const QString& taskName, const QJ } } +//-------------------------------------------------------------------------------------------------- +/// The control edge of a run condition, `when: {task, field, op, values}` in describe +//-------------------------------------------------------------------------------------------------- +void appendGuardEdge( QJsonArray& edges, const QString& taskName, const QJsonValue& when, const QStringList& switches ) +{ + const QJsonObject spec = when.toObject(); + if ( spec.isEmpty() ) return; + + using Op = RimWorkflowTaskGuard::Op; + RimWorkflowTaskGuard guard; + guard.source = spec.value( "task" ).toString(); + guard.values = spec.value( "values" ).toArray(); + const bool branch = switches.contains( guard.source ) && spec.value( "field" ).toString() == "branch"; + guard.field = branch ? QString() : spec.value( "field" ).toString(); + const QString op = spec.value( "op" ).toString(); + if ( op == "otherwise" ) + guard.op = Op::Otherwise; + else if ( op == "truthy" ) + guard.op = Op::Truthy; + else if ( op == "falsy" ) + guard.op = Op::Falsy; + else if ( branch ) + guard.op = Op::Case; + else + // Describe reports equals as `in` with one value, like taskmaestro writes it back + guard.op = guard.values.size() == 1 ? Op::Equals : Op::In; + if ( guard.source.isEmpty() ) return; + + edges.append( QJsonObject{ { "from", guard.source }, + { "to", taskName }, + { "kind", "when" }, + { "label", RimWorkflowDefinitionTools::guardLabel( guard ) } } ); +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- @@ -119,7 +169,9 @@ QJsonObject taskFromDescribe( const QJsonObject& task ) { "input_types", RimWorkflowPortCompatibility::portTypes( RimWorkflowPortCompatibility::inputPorts( task ) ) }, { "output_types", RimWorkflowPortCompatibility::portTypes( RimWorkflowPortCompatibility::outputPorts( task ) ) }, { "input_icons", RimWorkflowPortCompatibility::portIcons( RimWorkflowPortCompatibility::inputPorts( task ) ) }, - { "output_icons", RimWorkflowPortCompatibility::portIcons( RimWorkflowPortCompatibility::outputPorts( task ) ) } }; + { "output_icons", RimWorkflowPortCompatibility::portIcons( RimWorkflowPortCompatibility::outputPorts( task ) ) }, + { "switch", task.value( "branches" ).isArray() }, + { "branches", task.value( "branches" ).toArray() } }; } } // namespace @@ -128,6 +180,12 @@ QJsonObject taskFromDescribe( const QJsonObject& task ) //-------------------------------------------------------------------------------------------------- QJsonObject RimWorkflowDescribeTools::graphFromDescribe( const QJsonObject& describe ) { + QStringList switches; + for ( const QJsonValue& value : describe.value( "tasks" ).toArray() ) + { + if ( value.toObject().value( "branches" ).isArray() ) switches.append( value.toObject().value( "name" ).toString() ); + } + QJsonArray tasks; QJsonArray edges; for ( const QJsonValue& value : describe.value( "tasks" ).toArray() ) @@ -135,6 +193,7 @@ QJsonObject RimWorkflowDescribeTools::graphFromDescribe( const QJsonObject& desc const QJsonObject task = value.toObject(); tasks.append( taskFromDescribe( task ) ); appendDependencyEdges( edges, task.value( "name" ).toString(), task.value( "depends_on" ) ); + appendGuardEdge( edges, task.value( "name" ).toString(), task.value( "when" ), switches ); } return QJsonObject{ { "name", describe.value( "workflow" ).toString() }, { "description", "" }, { "tasks", tasks }, { "edges", edges } }; diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowFieldBinding.cpp b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowFieldBinding.cpp index e7d3d71509..b953c2462d 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowFieldBinding.cpp +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowFieldBinding.cpp @@ -118,6 +118,14 @@ bool RimWorkflowFieldBinding::hasValue() const return m_hasValue(); } +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void RimWorkflowFieldBinding::setHasValue( bool hasValue ) +{ + m_hasValue = hasValue; +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- @@ -126,6 +134,14 @@ bool RimWorkflowFieldBinding::isObjectReference() const return false; } +//-------------------------------------------------------------------------------------------------- +/// Whether a null value is an entered value (None) rather than a value that is not set +//-------------------------------------------------------------------------------------------------- +bool RimWorkflowFieldBinding::isExplicitNull() const +{ + return false; +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowFieldBinding.h b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowFieldBinding.h index f571dba6a5..8495702d93 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowFieldBinding.h +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowFieldBinding.h @@ -40,6 +40,7 @@ class RimWorkflowFieldBinding : public caf::PdmObject virtual void applySchema( const QJsonObject& fieldSchema ); virtual QJsonValue toJsonValue() const = 0; virtual bool isObjectReference() const; + virtual bool isExplicitNull() const; virtual QString displayValue() const; virtual caf::PdmFieldHandle* valueField() = 0; @@ -48,6 +49,7 @@ class RimWorkflowFieldBinding : public caf::PdmObject void fieldChangedByUi( const caf::PdmFieldHandle* changedField, const QVariant& oldValue, const QVariant& newValue ) override; bool hasValue() const; + void setHasValue( bool hasValue ); caf::PdmField m_fieldName; caf::PdmField m_description; diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowJob.cpp b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowJob.cpp index ed178cdc33..378e3ee2eb 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowJob.cpp +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowJob.cpp @@ -189,7 +189,8 @@ QString RimWorkflowJob::runStatus() const //-------------------------------------------------------------------------------------------------- void RimWorkflowJob::updateTaskState( const QString& runId, const QString& taskName, const QString& state, const QString& error ) { - if ( runId != m_runId || ( state != "running" && state != "completed" && state != "failed" ) ) return; + static const QStringList knownStates{ "running", "completed", "failed", "skipped" }; + if ( runId != m_runId || !knownStates.contains( state ) ) return; auto* workflow = firstAncestorOrThisOfType(); if ( !workflow ) return; bool knownTask = false; @@ -206,6 +207,9 @@ void RimWorkflowJob::updateTaskState( const QString& runId, const QString& taskN m_taskErrors[taskName] = error; m_runStatus = "Failed: " + error; } + + // A task whose run condition did not hold, or whose alternatives (first of) were all skipped + if ( state == "skipped" && !error.isEmpty() ) m_taskErrors[taskName] = "Skipped: " + error; if ( auto* window = RiuMainWindow::instance() ) window->workflowJobStateChanged( this ); } @@ -349,7 +353,7 @@ void RimWorkflowJob::syncTaskInputs( const QJsonArray& graphTasks, const QMapitems() ) { const QJsonValue value = binding->toJsonValue(); - if ( !value.isNull() ) + if ( !value.isNull() || binding->isExplicitNull() ) detached.insert( name + "/" + RimWorkflowTaskInput::detachedKey( binding->fieldName(), binding->classKeyword() ), value ); } delete removed; diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowJob.h b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowJob.h index 17527ddb0e..0e54b965af 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowJob.h +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowJob.h @@ -55,6 +55,7 @@ class RimWorkflowJob : public caf::PdmObject bool isRunning() const; QMap taskStates() const; + // Error of a failed task, or the reason a task was skipped QMap taskErrors() const; QString runStatus() const; void updateTaskState( const QString& runId, const QString& taskName, const QString& state, const QString& error ); diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.cpp b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.cpp index 2558658d25..ee25602203 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.cpp +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.cpp @@ -485,7 +485,7 @@ QStringList RimWorkflowPortCompatibility::coveredByWholeOutput( const QJsonObjec } //-------------------------------------------------------------------------------------------------- -/// Whether adding the connection from -> to closes a cycle +/// Whether adding the connection from -> to closes a cycle. Run conditions count as connections. //-------------------------------------------------------------------------------------------------- bool RimWorkflowPortCompatibility::wouldCreateCycle( const RimWorkflowDefinition& definition, const QString& from, const QString& to ) { @@ -503,6 +503,10 @@ bool RimWorkflowPortCompatibility::wouldCreateCycle( const RimWorkflowDefinition { if ( edge.from == current ) pending.append( edge.to ); } + for ( const auto& node : definition.nodes ) + { + if ( node.when && node.when->source == current ) pending.append( node.name ); + } } return false; } @@ -714,3 +718,28 @@ std::expected RimWorkflowPortCompatibility::checkCollectMember( c } return {}; } + +//-------------------------------------------------------------------------------------------------- +/// Type check of one alternative of a `first_of` input: it must fill the input on its own +//-------------------------------------------------------------------------------------------------- +std::expected RimWorkflowPortCompatibility::checkFirstOfMember( const RimWorkflowDefinition& definition, + const RimWorkflowDefinitionEdge& edge ) +{ + const QJsonObject upstreamType = definition.taskTypeForNode( edge.from ); + const QJsonObject downstreamType = definition.taskTypeForNode( edge.to ); + if ( upstreamType.isEmpty() || downstreamType.isEmpty() ) return {}; + + const auto producedPort = outputPort( upstreamType, edge.output ); + if ( !producedPort ) return std::unexpected( QString( "'%1' has no output '%2'" ).arg( edge.from, edge.output ) ); + const auto expectedPort = inputPort( downstreamType, edge.input ); + if ( !expectedPort || expectedPort->isWhole() ) + return std::unexpected( QString( "Alternatives (first of) need a named input; '%1' has no input '%2'" ).arg( edge.to, edge.input ) ); + + if ( !isCompatible( *producedPort, *expectedPort ) ) + { + const QString source = edge.output.isEmpty() ? edge.from : edge.from + "." + edge.output; + return std::unexpected( + QString( "%1 is %2, but %3.%4 expects %5" ).arg( source, producedPort->typeName, edge.to, edge.input, expectedPort->typeName ) ); + } + return {}; +} diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.h b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.h index a86a068df8..ebfd53b9ad 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.h +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.h @@ -84,4 +84,5 @@ std::expected resolveConnection( const RimWo std::expected canConnect( const RimWorkflowDefinition& definition, const QString& from, const QString& output, const QString& to, const QString& input ); std::expected checkCollectMember( const RimWorkflowDefinition& definition, const RimWorkflowDefinitionEdge& edge ); +std::expected checkFirstOfMember( const RimWorkflowDefinition& definition, const RimWorkflowDefinitionEdge& edge ); } // namespace RimWorkflowPortCompatibility diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowSchemaTools.cpp b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowSchemaTools.cpp index bed16d3df0..e746bee50e 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowSchemaTools.cpp +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowSchemaTools.cpp @@ -19,9 +19,12 @@ #include "RimWorkflowSchemaTools.h" #include +#include #include #include +#include + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- @@ -129,7 +132,61 @@ QString RimWorkflowSchemaTools::jsonType( const QJsonObject& typeSchema ) { const QString type = typeSchema.value( "type" ).toString(); if ( type == "boolean" || type == "integer" || type == "number" || type == "array" ) return type; - return "string"; + + // int | float + const QJsonArray anyOf = typeSchema.value( "anyOf" ).toArray(); + if ( !anyOf.isEmpty() && std::all_of( anyOf.begin(), + anyOf.end(), + []( const QJsonValue& arm ) + { + const QString armType = arm.toObject().value( "type" ).toString(); + return armType == "integer" || armType == "number"; + } ) ) + return "number"; + + // Any: the value is typed as a literal + for ( const char* key : { "type", "anyOf", "oneOf", "allOf", "enum", "const", "$ref", "properties", "items" } ) + { + if ( typeSchema.contains( key ) ) return "string"; + } + return "any"; +} + +//-------------------------------------------------------------------------------------------------- +/// A value typed for an untyped (Any) field, read like a YAML scalar: true/false, numbers, +/// null/None/~ and JSON arrays, objects and quoted strings. Other text is a string; empty text is null. +//-------------------------------------------------------------------------------------------------- +QJsonValue RimWorkflowSchemaTools::literalFromText( const QString& text ) +{ + const QString token = text.trimmed(); + const QString lower = token.toLower(); + if ( token.isEmpty() || lower == "null" || lower == "none" || token == "~" ) return QJsonValue( QJsonValue::Null ); + if ( lower == "true" ) return true; + if ( lower == "false" ) return false; + + const QJsonDocument document = QJsonDocument::fromJson( ( "[" + token + "]" ).toUtf8() ); + if ( document.isArray() && document.array().size() == 1 ) return document.array().first(); + return token; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +QString RimWorkflowSchemaTools::explicitNullKey() +{ + return "explicit_null"; +} + +//-------------------------------------------------------------------------------------------------- +/// The text that `literalFromText` reads back as `value`; null is written as `null` +//-------------------------------------------------------------------------------------------------- +QString RimWorkflowSchemaTools::textFromLiteral( const QJsonValue& value ) +{ + if ( value.isString() && literalFromText( value.toString() ) == value ) return value.toString(); + if ( value.isUndefined() ) return {}; + + const QByteArray json = QJsonDocument( QJsonArray{ value } ).toJson( QJsonDocument::Compact ); + return QString::fromUtf8( json.mid( 1, json.size() - 2 ) ); } //-------------------------------------------------------------------------------------------------- @@ -252,7 +309,11 @@ QJsonObject RimWorkflowSchemaTools::configFieldSchema( const QString& fieldN // Prefer the value already in input.yaml; fall back to the model default if ( configValues.contains( fieldName ) ) + { entry["default"] = configValues.value( fieldName ); + // A null in input.yaml is an entered None, unlike a model default of None + if ( entry["default"].isNull() ) entry[explicitNullKey()] = true; + } else if ( property.contains( "default" ) ) entry["default"] = property.value( "default" ); diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowSchemaTools.h b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowSchemaTools.h index f1bb24357f..2f9e9c7c43 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowSchemaTools.h +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowSchemaTools.h @@ -19,6 +19,7 @@ #pragma once #include +#include #include #include @@ -38,6 +39,11 @@ QString resinsightTypeFromPythonType( const QString& pythonType ); QStringList requiredFields( const QJsonObject& modelSchema ); QStringList propertyNames( const QJsonObject& modelSchema ); bool isConfigurable( const QJsonObject& property, const QJsonObject& rootSchema ); +QJsonValue literalFromText( const QString& text ); +QString textFromLiteral( const QJsonValue& value ); + +// Config field schema key: a null default is an entered None rather than an unset value +QString explicitNullKey(); QJsonObject configFieldSchema( const QString& fieldName, const QJsonObject& inputSchema, diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowStringBinding.cpp b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowStringBinding.cpp index 9cc076a303..dd5953bd1a 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowStringBinding.cpp +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowStringBinding.cpp @@ -18,6 +18,8 @@ #include "RimWorkflowStringBinding.h" +#include "RimWorkflowSchemaTools.h" + #include CAF_PDM_SOURCE_INIT( RimWorkflowStringBinding, "WorkflowStringBinding" ); @@ -28,6 +30,10 @@ CAF_PDM_SOURCE_INIT( RimWorkflowStringBinding, "WorkflowStringBinding" ); RimWorkflowStringBinding::RimWorkflowStringBinding() { CAF_PDM_InitFieldNoDefault( &m_value, "Value", "Value" ); + + // Untyped (Any) fields read the text as a literal, see RimWorkflowSchemaTools::literalFromText + CAF_PDM_InitField( &m_isLiteral, "IsLiteral", false, "Is Literal" ); + m_isLiteral.uiCapability()->setUiHidden( true ); } //-------------------------------------------------------------------------------------------------- @@ -35,10 +41,39 @@ RimWorkflowStringBinding::RimWorkflowStringBinding() //-------------------------------------------------------------------------------------------------- void RimWorkflowStringBinding::applySchema( const QJsonObject& fieldSchema ) { + // A binding without a field name is new, and its default is a typed value + const bool isNew = m_fieldName().isEmpty(); RimWorkflowFieldBinding::applySchema( fieldSchema ); + + const bool wasLiteral = m_isLiteral(); + m_isLiteral = fieldSchema.value( "type" ).toString() == "any"; + if ( m_isLiteral() ) + { + QString toolTip = fieldSchema.value( "description" ).toString(); + if ( !toolTip.isEmpty() ) toolTip += "\n\n"; + toolTip += "Any value: true, false, a number, null (None) or JSON. Other text is a string; quote it to keep \"true\" as text."; + m_value.uiCapability()->setUiToolTip( toolTip ); + } + if ( fieldSchema.contains( "default" ) ) { - m_value = fieldSchema.value( "default" ).toString(); + const QJsonValue value = fieldSchema.value( "default" ); + if ( !m_isLiteral() ) + m_value = value.toString(); + else if ( value.isNull() ) + { + // Only an entered None is a value; a model default of None leaves the field unset + const bool entered = fieldSchema.value( RimWorkflowSchemaTools::explicitNullKey() ).toBool(); + m_value = entered ? RimWorkflowSchemaTools::textFromLiteral( value ) : QString(); + setHasValue( entered ); + } + else + { + // The text of an existing binding that did not read literals is kept, and now read as a + // literal. Typed values (input.yaml, model defaults, detached values) keep their type. + const bool legacyText = !isNew && !wasLiteral && value.isString(); + m_value = legacyText ? value.toString() : RimWorkflowSchemaTools::textFromLiteral( value ); + } } } @@ -47,5 +82,14 @@ void RimWorkflowStringBinding::applySchema( const QJsonObject& fieldSchema ) //-------------------------------------------------------------------------------------------------- QJsonValue RimWorkflowStringBinding::toJsonValue() const { - return hasValue() ? QJsonValue( m_value() ) : QJsonValue( QJsonValue::Null ); + if ( !hasValue() ) return QJsonValue( QJsonValue::Null ); + return m_isLiteral() ? RimWorkflowSchemaTools::literalFromText( m_value() ) : QJsonValue( m_value() ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +bool RimWorkflowStringBinding::isExplicitNull() const +{ + return hasValue() && m_isLiteral() && !m_value().trimmed().isEmpty() && toJsonValue().isNull(); } diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowStringBinding.h b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowStringBinding.h index 14fc1bda80..053bee9198 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowStringBinding.h +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowStringBinding.h @@ -29,9 +29,11 @@ class RimWorkflowStringBinding : public RimWorkflowFieldBinding void applySchema( const QJsonObject& fieldSchema ) override; QJsonValue toJsonValue() const override; + bool isExplicitNull() const override; caf::PdmFieldHandle* valueField() override { return &m_value; } private: caf::PdmField m_value; + caf::PdmField m_isLiteral; }; diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowTaskInput.cpp b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowTaskInput.cpp index 8cb85812f6..e0844921bd 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowTaskInput.cpp +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowTaskInput.cpp @@ -25,6 +25,7 @@ #include "RimWorkflowFloatBinding.h" #include "RimWorkflowIntBinding.h" #include "RimWorkflowMappingBinding.h" +#include "RimWorkflowSchemaTools.h" #include "RimWorkflowStringBinding.h" #include "RimWorkflowVec3Binding.h" #include "RimWorkflowViewBinding.h" @@ -185,16 +186,22 @@ void RimWorkflowTaskInput::syncFromSchema( const QJsonArray& configFields, QMap< { const QJsonValue current = binding->toJsonValue(); // Keep a cleared value cleared instead of restoring the default - if ( current.isNull() ) + if ( current.isNull() && !binding->isExplicitNull() ) schema.remove( "default" ); else schema["default"] = current; + schema[RimWorkflowSchemaTools::explicitNullKey()] = binding->isExplicitNull(); } else { binding = createBinding( schema ); const QString key = detachedKey( field, keyword ); - if ( detachedValues.contains( key ) ) schema["default"] = detachedValues.take( key ); + if ( detachedValues.contains( key ) ) + { + // Only values that were set are detached, so a null is an entered None + schema["default"] = detachedValues.take( key ); + schema[RimWorkflowSchemaTools::explicitNullKey()] = schema["default"].isNull(); + } } binding->applySchema( schema ); addItem( binding ); @@ -204,7 +211,8 @@ void RimWorkflowTaskInput::syncFromSchema( const QJsonArray& configFields, QMap< { if ( !removed ) continue; const QJsonValue value = removed->toJsonValue(); - if ( !value.isNull() ) detachedValues.insert( detachedKey( removed->fieldName(), removed->classKeyword() ), value ); + if ( !value.isNull() || removed->isExplicitNull() ) + detachedValues.insert( detachedKey( removed->fieldName(), removed->classKeyword() ), value ); delete removed; } } @@ -218,7 +226,7 @@ QJsonObject RimWorkflowTaskInput::jsonValues() const for ( const RimWorkflowFieldBinding* binding : items() ) { const QJsonValue value = binding->toJsonValue(); - if ( !value.isNull() ) values.insert( binding->fieldName(), value ); + if ( !value.isNull() || binding->isExplicitNull() ) values.insert( binding->fieldName(), value ); } return values; } @@ -233,7 +241,7 @@ QJsonObject RimWorkflowTaskInput::literalValues() const { if ( binding->isObjectReference() ) continue; const QJsonValue value = binding->toJsonValue(); - if ( !value.isNull() ) values.insert( binding->fieldName(), value ); + if ( !value.isNull() || binding->isExplicitNull() ) values.insert( binding->fieldName(), value ); } return values; } diff --git a/ApplicationLibCode/UnitTests/RimWorkflowDefinition-Test.cpp b/ApplicationLibCode/UnitTests/RimWorkflowDefinition-Test.cpp index 33b20b0fd3..c1a7306bdd 100644 --- a/ApplicationLibCode/UnitTests/RimWorkflowDefinition-Test.cpp +++ b/ApplicationLibCode/UnitTests/RimWorkflowDefinition-Test.cpp @@ -20,6 +20,7 @@ #include "RiaTestDataDirectory.h" #include "RimWorkflowDefinitionTools.h" +#include "RimWorkflowDescribeTools.h" #include "RimWorkflowPortCompatibility.h" #include "RimWorkflowTaskCatalog.h" @@ -34,6 +35,9 @@ // python -m rips.taskmaestro_helper load rips/tests/test_data/taskmaestro_workflows/resinsight_completions --describe > completions.json // python -m rips.taskmaestro_helper load rips/tests/test_data/taskmaestro_workflows/resinsight_load_data --describe > load_data.json // python -m rips.taskmaestro_helper catalog > catalog.json +// conditional.json needs taskmaestro with conditions support and the test workflows on PYTHONPATH: +// PYTHONPATH=.:rips/tests/test_data/taskmaestro_workflows python -m rips.taskmaestro_helper load +// rips/tests/test_data/taskmaestro_workflows/conditional --describe > conditional.json // namespace { @@ -608,3 +612,339 @@ TEST( RimWorkflowDefinitionEdit, mapTask ) EXPECT_EQ( QStringList( { "name", "factor" } ), definition.findNode( "a" )->configFields ); EXPECT_FALSE( definition.inputs.value( "a" ).toObject().contains( "cases" ) ); } + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowConditional, fromJsonAndRoundTrip ) +{ + using Op = RimWorkflowTaskGuard::Op; + const auto definition = definitionFixture( "conditional" ); + const auto guardOf = [&definition]( const QString& name ) { return definition.findNode( name )->when; }; + const auto largeGuard = guardOf( "double_large" ); + const auto smallGuard = guardOf( "double_small" ); + const auto positiveGuard = guardOf( "double_positive" ); + const auto specialGuard = guardOf( "special" ); + + EXPECT_TRUE( definition.editable ); + EXPECT_TRUE( RimWorkflowDefinitionTools::structuralReadOnlyReasons( definition ).isEmpty() ); + ASSERT_TRUE( largeGuard && smallGuard && positiveGuard && specialGuard ); + EXPECT_EQ( ( RimWorkflowTaskGuard{ .source = "choose_size", .op = Op::Case, .values = { "large", "huge" }, .caseAsList = true } ), + *largeGuard ); + EXPECT_EQ( ( RimWorkflowTaskGuard{ .source = "choose_size", .op = Op::Otherwise } ), *smallGuard ); + EXPECT_EQ( ( RimWorkflowTaskGuard{ .source = "is_positive", .op = Op::Case, .values = { true } } ), *positiveGuard ); + EXPECT_EQ( ( RimWorkflowTaskGuard{ .source = "start", .field = "value", .op = Op::In, .values = { 1.0, 2.0 } } ), *specialGuard ); + EXPECT_FALSE( guardOf( "report" ).has_value() ); + + int firstOf = 0; + for ( const auto& edge : definition.edges ) + { + if ( edge.to == "report" ) EXPECT_TRUE( edge.isFirstOf() ); + if ( edge.isFirstOf() ) ++firstOf; + } + EXPECT_EQ( 4, firstOf ); + + // The YAML forms of the guards and the order of the alternatives survive a round trip + const QJsonObject json = RimWorkflowDefinitionTools::toJson( definition ); + // Written nodes also carry ResInsight's explicit config field marker + QJsonArray writtenNodes; + for ( const QJsonValue& node : json.value( "nodes" ).toArray() ) + { + QJsonObject object = node.toObject(); + object.remove( "x_ri_explicit_config_fields" ); + writtenNodes.append( object ); + } + EXPECT_EQ( fixture( "conditional" ).value( "nodes" ).toArray(), writtenNodes ); + EXPECT_EQ( fixture( "conditional" ).value( "edges" ), json.value( "edges" ) ); + const auto roundTrip = RimWorkflowDefinitionTools::fromJson( json ); + ASSERT_TRUE( roundTrip.has_value() ); + EXPECT_EQ( definition, roundTrip.value() ); +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowConditional, validFixtureAndSinks ) +{ + const auto definition = definitionFixture( "conditional" ); + EXPECT_TRUE( RimWorkflowDefinitionTools::validate( definition ).empty() ) + << issueMessages( RimWorkflowDefinitionTools::validate( definition ) ).join( "\n" ).toStdString(); + EXPECT_EQ( definition, RimWorkflowDefinitionTools::normalize( definition ) ); + + // Switch tasks used only by run conditions are not end tasks + QStringList sinks = RimWorkflowDefinitionTools::sinkTasks( definition ); + sinks.sort(); + EXPECT_EQ( QStringList( { "report", "special" } ), sinks ); +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowConditional, switchBranches ) +{ + const auto definition = definitionFixture( "conditional" ); + const auto typeOf = [&definition]( const QString& name ) { return definition.taskType( definition.findNode( name )->taskId ); }; + + EXPECT_TRUE( RimWorkflowDefinitionTools::isSwitchTask( typeOf( "choose_size" ) ) ); + EXPECT_TRUE( RimWorkflowDefinitionTools::isSwitchTask( typeOf( "is_positive" ) ) ); + EXPECT_FALSE( RimWorkflowDefinitionTools::isSwitchTask( typeOf( "start" ) ) ); + EXPECT_EQ( QJsonArray( { "small", "large", "huge" } ), RimWorkflowDefinitionTools::switchBranches( typeOf( "choose_size" ) ) ); + EXPECT_EQ( QJsonArray( { true, false } ), RimWorkflowDefinitionTools::switchBranches( typeOf( "is_positive" ) ) ); + EXPECT_TRUE( RimWorkflowDefinitionTools::switchBranches( typeOf( "start" ) ).isEmpty() ); +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowConditional, checkTaskGuard ) +{ + using Op = RimWorkflowTaskGuard::Op; + const auto definition = definitionFixture( "conditional" ); + const auto check = [&definition]( const QString& node, const RimWorkflowTaskGuard& guard ) + { + const auto result = RimWorkflowDefinitionTools::checkTaskGuard( definition, node, guard ); + return result ? QString() : result.error(); + }; + + EXPECT_EQ( "", check( "special", { .source = "choose_size", .op = Op::Case, .values = { "small" } } ) ); + EXPECT_EQ( "", check( "special", { .source = "start", .field = "value", .op = Op::Equals, .values = { 3 } } ) ); + EXPECT_EQ( "", check( "special", { .source = "choose_size", .op = Op::Otherwise } ) ); + + EXPECT_TRUE( check( "special", { .source = "missing", .op = Op::Otherwise } ).contains( "unknown task" ) ); + EXPECT_TRUE( check( "special", { .source = "special", .field = "value", .op = Op::Truthy } ).contains( "its own output" ) ); + EXPECT_TRUE( check( "special", { .source = "choose_size", .op = Op::Case, .values = { "tiny" } } ).contains( "not a valid value" ) ); + EXPECT_TRUE( check( "special", { .source = "choose_size", .op = Op::Case } ).contains( "at least one branch" ) ); + EXPECT_TRUE( check( "special", { .source = "start", .op = Op::Case, .values = { 1 } } ).contains( "not a switch task" ) ); + EXPECT_TRUE( check( "special", { .source = "start", .field = "value", .op = Op::Otherwise } ).contains( "requires a switch" ) ); + EXPECT_TRUE( check( "special", { .source = "start", .field = "value", .op = Op::Truthy } ).contains( "bool or optional" ) ); + EXPECT_TRUE( + check( "special", { .source = "start", .field = "value", .op = Op::Equals, .values = { "text" } } ).contains( "not a valid value" ) ); + EXPECT_TRUE( check( "special", { .source = "start", .field = "missing", .op = Op::Truthy } ).contains( "no output field" ) ); + EXPECT_TRUE( check( "choose_size", { .source = "report", .field = "sized", .op = Op::Truthy } ).contains( "cycle" ) ); +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowConditional, validateSwitchCoverage ) +{ + using Op = RimWorkflowTaskGuard::Op; + const auto definition = definitionFixture( "conditional" ); + + // Without the otherwise branch, `small` is not handled by any task + const auto uncovered = edited( RimWorkflowDefinitionTools::setTaskGuard( definition, "double_small", std::nullopt ) ); + EXPECT_TRUE( issueMessages( RimWorkflowDefinitionTools::validate( uncovered ) ).join( "\n" ).contains( "small" ) ); + + // An otherwise branch that can never match + const auto allLabels = edited( RimWorkflowDefinitionTools::setTaskGuard( definition, + "double_large", + RimWorkflowTaskGuard{ .source = "choose_size", + .op = Op::Case, + .values = { "small", "large", "huge" } } ) ); + const auto issues = RimWorkflowDefinitionTools::validate( allLabels ); + ASSERT_FALSE( issues.empty() ); + EXPECT_EQ( "choose_size", issues.front().task ); +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowConditional, guardsFollowRenamesAndRemovals ) +{ + const auto definition = definitionFixture( "conditional" ); + + const auto renamed = edited( RimWorkflowDefinitionTools::renameTask( definition, "choose_size", "picker" ) ); + EXPECT_EQ( "picker", renamed.findNode( "double_large" )->when->source ); + EXPECT_EQ( "picker", renamed.findNode( "double_small" )->when->source ); + + const auto removed = edited( RimWorkflowDefinitionTools::removeTask( definition, "is_positive" ) ); + EXPECT_FALSE( removed.findNode( "double_positive" )->when.has_value() ); + EXPECT_FALSE( removed.findNode( "constant" )->when.has_value() ); + EXPECT_TRUE( removed.findNode( "double_large" )->when.has_value() ); + + const auto cleared = edited( RimWorkflowDefinitionTools::setTaskGuard( definition, "special", std::nullopt ) ); + EXPECT_FALSE( cleared.findNode( "special" )->when.has_value() ); +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowConditional, firstOfAlternatives ) +{ + using Collect = RimWorkflowDefinitionEdge::Collect; + const auto definition = definitionFixture( "conditional" ); + + // Removing one of two alternatives leaves a plain connection + const auto single = + edited( RimWorkflowDefinitionTools::disconnect( definition, { .from = "double_small", .to = "report", .input = "sized" } ) ); + EXPECT_TRUE( hasEdge( single, { .from = "double_large", .to = "report", .input = "sized" } ) ); + + // ... and adding an alternative to it makes the connection the first alternative + const auto again = + edited( RimWorkflowDefinitionTools::addFirstOfMember( single, { .from = "double_small", .to = "report", .input = "sized" } ) ); + EXPECT_TRUE( hasEdge( again, { .from = "double_large", .to = "report", .input = "sized", .collect = Collect::FirstOf } ) ); + EXPECT_TRUE( hasEdge( again, { .from = "double_small", .to = "report", .input = "sized", .collect = Collect::FirstOf } ) ); + EXPECT_EQ( definition, again ); + + const auto third = + edited( RimWorkflowDefinitionTools::addFirstOfMember( definition, { .from = "double_positive", .to = "report", .input = "sized" } ) ); + EXPECT_TRUE( hasEdge( third, { .from = "double_positive", .to = "report", .input = "sized", .collect = Collect::FirstOf } ) ); + const auto moved = edited( + RimWorkflowDefinitionTools::moveCollectMember( third, + { .from = "double_positive", .to = "report", .input = "sized", .collect = Collect::FirstOf }, + -2 ) ); + for ( const auto& edge : moved.edges ) + { + if ( edge.input != "sized" ) continue; + EXPECT_EQ( "double_positive", edge.from ); + break; + } + + EXPECT_FALSE( + RimWorkflowDefinitionTools::addFirstOfMember( definition, { .from = "start", .to = "special", .input = "value" } ).has_value() ); + EXPECT_FALSE( + RimWorkflowDefinitionTools::addFirstOfMember( definition, { .from = "double_small", .to = "report", .input = "sized" } ).has_value() ); + + // A plain connection replaces all alternatives + const auto replaced = + edited( RimWorkflowDefinitionTools::connect( definition, { .from = "double_positive", .to = "report", .input = "sized" } ) ); + EXPECT_FALSE( hasEdge( replaced, { .from = "double_large", .to = "report", .input = "sized", .collect = Collect::FirstOf } ) ); + EXPECT_TRUE( hasEdge( replaced, { .from = "double_positive", .to = "report", .input = "sized" } ) ); +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowConditional, graphFromDefinition ) +{ + const auto definition = definitionFixture( "conditional" ); + const QJsonObject graph = RimWorkflowDefinitionTools::graphFromDefinition( definition, true ); + + EXPECT_TRUE( definitionTaskByName( graph, "choose_size" ).value( "switch" ).toBool() ); + EXPECT_EQ( QJsonArray( { "small", "large", "huge" } ), definitionTaskByName( graph, "choose_size" ).value( "branches" ).toArray() ); + EXPECT_FALSE( definitionTaskByName( graph, "start" ).value( "switch" ).toBool() ); + EXPECT_EQ( "large, huge", definitionTaskByName( graph, "double_large" ).value( "when_label" ).toString() ); + + QMap conditions; + QMap alternatives; + for ( const QJsonValue& value : graph.value( "edges" ).toArray() ) + { + const QJsonObject edge = value.toObject(); + if ( edge.value( "kind" ).toString() == "when" ) + conditions.insert( edge.value( "to" ).toString(), edge.value( "from" ).toString() + ": " + edge.value( "label" ).toString() ); + if ( edge.value( "collect" ).toString() == "first_of" ) + alternatives.insert( edge.value( "from" ).toString(), edge.value( "label" ).toString() ); + } + const QMap expectedConditions{ { "double_large", "choose_size: large, huge" }, + { "double_small", "choose_size: otherwise" }, + { "double_positive", "is_positive: true" }, + { "constant", "is_positive: false" }, + { "special", "start: value in {1, 2}" } }; + EXPECT_EQ( expectedConditions, conditions ); + const QMap expectedAlternatives{ { "double_large", "1st" }, + { "double_small", "2nd" }, + { "double_positive", "1st" }, + { "constant", "2nd" } }; + EXPECT_EQ( expectedAlternatives, alternatives ); + + for ( const QJsonValue& port : definitionTaskByName( graph, "report" ).value( "input_ports" ).toArray() ) + { + const QString name = port.toObject().value( "name" ).toString(); + if ( name == "sized" || name == "signed" ) EXPECT_TRUE( port.toObject().value( "first_of" ).toBool() ) << name.toStdString(); + } +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowConditional, graphFromDescribe ) +{ + const QJsonObject graph = RimWorkflowDescribeTools::graphFromDescribe( fixture( "conditional" ).value( "describe" ).toObject() ); + + EXPECT_TRUE( definitionTaskByName( graph, "choose_size" ).value( "switch" ).toBool() ); + EXPECT_FALSE( definitionTaskByName( graph, "report" ).value( "switch" ).toBool() ); + + QStringList conditions; + QStringList alternatives; + for ( const QJsonValue& value : graph.value( "edges" ).toArray() ) + { + const QJsonObject edge = value.toObject(); + if ( edge.value( "kind" ).toString() == "when" ) + conditions.append( + QString( "%1->%2: %3" ).arg( edge.value( "from" ).toString(), edge.value( "to" ).toString(), edge.value( "label" ).toString() ) ); + if ( edge.value( "collect" ).toString() == "first_of" ) + alternatives.append( QString( "%1->%2.%3 %4" ) + .arg( edge.value( "from" ).toString(), + edge.value( "to" ).toString(), + edge.value( "input" ).toString(), + edge.value( "label" ).toString() ) ); + } + conditions.sort(); + alternatives.sort(); + EXPECT_EQ( QStringList( { "choose_size->double_large: large, huge", + "choose_size->double_small: otherwise", + "is_positive->constant: false", + "is_positive->double_positive: true", + "start->special: value in {1, 2}" } ), + conditions ); + EXPECT_EQ( QStringList( { "constant->report.signed 2nd", + "double_large->report.sized 1st", + "double_positive->report.signed 1st", + "double_small->report.sized 2nd" } ), + alternatives ); +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowConditional, guardValuesText ) +{ + using RimWorkflowDefinitionTools::guardValuesFromText; + using RimWorkflowDefinitionTools::guardValuesToText; + const QJsonObject numberField{ { "type", "number" } }; + const QJsonObject stringField{ { "type", "string" } }; + + EXPECT_EQ( QJsonArray( { 1, "a", true } ), guardValuesFromText( "1, a, true", numberField ) ); + EXPECT_EQ( QJsonArray( { "1", "a", "true" } ), guardValuesFromText( "1, a, true", stringField ) ); + + // Quoted values are strings and may contain commas and escaped quotes + EXPECT_EQ( QJsonArray( { "a, b", "1", "say \"hi\"" } ), guardValuesFromText( "\"a, b\", \"1\", \"say \\\"hi\\\"\"", numberField ) ); + EXPECT_EQ( QJsonArray( { "a, b", "c" } ), guardValuesFromText( "\"a, b\", c", stringField ) ); + + // Values read back as written, with strings quoted only when needed + const QJsonArray values{ "1", "a, b", "text", 2.5, false, QJsonValue::Null, "say \"hi\"" }; + EXPECT_EQ( values, guardValuesFromText( guardValuesToText( values, numberField ), numberField ) ); + const QJsonArray strings{ "1", "a, b", "text", "\"quoted\"", "say \"hi\"" }; + EXPECT_EQ( strings, guardValuesFromText( guardValuesToText( strings, stringField ), stringField ) ); + EXPECT_EQ( "\"1\", text", guardValuesToText( QJsonArray( { "1", "text" } ), numberField ) ); + EXPECT_EQ( "1, text", guardValuesToText( QJsonArray( { "1", "text" } ), stringField ) ); + + // A string Literal has enum or const but no type + EXPECT_EQ( QJsonArray( { "1", "true" } ), guardValuesFromText( "1, true", QJsonObject{ { "enum", QJsonArray( { "1", "true" } ) } } ) ); + EXPECT_EQ( QJsonArray( { "1" } ), guardValuesFromText( "1", QJsonObject{ { "const", "1" } } ) ); + EXPECT_EQ( QJsonArray( { 1 } ), guardValuesFromText( "1", QJsonObject{ { "enum", QJsonArray( { 1, "a" } ) } } ) ); +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowConditional, malformedFieldRef ) +{ + QJsonObject json = fixture( "conditional" ); + QJsonArray nodes = json.value( "nodes" ).toArray(); + for ( qsizetype i = 0; i < nodes.size(); ++i ) + { + QJsonObject node = nodes.at( i ).toObject(); + if ( node.value( "name" ) != "double_large" ) continue; + node["when"] = QJsonObject{ { "ref", QJsonArray( { "choose_size" } ) }, { "truthy", true } }; + nodes.replace( i, node ); + } + json["nodes"] = nodes; + + const auto definition = RimWorkflowDefinitionTools::fromJson( json ); + ASSERT_TRUE( definition.has_value() ); + const auto guard = definition->findNode( "double_large" )->when; + ASSERT_TRUE( guard.has_value() ); + EXPECT_FALSE( guard->isSwitchBranch() ); + EXPECT_TRUE( issueMessages( RimWorkflowDefinitionTools::validate( *definition ) ).join( "\n" ).contains( "task and an output field" ) ); +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowConditional, switchWithoutCase ) +{ + QJsonObject json = fixture( "conditional" ); + QJsonArray nodes = json.value( "nodes" ).toArray(); + for ( qsizetype i = 0; i < nodes.size(); ++i ) + { + QJsonObject node = nodes.at( i ).toObject(); + if ( node.value( "name" ) != "double_large" ) continue; + node["when"] = QJsonObject{ { "switch", "choose_size" } }; + nodes.replace( i, node ); + } + json["nodes"] = nodes; + + const auto definition = RimWorkflowDefinitionTools::fromJson( json ); + ASSERT_TRUE( definition.has_value() ); + const auto guard = definition->findNode( "double_large" )->when; + ASSERT_TRUE( guard.has_value() ); + EXPECT_TRUE( guard->values.isEmpty() ); + EXPECT_TRUE( issueMessages( RimWorkflowDefinitionTools::validate( *definition ) ).join( "\n" ).contains( "at least one branch" ) ); +} diff --git a/ApplicationLibCode/UnitTests/RimWorkflowDescribeTools-Test.cpp b/ApplicationLibCode/UnitTests/RimWorkflowDescribeTools-Test.cpp index 0addc96d8b..bc1a12f1f2 100644 --- a/ApplicationLibCode/UnitTests/RimWorkflowDescribeTools-Test.cpp +++ b/ApplicationLibCode/UnitTests/RimWorkflowDescribeTools-Test.cpp @@ -2,6 +2,7 @@ #include "RiaTestDataDirectory.h" #include "RimWorkflowDescribeTools.h" +#include "RimWorkflowSchemaTools.h" #include #include @@ -158,6 +159,51 @@ TEST( RimWorkflowDescribeTools, vec3FieldIsRecognizedStructurally ) EXPECT_TRUE( referencePoint.value( "required" ).toBool() ); } +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowDescribeTools, numericUnionFieldIsNumber ) +{ + const QJsonObject inputSchema{ + { "properties", + QJsonObject{ { "value", + QJsonObject{ { "anyOf", QJsonArray{ QJsonObject{ { "type", "integer" } }, QJsonObject{ { "type", "number" } } } } } }, + { "mixed", + QJsonObject{ { "anyOf", QJsonArray{ QJsonObject{ { "type", "integer" } }, QJsonObject{ { "type", "string" } } } } } } } } }; + + EXPECT_EQ( "number", RimWorkflowSchemaTools::configFieldSchema( "value", inputSchema, {}, {} ).value( "type" ).toString() ); + EXPECT_EQ( "string", RimWorkflowSchemaTools::configFieldSchema( "mixed", inputSchema, {}, {} ).value( "type" ).toString() ); +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowDescribeTools, untypedFieldIsLiteral ) +{ + const QJsonObject inputSchema{ + { "properties", QJsonObject{ { "value", QJsonObject{ { "title", "Value" }, { "description", "Value to test" } } } } } }; + EXPECT_EQ( "any", RimWorkflowSchemaTools::configFieldSchema( "value", inputSchema, {}, {} ).value( "type" ).toString() ); + + using RimWorkflowSchemaTools::literalFromText; + EXPECT_EQ( QJsonValue( false ), literalFromText( "False" ) ); + EXPECT_EQ( QJsonValue( true ), literalFromText( " true " ) ); + EXPECT_EQ( QJsonValue( 0 ), literalFromText( "0" ) ); + EXPECT_EQ( QJsonValue( 2.5 ), literalFromText( "2.5" ) ); + EXPECT_TRUE( literalFromText( "None" ).isNull() ); + EXPECT_TRUE( literalFromText( "" ).isNull() ); + EXPECT_EQ( QJsonValue( "text" ), literalFromText( "text" ) ); + EXPECT_EQ( QJsonValue( "true" ), literalFromText( "\"true\"" ) ); + EXPECT_EQ( QJsonValue( QJsonArray( { 1, "a" } ) ), literalFromText( "[1, \"a\"]" ) ); + + for ( const QJsonValue& value : { QJsonValue( false ), + QJsonValue( 3 ), + QJsonValue( "text" ), + QJsonValue( "true" ), + QJsonValue( "0" ), + QJsonValue( QJsonValue::Null ), + QJsonValue( QJsonObject{ { "a", 1 } } ) } ) + { + EXPECT_EQ( value, literalFromText( RimWorkflowSchemaTools::textFromLiteral( value ) ) ) + << RimWorkflowSchemaTools::textFromLiteral( value ).toStdString(); + } +} + //-------------------------------------------------------------------------------------------------- TEST( RimWorkflowDescribeTools, dependencyEdges ) { diff --git a/ApplicationLibCode/UnitTests/RimWorkflowJobSync-Test.cpp b/ApplicationLibCode/UnitTests/RimWorkflowJobSync-Test.cpp index 4164d522f1..ee4a541247 100644 --- a/ApplicationLibCode/UnitTests/RimWorkflowJobSync-Test.cpp +++ b/ApplicationLibCode/UnitTests/RimWorkflowJobSync-Test.cpp @@ -23,6 +23,7 @@ #include "RimWorkflowDefinitionTools.h" #include "RimWorkflowHelperProcess.h" #include "RimWorkflowJob.h" +#include "RimWorkflowSchemaTools.h" #include "RimWorkflowTaskInput.h" #include "RimWorkflowValidationTools.h" @@ -121,6 +122,59 @@ TEST( RimWorkflowJobSync, valuesFollowRenamesAndRemovals ) EXPECT_EQ( "/path/to/grid.roff", pathValue( job, "load_model" ) ); } +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowJobSync, untypedFieldValuesAreLiterals ) +{ + const QString explicitNull = RimWorkflowSchemaTools::explicitNullKey(); + + // Text entered before the fields were untyped is read as a literal + RimWorkflowTaskInput input; + input.buildFromSchema( QJsonArray{ QJsonObject{ { "name", "flag" }, { "type", "string" }, { "default", "False" } }, + QJsonObject{ { "name", "nothing" }, { "type", "string" }, { "default", "None" } }, + QJsonObject{ { "name", "text" }, { "type", "string" }, { "default", "False" } } } ); + const QJsonArray untyped{ QJsonObject{ { "name", "flag" }, { "type", "any" } }, + QJsonObject{ { "name", "nothing" }, { "type", "any" } }, + QJsonObject{ { "name", "text" }, { "type", "string" } } }; + QMap detached; + input.syncFromSchema( untyped, detached ); + + const QJsonObject expected{ { "flag", false }, { "nothing", QJsonValue::Null }, { "text", "False" } }; + EXPECT_EQ( expected, input.jsonValues() ); + + // Values, including an entered None, survive a sync + input.syncFromSchema( untyped, detached ); + EXPECT_EQ( expected, input.jsonValues() ); + + // ... and a removal and return of the fields + input.syncFromSchema( QJsonArray{}, detached ); + EXPECT_TRUE( input.jsonValues().isEmpty() ); + input.syncFromSchema( untyped, detached ); + EXPECT_EQ( expected, input.jsonValues() ); + + // New bindings keep the type of typed values; a model default of None is not a value + RimWorkflowTaskInput typed; + typed.buildFromSchema( + QJsonArray{ QJsonObject{ { "name", "quoted" }, { "type", "any" }, { "default", "true" } }, + QJsonObject{ { "name", "number" }, { "type", "any" }, { "default", "1" } }, + QJsonObject{ { "name", "flag" }, { "type", "any" }, { "default", true } }, + QJsonObject{ { "name", "model_none" }, { "type", "any" }, { "default", QJsonValue::Null } }, + QJsonObject{ { "name", "entered_none" }, { "type", "any" }, { "default", QJsonValue::Null }, { explicitNull, true } } } ); + const QJsonObject expectedTyped{ { "quoted", "true" }, { "number", "1" }, { "flag", true }, { "entered_none", QJsonValue::Null } }; + EXPECT_EQ( expectedTyped, typed.jsonValues() ); +} + +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowJobSync, inputYamlNullIsExplicit ) +{ + const QJsonObject inputSchema{ { "properties", QJsonObject{ { "value", QJsonObject{ { "default", QJsonValue::Null } } } } } }; + const QString explicitNull = RimWorkflowSchemaTools::explicitNullKey(); + + EXPECT_FALSE( RimWorkflowSchemaTools::configFieldSchema( "value", inputSchema, {}, {} ).value( explicitNull ).toBool() ); + EXPECT_TRUE( RimWorkflowSchemaTools::configFieldSchema( "value", inputSchema, {}, QJsonObject{ { "value", QJsonValue::Null } } ) + .value( explicitNull ) + .toBool() ); +} + //-------------------------------------------------------------------------------------------------- TEST( RimWorkflowEditing, copyPythonModules ) { diff --git a/ApplicationLibCode/UnitTests/TestData/RimWorkflowDefinition/conditional.json b/ApplicationLibCode/UnitTests/TestData/RimWorkflowDefinition/conditional.json new file mode 100644 index 0000000000..d67ee9e6d1 --- /dev/null +++ b/ApplicationLibCode/UnitTests/TestData/RimWorkflowDefinition/conditional.json @@ -0,0 +1,1084 @@ +{ + "format": 1, + "name": "sizes", + "result_task": "report", + "editable": true, + "readonly_reasons": [], + "warnings": [], + "source": { + "workflow_yaml": "test_data/taskmaestro_workflows/conditional/workflow.yaml", + "input_yaml": "test_data/taskmaestro_workflows/conditional/input.yaml", + "registered_id": null + }, + "header_comment": "", + "passthrough": {}, + "nodes": [ + { + "name": "start", + "task": "tm_helper_pipeline.Start", + "config_fields": [ + "value" + ] + }, + { + "name": "choose_size", + "task": "tm_helper_conditional.ChooseSize", + "config_fields": [] + }, + { + "name": "is_positive", + "task": "tm_helper_conditional.IsPositive", + "config_fields": [] + }, + { + "name": "double_large", + "task": "tm_helper_pipeline.Double", + "config_fields": [], + "when": { + "switch": "choose_size", + "case": [ + "large", + "huge" + ] + } + }, + { + "name": "double_small", + "task": "tm_helper_pipeline.Double", + "config_fields": [], + "when": { + "switch": "choose_size", + "otherwise": true + } + }, + { + "name": "double_positive", + "task": "tm_helper_pipeline.Double", + "config_fields": [], + "when": { + "switch": "is_positive", + "case": true + } + }, + { + "name": "constant", + "task": "tm_helper_pipeline.Constant", + "config_fields": [], + "when": { + "switch": "is_positive", + "case": false + } + }, + { + "name": "special", + "task": "tm_helper_pipeline.Special", + "config_fields": [], + "when": { + "ref": [ + "start", + "value" + ], + "in": [ + 1.0, + 2.0 + ] + } + }, + { + "name": "report", + "task": "tm_helper_conditional.Report", + "config_fields": [] + } + ], + "edges": [ + { + "from": "start", + "output": null, + "to": "choose_size", + "input": null + }, + { + "from": "start", + "output": null, + "to": "is_positive", + "input": null + }, + { + "from": "start", + "output": null, + "to": "double_large", + "input": null + }, + { + "from": "start", + "output": null, + "to": "double_small", + "input": null + }, + { + "from": "start", + "output": null, + "to": "double_positive", + "input": null + }, + { + "from": "start", + "output": null, + "to": "special", + "input": null + }, + { + "from": "double_large", + "output": null, + "to": "report", + "input": "sized", + "collect": "first_of", + "key": null + }, + { + "from": "double_small", + "output": null, + "to": "report", + "input": "sized", + "collect": "first_of", + "key": null + }, + { + "from": "double_positive", + "output": null, + "to": "report", + "input": "signed", + "collect": "first_of", + "key": null + }, + { + "from": "constant", + "output": null, + "to": "report", + "input": "signed", + "collect": "first_of", + "key": null + } + ], + "inputs": { + "start": { + "value": 2.0 + }, + "choose_size": {}, + "is_positive": {}, + "double_large": {}, + "double_small": {}, + "double_positive": {}, + "constant": {}, + "special": {}, + "report": {} + }, + "task_types": { + "tm_helper_pipeline.Start": { + "id": "tm_helper_pipeline.Start", + "name": "start", + "python_type": "tm_helper_pipeline.Start", + "description": "Produce the configured value.", + "timeout_seconds": null, + "input_schema": { + "description": "Configured start value.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "StartInput", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.StartInput" + ], + "x-ri-python-type": "tm_helper_pipeline.StartInput" + }, + "output_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + } + }, + "tm_helper_conditional.ChooseSize": { + "id": "tm_helper_conditional.ChooseSize", + "name": "choose_size", + "python_type": "tm_helper_conditional.ChooseSize", + "description": "Pick a size label from a number.", + "timeout_seconds": null, + "input_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + }, + "output_schema": { + "properties": { + "branch": { + "enum": [ + "small", + "large", + "huge" + ], + "title": "Branch", + "type": "string" + } + }, + "required": [ + "branch" + ], + "title": "ChooseSizeBranch", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_conditional.ChooseSizeBranch", + "taskmaestro.conditions.Branch" + ], + "x-ri-python-type": "tm_helper_conditional.ChooseSizeBranch" + } + }, + "tm_helper_conditional.IsPositive": { + "id": "tm_helper_conditional.IsPositive", + "name": "is_positive", + "python_type": "tm_helper_conditional.IsPositive", + "description": "True when the number is positive.", + "timeout_seconds": null, + "input_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + }, + "output_schema": { + "properties": { + "branch": { + "title": "Branch", + "type": "boolean" + } + }, + "required": [ + "branch" + ], + "title": "IsPositiveBranch", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_conditional.IsPositiveBranch", + "taskmaestro.conditions.Branch" + ], + "x-ri-python-type": "tm_helper_conditional.IsPositiveBranch" + } + }, + "tm_helper_pipeline.Double": { + "id": "tm_helper_pipeline.Double", + "name": "double", + "python_type": "tm_helper_pipeline.Double", + "description": "Double a number.", + "timeout_seconds": null, + "input_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + }, + "output_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + } + }, + "tm_helper_pipeline.Constant": { + "id": "tm_helper_pipeline.Constant", + "name": "constant", + "python_type": "tm_helper_pipeline.Constant", + "description": "Produce the number one.", + "timeout_seconds": null, + "input_schema": { + "description": "Sentinel config for workflows where all root tasks use JobConfiguration.", + "properties": {}, + "title": "EmptyConfig", + "type": "object", + "x-ri-python-bases": [ + "taskmaestro.job.EmptyConfig" + ], + "x-ri-python-type": "taskmaestro.job.EmptyConfig" + }, + "output_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + } + }, + "tm_helper_pipeline.Special": { + "id": "tm_helper_pipeline.Special", + "name": "special", + "python_type": "tm_helper_pipeline.Special", + "description": "Produce a SpecialNumber.", + "timeout_seconds": null, + "input_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + }, + "output_schema": { + "description": "A number subclass, compatible with Number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "SpecialNumber", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.SpecialNumber", + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.SpecialNumber" + } + }, + "tm_helper_conditional.Report": { + "id": "tm_helper_conditional.Report", + "name": "report", + "python_type": "tm_helper_conditional.Report", + "description": "Add the two numbers.", + "timeout_seconds": null, + "input_schema": { + "$defs": { + "Number": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + } + }, + "description": "Two numbers, each taken from whichever branch ran.", + "properties": { + "sized": { + "$ref": "#/$defs/Number" + }, + "signed": { + "$ref": "#/$defs/Number" + } + }, + "required": [ + "sized", + "signed" + ], + "title": "ReportInput", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_conditional.ReportInput" + ], + "x-ri-python-type": "tm_helper_conditional.ReportInput" + }, + "output_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + } + } + }, + "describe": { + "workflow": "sizes", + "result_task": "report", + "tasks": [ + { + "name": "start", + "python_type": "tm_helper_pipeline.Start", + "depends_on": null, + "config_fields": [ + "value" + ], + "provided_config_fields": [ + "value" + ], + "missing_config_fields": [], + "config_values": { + "value": 2.0 + }, + "required_input_fields": [ + "value" + ], + "map": null, + "when": null, + "branches": null, + "input_schema": { + "description": "Configured start value.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "StartInput", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.StartInput" + ], + "x-ri-python-type": "tm_helper_pipeline.StartInput" + }, + "output_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + } + }, + { + "name": "choose_size", + "python_type": "tm_helper_conditional.ChooseSize", + "depends_on": { + "task": "start", + "field": null + }, + "config_fields": [], + "provided_config_fields": [], + "missing_config_fields": [], + "config_values": {}, + "required_input_fields": [ + "value" + ], + "map": null, + "when": null, + "branches": [ + "small", + "large", + "huge" + ], + "input_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + }, + "output_schema": { + "properties": { + "branch": { + "enum": [ + "small", + "large", + "huge" + ], + "title": "Branch", + "type": "string" + } + }, + "required": [ + "branch" + ], + "title": "ChooseSizeBranch", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_conditional.ChooseSizeBranch", + "taskmaestro.conditions.Branch" + ], + "x-ri-python-type": "tm_helper_conditional.ChooseSizeBranch" + } + }, + { + "name": "double_large", + "python_type": "tm_helper_pipeline.Double", + "depends_on": { + "task": "start", + "field": null + }, + "config_fields": [], + "provided_config_fields": [], + "missing_config_fields": [], + "config_values": {}, + "required_input_fields": [ + "value" + ], + "map": null, + "when": { + "task": "choose_size", + "field": "branch", + "op": "in", + "values": [ + "large", + "huge" + ] + }, + "branches": null, + "input_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + }, + "output_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + } + }, + { + "name": "double_small", + "python_type": "tm_helper_pipeline.Double", + "depends_on": { + "task": "start", + "field": null + }, + "config_fields": [], + "provided_config_fields": [], + "missing_config_fields": [], + "config_values": {}, + "required_input_fields": [ + "value" + ], + "map": null, + "when": { + "task": "choose_size", + "field": "branch", + "op": "otherwise", + "values": [ + "small" + ] + }, + "branches": null, + "input_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + }, + "output_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + } + }, + { + "name": "is_positive", + "python_type": "tm_helper_conditional.IsPositive", + "depends_on": { + "task": "start", + "field": null + }, + "config_fields": [], + "provided_config_fields": [], + "missing_config_fields": [], + "config_values": {}, + "required_input_fields": [ + "value" + ], + "map": null, + "when": null, + "branches": [ + true, + false + ], + "input_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + }, + "output_schema": { + "properties": { + "branch": { + "title": "Branch", + "type": "boolean" + } + }, + "required": [ + "branch" + ], + "title": "IsPositiveBranch", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_conditional.IsPositiveBranch", + "taskmaestro.conditions.Branch" + ], + "x-ri-python-type": "tm_helper_conditional.IsPositiveBranch" + } + }, + { + "name": "constant", + "python_type": "tm_helper_pipeline.Constant", + "depends_on": null, + "config_fields": [], + "provided_config_fields": [], + "missing_config_fields": [], + "config_values": {}, + "required_input_fields": [], + "map": null, + "when": { + "task": "is_positive", + "field": "branch", + "op": "in", + "values": [ + false + ] + }, + "branches": null, + "input_schema": { + "description": "Sentinel config for workflows where all root tasks use JobConfiguration.", + "properties": {}, + "title": "EmptyConfig", + "type": "object", + "x-ri-python-bases": [ + "taskmaestro.job.EmptyConfig" + ], + "x-ri-python-type": "taskmaestro.job.EmptyConfig" + }, + "output_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + } + }, + { + "name": "double_positive", + "python_type": "tm_helper_pipeline.Double", + "depends_on": { + "task": "start", + "field": null + }, + "config_fields": [], + "provided_config_fields": [], + "missing_config_fields": [], + "config_values": {}, + "required_input_fields": [ + "value" + ], + "map": null, + "when": { + "task": "is_positive", + "field": "branch", + "op": "in", + "values": [ + true + ] + }, + "branches": null, + "input_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + }, + "output_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + } + }, + { + "name": "report", + "python_type": "tm_helper_conditional.Report", + "depends_on": { + "sized": { + "first_of": [ + { + "task": "double_large", + "field": null + }, + { + "task": "double_small", + "field": null + } + ] + }, + "signed": { + "first_of": [ + { + "task": "double_positive", + "field": null + }, + { + "task": "constant", + "field": null + } + ] + } + }, + "config_fields": [], + "provided_config_fields": [], + "missing_config_fields": [], + "config_values": {}, + "required_input_fields": [ + "signed", + "sized" + ], + "map": null, + "when": null, + "branches": null, + "input_schema": { + "$defs": { + "Number": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + } + }, + "description": "Two numbers, each taken from whichever branch ran.", + "properties": { + "sized": { + "$ref": "#/$defs/Number" + }, + "signed": { + "$ref": "#/$defs/Number" + } + }, + "required": [ + "sized", + "signed" + ], + "title": "ReportInput", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_conditional.ReportInput" + ], + "x-ri-python-type": "tm_helper_conditional.ReportInput" + }, + "output_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + } + }, + { + "name": "special", + "python_type": "tm_helper_pipeline.Special", + "depends_on": { + "task": "start", + "field": null + }, + "config_fields": [], + "provided_config_fields": [], + "missing_config_fields": [], + "config_values": {}, + "required_input_fields": [ + "value" + ], + "map": null, + "when": { + "task": "start", + "field": "value", + "op": "in", + "values": [ + 1.0, + 2.0 + ] + }, + "branches": null, + "input_schema": { + "description": "A single number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "Number", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.Number" + }, + "output_schema": { + "description": "A number subclass, compatible with Number.", + "properties": { + "value": { + "title": "Value", + "type": "number" + } + }, + "required": [ + "value" + ], + "title": "SpecialNumber", + "type": "object", + "x-ri-python-bases": [ + "tm_helper_pipeline.SpecialNumber", + "tm_helper_pipeline.Number" + ], + "x-ri-python-type": "tm_helper_pipeline.SpecialNumber" + } + } + ], + "source": { + "kind": "yaml", + "path": "test_data/taskmaestro_workflows/conditional/workflow.yaml" + } + }, + "describe_error": null, + "status": "ok" +} \ No newline at end of file diff --git a/ApplicationLibCode/UserInterface/RiuWorkflowEditorWidget.cpp b/ApplicationLibCode/UserInterface/RiuWorkflowEditorWidget.cpp index b0351eadb0..87c93e00b6 100644 --- a/ApplicationLibCode/UserInterface/RiuWorkflowEditorWidget.cpp +++ b/ApplicationLibCode/UserInterface/RiuWorkflowEditorWidget.cpp @@ -34,7 +34,9 @@ #include "Workflow/RimWorkflowValidationTools.h" #include +#include #include +#include #include #include #include @@ -42,9 +44,11 @@ #include #include #include +#include #include #include #include +#include #include #include @@ -290,7 +294,8 @@ void RiuWorkflowEditorWidget::updateTaskIssues() { for ( const RimWorkflowFieldBinding* binding : taskInput->items() ) { - if ( binding && binding->fieldName() == fieldName ) hasValue = !binding->toJsonValue().isNull(); + if ( binding && binding->fieldName() == fieldName ) + hasValue = !binding->toJsonValue().isNull() || binding->isExplicitNull(); } } fieldHasValue[fieldName] = hasValue || !schema.value( "required" ).toBool(); @@ -369,6 +374,10 @@ void RiuWorkflowEditorWidget::deleteItem( const RiuWorkflowGraphView::ItemRef& i const RimWorkflowDefinitionEdge edge{ .from = item.from, .output = item.output, .to = item.to, .input = item.input }; applyEdit( RimWorkflowDefinitionTools::disconnect( m_workflow->definition(), edge ) ); } + else if ( item.kind == RiuWorkflowGraphView::ItemRef::Kind::Condition ) + { + applyEdit( RimWorkflowDefinitionTools::setTaskGuard( m_workflow->definition(), item.to, std::nullopt ) ); + } } //-------------------------------------------------------------------------------------------------- @@ -432,6 +441,26 @@ void RiuWorkflowEditorWidget::connectPorts( const QString& from, const QString& { if ( !canEdit() ) return; const RimWorkflowDefinitionEdge edge{ .from = from, .output = output, .to = to, .input = input }; + + // A connected input may instead take the new output as an alternative, used when the earlier ones did not run + auto alternative = RimWorkflowDefinitionTools::addFirstOfMember( m_workflow->definition(), edge ); + if ( alternative ) + { + QMenu menu; + QAction* replace = menu.addAction( "Replace Connection" ); + replace->setToolTip( QString( "Connect %1 to %2.%3 instead of the current connection" ).arg( from, to, input ) ); + QAction* add = menu.addAction( "Add as Alternative (First Of)" ); + add->setToolTip( QString( "%1.%2 takes the output of the first connected task that ran" ).arg( to, input ) ); + menu.setToolTipsVisible( true ); + + QAction* chosen = menu.exec( QCursor::pos() ); + if ( !chosen || !canEdit() ) return; + if ( chosen == add ) + { + applyEdit( RimWorkflowDefinitionTools::addFirstOfMember( m_workflow->definition(), edge ) ); + return; + } + } applyEdit( RimWorkflowDefinitionTools::connect( m_workflow->definition(), edge ) ); } @@ -561,6 +590,244 @@ void RiuWorkflowEditorWidget::removeTaskMap( const QString& taskName ) applyEdit( RimWorkflowDefinitionTools::setTaskMap( m_workflow->definition(), taskName, std::nullopt ) ); } +//-------------------------------------------------------------------------------------------------- +/// Run the task only for some branches of a switch task, or when an output field of another task +/// has a value. The dialog edits the current condition, if any. +//-------------------------------------------------------------------------------------------------- +void RiuWorkflowEditorWidget::editTaskGuard( const QString& taskName ) +{ + using Op = RimWorkflowTaskGuard::Op; + if ( !canEdit() ) return; + + // A copy, as the workflow may change or be deleted while the dialog is open + const RimWorkflowDefinition definition = m_workflow->definition(); + const RimWorkflowDefinitionNode* node = definition.findNode( taskName ); + if ( !node ) return; + + QStringList sources; + for ( const auto& other : definition.nodes ) + { + if ( other.name != taskName ) sources.append( other.name ); + } + if ( sources.isEmpty() ) + { + showMessage( QString( "Add a task for '%1' to depend on first" ).arg( taskName ) ); + return; + } + + // Default to the first switch task, the usual source of a run condition + QString defaultSource = sources.front(); + for ( const QString& name : sources ) + { + if ( RimWorkflowDefinitionTools::isSwitchTask( definition.taskType( definition.findNode( name )->taskId ) ) ) + { + defaultSource = name; + break; + } + } + const RimWorkflowTaskGuard current = node->when.value_or( RimWorkflowTaskGuard{ .source = defaultSource } ); + + QDialog dialog( this ); + dialog.setWindowTitle( QString( "Run When - %1" ).arg( taskName ) ); + auto* form = new QFormLayout( &dialog ); + form->addRow( new QLabel( QString( "'%1' runs only when the condition holds; otherwise it is skipped, " + "and so are the tasks that need its output." ) + .arg( taskName ), + &dialog ) ); + + auto* sourceCombo = new QComboBox( &dialog ); + sourceCombo->addItems( sources ); + sourceCombo->setCurrentText( sources.contains( current.source ) ? current.source : defaultSource ); + sourceCombo->setToolTip( "The task whose output decides whether this task runs" ); + form->addRow( "Depends on:", sourceCombo ); + + auto* fieldCombo = new QComboBox( &dialog ); + fieldCombo->setToolTip( "The branch of a switch task, or an output field to test" ); + form->addRow( "Test:", fieldCombo ); + + auto* pages = new QStackedWidget( &dialog ); + form->addRow( pages ); + + // Page 0: branches of a switch task + auto* branchPage = new QWidget( pages ); + auto* branchLayout = new QVBoxLayout( branchPage ); + branchLayout->setContentsMargins( 0, 0, 0, 0 ); + auto* branchList = new QListWidget( branchPage ); + branchList->setToolTip( "Run for any of the checked branches" ); + auto* otherwiseCheck = new QCheckBox( "Otherwise (no other condition on the switch matches)", branchPage ); + branchLayout->addWidget( branchList ); + branchLayout->addWidget( otherwiseCheck ); + pages->addWidget( branchPage ); + + // Page 1: a field value + auto* valuePage = new QWidget( pages ); + auto* valueForm = new QFormLayout( valuePage ); + valueForm->setContentsMargins( 0, 0, 0, 0 ); + auto* opCombo = new QComboBox( valuePage ); + opCombo->addItem( "equals", static_cast( Op::Equals ) ); + opCombo->addItem( "is one of", static_cast( Op::In ) ); + opCombo->addItem( "is true", static_cast( Op::Truthy ) ); + opCombo->addItem( "is false or empty", static_cast( Op::Falsy ) ); + auto* valuesEdit = new QLineEdit( valuePage ); + valuesEdit->setPlaceholderText( "value, or several separated by commas" ); + valueForm->addRow( "Condition:", opCombo ); + valueForm->addRow( "Value:", valuesEdit ); + pages->addWidget( valuePage ); + + auto* errorLabel = new QLabel( &dialog ); + errorLabel->setStyleSheet( "color: #b00020;" ); + errorLabel->setWordWrap( true ); + form->addRow( errorLabel ); + + auto* buttons = new QDialogButtonBox( QDialogButtonBox::Ok | QDialogButtonBox::Cancel, &dialog ); + form->addRow( buttons ); + + auto sourceIsSwitch = [&]() + { + const auto* source = definition.findNode( sourceCombo->currentText() ); + return source && RimWorkflowDefinitionTools::isSwitchTask( definition.taskType( source->taskId ) ); + }; + auto isBranchSelected = [&]() { return sourceIsSwitch() && fieldCombo->currentIndex() == 0; }; + + // A condition that picks branches: a case, or equals/in on the branch of the switch + auto isBranchValueGuard = []( const RimWorkflowTaskGuard& guard ) + { + const bool onBranch = guard.isSwitchBranch() || guard.field == "branch"; + return onBranch && ( guard.op == Op::Case || guard.op == Op::Equals || guard.op == Op::In ); + }; + + auto guardFromDialog = [&]() + { + RimWorkflowTaskGuard guard{ .source = sourceCombo->currentText() }; + if ( isBranchSelected() ) + { + if ( otherwiseCheck->isChecked() ) + { + guard.op = Op::Otherwise; + return guard; + } + for ( int row = 0; row < branchList->count(); ++row ) + { + const QListWidgetItem* item = branchList->item( row ); + if ( item->checkState() == Qt::Checked ) guard.values.append( QJsonValue::fromVariant( item->data( Qt::UserRole ) ) ); + } + + // Keep the `ref: ..., equals/in: ...` form of an existing condition on the branch + if ( guard.source == current.source && isBranchValueGuard( current ) && current.op != Op::Case ) + { + guard.field = current.field; + guard.op = current.op == Op::Equals && guard.values.size() == 1 ? Op::Equals : Op::In; + return guard; + } + guard.op = Op::Case; + guard.caseAsList = guard.values.size() > 1 || ( current.op == Op::Case && current.caseAsList ); + return guard; + } + + guard.field = fieldCombo->currentText(); + guard.op = static_cast( opCombo->currentData().toInt() ); + if ( guard.op == Op::Equals || guard.op == Op::In ) + { + guard.values = RimWorkflowDefinitionTools::guardValuesFromText( valuesEdit->text(), + RimWorkflowDefinitionTools::guardFieldSchema( definition, + guard.source, + guard.field ) ); + } + return guard; + }; + + auto updateValidity = [&]() + { + const bool branch = isBranchSelected(); + pages->setCurrentIndex( branch ? 0 : 1 ); + branchList->setEnabled( !otherwiseCheck->isChecked() ); + const auto op = static_cast( opCombo->currentData().toInt() ); + valuesEdit->setEnabled( op == Op::Equals || op == Op::In ); + + const auto valid = RimWorkflowDefinitionTools::checkTaskGuard( definition, taskName, guardFromDialog() ); + errorLabel->setText( valid ? QString() : valid.error() ); + errorLabel->setVisible( !valid ); + buttons->button( QDialogButtonBox::Ok )->setEnabled( valid.has_value() ); + }; + + auto fillBranches = [&]( const RimWorkflowTaskGuard& checked ) + { + const QSignalBlocker blocker( branchList ); + branchList->clear(); + const auto* source = definition.findNode( sourceCombo->currentText() ); + if ( !source ) return; + for ( const QJsonValue& label : RimWorkflowDefinitionTools::switchBranches( definition.taskType( source->taskId ) ) ) + { + auto* item = new QListWidgetItem( RimWorkflowDefinitionTools::guardValueText( label ), branchList ); + item->setData( Qt::UserRole, label.toVariant() ); + item->setFlags( item->flags() | Qt::ItemIsUserCheckable ); + const bool isChecked = checked.source == source->name && isBranchValueGuard( checked ) && checked.values.contains( label ); + item->setCheckState( isChecked ? Qt::Checked : Qt::Unchecked ); + } + }; + + auto fillFields = [&]( const RimWorkflowTaskGuard& selected ) + { + const QSignalBlocker blocker( fieldCombo ); + fieldCombo->clear(); + if ( sourceIsSwitch() ) fieldCombo->addItem( "branch (switch)" ); + for ( const QString& field : RimWorkflowDefinitionTools::guardFields( definition, sourceCombo->currentText() ) ) + { + if ( sourceIsSwitch() && field == "branch" ) continue; + fieldCombo->addItem( field ); + } + const bool sameSource = selected.source == sourceCombo->currentText(); + if ( sameSource && !selected.isSwitchBranch() ) fieldCombo->setCurrentText( selected.field ); + fillBranches( sameSource ? selected : RimWorkflowTaskGuard{} ); + otherwiseCheck->setChecked( sameSource && selected.op == Op::Otherwise ); + }; + + if ( !current.isSwitchBranch() ) + { + opCombo->setCurrentIndex( std::max( 0, opCombo->findData( static_cast( current.op ) ) ) ); + valuesEdit->setText( RimWorkflowDefinitionTools::guardValuesToText( current.values, + RimWorkflowDefinitionTools::guardFieldSchema( definition, + current.source, + current.field ) ) ); + } + fillFields( current ); + + QObject::connect( sourceCombo, + &QComboBox::currentIndexChanged, + &dialog, + [&]() + { + fillFields( current ); + updateValidity(); + } ); + QObject::connect( fieldCombo, &QComboBox::currentIndexChanged, &dialog, updateValidity ); + QObject::connect( opCombo, &QComboBox::currentIndexChanged, &dialog, updateValidity ); + QObject::connect( valuesEdit, &QLineEdit::textChanged, &dialog, updateValidity ); + QObject::connect( branchList, &QListWidget::itemChanged, &dialog, updateValidity ); + QObject::connect( otherwiseCheck, &QCheckBox::toggled, &dialog, updateValidity ); + QObject::connect( buttons, &QDialogButtonBox::accepted, &dialog, &QDialog::accept ); + QObject::connect( buttons, &QDialogButtonBox::rejected, &dialog, &QDialog::reject ); + updateValidity(); + + if ( dialog.exec() != QDialog::Accepted || !canEdit() ) return; + + const RimWorkflowDefinitionNode* currentNode = m_workflow->definition().findNode( taskName ); + if ( !currentNode ) return; + + const RimWorkflowTaskGuard guard = guardFromDialog(); + if ( currentNode->when && *currentNode->when == guard ) return; + applyEdit( RimWorkflowDefinitionTools::setTaskGuard( m_workflow->definition(), taskName, guard ) ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void RiuWorkflowEditorWidget::removeTaskGuard( const QString& taskName ) +{ + if ( !canEdit() ) return; + applyEdit( RimWorkflowDefinitionTools::setTaskGuard( m_workflow->definition(), taskName, std::nullopt ) ); +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- @@ -645,6 +912,7 @@ void RiuWorkflowEditorWidget::showContextMenu( const RiuWorkflowGraphView::ItemR m_workflow->definition().resultTask.isEmpty() ); appendOptionalInputsMenu( &menu, task ); appendMapMenu( &menu, task ); + appendConditionMenu( &menu, task ); menu.addSeparator(); addMenuAction( &menu, "Delete Task", [this, item]() { deleteItem( item ); }, ":/Erase.svg" ); } @@ -653,6 +921,12 @@ void RiuWorkflowEditorWidget::showContextMenu( const RiuWorkflowGraphView::ItemR appendCollectMenu( &menu, item ); addMenuAction( &menu, "Delete Connection", [this, item]() { deleteItem( item ); }, ":/Erase.svg" ); } + else if ( item.kind == RiuWorkflowGraphView::ItemRef::Kind::Condition && editable ) + { + const QString task = item.to; + addMenuAction( &menu, "Edit Condition...", [this, task]() { editTaskGuard( task ); } ); + addMenuAction( &menu, "Remove Condition", [this, task]() { removeTaskGuard( task ); }, ":/Erase.svg" ); + } else { if ( editable ) @@ -729,13 +1003,26 @@ void RiuWorkflowEditorWidget::appendMapMenu( QMenu* menu, const QString& taskNam if ( node->map ) addMenuAction( menu, "Remove Map", [this, taskName]() { removeTaskMap( taskName ); } ); } +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void RiuWorkflowEditorWidget::appendConditionMenu( QMenu* menu, const QString& taskName ) +{ + const RimWorkflowDefinitionNode* node = m_workflow->definition().findNode( taskName ); + if ( !node ) return; + + addMenuAction( menu, node->when ? "Edit Condition..." : "Run When...", [this, taskName]() { editTaskGuard( taskName ); } ) + ->setToolTip( "Run the task only for some branches of a switch task, or when an output of another task has a value" ); + if ( node->when ) addMenuAction( menu, "Remove Condition", [this, taskName]() { removeTaskGuard( taskName ); } ); +} + //-------------------------------------------------------------------------------------------------- /// Collected connections: dict members have a key, list members an order //-------------------------------------------------------------------------------------------------- void RiuWorkflowEditorWidget::appendCollectMenu( QMenu* menu, const RiuWorkflowGraphView::ItemRef& item ) { const RimWorkflowDefinitionEdge* edge = findEdge( item ); - if ( !edge || !edge->isCollected() ) return; + if ( !edge || ( !edge->isCollected() && !edge->isFirstOf() ) ) return; if ( edge->collect == RimWorkflowDefinitionEdge::Collect::Dict ) { @@ -747,7 +1034,7 @@ void RiuWorkflowEditorWidget::appendCollectMenu( QMenu* menu, const RiuWorkflowG int count = 0; for ( const auto& other : m_workflow->definition().edges ) { - if ( other.to != edge->to || other.input != edge->input || !other.isCollected() ) continue; + if ( other.to != edge->to || other.input != edge->input || other.collect != edge->collect ) continue; if ( &other == edge ) index = count; ++count; } diff --git a/ApplicationLibCode/UserInterface/RiuWorkflowEditorWidget.h b/ApplicationLibCode/UserInterface/RiuWorkflowEditorWidget.h index fef5111d3c..473661f976 100644 --- a/ApplicationLibCode/UserInterface/RiuWorkflowEditorWidget.h +++ b/ApplicationLibCode/UserInterface/RiuWorkflowEditorWidget.h @@ -72,6 +72,8 @@ class RiuWorkflowEditorWidget : public QWidget void setOptionalInputConfigured( const QString& taskName, const QString& field, bool configured ); void editTaskMap( const QString& taskName ); void removeTaskMap( const QString& taskName ); + void editTaskGuard( const QString& taskName ); + void removeTaskGuard( const QString& taskName ); void renameCollectKey( const RiuWorkflowGraphView::ItemRef& edge ); void moveCollectMember( const RiuWorkflowGraphView::ItemRef& edge, int delta ); void connectPorts( const QString& from, const QString& output, const QString& to, const QString& input ); @@ -86,6 +88,7 @@ class RiuWorkflowEditorWidget : public QWidget void showContextMenu( const RiuWorkflowGraphView::ItemRef& item, const QPoint& globalPos, const QPointF& scenePos ); void appendAddTaskMenu( QMenu* menu, const QPointF& scenePos ); void appendMapMenu( QMenu* menu, const QString& taskName ); + void appendConditionMenu( QMenu* menu, const QString& taskName ); void appendCollectMenu( QMenu* menu, const RiuWorkflowGraphView::ItemRef& item ); bool canEdit(); void renameNodePositions( const QMap& renames ); diff --git a/ApplicationLibCode/UserInterface/RiuWorkflowGraphView.cpp b/ApplicationLibCode/UserInterface/RiuWorkflowGraphView.cpp index d1a04aa198..ff7fdb09f9 100644 --- a/ApplicationLibCode/UserInterface/RiuWorkflowGraphView.cpp +++ b/ApplicationLibCode/UserInterface/RiuWorkflowGraphView.cpp @@ -80,6 +80,10 @@ const QColor warningColor( 200, 130, 20 ); const QColor edgeColor( 75, 95, 115 ); const QColor selectedEdgeColor( 30, 120, 220 ); const QColor mapColor( 95, 80, 160 ); +const QColor conditionColor( 135, 75, 150 ); + +constexpr qreal diamondMinHeight = 130.0; +constexpr qreal diamondPortStep = 24.0; QString keyForField( const QString& field ) { @@ -149,6 +153,7 @@ struct PortInfo QString iconResource{}; Style style = Style::Normal; bool collect = false; + bool firstOf = false; }; //================================================================================================== @@ -194,10 +199,15 @@ class GraphNode : public QGraphicsRectItem const std::vector& inputs, const std::vector& outputs, bool isConfig = false, - const QString& subtitle = {} ); + const QString& subtitle = {}, + bool diamond = false ); qreal height() const; QPointF portPosition( const QString& key, bool output ) const; + QPointF controlAnchor( bool outgoing ) const; + void setBranches( const QStringList& branches ); + void paint( QPainter* painter, const QStyleOptionGraphicsItem* option, QWidget* widget ) override; + QPainterPath shape() const override; void addEdge( GraphEdge* edge ); QString name() const; bool isConfig() const; @@ -214,32 +224,36 @@ class GraphNode : public QGraphicsRectItem QVariant itemChange( GraphicsItemChange change, const QVariant& value ) override; private: - void addPorts( const std::vector& ports, bool output ); - void applyStyle(); - - QString m_name; - bool m_isConfig; - qreal m_width; - QMap m_inputs; - QMap m_outputs; - QMap m_configValues; - QMap m_configValueRight; // Where the value must end to stay clear of the type - std::vector m_edges; - QGraphicsTextItem* m_title = nullptr; - QGraphicsTextItem* m_stateLabel = nullptr; - QGraphicsTextItem* m_issueLabel = nullptr; - QGraphicsTextItem* m_subtitle = nullptr; - std::vector m_stack; - qreal m_portTop = portTop; - QMap m_itemStates; - QMap m_itemErrors; - QString m_state; - QString m_stateError; - QString m_baseToolTip; - QString m_issueText; - int m_errorCount = 0; - int m_warningCount = 0; - bool m_unknownType = false; + void addPorts( const std::vector& ports, bool output ); + void addDiamondPorts( const std::vector& ports, bool output ); + void applyStyle(); + QPolygonF diamondPolygon( const QRectF& bounds ) const; + + QString m_name; + bool m_isConfig; + bool m_diamond; + qreal m_width; + QMap m_inputs; + QMap m_outputs; + QMap m_configValues; + QMap m_configValueRight; // Where the value must end to stay clear of the type + std::vector m_edges; + QGraphicsTextItem* m_title = nullptr; + QGraphicsTextItem* m_stateLabel = nullptr; + QGraphicsTextItem* m_issueLabel = nullptr; + QGraphicsTextItem* m_subtitle = nullptr; + QGraphicsTextItem* m_branches = nullptr; + std::vector m_stack; + qreal m_portTop = portTop; + QMap m_itemStates; + QMap m_itemErrors; + QString m_state; + QString m_stateError; + QString m_baseToolTip; + QString m_issueText; + int m_errorCount = 0; + int m_warningCount = 0; + bool m_unknownType = false; }; //================================================================================================== @@ -249,10 +263,13 @@ class GraphEdge : public QGraphicsPathItem { public: GraphEdge( GraphNode* from, QString outputKey, GraphNode* to, QString inputKey, bool isConfig, bool selectable ); + // A control edge: `to` only runs when the condition on the output of `from` holds + GraphEdge( GraphNode* from, GraphNode* to, const QString& condition, bool selectable ); void updatePath(); QPainterPath shape() const override; bool isConfig() const { return m_isConfig; } + bool isControl() const { return m_isControl; } GraphNode* fromNode() const { return m_from; } GraphNode* toNode() const { return m_to; } QString outputField() const { return fieldForKey( m_outputKey ); } @@ -270,6 +287,7 @@ class GraphEdge : public QGraphicsPathItem QString m_outputKey; QString m_inputKey; bool m_isConfig; + bool m_isControl = false; QGraphicsPolygonItem* m_arrow; QGraphicsTextItem* m_label = nullptr; }; @@ -339,12 +357,12 @@ PortItem::PortItem( GraphNode* node, const PortInfo& info, bool output, bool con setAcceptedMouseButtons( Qt::NoButton ); setZValue( 2 ); - // An outer ring marks inputs that collect several connections - if ( info.collect ) + // An outer ring marks inputs that collect several connections, a dashed one alternatives + if ( info.collect || info.firstOf ) { const qreal ring = portRadius + 3.0; auto* outer = new QGraphicsEllipseItem( center.x() - ring, center.y() - ring, 2 * ring, 2 * ring, node ); - outer->setPen( QPen( color, 1.2 ) ); + outer->setPen( QPen( color, 1.2, info.firstOf ? Qt::DotLine : Qt::SolidLine ) ); outer->setBrush( Qt::NoBrush ); outer->setAcceptedMouseButtons( Qt::NoButton ); outer->setZValue( 1.5 ); @@ -385,15 +403,20 @@ GraphNode::GraphNode( const QString& name, const std::vector& inputs, const std::vector& outputs, bool isConfig, - const QString& subtitle ) + const QString& subtitle, + bool diamond ) : m_name( name ) , m_isConfig( isConfig ) + , m_diamond( diamond && !isConfig ) , m_width( isConfig ? configWidth : nodeWidth ) , m_portTop( subtitle.isEmpty() ? portTop : portTop + subtitleGap ) , m_baseToolTip( name ) { const qsizetype rows = std::max( { qsizetype( 1 ), qsizetype( inputs.size() ), qsizetype( outputs.size() ) } ); - setRect( 0, 0, m_width, m_portTop + portStep * rows + 12 ); + if ( m_diamond ) + setRect( 0, 0, m_width, std::max( diamondMinHeight, ( rows - 1 ) * diamondPortStep + 60.0 ) ); + else + setRect( 0, 0, m_width, m_portTop + portStep * rows + 12 ); setFlags( ItemIsMovable | ItemIsSelectable | ItemSendsGeometryChanges ); setZValue( 1 ); @@ -416,7 +439,8 @@ GraphNode::GraphNode( const QString& name, m_issueLabel = new QGraphicsTextItem( this ); m_issueLabel->setFont( font ); - m_issueLabel->setPos( 8, rect().height() - 2 ); + // A diamond has its badge beside the bottom vertex, where control edges leave + m_issueLabel->setPos( m_diamond ? m_width / 2 + 10 : 8, rect().height() - ( m_diamond ? 16 : 2 ) ); m_issueLabel->setAcceptedMouseButtons( Qt::NoButton ); } @@ -425,7 +449,12 @@ GraphNode::GraphNode( const QString& name, { for ( int depth = 2; depth >= 1; --depth ) { - auto* card = new QGraphicsRectItem( rect().translated( depth * stackOffset, depth * stackOffset ), this ); + const QRectF bounds = rect().translated( depth * stackOffset, depth * stackOffset ); + QAbstractGraphicsShapeItem* card = nullptr; + if ( m_diamond ) + card = new QGraphicsPolygonItem( diamondPolygon( bounds ), this ); + else + card = new QGraphicsRectItem( bounds, this ); card->setFlag( ItemStacksBehindParent ); card->setAcceptedMouseButtons( Qt::NoButton ); m_stack.push_back( card ); @@ -437,13 +466,24 @@ GraphNode::GraphNode( const QString& name, font.setItalic( true ); m_subtitle->setFont( font ); m_subtitle->setDefaultTextColor( mapColor ); - m_subtitle->setPlainText( QFontMetrics( font ).elidedText( subtitle, Qt::ElideRight, m_width - 20 ) ); - m_subtitle->setPos( 10, 24 ); + m_subtitle->setPlainText( QFontMetrics( font ).elidedText( subtitle, Qt::ElideRight, m_diamond ? 100 : m_width - 20 ) ); + if ( m_diamond ) + m_subtitle->setPos( ( m_width - m_subtitle->boundingRect().width() ) / 2, rect().height() / 2 - 42 ); + else + m_subtitle->setPos( 10, 24 ); m_subtitle->setAcceptedMouseButtons( Qt::NoButton ); } - addPorts( inputs, false ); - addPorts( outputs, true ); + if ( m_diamond ) + { + addDiamondPorts( inputs, false ); + addDiamondPorts( outputs, true ); + } + else + { + addPorts( inputs, false ); + addPorts( outputs, true ); + } setTitle( isConfig ? "Config: " + name : name, {} ); } @@ -460,8 +500,10 @@ qreal GraphNode::height() const //-------------------------------------------------------------------------------------------------- void GraphNode::setTitle( const QString& title, const QString& toolTip ) { - m_title->setPlainText( QFontMetrics( m_title->font() ).elidedText( title, Qt::ElideRight, m_width - ( m_isConfig ? 20 : 100 ) ) ); + const qreal available = m_diamond ? m_width * 0.62 : m_width - ( m_isConfig ? 20 : 100 ); + m_title->setPlainText( QFontMetrics( m_title->font() ).elidedText( title, Qt::ElideRight, static_cast( available ) ) ); m_title->setToolTip( title ); + if ( m_diamond ) m_title->setPos( ( m_width - m_title->boundingRect().width() ) / 2, rect().height() / 2 - 24 ); if ( !toolTip.isEmpty() ) m_baseToolTip = toolTip; applyStyle(); } @@ -534,6 +576,91 @@ void GraphNode::addPorts( const std::vector& ports, bool output ) } } +//-------------------------------------------------------------------------------------------------- +/// Ports of a switch task sit on the left and right sides of its diamond, labels go in tooltips +//-------------------------------------------------------------------------------------------------- +void GraphNode::addDiamondPorts( const std::vector& ports, bool output ) +{ + auto& portMap = output ? m_outputs : m_inputs; + const qreal halfHeight = rect().height() / 2; + for ( size_t row = 0; row < ports.size(); ++row ) + { + const qreal offset = ( row - ( ports.size() - 1 ) / 2.0 ) * diamondPortStep; + const qreal inset = m_width / 2 * std::abs( offset ) / halfHeight; + const qreal x = output ? m_width - inset : inset; + portMap.insert( ports[row].key, new PortItem( this, ports[row], output, false, QPointF( x, halfHeight + offset ) ) ); + } +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +QPolygonF GraphNode::diamondPolygon( const QRectF& bounds ) const +{ + return QPolygonF{ QPointF( bounds.left(), bounds.center().y() ), + QPointF( bounds.center().x(), bounds.top() ), + QPointF( bounds.right(), bounds.center().y() ), + QPointF( bounds.center().x(), bounds.bottom() ), + QPointF( bounds.left(), bounds.center().y() ) }; +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +void GraphNode::paint( QPainter* painter, const QStyleOptionGraphicsItem* option, QWidget* widget ) +{ + if ( !m_diamond ) + { + QGraphicsRectItem::paint( painter, option, widget ); + return; + } + painter->setPen( pen() ); + painter->setBrush( brush() ); + painter->drawPolygon( diamondPolygon( rect() ) ); +} + +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +QPainterPath GraphNode::shape() const +{ + if ( !m_diamond ) return QGraphicsRectItem::shape(); + QPainterPath path; + path.addPolygon( diamondPolygon( rect() ) ); + return path; +} + +//-------------------------------------------------------------------------------------------------- +/// Where control edges leave (the bottom) or enter (the top) the node, in scene coordinates +//-------------------------------------------------------------------------------------------------- +QPointF GraphNode::controlAnchor( bool outgoing ) const +{ + return mapToScene( QPointF( m_width / 2, outgoing ? rect().height() : 0.0 ) ); +} + +//-------------------------------------------------------------------------------------------------- +/// The branch labels of a switch task, shown below its name +//-------------------------------------------------------------------------------------------------- +void GraphNode::setBranches( const QStringList& branches ) +{ + if ( branches.isEmpty() ) return; + if ( !m_branches ) + { + m_branches = new QGraphicsTextItem( this ); + QFont font; + font.setPointSize( 8 ); + font.setItalic( true ); + m_branches->setFont( font ); + m_branches->setDefaultTextColor( conditionColor ); + m_branches->setAcceptedMouseButtons( Qt::NoButton ); + } + const QString text = branches.join( " | " ); + m_branches->setPlainText( QFontMetrics( m_branches->font() ).elidedText( text, Qt::ElideRight, static_cast( m_width * 0.5 ) ) ); + m_branches->setToolTip( "Branches: " + branches.join( ", " ) ); + const qreal x = m_diamond ? ( m_width - m_branches->boundingRect().width() ) / 2 : 10; + m_branches->setPos( x, m_diamond ? rect().height() / 2 - 4 : 24 ); +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- @@ -686,6 +813,12 @@ void GraphNode::applyStyle() border = QColor( 140, 85, 50 ); background = QColor( 245, 228, 210 ); } + else if ( m_state == "skipped" ) + { + border = QColor( 150, 150, 150 ); + background = QColor( 240, 240, 240 ); + style = Qt::DashLine; + } else if ( m_errorCount > 0 || m_warningCount > 0 ) { border = m_errorCount > 0 ? errorColor : warningColor; @@ -698,7 +831,7 @@ void GraphNode::applyStyle() pen.setStyle( style ); setPen( pen ); setBrush( background ); - for ( QGraphicsRectItem* card : m_stack ) + for ( QAbstractGraphicsShapeItem* card : m_stack ) { card->setPen( QPen( border, 1.0 ) ); card->setBrush( background.darker( 104 ) ); @@ -722,7 +855,10 @@ void GraphNode::applyStyle() } m_stateLabel->setDefaultTextColor( border ); m_stateLabel->setPlainText( text ); - m_stateLabel->setPos( m_width - m_stateLabel->boundingRect().width() - 6, 5 ); + if ( m_diamond ) + m_stateLabel->setPos( ( m_width - m_stateLabel->boundingRect().width() ) / 2, rect().height() / 2 + 12 ); + else + m_stateLabel->setPos( m_width - m_stateLabel->boundingRect().width() - 6, 5 ); } if ( m_issueLabel ) @@ -750,6 +886,7 @@ void GraphNode::applyStyle() } if ( !m_issueText.isEmpty() ) toolTip << m_issueText; setToolTip( toolTip.join( "\n\n" ) ); + setOpacity( m_state == "skipped" ? 0.6 : 1.0 ); } //-------------------------------------------------------------------------------------------------- @@ -828,6 +965,27 @@ GraphEdge::GraphEdge( GraphNode* from, QString outputKey, GraphNode* to, QString updatePath(); } +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +GraphEdge::GraphEdge( GraphNode* from, GraphNode* to, const QString& condition, bool selectable ) + : m_from( from ) + , m_to( to ) + , m_isConfig( false ) + , m_isControl( true ) + , m_arrow( new QGraphicsPolygonItem( this ) ) +{ + setZValue( -1 ); + if ( selectable ) setFlag( ItemIsSelectable ); + setToolTip( QString( "'%1' runs only when %2 of '%3' matches; otherwise it is skipped" ).arg( to->name(), condition, from->name() ) ); + m_arrow->setPen( Qt::NoPen ); + applyPen(); + m_from->addEdge( this ); + m_to->addEdge( this ); + updatePath(); + setLabel( condition, toolTip() ); +} + //-------------------------------------------------------------------------------------------------- /// A label near the target, such as the index or key of a collected member //-------------------------------------------------------------------------------------------------- @@ -840,7 +998,7 @@ void GraphEdge::setLabel( const QString& label, const QString& toolTip ) QFont font; font.setPointSize( 8 ); m_label->setFont( font ); - m_label->setDefaultTextColor( edgeColor ); + m_label->setDefaultTextColor( m_isControl ? conditionColor : edgeColor ); m_label->setAcceptedMouseButtons( Qt::NoButton ); } m_label->setPlainText( label ); @@ -853,9 +1011,10 @@ void GraphEdge::setLabel( const QString& label, const QString& toolTip ) //-------------------------------------------------------------------------------------------------- void GraphEdge::applyPen() { - const QColor color = isSelected() ? selectedEdgeColor : ( m_isConfig ? configPortColor : edgeColor ); + const QColor color = isSelected() ? selectedEdgeColor : ( m_isConfig ? configPortColor : m_isControl ? conditionColor : edgeColor ); QPen pen( color, isSelected() ? 3.0 : 1.7 ); if ( m_isConfig ) pen.setStyle( Qt::DashLine ); + if ( m_isControl ) pen.setStyle( Qt::DashDotLine ); setPen( pen ); m_arrow->setBrush( color ); } @@ -865,6 +1024,23 @@ void GraphEdge::applyPen() //-------------------------------------------------------------------------------------------------- void GraphEdge::updatePath() { + if ( m_isControl ) + { + const QPointF start = m_from->controlAnchor( true ); + const QPointF end = m_to->controlAnchor( false ); + const qreal bend = std::max( 40.0, std::abs( end.y() - start.y() ) / 2.0 ); + QPainterPath path( start ); + path.cubicTo( start + QPointF( 0, bend ), end - QPointF( 0, bend ), end ); + setPath( path ); + m_arrow->setPolygon( QPolygonF{ end, end + QPointF( -5, -9 ), end + QPointF( 5, -9 ) } ); + if ( m_label ) + { + const QPointF anchor = path.pointAtPercent( 0.5 ); + m_label->setPos( anchor.x() + 4, anchor.y() - m_label->boundingRect().height() / 2 ); + } + return; + } + const QPointF start = m_from->portPosition( m_outputKey, true ); const QPointF end = m_to->portPosition( m_inputKey, false ); const qreal bend = std::max( 35.0, std::abs( end.x() - start.x() ) / 2.0 ); @@ -963,6 +1139,11 @@ std::pair, std::vector> editModePorts( const QJs toolTip << ( required ? "Required" : "Optional" ); if ( port.value( "configured" ).toBool() ) toolTip << "Set in the job configuration"; if ( port.value( "covered" ).toBool() ) toolTip << "Provided by a whole-output connection"; + if ( port.value( "first_of" ).toBool() ) + { + info.firstOf = true; + toolTip << "Alternatives: the output of the first connected task that ran is used"; + } if ( !port.value( "collect" ).toString().isEmpty() ) { info.collect = true; @@ -981,6 +1162,7 @@ std::pair, std::vector> editModePorts( const QJs for ( const QJsonValue& value : edges ) { const QJsonObject edge = value.toObject(); + if ( edge.value( "kind" ).toString() == "when" ) continue; if ( edge.value( "to" ).toString() == name && edge.value( "input" ).toString().isEmpty() && !hasWholeInput ) { inputs.insert( inputs.begin(), wholeModelPort( task, false ) ); @@ -1180,7 +1362,7 @@ void RiuWorkflowGraphView::showGraph( const QJsonObject& graph, const QString& e const QString from = edge.value( "from" ).toString(); const QString to = edge.value( "to" ).toString(); layoutEdges.emplace_back( from, to ); - if ( !taskData.contains( from ) || !taskData.contains( to ) || from == to ) continue; + if ( !taskData.contains( from ) || !taskData.contains( to ) || from == to || edge.value( "kind" ).toString() == "when" ) continue; usedInputs[to].append( keyForField( edge.value( "input" ).toString() ) ); usedOutputs[from].append( keyForField( edge.value( "output" ).toString() ) ); } @@ -1224,7 +1406,15 @@ void RiuWorkflowGraphView::showGraph( const QJsonObject& graph, const QString& e const QJsonObject map = data.value( "map" ).toObject(); QString subtitle; if ( !map.isEmpty() ) subtitle = QString::fromUtf8( "⟳ map over %1" ).arg( map.value( "over" ).toString() ); - auto* node = new GraphNode( name, inputs, outputs, false, subtitle ); + const bool isSwitch = data.value( "switch" ).toBool(); + auto* node = new GraphNode( name, inputs, outputs, false, subtitle, isSwitch ); + if ( isSwitch ) + { + QStringList branches; + for ( const QJsonValue& branch : data.value( "branches" ).toArray() ) + branches.append( branch.toVariant().toString() ); + node->setBranches( branches ); + } if ( !map.isEmpty() ) { QStringList mapToolTip{ @@ -1245,6 +1435,7 @@ void RiuWorkflowGraphView::showGraph( const QJsonObject& graph, const QString& e if ( !description.isEmpty() ) toolTip << description; const bool unknownType = data.contains( "known_type" ) && !data.value( "known_type" ).toBool(); if ( name == result ) toolTip << "Result task: its output is the result of the workflow"; + if ( isSwitch ) toolTip << "Switch: picks a branch; tasks with a run condition on it run only for their branches"; if ( unknownType ) toolTip << "Unknown task type: the package providing it is not installed"; node->setTitle( name == result ? QString::fromUtf8( "\u2605 " ) + name : name, toolTip.join( "\n" ) ); node->setUnknownType( unknownType ); @@ -1331,13 +1522,24 @@ void RiuWorkflowGraphView::showGraph( const QJsonObject& graph, const QString& e const QString from = edge.value( "from" ).toString(); const QString to = edge.value( "to" ).toString(); if ( !m_taskNodes.contains( from ) || !m_taskNodes.contains( to ) || from == to ) continue; + if ( edge.value( "kind" ).toString() == "when" ) + { + m_scene->addItem( + new GraphEdge( m_taskNodes.value( from ), m_taskNodes.value( to ), edge.value( "label" ).toString(), selectableEdges ) ); + continue; + } auto* graphEdge = new GraphEdge( m_taskNodes.value( from ), keyForField( edge.value( "output" ).toString() ), m_taskNodes.value( to ), keyForField( edge.value( "input" ).toString() ), false, selectableEdges ); - if ( edge.contains( "collect" ) ) + if ( edge.value( "collect" ).toString() == "first_of" ) + { + graphEdge->setLabel( edge.value( "label" ).toString(), + QString( "Alternative %1: used when no earlier alternative ran" ).arg( edge.value( "label" ).toString() ) ); + } + else if ( edge.contains( "collect" ) ) { const bool keyed = edge.value( "collect" ).toString() == "dict"; graphEdge->setLabel( edge.value( "label" ).toString(), @@ -1429,6 +1631,18 @@ RiuWorkflowGraphView::ItemRef return item; } +//-------------------------------------------------------------------------------------------------- +/// +//-------------------------------------------------------------------------------------------------- +RiuWorkflowGraphView::ItemRef RiuWorkflowGraphView::ItemRef::forCondition( const QString& from, const QString& to ) +{ + ItemRef item; + item.kind = Kind::Condition; + item.from = from; + item.to = to; + return item; +} + //-------------------------------------------------------------------------------------------------- /// //-------------------------------------------------------------------------------------------------- @@ -1439,6 +1653,7 @@ RiuWorkflowGraphView::ItemRef RiuWorkflowGraphView::selectedItem() const if ( auto* node = dynamic_cast( item ) ) return ItemRef::forTask( node->name() ); if ( auto* edge = dynamic_cast( item ); edge && !edge->isConfig() ) { + if ( edge->isControl() ) return ItemRef::forCondition( edge->fromNode()->name(), edge->toNode()->name() ); return ItemRef::forEdge( edge->fromNode()->name(), edge->outputField(), edge->toNode()->name(), edge->inputField() ); } } @@ -1540,6 +1755,7 @@ RiuWorkflowGraphView::ItemRef RiuWorkflowGraphView::itemRefAt( const QPoint& vie if ( auto* edge = dynamic_cast( current ) ) { if ( edge->isConfig() ) break; + if ( edge->isControl() ) return ItemRef::forCondition( edge->fromNode()->name(), edge->toNode()->name() ); return ItemRef::forEdge( edge->fromNode()->name(), edge->outputField(), edge->toNode()->name(), edge->inputField() ); } current = current->parentItem(); diff --git a/ApplicationLibCode/UserInterface/RiuWorkflowGraphView.h b/ApplicationLibCode/UserInterface/RiuWorkflowGraphView.h index dcaabdd44f..6185c40e89 100644 --- a/ApplicationLibCode/UserInterface/RiuWorkflowGraphView.h +++ b/ApplicationLibCode/UserInterface/RiuWorkflowGraphView.h @@ -58,7 +58,8 @@ class RiuWorkflowGraphView : public QGraphicsView { None, Task, - Edge + Edge, + Condition // The run condition of `to`, a control edge from `from` }; Kind kind = Kind::None; @@ -70,6 +71,7 @@ class RiuWorkflowGraphView : public QGraphicsView static ItemRef forTask( const QString& taskName ); static ItemRef forEdge( const QString& from, const QString& output, const QString& to, const QString& input ); + static ItemRef forCondition( const QString& from, const QString& to ); }; // Returns why a connection is not possible. Fields are empty for the whole input or output. diff --git a/ApplicationLibCode/UserInterface/RiuWorkflowJobRunner.cpp b/ApplicationLibCode/UserInterface/RiuWorkflowJobRunner.cpp index 3e078f0225..03818076a3 100644 --- a/ApplicationLibCode/UserInterface/RiuWorkflowJobRunner.cpp +++ b/ApplicationLibCode/UserInterface/RiuWorkflowJobRunner.cpp @@ -163,10 +163,12 @@ void RiuWorkflowJobRunner::drainLines( QByteArray& buffer, RILogLevel level ) if ( event.value( "event" ) == "task_state" && event.value( "run_id" ).isString() && event.value( "task" ).isString() && event.value( "state" ).isString() ) { + // Skipped tasks report a reason instead of an error + const QString message = event.contains( "error" ) ? event.value( "error" ).toString() : event.value( "reason" ).toString(); emit taskStateChanged( event.value( "run_id" ).toString(), event.value( "task" ).toString(), event.value( "state" ).toString(), - event.value( "error" ).toString() ); + message ); continue; } if ( event.value( "event" ) == "map_item_state" && event.value( "run_id" ).isString() && event.value( "task" ).isString() && diff --git a/GrpcInterface/Python/rips/taskmaestro_helper/run.py b/GrpcInterface/Python/rips/taskmaestro_helper/run.py index dd1ed6d73d..d105460ed6 100644 --- a/GrpcInterface/Python/rips/taskmaestro_helper/run.py +++ b/GrpcInterface/Python/rips/taskmaestro_helper/run.py @@ -27,7 +27,12 @@ def _emit_progress( - run_id: str, task: str, state: str, error: str = "", item: str | None = None + run_id: str, + task: str, + state: str, + error: str = "", + item: str | None = None, + reason: str = "", ) -> None: payload = {"event": "task_state", "run_id": run_id, "task": task, "state": state} if item is not None: @@ -36,6 +41,8 @@ def _emit_progress( payload["item"] = item if error: payload["error"] = error + if reason: + payload["reason"] = reason sys.stdout.write(PROGRESS_PREFIX + json.dumps(payload) + "\n") sys.stdout.flush() @@ -55,6 +62,10 @@ def on_task_complete(self, job: Any, task: Any, output: Any) -> None: def on_task_fail(self, job: Any, task: Any, error: Exception) -> None: _emit_progress(self.run_id, task.name, "failed", str(error)) + def on_task_skip(self, job: Any, task_name: str, reason: str) -> None: + # Conditional branches: the task never starts, so only its name is known + _emit_progress(self.run_id, task_name, "skipped", reason=reason) + def on_map_item_start(self, job: Any, task: Any, key: str) -> None: _emit_progress(self.run_id, task.name, "running", item=key) diff --git a/GrpcInterface/Python/rips/tests/test_data/taskmaestro_workflows/conditional/input.yaml b/GrpcInterface/Python/rips/tests/test_data/taskmaestro_workflows/conditional/input.yaml new file mode 100644 index 0000000000..eb6251b0e4 --- /dev/null +++ b/GrpcInterface/Python/rips/tests/test_data/taskmaestro_workflows/conditional/input.yaml @@ -0,0 +1 @@ +start: {value: 2.0} diff --git a/GrpcInterface/Python/rips/tests/test_data/taskmaestro_workflows/conditional/workflow.yaml b/GrpcInterface/Python/rips/tests/test_data/taskmaestro_workflows/conditional/workflow.yaml new file mode 100644 index 0000000000..824f849049 --- /dev/null +++ b/GrpcInterface/Python/rips/tests/test_data/taskmaestro_workflows/conditional/workflow.yaml @@ -0,0 +1,33 @@ +workflow: + name: sizes + result_task: tm_helper_conditional.Report + tasks: + - task: tm_helper_pipeline.Start + config_fields: [value] + - task: tm_helper_conditional.ChooseSize + depends_on: tm_helper_pipeline.Start + - task: tm_helper_conditional.IsPositive + depends_on: tm_helper_pipeline.Start + - task: tm_helper_pipeline.Double + name: double_large + depends_on: tm_helper_pipeline.Start + when: {switch: tm_helper_conditional.ChooseSize, case: [large, huge]} + - task: tm_helper_pipeline.Double + name: double_small + depends_on: tm_helper_pipeline.Start + when: {switch: tm_helper_conditional.ChooseSize, otherwise: true} + - task: tm_helper_pipeline.Double + name: double_positive + depends_on: tm_helper_pipeline.Start + when: {switch: tm_helper_conditional.IsPositive, case: true} + - task: tm_helper_pipeline.Constant + when: {switch: tm_helper_conditional.IsPositive, case: false} + - task: tm_helper_pipeline.Special + depends_on: tm_helper_pipeline.Start + when: {ref: [tm_helper_pipeline.Start, value], in: [1.0, 2.0]} + - task: tm_helper_conditional.Report + depends_on: + sized: + first_of: [double_large, double_small] + signed: + first_of: [double_positive, tm_helper_pipeline.Constant] diff --git a/GrpcInterface/Python/rips/tests/test_data/taskmaestro_workflows/tm_helper_conditional.py b/GrpcInterface/Python/rips/tests/test_data/taskmaestro_workflows/tm_helper_conditional.py new file mode 100644 index 0000000000..59683207d5 --- /dev/null +++ b/GrpcInterface/Python/rips/tests/test_data/taskmaestro_workflows/tm_helper_conditional.py @@ -0,0 +1,51 @@ +"""Conditional-branch tasks used by the taskmaestro helper tests. + +Kept apart from ``tm_helper_pipeline`` because they need a taskmaestro with +``taskmaestro.conditions``. +""" + +from __future__ import annotations + +from typing import Literal + +from pydantic import BaseModel +from taskmaestro import ConditionTask, ExecutionContext, SwitchTask, Task +from tm_helper_pipeline import Number + + +class ChooseSize(SwitchTask[Number, Literal["small", "large", "huge"]]): + """Pick a size label from a number.""" + + name = "choose_size" + + def decide( + self, input: Number, ctx: ExecutionContext + ) -> Literal["small", "large", "huge"]: + if input.value > 100: + return "huge" + return "large" if input.value > 10 else "small" + + +class IsPositive(ConditionTask[Number]): + """True when the number is positive.""" + + name = "is_positive" + + def decide(self, input: Number, ctx: ExecutionContext) -> bool: + return input.value > 0 + + +class ReportInput(BaseModel): + """Two numbers, each taken from whichever branch ran.""" + + sized: Number + signed: Number + + +class Report(Task[ReportInput, Number]): + """Add the two numbers.""" + + name = "report" + + def run(self, input: ReportInput, ctx: ExecutionContext) -> Number: + return Number(value=input.sized.value + input.signed.value) diff --git a/GrpcInterface/Python/rips/tests/test_taskmaestro_helper.py b/GrpcInterface/Python/rips/tests/test_taskmaestro_helper.py index 4252074631..58714586a0 100644 --- a/GrpcInterface/Python/rips/tests/test_taskmaestro_helper.py +++ b/GrpcInterface/Python/rips/tests/test_taskmaestro_helper.py @@ -206,6 +206,67 @@ def run(self, input: Item, ctx: ExecutionContext) -> Out: assert events[4]["error"] == "negative" +def test_progress_hook_reports_skipped_branch( + capsys: pytest.CaptureFixture[str], +) -> None: + pytest.importorskip("taskmaestro.conditions") + from pydantic import BaseModel + from rips.taskmaestro_helper.run import PROGRESS_PREFIX, ProgressHook + from taskmaestro import ( + ConditionTask, + EmptyConfig, + ExecutionContext, + Job, + Runner, + Task, + Workflow, + first_of, + ) + + class Value(BaseModel): + value: int + + class Join(BaseModel): + value: Value + + class Source(Task[EmptyConfig, Value]): + def run(self, input: EmptyConfig, ctx: ExecutionContext) -> Value: + return Value(value=1) + + class Positive(ConditionTask[Value]): + def decide(self, input: Value, ctx: ExecutionContext) -> bool: + return input.value > 0 + + class Copy(Task[Value, Value]): + def run(self, input: Value, ctx: ExecutionContext) -> Value: + return input + + class Take(Task[Join, Value]): + def run(self, input: Join, ctx: ExecutionContext) -> Value: + return input.value + + builder = Workflow.builder("branches") + source = builder.task(Source, name="source") + check = builder.task(Positive, name="check", depends_on=source) + yes = builder.task(Copy, name="yes", depends_on=source, when=check.if_true()) + no = builder.task(Copy, name="no", depends_on=source, when=check.if_false()) + builder.task(Take, name="take", value=first_of(yes, no)) + Runner(hooks=[ProgressHook("run-branch")]).run( + Job(builder.build(), EmptyConfig()), ctx=ExecutionContext() + ) + events = [ + json.loads(line.removeprefix(PROGRESS_PREFIX)) + for line in capsys.readouterr().out.splitlines() + if line.startswith(PROGRESS_PREFIX) + ] + skipped = [event for event in events if event["state"] == "skipped"] + assert [event["task"] for event in skipped] == ["no"] + assert skipped[0]["run_id"] == "run-branch" + assert skipped[0]["reason"] + assert ("no", "running") not in [(e["task"], e["state"]) for e in events] + assert ("take", "completed") in [(e["task"], e["state"]) for e in events] + + def test_run_helper_emits_progress_without_grpc( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: diff --git a/GrpcInterface/Python/rips/tests/test_taskmaestro_helper_definition.py b/GrpcInterface/Python/rips/tests/test_taskmaestro_helper_definition.py index b6363f2f15..4c6f23b3f2 100644 --- a/GrpcInterface/Python/rips/tests/test_taskmaestro_helper_definition.py +++ b/GrpcInterface/Python/rips/tests/test_taskmaestro_helper_definition.py @@ -242,6 +242,76 @@ def test_round_trip_whole_output_dependency(tmp_path: Path) -> None: assert "result_task" not in text +CONDITIONAL_WORKFLOW = f"""\ +workflow: + name: sizes + result_task: tm_helper_conditional.Report + tasks: + - task: {PIPELINE}.Start + config_fields: [value] + - task: tm_helper_conditional.ChooseSize + depends_on: {PIPELINE}.Start + - task: tm_helper_conditional.IsPositive + depends_on: {PIPELINE}.Start + - task: {PIPELINE}.Double + name: double_large + depends_on: {PIPELINE}.Start + when: {{switch: tm_helper_conditional.ChooseSize, case: [large, huge]}} + - task: {PIPELINE}.Double + name: double_small + depends_on: {PIPELINE}.Start + when: {{switch: tm_helper_conditional.ChooseSize, otherwise: true}} + - task: {PIPELINE}.Double + name: double_positive + depends_on: {PIPELINE}.Start + when: {{switch: tm_helper_conditional.IsPositive, case: true}} + - task: {PIPELINE}.Constant + when: {{switch: tm_helper_conditional.IsPositive, case: false}} + - task: {PIPELINE}.Special + depends_on: {PIPELINE}.Start + when: {{ref: [{PIPELINE}.Start, value], in: [1.0, 2.0]}} + - task: tm_helper_conditional.Report + depends_on: + sized: + first_of: [double_large, double_small] + signed: + first_of: [double_positive, {PIPELINE}.Constant] +""" + + +def test_round_trip_conditional_branches(tmp_path: Path) -> None: + pytest.importorskip("taskmaestro.conditions") + source = _write_workflow( + tmp_path / "src", CONDITIONAL_WORKFLOW, "start:\n value: 2.0\n" + ) + first, _ = _round_trip(source, tmp_path / "out") + when = {node["name"]: node.get("when") for node in first["nodes"]} + assert when["double_large"] == {"switch": "choose_size", "case": ["large", "huge"]} + assert when["double_small"] == {"switch": "choose_size", "otherwise": True} + assert when["double_positive"] == {"switch": "is_positive", "case": True} + assert when["special"] == {"ref": ["start", "value"], "in": [1.0, 2.0]} + first_of = [ + (edge["from"], edge["input"]) + for edge in first["edges"] + if edge.get("collect") == "first_of" + ] + assert first_of == [ + ("double_large", "sized"), + ("double_small", "sized"), + ("double_positive", "signed"), + ("constant", "signed"), + ] + switch_output = first["task_types"]["tm_helper_conditional.ChooseSize"][ + "output_schema" + ] + assert "taskmaestro.conditions.Branch" in switch_output["x-ri-python-bases"] + assert switch_output["properties"]["branch"]["enum"] == ["small", "large", "huge"] + + tasks = {task["name"]: task for task in first["describe"]["tasks"]} + assert tasks["is_positive"]["branches"] == [True, False] + assert tasks["double_small"]["when"]["values"] == ["small"] + + # --- References --- diff --git a/README-experimental-workflow-ui.md b/README-experimental-workflow-ui.md index 22b398de40..90f7ca2475 100644 --- a/README-experimental-workflow-ui.md +++ b/README-experimental-workflow-ui.md @@ -110,6 +110,23 @@ for Undo, Redo, Save, Save As, and Duplicate as Editable. once per item of a mapping. Choose the inputs that receive the key and the value of each item. The items are entered in the property view, either as JSON or by selecting cases, well paths or views. **Remove Map** undoes it. +- **Conditional branches:** right-click a task and choose **Run When…** to + run it only for some branches of a switch task (drawn as a diamond), or + when a field of another task has a given value. The condition is drawn as + a dashed purple arrow. Right-click the task or the arrow to edit or remove + it. Tasks that did not run are shown greyed out as *skipped*. This + requires a taskmaestro version with conditional branch support. For simple + conditions, add **condition.compare** (value compared to a threshold), + **condition.is_true** or **condition.path_exists** from the palette, + connect its input and use **Run When…** with *true* or *false* on the + tasks that should depend on it. Fields that accept any value (such as + the value of **condition.is_true**) read the text as a literal: `false`, + `0`, `None`/`null`, numbers and JSON are typed values; other text is a + string (quote it, e.g. `"false"`, to keep it as text). +- **Alternatives (first of):** dropping a connection on an input that is + already connected asks whether to replace the connection or add the new + one as an alternative. The input then takes the first alternative whose + task ran. Right-click an alternative to **Move Earlier/Later**. - **Rename, Set as Result Task, Delete:** right-click a task or connection. F2 renames and Del deletes the selection. Ctrl+Z and Ctrl+Y undo and redo. diff --git a/docs/workflow-ui-design.md b/docs/workflow-ui-design.md index 874dfc78e5..9970031dbc 100644 --- a/docs/workflow-ui-design.md +++ b/docs/workflow-ui-design.md @@ -210,6 +210,65 @@ object, or a multi-selection of cases, well paths or views keyed by name. During a run the helper emits `map_item_state` events; the node shows "done/seen" counts and failures, and the tooltip lists each item. +### Conditional branches + +Conditional branches need a taskmaestro with `taskmaestro.conditions` +(`SwitchTask`, `ConditionTask`). A switch task outputs a `Branch` whose +`branch` field holds the chosen label; describe lists the labels as the +task's `branches`. + +In the definition a node may carry a `when` guard, stored as +`RimWorkflowTaskGuard` (`source`, `field`, `op`, `values`): + +| YAML form | `op` | +|------------------------------------|--------------------| +| `{switch: s, case: a}` or `[a, b]` | `Case` | +| `{switch: s, otherwise: true}` | `Otherwise` | +| `{ref: [t, f], equals: v}` | `Equals` | +| `{ref: [t, f], in: [v, …]}` | `In` | +| `{ref: [t, f], truthy: true/false}`| `Truthy` / `Falsy` | + +A guard is a control dependency, not data. It orders the guarded task after +its source for cycle checks and layout, and a source that is used only by +guards is not a sink. `first_of` joins are edges with +`collect: "first_of"`. The input takes the output of the first alternative, +in edge order, whose task ran. + +Edits: `setTaskGuard` and `checkTaskGuard` set and check a guard. +`addFirstOfMember` turns an existing plain connection into the first +alternative. `disconnect` collapses a single remaining alternative back +to a plain edge, and `connect` replaces all alternatives. `moveCollectMember` +reorders alternatives. `renameTask` updates guards and `removeTask` drops +them. + +`validate` checks the following: + +- the guard's source exists and is not the task itself; +- the guard does not test a field of a mapped task; +- the field exists; +- values match the field schema; +- case labels are among the `branches`; +- `otherwise` is used only on switches; +- truthy and falsy are used only on bool or optional fields; +- every switch label is covered when a switch has guards; +- `first_of` members are type compatible and not mixed with other edges. + +Drawing: + +- Switch tasks (`switch: true` in the graph JSON) are diamonds and show their + labels. +- Guards are purple dash-dot control edges from the bottom of the source to + the top of the guarded task, labelled with `when_label`. +- `first_of` inputs have a dotted ring, and their edges are labelled `1st`, + `2nd`, …. +- Right-click a task for Run When… and Remove Condition, or a control edge + for Edit Condition…. Dropping a compatible output on a wired input asks + whether to replace the connection or add it as an alternative. + +When running, the helper's `on_task_skip` hook emits a `task_state` event +with `state: "skipped"` and a `reason`. Skipped tasks are drawn greyed and +dashed, with the reason in the tooltip. + Selecting a task shows its `RimWorkflowTaskInput` from the displayed job in the property view. Editing is locked while a job of the workflow is running. During a run, task states are drawn on the nodes. From 57ffb1b329ce0c8c86ecb860b14ce6d991cfcb2b Mon Sep 17 00:00:00 2001 From: Kristian Bendiksen Date: Fri, 30 Oct 2026 12:55:50 +0100 Subject: [PATCH 2/3] Workflow UI: Support collect flattening Allow list and dict outputs, including mapped task outputs, to be collected into list inputs, and dicts to be merged into dict inputs, matching taskmaestro's collect flattening. Label flattened members in the graph and document the rules. --- .../Workflow/RimWorkflowDefinitionTools.cpp | 21 ++++++- .../Workflow/RimWorkflowPortCompatibility.cpp | 63 ++++++++++++++++--- .../Workflow/RimWorkflowPortCompatibility.h | 14 +++++ .../UnitTests/RimWorkflowDefinition-Test.cpp | 49 +++++++++++++++ .../UserInterface/RiuWorkflowGraphView.cpp | 20 ++++-- README-experimental-workflow-ui.md | 8 ++- 6 files changed, 157 insertions(+), 18 deletions(-) diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinitionTools.cpp b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinitionTools.cpp index 641bb3826d..a8e7154a2a 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinitionTools.cpp +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowDefinitionTools.cpp @@ -159,6 +159,18 @@ QJsonObject mapToJson( const RimWorkflowTaskMap& map ) return QJsonObject{ { "over", map.over }, { "key_as", map.keyAs }, { "value_as", map.valueAs }, { "error_mode", map.errorMode } }; } +// A member that is a list or dict spliced into the collection, rather than one item +bool isFlattenedMember( const RimWorkflowDefinition& definition, const RimWorkflowDefinitionEdge& edge ) +{ + const auto producedPort = RimWorkflowPortCompatibility::outputPort( definition.taskTypeForNode( edge.from ), edge.output ); + const auto expectedPort = RimWorkflowPortCompatibility::inputPort( definition.taskTypeForNode( edge.to ), edge.input ); + if ( !producedPort || !expectedPort ) return false; + const auto target = RimWorkflowPortCompatibility::collectTarget( *expectedPort ); + if ( !target || target->first != edge.collect ) return false; + const auto mode = RimWorkflowPortCompatibility::collectMember( *producedPort, target->first, target->second, expectedPort->rootSchema ); + return mode && *mode != RimWorkflowPortCompatibility::CollectMember::Item; +} + QString collectName( RimWorkflowDefinitionEdge::Collect collect ) { switch ( collect ) @@ -1834,8 +1846,13 @@ QJsonObject RimWorkflowDefinitionTools::graphFromDefinition( const RimWorkflowDe if ( !edge.output.isEmpty() ) json["output"] = edge.output; if ( edge.isCollected() ) { - json["collect"] = collectName( edge.collect ); - json["label"] = edge.collect == RimWorkflowDefinitionEdge::Collect::Dict ? edge.key : QString( "[%1]" ).arg( listIndex[&edge] ); + json["collect"] = collectName( edge.collect ); + const bool flattened = isFlattenedMember( definition, edge ); + if ( flattened ) json["flatten"] = true; + if ( edge.collect == RimWorkflowDefinitionEdge::Collect::Dict ) + json["label"] = flattened ? QString( "merged" ) : edge.key; + else + json["label"] = QString( flattened ? "[%1+]" : "[%1]" ).arg( listIndex[&edge] ); } else if ( edge.isFirstOf() ) { diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.cpp b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.cpp index ee25602203..aadc40540d 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.cpp +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.cpp @@ -565,8 +565,16 @@ std::optional> RimWorkflowPortCompatibility::collectTarget( const RimWorkflowPort& expected ) { if ( expected.isWhole() ) return std::nullopt; + return collectKind( expected.schema, expected.rootSchema ); +} - const QJsonObject resolved = RimWorkflowSchemaTools::resolveReferences( expected.schema, expected.rootSchema ); +//-------------------------------------------------------------------------------------------------- +/// The kind and element schema of a `list[T]` or `dict[str, T]` schema +//-------------------------------------------------------------------------------------------------- +std::optional> + RimWorkflowPortCompatibility::collectKind( const QJsonObject& schema, const QJsonObject& rootSchema ) +{ + const QJsonObject resolved = RimWorkflowSchemaTools::resolveReferences( schema, rootSchema ); QJsonArray options; for ( const char* unionKey : { "anyOf", "oneOf" } ) { @@ -577,7 +585,7 @@ std::optional> for ( const QJsonValue& value : options ) { - const QJsonObject option = RimWorkflowSchemaTools::resolveReferences( value.toObject(), expected.rootSchema ); + const QJsonObject option = RimWorkflowSchemaTools::resolveReferences( value.toObject(), rootSchema ); const QString type = option.value( "type" ).toString(); if ( type == "array" && option.value( "items" ).isObject() ) return std::make_pair( RimWorkflowDefinitionEdge::Collect::List, option.value( "items" ).toObject() ); @@ -587,6 +595,28 @@ std::optional> return std::nullopt; } +//-------------------------------------------------------------------------------------------------- +/// How a produced value joins a collected input. A value that matches the element type is one +/// item. Otherwise a `list[T]` or `dict[str, T]` is flattened, like taskmaestro does: a list +/// contributes its items and a dict its values (a keyed input merges the dict by its own keys). +/// Lists cannot be merged into a keyed input. +//-------------------------------------------------------------------------------------------------- +std::optional RimWorkflowPortCompatibility::collectMember( const RimWorkflowPort& produced, + RimWorkflowDefinitionEdge::Collect kind, + const QJsonObject& element, + const QJsonObject& expectedRoot ) +{ + if ( isCompatible( produced.schema, produced.rootSchema, element, expectedRoot ) ) return CollectMember::Item; + + const auto source = collectKind( produced.schema, produced.rootSchema ); + if ( !source || !isCompatible( source->second, produced.rootSchema, element, expectedRoot ) ) return std::nullopt; + + using Collect = RimWorkflowDefinitionEdge::Collect; + if ( source->first == Collect::List && kind == Collect::List ) return CollectMember::FlattenList; + if ( source->first == Collect::Dict ) return CollectMember::FlattenDict; + return std::nullopt; +} + //-------------------------------------------------------------------------------------------------- /// Checks a new connection and returns the edge to add //-------------------------------------------------------------------------------------------------- @@ -616,10 +646,21 @@ std::expected RimWorkflowPortCompatibility:: return std::unexpected( QString( "'%1' is mapped and takes its inputs field by field" ).arg( to ) ); RimWorkflowDefinitionEdge edge{ .from = from, .output = output, .to = to, .input = input }; - if ( !input.isEmpty() && !isCompatible( *producedPort, *expectedPort ) ) + // A list or dict that fits the input directly joins the collection when the input already has members + const auto target = input.isEmpty() ? std::nullopt : collectTarget( *expectedPort ); + const auto& edges = definition.edges; + const bool joinsCollection = + target && std::any_of( edges.begin(), + edges.end(), + [&]( const RimWorkflowDefinitionEdge& existing ) + { return existing.to == to && existing.input == input && existing.collect == target->first; } ); + if ( joinsCollection && collectMember( *producedPort, target->first, target->second, expectedPort->rootSchema ) ) + { + edge.collect = target->first; + } + else if ( !input.isEmpty() && !isCompatible( *producedPort, *expectedPort ) ) { - const auto target = collectTarget( *expectedPort ); - if ( target && isCompatible( producedPort->schema, producedPort->rootSchema, target->second, expectedPort->rootSchema ) ) + if ( target && collectMember( *producedPort, target->first, target->second, expectedPort->rootSchema ) ) { edge.collect = target->first; } @@ -709,12 +750,16 @@ std::expected RimWorkflowPortCompatibility::checkCollectMember( c return std::unexpected( QString( "Input '%1' is %2 and cannot collect %3" ) .arg( edge.input, expectedPort->typeName, isList ? "a list of members" : "keyed members" ) ); } - if ( !isCompatible( producedPort->schema, producedPort->rootSchema, target->second, expectedPort->rootSchema ) ) + if ( !collectMember( *producedPort, edge.collect, target->second, expectedPort->rootSchema ) ) { const QString source = edge.output.isEmpty() ? edge.from : edge.from + "." + edge.output; - return std::unexpected( - QString( "%1 is %2, but the members of %3.%4 must be %5" ) - .arg( source, producedPort->typeName, edge.to, edge.input, typeName( target->second, expectedPort->rootSchema ) ) ); + return std::unexpected( QString( "%1 is %2, but the members of %3.%4 must be %5, or %6 of them" ) + .arg( source, + producedPort->typeName, + edge.to, + edge.input, + typeName( target->second, expectedPort->rootSchema ), + isList ? "a list or dict" : "a dict" ) ); } return {}; } diff --git a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.h b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.h index ebfd53b9ad..2f2c9de2f9 100644 --- a/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.h +++ b/ApplicationLibCode/ProjectDataModel/Workflow/RimWorkflowPortCompatibility.h @@ -72,6 +72,20 @@ QJsonObject mappedOutputSchema( const QJsonObject& outputSchema ); // Collected inputs: the kind and element schema of a `list[T]` or `dict[str, T]` input std::optional> collectTarget( const RimWorkflowPort& expected ); +std::optional> collectKind( const QJsonObject& schema, + const QJsonObject& rootSchema ); + +// How a produced value joins a collected input: as one item, or flattened from a list or dict +enum class CollectMember +{ + Item, + FlattenList, + FlattenDict +}; +std::optional collectMember( const RimWorkflowPort& produced, + RimWorkflowDefinitionEdge::Collect kind, + const QJsonObject& element, + const QJsonObject& expectedRoot ); // The edge to add for a new connection, collecting into a list or dict input when the upstream // value is an element of it. An existing connection to the same input is replaced, except for diff --git a/ApplicationLibCode/UnitTests/RimWorkflowDefinition-Test.cpp b/ApplicationLibCode/UnitTests/RimWorkflowDefinition-Test.cpp index c1a7306bdd..dda276cedc 100644 --- a/ApplicationLibCode/UnitTests/RimWorkflowDefinition-Test.cpp +++ b/ApplicationLibCode/UnitTests/RimWorkflowDefinition-Test.cpp @@ -544,6 +544,55 @@ TEST( RimWorkflowDefinitionEdit, collectIntoDict ) EXPECT_FALSE( taskIssues( definition, "sum_dict" ).isEmpty() ); } +//-------------------------------------------------------------------------------------------------- +TEST( RimWorkflowDefinitionEdit, collectFlattensListsAndDicts ) +{ + using Collect = RimWorkflowDefinitionEdge::Collect; + auto definition = collectDefinition(); + + const QJsonObject number{ { "type", "number" } }; + const QJsonObject listOfNumbers{ { "type", "array" }, { "items", number } }; + const QJsonObject dictOfNumbers{ { "type", "object" }, { "additionalProperties", number } }; + definition.taskTypes.insert( "t.make_list", + QJsonObject{ { "name", "make_list" }, + { "input_schema", QJsonObject{ { "type", "object" } } }, + { "output_schema", numberModel( "Listed", "values", listOfNumbers ) } } ); + definition.taskTypes.insert( "t.make_dict", + QJsonObject{ { "name", "make_dict" }, + { "input_schema", QJsonObject{ { "type", "object" } } }, + { "output_schema", numberModel( "Dicted", "values", dictOfNumbers ) } } ); + for ( const auto& [name, taskId] : std::vector>{ { "lister", "t.make_list" }, { "dicter", "t.make_dict" } } ) + { + definition.nodes.push_back( { .name = name, .taskId = taskId } ); + definition.inputs.insert( name, QJsonObject() ); + } + definition = RimWorkflowDefinitionTools::normalize( definition ); + + // A single item, a dict and a list all join one list input + definition = + edited( RimWorkflowDefinitionTools::connect( definition, { .from = "a", .output = "value", .to = "sum_list", .input = "values" } ) ); + definition = edited( + RimWorkflowDefinitionTools::connect( definition, { .from = "dicter", .output = "values", .to = "sum_list", .input = "values" } ) ); + definition = edited( + RimWorkflowDefinitionTools::connect( definition, { .from = "lister", .output = "values", .to = "sum_list", .input = "values" } ) ); + const auto members = definition.incomingEdges( "sum_list" ); + ASSERT_EQ( 3u, members.size() ); + for ( const auto& member : members ) + EXPECT_EQ( Collect::List, member.collect ); + EXPECT_TRUE( taskIssues( definition, "sum_list" ).isEmpty() ) << taskIssues( definition, "sum_list" ).join( "\n" ).toStdString(); + + // A dict is merged into a keyed input that has members, a list is not + definition = + edited( RimWorkflowDefinitionTools::connect( definition, { .from = "a", .output = "value", .to = "sum_dict", .input = "values" } ) ); + definition = edited( + RimWorkflowDefinitionTools::connect( definition, { .from = "dicter", .output = "values", .to = "sum_dict", .input = "values" } ) ); + ASSERT_EQ( 2u, definition.incomingEdges( "sum_dict" ).size() ); + EXPECT_EQ( Collect::Dict, definition.incomingEdges( "sum_dict" )[1].collect ); + EXPECT_FALSE( + RimWorkflowDefinitionTools::connect( definition, { .from = "lister", .output = "values", .to = "sum_dict", .input = "values" } ) ); + EXPECT_TRUE( taskIssues( definition, "sum_dict" ).isEmpty() ) << taskIssues( definition, "sum_dict" ).join( "\n" ).toStdString(); +} + //-------------------------------------------------------------------------------------------------- TEST( RimWorkflowDefinitionEdit, mapTask ) { diff --git a/ApplicationLibCode/UserInterface/RiuWorkflowGraphView.cpp b/ApplicationLibCode/UserInterface/RiuWorkflowGraphView.cpp index ff7fdb09f9..0b8092f1b5 100644 --- a/ApplicationLibCode/UserInterface/RiuWorkflowGraphView.cpp +++ b/ApplicationLibCode/UserInterface/RiuWorkflowGraphView.cpp @@ -1147,8 +1147,9 @@ std::pair, std::vector> editModePorts( const QJs if ( !port.value( "collect" ).toString().isEmpty() ) { info.collect = true; - toolTip << ( port.value( "collect" ).toString() == "dict" ? "Connect several outputs to collect them, each under its own key" - : "Connect several outputs to collect them into a list" ); + toolTip << ( port.value( "collect" ).toString() == "dict" + ? "Connect several outputs to collect them, each under its own key; a dict output is merged by its own keys" + : "Connect several outputs to collect them into a list; list and dict outputs are flattened" ); } info.toolTip = toolTip.join( "\n" ); if ( port.value( "map_over" ).toBool() ) @@ -1541,10 +1542,17 @@ void RiuWorkflowGraphView::showGraph( const QJsonObject& graph, const QString& e } else if ( edge.contains( "collect" ) ) { - const bool keyed = edge.value( "collect" ).toString() == "dict"; - graphEdge->setLabel( edge.value( "label" ).toString(), - keyed ? QString( "Collected under the key '%1'" ).arg( edge.value( "label" ).toString() ) - : QString( "Collected as item %1 of the list" ).arg( edge.value( "label" ).toString() ) ); + const bool keyed = edge.value( "collect" ).toString() == "dict"; + const bool flattened = edge.value( "flatten" ).toBool(); + QString toolTip = keyed ? QString( "Collected under the key '%1'" ).arg( edge.value( "label" ).toString() ) + : QString( "Collected as item %1 of the list" ).arg( edge.value( "label" ).toString() ); + if ( flattened ) + { + toolTip = keyed ? "A dict merged into the collection by its own keys" + : QString( "Collected at position %1: the items of a list, or the values of a dict, are added in order" ) + .arg( edge.value( "label" ).toString() ); + } + graphEdge->setLabel( edge.value( "label" ).toString(), toolTip ); } m_scene->addItem( graphEdge ); } diff --git a/README-experimental-workflow-ui.md b/README-experimental-workflow-ui.md index 90f7ca2475..86a0168962 100644 --- a/README-experimental-workflow-ui.md +++ b/README-experimental-workflow-ui.md @@ -105,7 +105,13 @@ for Undo, Redo, Save, Save As, and Duplicate as Editable. - **Collect outputs:** an input that takes a list or a dict (ring around the port) accepts several connections. Each connection is labelled with its position or key. Right-click a connection to **Rename Key** or **Move - Earlier/Later**. + Earlier/Later**. A list or dict output can also be collected: into a list + input its items (or the values of a dict, in order) are added one by one, + shown as `[n+]`. Into a dict input a dict output is merged by its own keys + (labelled *merged*). This includes the output of a mapped task. Lists cannot + be merged into a dict input, and only one level is flattened. To add such an + output to an existing collection, connect it after the first member. This + requires a taskmaestro version with collect flattening. - **Map over items:** right-click a task and choose **Map Over** to run it once per item of a mapping. Choose the inputs that receive the key and the value of each item. The items are entered in the property view, either as From cbd8a0ae1788932738b351dbe25c45e501631ddf Mon Sep 17 00:00:00 2001 From: Kristian Bendiksen Date: Fri, 30 Oct 2026 15:32:42 +0100 Subject: [PATCH 3/3] Docs: use mermaid for workflow UI architecture diagram --- docs/workflow-ui-design.md | 41 ++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/docs/workflow-ui-design.md b/docs/workflow-ui-design.md index 9970031dbc..15025a4694 100644 --- a/docs/workflow-ui-design.md +++ b/docs/workflow-ui-design.md @@ -20,23 +20,30 @@ manifest file or plugin descriptor. ## Architecture at a glance -``` -ResInsight (C++) Python helper (rips.taskmaestro_helper) -───────────────── ─────────────────────────────────────── -RimWorkflowTaskCatalog ────── catalog ─────► entry points → task descriptors + registered workflows -RimWorkflowCollection - folders in ~/.taskmaestro/workflows ─ load ─► workflow.yaml + input.yaml → definition JSON - Installed (RimWorkflowInstalledCollection) (from the catalog) -RimWorkflow - RimWorkflowDefinition (value struct) - edits → applyEdit → undo/redo, dirty - Save ──────────────── save (stdin JSON) ──► definition JSON → workflow.yaml + input.yaml - RiuWorkflowValidationRunner ─ save --describe ► temp folder + taskmaestro describe - RimWorkflowJob (one or more) - RimWorkflowTaskInput (one per task) - RimWorkflowBinding (one per config field, see table below) - Run ─ writes input.yaml ─ QProcess ─────► run | run --registered - ◄───── JSON events → task states in the graph +```mermaid +flowchart LR + subgraph RI["ResInsight (C++)"] + direction TB + Catalog["RimWorkflowTaskCatalog"] + Coll["RimWorkflowCollection
(Installed: RimWorkflowInstalledCollection)"] + WF["RimWorkflow
RimWorkflowDefinition (value struct)
edits → applyEdit → undo/redo, dirty"] + Valid["RiuWorkflowValidationRunner"] + Job["RimWorkflowJob (one or more)
RimWorkflowTaskInput (one per task)
RimWorkflow<Type>Binding (one per config field)"] + end + subgraph PY["Python helper (rips.taskmaestro_helper)"] + direction TB + PCat["entry points → task descriptors
+ registered workflows"] + PLoad["workflow.yaml + input.yaml
→ definition JSON"] + PSave["definition JSON
→ workflow.yaml + input.yaml"] + PDesc["temp folder + taskmaestro describe"] + PRun["run <dir> | run --registered <id>"] + end + PCat -- "catalog" --> Catalog + Coll -- "load
(folders in ~/.taskmaestro/workflows)" --> PLoad + WF -- "save (stdin JSON)" --> PSave + Valid -- "save --describe" --> PDesc + Job -- "Run: writes input.yaml, QProcess" --> PRun + PRun -. "JSON events → task states in the graph" .-> Job ``` The C++ side never parses YAML and doesn't import Pydantic. It only handles