Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions openc3-cosmos-cmd-tlm-api/app/controllers/reaction_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,11 @@ def show
def create
return unless authorization('script_run')
begin
hash = params.to_unsafe_h.slice(:triggers, :trigger_level, :actions, :snooze).to_h
name = @model_class.create_unique_name(scope: params[:scope])
hash = params.to_unsafe_h.slice(:triggers, :trigger_level, :actions, :snooze, :label, :name).to_h
name = hash.delete('name')
if name.nil? || name.strip.empty?
name = @model_class.create_unique_name(scope: params[:scope])
end
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need the name stuff anymore now that we're using the label field

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

name will remain as the primary key!

hash[:username] = username()
model = @model_class.from_json(hash.symbolize_keys, name: name, scope: params[:scope])
model.create()
Expand Down Expand Up @@ -146,11 +149,12 @@ def update
render json: { status: 'error', message: NOT_FOUND }, status: :not_found
return
end
hash = params.to_unsafe_h.slice(:snooze, :triggers, :trigger_level, :actions).to_h
hash = params.to_unsafe_h.slice(:snooze, :triggers, :trigger_level, :actions, :label).to_h
model.triggers = hash['triggers'] if hash['triggers']
model.actions = hash['actions'] if hash['actions']
model.trigger_level = hash['trigger_level'] if hash['trigger_level']
model.snooze = hash['snooze'] if hash['snooze']
model.label = hash['label'] if hash.key?('label')
# Update the timestamp before notifying so the event has the current time
model.updated_at = Time.now.to_nsec_from_epoch
# Notify the ReactionMicroservice to update the ReactionModel
Expand Down
10 changes: 7 additions & 3 deletions openc3-cosmos-cmd-tlm-api/app/controllers/trigger_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,11 @@ def create
return unless authorization('script_run')
hash = nil
begin
hash = params.to_unsafe_h.slice(:group, :left, :operator, :right).to_h
name = @model_class.create_unique_name(group: hash['group'], scope: params[:scope])
hash = params.to_unsafe_h.slice(:group, :left, :operator, :right, :label, :name).to_h
name = hash.delete('name')
if name.nil? || name.strip.empty?
name = @model_class.create_unique_name(group: hash['group'], scope: params[:scope])
end
model = @model_class.from_json(hash.symbolize_keys, name: name, scope: params[:scope])
model.create() # Create sends a notification
render json: model.as_json(), status: :created
Expand Down Expand Up @@ -136,10 +139,11 @@ def update
render json: { status: 'error', message: NOT_FOUND }, status: :not_found
return
end
hash = params.to_unsafe_h.slice(:left, :operator, :right).to_h
hash = params.to_unsafe_h.slice(:left, :operator, :right, :label).to_h
model.left = hash['left'] if hash['left']
model.operator = hash['operator'] if hash['operator']
model.right = hash['right'] if hash['right']
model.label = hash['label'] if hash.key?('label')
# Update the timestamp before notifying so the event has the current time
model.updated_at = Time.now.to_nsec_from_epoch
# Notify the TriggerGroupMicroservice to update the TriggerModel
Expand Down
36 changes: 27 additions & 9 deletions openc3/lib/openc3/models/reaction_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ class ReactionModel < Model
ACTION_TYPES = [SCRIPT_REACTION, COMMAND_REACTION, NOTIFY_REACTION]

def self.create_unique_name(scope:)
reaction_names = self.names(scope: scope) # comes back sorted
reaction_names = self.names(scope: scope)
num = 1 # Users count with 1
if reaction_names[-1]
num = reaction_names[-1][5..-1].to_i + 1
unless reaction_names.empty?
# Extract numeric suffixes and find the max to avoid lexicographic sort issues
max_num = reaction_names.map { |name| name[5..-1].to_i }.max
num = max_num + 1
end
return "REACT#{num}"
end
Expand Down Expand Up @@ -77,7 +79,7 @@ def self.delete(name:, scope:)
end

attr_reader :name, :scope, :snooze, :triggers, :actions, :enabled, :trigger_level, :snoozed_until
attr_accessor :username, :shard
attr_accessor :username, :shard, :label

def initialize(
name:,
Expand All @@ -90,6 +92,7 @@ def initialize(
snoozed_until: nil,
username: nil,
shard: 0,
label: nil,
updated_at: nil
)
super("#{scope}#{PRIMARY_KEY}", name: name, scope: scope)
Expand All @@ -102,6 +105,7 @@ def initialize(
@triggers = validate_triggers(triggers)
@username = username
@shard = shard.to_i # to_i to handle nil
@label = label
@updated_at = updated_at
end

Expand Down Expand Up @@ -177,28 +181,40 @@ def validate_actions(actions)
return actions
end

def verify_triggers
# Validate that all triggers exist, but do not persist dependent changes yet.
# Returns the list of trigger models that need updating.
def validate_triggers_exist
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The change below is to ensure that the dependencies are not updated until after the trigger / reaction is created. This prevents an edge case where dependents are defined but the dependency doesn't exist.

if @triggers.empty?
raise ReactionInputError.new "reaction must contain at least one valid trigger: #{@triggers}"
end

models_to_update = []
@triggers.each do | trigger |
model = TriggerModel.get(name: trigger['name'], group: trigger['group'], scope: @scope)
if model.nil?
raise ReactionInputError.new "failed to find trigger: #{trigger}"
end
model.update_dependents(dependent: @name)
model.update()
unless model.dependents.include?(@name)
model.update_dependents(dependent: @name)
models_to_update << model
end
end
models_to_update
end

# Persist dependent changes to trigger models
def commit_trigger_dependents(models)
models.each { |model| model.update() }
end

def create
unless Store.hget(@primary_key, @name).nil?
raise ReactionInputError.new "existing reaction found: #{@name}"
end
verify_triggers()
models = validate_triggers_exist()
@updated_at = Time.now.to_nsec_from_epoch
Store.hset(@primary_key, @name, JSON.generate(as_json, allow_nan: true))
commit_trigger_dependents(models)
notify(kind: 'created')
end

Expand All @@ -222,9 +238,10 @@ def update
end
end

verify_triggers()
models = validate_triggers_exist()
@updated_at = Time.now.to_nsec_from_epoch
Store.hset(@primary_key, @name, JSON.generate(as_json, allow_nan: true))
commit_trigger_dependents(models)
# No notification as this is only called via reaction_controller which already notifies
end

Expand Down Expand Up @@ -281,6 +298,7 @@ def as_json(*a)
'actions' => @actions,
'username' => @username,
'shard' => @shard,
'label' => @label,
'updated_at' => @updated_at
}
end
Expand Down
31 changes: 24 additions & 7 deletions openc3/lib/openc3/models/trigger_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,12 @@ class TriggerModel < Model
TRIGGER_TYPE = 'trigger'.freeze

def self.create_unique_name(group:, scope:)
trigger_names = self.names(group: group, scope: scope) # comes back sorted
trigger_names = self.names(group: group, scope: scope)
num = 1 # Users count with 1
if trigger_names[-1]
num = trigger_names[-1][4..-1].to_i + 1
unless trigger_names.empty?
# Extract numeric suffixes and find the max to avoid lexicographic sort issues
max_num = trigger_names.map { |name| name[4..-1].to_i }.max
num = max_num + 1
end
return "TRIG#{num}"
end
Expand Down Expand Up @@ -97,6 +99,7 @@ def self.delete(name:, group:, scope:)
end

attr_reader :name, :scope, :state, :group, :enabled, :left, :operator, :right, :dependents, :roots
attr_accessor :label

def initialize(
name:,
Expand All @@ -108,6 +111,7 @@ def initialize(
state: false,
enabled: true,
dependents: nil,
label: nil,
updated_at: nil
)
super("#{scope}#{PRIMARY_KEY}#{group}", name: name, scope: scope)
Expand All @@ -119,6 +123,7 @@ def initialize(
@operator = validate_operator(operator: operator)
@right = validate_operand(operand: right, right: true)
@dependents = dependents
@label = label
@updated_at = updated_at
selected_group = TriggerGroupModel.get(name: @group, scope: @scope)
if selected_group.nil?
Expand Down Expand Up @@ -175,34 +180,45 @@ def validate_operator(operator:)
end
end

def verify_triggers
# Validate that all root triggers exist, but do not persist dependent changes yet.
# Returns the list of root trigger models that need updating.
def validate_roots
@dependents = [] if @dependents.nil?
models_to_update = []
@roots.each do | trigger |
model = TriggerModel.get(name: trigger, group: @group, scope: @scope)
if model.nil?
raise TriggerInputError.new "failed to find dependent trigger: '#{@group}:#{trigger}'"
end
unless model.dependents.include?(@name)
model.update_dependents(dependent: @name)
model.update()
models_to_update << model
end
end
models_to_update
end

# Persist dependent changes to root triggers
def commit_roots(models)
models.each { |model| model.update() }
end

def create
unless Store.hget(@primary_key, @name).nil?
raise TriggerInputError.new "existing trigger found: '#{@name}'"
end
verify_triggers()
models = validate_roots()
@updated_at = Time.now.to_nsec_from_epoch
Store.hset(@primary_key, @name, JSON.generate(as_json, allow_nan: true))
commit_roots(models)
notify(kind: 'created')
end

def update
verify_triggers()
models = validate_roots()
@updated_at = Time.now.to_nsec_from_epoch
Store.hset(@primary_key, @name, JSON.generate(as_json, allow_nan: true))
commit_roots(models)
# No notification as this is only called via trigger_controller which already notifies
end

Expand Down Expand Up @@ -267,6 +283,7 @@ def as_json(*a)
'left' => @left,
'operator' => @operator,
'right' => @right,
'label' => @label,
'updated_at' => @updated_at,
}
end
Expand Down
45 changes: 45 additions & 0 deletions openc3/spec/models/reaction_model_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,22 @@ def generate_reaction(name: 'REACT1')
name = ReactionModel.create_unique_name(scope: $openc3_scope)
expect(name).to eql 'REACT10' # Previous is 9 so now 10
end

it "handles more than 10 reactions correctly" do
generate_custom_trigger().create()
(1..11).each { |i| generate_custom_reaction(name: "REACT#{i}").create() }
name = ReactionModel.create_unique_name(scope: $openc3_scope)
expect(name).to eql 'REACT12'
end

it "handles custom named reactions mixed with auto-generated" do
generate_custom_trigger().create()
generate_custom_reaction(name: 'REACT1').create()
generate_custom_reaction(name: 'MyReaction').create()
generate_custom_reaction(name: 'REACT5').create()
name = ReactionModel.create_unique_name(scope: $openc3_scope)
expect(name).to eql 'REACT6' # Max numeric is 5, so next is 6
end
end

describe "check attr_reader" do
Expand Down Expand Up @@ -225,6 +241,35 @@ def generate_reaction(name: 'REACT1')
expect(json['snooze']).to eql(300)
expect(json['triggers']).to_not be_nil()
expect(json['actions']).to_not be_nil()
expect(json['label']).to be_nil()
end
end

describe "label" do
it "defaults to nil" do
model = generate_reaction()
expect(model.label).to be_nil()
expect(model.as_json()['label']).to be_nil()
end

it "persists through create and reload" do
model = generate_reaction()
model.label = 'My Reaction'
model.update()
loaded = ReactionModel.get(name: 'REACT1', scope: $openc3_scope)
expect(loaded.label).to eql('My Reaction')
end

it "can be updated and cleared" do
model = generate_reaction()
model.label = 'Updated Label'
model.update()
loaded = ReactionModel.get(name: 'REACT1', scope: $openc3_scope)
expect(loaded.label).to eql('Updated Label')
loaded.label = nil
loaded.update()
cleared = ReactionModel.get(name: 'REACT1', scope: $openc3_scope)
expect(cleared.label).to be_nil()
end
end

Expand Down
44 changes: 44 additions & 0 deletions openc3/spec/models/trigger_model_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,20 @@ def generate_trigger_group_model(name: TMO_GROUP)
name = TriggerModel.create_unique_name(group: TMO_GROUP, scope: $openc3_scope)
expect(name).to eql 'TRIG10' # Previous is 9 so now 10
end

it "handles more than 10 triggers correctly" do
(1..11).each { |i| generate_trigger(name: "TRIG#{i}").create() }
name = TriggerModel.create_unique_name(group: TMO_GROUP, scope: $openc3_scope)
expect(name).to eql 'TRIG12'
end

it "handles custom named triggers mixed with auto-generated" do
generate_trigger(name: 'TRIG1').create()
generate_trigger(name: 'MyCustomTrig').create()
generate_trigger(name: 'TRIG5').create()
name = TriggerModel.create_unique_name(group: TMO_GROUP, scope: $openc3_scope)
expect(name).to eql 'TRIG6' # Max numeric is 5, so next is 6
end
end

describe "self.all" do
Expand Down Expand Up @@ -228,6 +242,36 @@ def generate_trigger_group_model(name: TMO_GROUP)
expect(json['operator']).to eql('>')
expect(json['right']).to_not be_nil()
expect(json['dependents']).to_not be_nil()
expect(json['label']).to be_nil()
end
end

describe "label" do
it "defaults to nil" do
model = generate_trigger()
expect(model.label).to be_nil()
expect(model.as_json()['label']).to be_nil()
end

it "persists through create and reload" do
model = generate_trigger()
model.label = 'My Trigger'
model.create()
loaded = TriggerModel.get(name: 'TRIG1', group: TMO_GROUP, scope: $openc3_scope)
expect(loaded.label).to eql('My Trigger')
end

it "can be updated and cleared" do
model = generate_trigger()
model.create()
model.label = 'Updated Label'
model.update()
loaded = TriggerModel.get(name: 'TRIG1', group: TMO_GROUP, scope: $openc3_scope)
expect(loaded.label).to eql('Updated Label')
loaded.label = nil
loaded.update()
cleared = TriggerModel.get(name: 'TRIG1', group: TMO_GROUP, scope: $openc3_scope)
expect(cleared.label).to be_nil()
end
end

Expand Down
Loading