Skip to content

Commit

Permalink
Merge pull request #1383 from OpenC3/overlap
Browse files Browse the repository at this point in the history
Allow overlapping activities
  • Loading branch information
jmthomas authored Jul 9, 2024
2 parents f905784 + fe1f527 commit d910a70
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,7 @@ def create
)
render :json => model.as_json(:allow_nan => true), :status => 201
rescue ArgumentError, TypeError => e
message = "Invalid input: #{JSON.parse(hash, :allow_nan => true, :create_additions => true)}"
render :json => { :status => 'error', :message => message, :type => e.class, :e => e.to_s }, :status => 400
render :json => { :status => 'error', :message => "Invalid input: #{hash}", :type => e.class, :e => e.to_s }, :status => 400
rescue OpenC3::ActivityInputError => e
render :json => { :status => 'error', :message => e.message, :type => e.class, :e => e.to_s }, :status => 400
rescue OpenC3::ActivityOverlapError => e
Expand Down Expand Up @@ -257,8 +256,7 @@ def update
)
render :json => model.as_json(:allow_nan => true), :status => 200
rescue ArgumentError, TypeError => e
message = "Invalid input: #{JSON.parse(hash, :allow_nan => true, :create_additions => true)}"
render :json => { :status => 'error', :message => message, :type => e.class, :e => e.to_s }, :status => 400
render :json => { :status => 'error', :message => "Invalid input: #{hash}", :type => e.class, :e => e.to_s }, :status => 400
rescue OpenC3::ActivityInputError => e
render :json => { :status => 'error', :message => e.message, :type => e.class, :e => e.to_s }, :status => 400
rescue OpenC3::ActivityOverlapError => e
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def generate_activity_hash(start)
expect(response).to have_http_status(:created)
end

xit "returns a hash and status code 400 with missing values" do
it "returns a hash and status code 400 with missing values" do
post :create, params: { 'scope'=>'DEFAULT', 'name'=>'test' }
ret = JSON.parse(response.body, :allow_nan => true, :create_additions => true)
expect(ret['status']).to eql('error')
Expand Down Expand Up @@ -125,9 +125,8 @@ def generate_activity_hash(start)
expect(response).to have_http_status(:created)
post :create, params: hash.merge({ 'scope'=>'DEFAULT', 'name'=>'test' })
ret = JSON.parse(response.body, :allow_nan => true, :create_additions => true)
expect(ret['status']).to eql('error')
expect(ret['message']).not_to be_nil
expect(response).to have_http_status(409)
expect(ret['name']).to eql('test')
expect(response).to have_http_status(201)
end
end

Expand Down Expand Up @@ -196,9 +195,8 @@ def generate_activity_hash(start)
hash = generate_activity_hash(2.0)
put :update, params: hash.merge({ 'scope'=>'DEFAULT', 'name'=>'test', 'id'=>created['start'] })
ret = JSON.parse(response.body, :allow_nan => true, :create_additions => true)
expect(ret['status']).to eql('error')
expect(ret['message']).not_to be_nil
expect(response).to have_http_status(409)
expect(ret['name']).to eql 'test'
expect(response).to have_http_status(200)
end

it "returns a hash and status code 404 with invalid start" do
Expand All @@ -209,7 +207,7 @@ def generate_activity_hash(start)
expect(response).to have_http_status(:not_found)
end

xit "returns a hash and status code 400 with valid params" do
it "returns a hash and status code 400 with valid params" do
hash = generate_activity_hash(1.0)
post :create, params: hash.merge({'scope'=>'DEFAULT', 'name'=>'test' })
created = JSON.parse(response.body, :allow_nan => true, :create_additions => true)
Expand All @@ -221,7 +219,7 @@ def generate_activity_hash(start)
expect(response).to have_http_status(400)
end

xit "returns a hash and status code 400 with negative time" do
it "returns a hash and status code 400 with negative time" do
hash = generate_activity_hash(1.0)
post :create, params: hash.merge({ 'scope'=>'DEFAULT', 'name'=>'test' })
expect(response).to have_http_status(:created)
Expand All @@ -235,7 +233,7 @@ def generate_activity_hash(start)
expect(response).to have_http_status(400)
end

xit "returns a hash and status code 400 with invalid json" do
it "returns a hash and status code 400 with invalid json" do
hash = generate_activity_hash(1.0)
post :create, params: hash.merge({ 'scope'=>'DEFAULT', 'name'=>'test' })
expect(response).to have_http_status(:created)
Expand Down
39 changes: 21 additions & 18 deletions openc3/lib/openc3/models/activity_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@

module OpenC3
class ActivityError < StandardError; end

class ActivityInputError < ActivityError; end

class ActivityOverlapError < ActivityError; end

class ActivityModel < Model
Expand All @@ -45,7 +43,7 @@ class ActivityModel < Model
# with 15 slots to make sure we don't miss a planned task.
# @return [Array|nil] Array of the next hour in the sorted set
def self.activities(name:, scope:)
now = Time.now.to_i
now = Time.now.to_f
start_score = now - 15
stop_score = (now + 3660)
array = Store.zrangebyscore("#{scope}#{PRIMARY_KEY}__#{name}", start_score, stop_score)
Expand Down Expand Up @@ -158,7 +156,7 @@ def initialize(
@updated_at = updated_at
end

# validate_time searches from the current activity @stop - 1 (because we allow overlap of stop with start)
# validate_time searches from the current activity @stop (exclusive because we allow overlap of stop with start)
# back through @start - MAX_DURATION. The method is trying to validate that this new activity does not
# overlap with anything else. The reason we search back past @start through MAX_DURATION is because we
# need to return all the activities that may start before us and verify that we don't overlap them.
Expand All @@ -170,7 +168,8 @@ def initialize(
#
# @param [Integer] ignore_score - should be nil unless you want to ignore a time when doing an update
def validate_time(ignore_score = nil)
array = Store.zrevrangebyscore(@primary_key, @stop - 1, @start - MAX_DURATION)
# Adding a '(' makes the max value exclusive
array = Store.zrevrangebyscore(@primary_key, "(#{@stop}", @start - MAX_DURATION)
array.each do |value|
activity = JSON.parse(value, :allow_nan => true, :create_additions => true)
if ignore_score == activity['start']
Expand All @@ -197,14 +196,14 @@ def validate_input(start:, stop:, kind:, data:)
rescue Date::Error
raise ActivityInputError.new "start and stop must be seconds: #{start}, #{stop}"
end
now_i = Time.now.to_i
now_f = Time.now.to_f
begin
duration = stop - start
rescue NoMethodError
raise ActivityInputError.new "start and stop must be seconds: #{start}, #{stop}"
end
if now_i >= start and kind != 'expire'
raise ActivityInputError.new "activity must be in the future, current_time: #{now_i} vs #{start}"
if now_f >= start and kind != 'expire'
raise ActivityInputError.new "activity must be in the future, current_time: #{now_f} vs #{start}"
elsif duration >= MAX_DURATION and kind != 'expire'
raise ActivityInputError.new "activity can not be longer than #{MAX_DURATION} seconds"
elsif duration <= 0
Expand Down Expand Up @@ -233,7 +232,7 @@ def set_input(start:, stop:, kind: nil, data: nil, events: nil, fulfillment: nil

# Update the Redis hash at primary_key and set the score equal to the start Epoch time
# the member is set to the JSON generated via calling as_json
def create
def create(overlap: true)
if @recurring['end'] and @recurring['frequency'] and @recurring['span']
# First validate the initial recurring activity ... all others are just offsets
validate_input(start: @start, stop: @stop, kind: @kind, data: @data)
Expand Down Expand Up @@ -285,11 +284,13 @@ def create
notify(kind: 'created')
else
validate_input(start: @start, stop: @stop, kind: @kind, data: @data)
collision = validate_time()
unless collision.nil?
raise ActivityOverlapError.new "activity overlaps existing at #{collision}"
if !overlap
# If we don't allow overlap we need to validate the time
collision = validate_time()
unless collision.nil?
raise ActivityOverlapError.new "activity overlaps existing at #{collision}"
end
end

@updated_at = Time.now.to_nsec_from_epoch
add_event(status: 'created')
Store.zadd(@primary_key, @start, JSON.generate(self.as_json(:allow_nan => true)))
Expand All @@ -300,7 +301,7 @@ def create
# Update the Redis hash at primary_key and remove the current activity at the current score
# and update the score to the new score equal to the start Epoch time this uses a multi
# to execute both the remove and create.
def update(start:, stop:, kind:, data:)
def update(start:, stop:, kind:, data:, overlap: true)
array = Store.zrangebyscore(@primary_key, @start, @start)
if array.length == 0
raise ActivityError.new "failed to find activity at: #{@start}"
Expand All @@ -309,10 +310,12 @@ def update(start:, stop:, kind:, data:)
old_start = @start
set_input(start: start, stop: stop, kind: kind, data: data, events: @events)
@updated_at = Time.now.to_nsec_from_epoch
# copy of create
collision = validate_time(old_start)
unless collision.nil?
raise ActivityOverlapError.new "failed to update #{old_start}, no activities can overlap, collision: #{collision}"
if !overlap
# If we don't allow overlap we need to validate the time
collision = validate_time(old_start)
unless collision.nil?
raise ActivityOverlapError.new "failed to update #{old_start}, no activities can overlap, collision: #{collision}"
end
end

add_event(status: 'updated')
Expand Down
2 changes: 1 addition & 1 deletion openc3/lib/openc3/models/sorted_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# GNU Affero General Public License for more details.

# Modified by OpenC3, Inc.
# All changes Copyright 2023, OpenC3, Inc.
# All changes Copyright 2024, OpenC3, Inc.
# All Rights Reserved
#
# This file may also be used under the terms of a commercial license
Expand Down
44 changes: 32 additions & 12 deletions openc3/spec/models/activity_model_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -238,19 +238,19 @@ def generate_activity(name:, scope:, start:, kind: "COMMAND", stop: 1.0)
it "returns all metrics between X and Y" do
name = "foobar"
scope = "scope"
activity = generate_activity(name: name, scope: scope, start: 1.5, kind: 'SCRIPT')
activity.create()
activity = generate_activity(name: name, scope: scope, start: 5.0, kind: 'SCRIPT')
activity.create()
activity1 = generate_activity(name: name, scope: scope, start: 1.5, kind: 'SCRIPT')
activity1.create()
activity2 = generate_activity(name: name, scope: scope, start: 5.0, kind: 'SCRIPT')
activity2.create()
dt = DateTime.now.new_offset(0)
start = (dt + (1 / 24.0)).strftime("%s").to_i
stop = (dt + (3 / 24.0)).strftime("%s").to_i
array = ActivityModel.get(name: name, scope: scope, start: start, stop: stop)
expect(array.empty?).to eql(false)
expect(array.length).to eql(1)
expect(array[0]["kind"]).to eql("script")
expect(array[0]["start"]).not_to be_nil
expect(array[0]["stop"]).not_to be_nil
expect(array[0]["start"]).to eql(activity1.start)
expect(array[0]["stop"]).to eql(activity1.stop)
end
end

Expand Down Expand Up @@ -288,6 +288,26 @@ def generate_activity(name:, scope:, start:, kind: "COMMAND", stop: 1.0)
expect(model.events.empty?).to eql(false)
expect(model.events.length).to eql(1)
end

it "supports floating point start and stop" do
name = "foobar"
scope = "scope"
activity = ActivityModel.new(
name: name,
scope: scope,
start: (Time.now + 1).to_f,
stop: (Time.now + 1.5).to_f,
kind: "COMMAND",
data: {}
)
activity.create()
model = ActivityModel.score(name: name, scope: scope, score: activity.start)
expect(model.fulfillment).to eql(false)
expect(model.start).to eql(activity.start)
expect(model.stop).to eql(activity.stop)
expect(model.events.empty?).to eql(false)
expect(model.events.length).to eql(1)
end
end

describe "self.count" do
Expand Down Expand Up @@ -409,7 +429,7 @@ def generate_activity(name:, scope:, start:, kind: "COMMAND", stop: 1.0)
activity.create()
model = generate_activity(name: name, scope: scope, start: 1.1, stop: 0.8)
expect {
model.create()
model.create(overlap: false)
}.to raise_error(ActivityOverlapError)
end

Expand All @@ -420,7 +440,7 @@ def generate_activity(name:, scope:, start:, kind: "COMMAND", stop: 1.0)
activity.create()
model = generate_activity(name: name, scope: scope, start: 0.5, stop: 1.0)
expect {
model.create()
model.create(overlap: false)
}.to raise_error(ActivityOverlapError)
end

Expand All @@ -431,7 +451,7 @@ def generate_activity(name:, scope:, start:, kind: "COMMAND", stop: 1.0)
activity.create()
model = generate_activity(name: name, scope: scope, start: 1.5, stop: 1.5)
expect {
model.create()
model.create(overlap: false)
}.to raise_error(ActivityOverlapError)
end

Expand All @@ -442,7 +462,7 @@ def generate_activity(name:, scope:, start:, kind: "COMMAND", stop: 1.0)
activity.create()
model = generate_activity(name: name, scope: scope, start: 0.5, stop: 1.5)
expect {
model.create()
model.create(overlap: false)
}.to raise_error(ActivityOverlapError)
end

Expand All @@ -455,7 +475,7 @@ def generate_activity(name:, scope:, start:, kind: "COMMAND", stop: 1.0)
bar.create()
activity = generate_activity(name: name, scope: scope, start: 0.5, stop: 1.7)
expect {
activity.create()
activity.create(overlap: false)
}.to raise_error(ActivityOverlapError)
end

Expand Down Expand Up @@ -559,7 +579,7 @@ def generate_activity(name:, scope:, start:, kind: "COMMAND", stop: 1.0)
new_start = activity.start + 3600
new_stop = activity.stop + 3600
expect {
activity.update(start: new_start, stop: new_stop, kind: "COMMAND", data: {})
activity.update(start: new_start, stop: new_stop, kind: "COMMAND", data: {}, overlap: false)
}.to raise_error(ActivityOverlapError)
end
end
Expand Down

0 comments on commit d910a70

Please sign in to comment.