From b7f09e51e856bfca5cfedd7fef3c572bebdbe809 Mon Sep 17 00:00:00 2001 From: Rick Carlino Date: Tue, 9 Jul 2019 20:40:31 -0500 Subject: [PATCH] Mark as dependency tracking (#1262) * Set resource_id to device.id instead of 0 (less surprising to users) * Changes to Device.current, tests for `resource_update` dep tracking * Re-enable demos --- app/controllers/application_controller.rb | 3 +- app/lib/celery_script/checker.rb | 7 +- app/models/celery_script_settings_bag.rb | 16 +-- app/models/device.rb | 1 - app/models/in_use_point.rb | 8 +- app/models/point.rb | 8 +- app/models/resource_update_step.rb | 21 ++++ app/mutations/points/destroy.rb | 40 +++--- ...0709194037_create_resource_update_steps.rb | 5 + db/structure.sql | 51 ++++++-- db/views/resource_update_steps_v01.sql | 31 +++++ spec/lib/celery_script/corpus_spec.rb | 112 ++++++++--------- spec/models/resource_update_step_spec.rb | 7 ++ spec/mutations/points/destroy_spec.rb | 117 +++++++++++------- 14 files changed, 285 insertions(+), 142 deletions(-) create mode 100644 app/models/resource_update_step.rb create mode 100644 db/migrate/20190709194037_create_resource_update_steps.rb create mode 100644 db/views/resource_update_steps_v01.sql create mode 100644 spec/models/resource_update_step_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1f177b7fa..6638c8540 100755 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -7,7 +7,8 @@ class ApplicationController < ActionController::Base @current_device else @current_device = (current_user.try(:device) || no_device) - Device.current = @current_device # Mutable state eww + # Mutable state eww + Device.send(:current=, @current_device) @current_device end end diff --git a/app/lib/celery_script/checker.rb b/app/lib/celery_script/checker.rb index 9f1d236de..a67eb8b82 100644 --- a/app/lib/celery_script/checker.rb +++ b/app/lib/celery_script/checker.rb @@ -44,7 +44,12 @@ module CeleryScript # This is the type checker entry point after initialization. def run! - CeleryScript::TreeClimber.travel(tree, method(:validate).to_proc) + # NOTE: Some nodes require knowledge of + # `Device.current` in order to validate + # properly + device.auto_sync_transaction do + CeleryScript::TreeClimber.travel(tree, method(:validate).to_proc) + end tree end diff --git a/app/models/celery_script_settings_bag.rb b/app/models/celery_script_settings_bag.rb index 787fe1222..1d69f25c5 100644 --- a/app/models/celery_script_settings_bag.rb +++ b/app/models/celery_script_settings_bag.rb @@ -30,8 +30,7 @@ module CeleryScriptSettingsBag ALLOWED_PIN_MODES = [DIGITAL = 0, ANALOG = 1] ALLOWED_PIN_TYPES = PIN_TYPE_MAP.keys ALLOWED_POINTER_TYPE = %w(GenericPointer ToolSlot Plant) - ALLOWED_RESOURCE_TYPE = %w(Device FarmEvent Image Log Peripheral Plant Point - Regimen Sequence Tool ToolSlot User GenericPointer) + ALLOWED_RESOURCE_TYPE = %w(Device Point Plant ToolSlot GenericPointer) ALLOWED_RPC_NODES = %w(calibrate change_ownership check_updates dump_info emergency_lock emergency_unlock execute execute_script @@ -488,10 +487,10 @@ module CeleryScriptSettingsBag resource_update: { args: RESOURCE_UPDATE_ARGS, tags: [:function, :api_writer, :network_user], - blk: ->(x) do - resource_type = x.args.fetch(:resource_type).value - resource_id = x.args.fetch(:resource_id).value - check_resource_type(x, resource_type, resource_id) + blk: ->(n) do + resource_type = n.args.fetch(:resource_type).value + resource_id = n.args.fetch(:resource_id).value + check_resource_type(n, resource_type, resource_id, Device.current) end, }, }.map { |(name, list)| Corpus.node(name, **list) } @@ -507,13 +506,14 @@ module CeleryScriptSettingsBag node.invalidate!(BAD_RESOURCE_ID % [klass.name, resource_id]) end - def self.check_resource_type(node, resource_type, resource_id) + def self.check_resource_type(node, resource_type, resource_id, owner) + raise "OPPS!" unless owner case resource_type # <= Security critical code (for const_get'ing) when "Device" # When "resource_type" is "Device", resource_id always refers to # the current_device. # For convenience, we try to set it here, defaulting to 0 - node.args[:resource_id].instance_variable_set("@value", 0) + node.args[:resource_id].instance_variable_set("@value", owner.id) when *ALLOWED_RESOURCE_TYPE.without("Device") klass = Kernel.const_get(resource_type) resource_ok = klass.exists?(resource_id) diff --git a/app/models/device.rb b/app/models/device.rb index b20ff828f..70a6c7e21 100644 --- a/app/models/device.rb +++ b/app/models/device.rb @@ -64,7 +64,6 @@ class Device < ApplicationRecord def self.current=(dev) RequestStore.store[:device] = dev end - # Sets Device.current to `self` and returns it to the previous value when # finished running block. Usually this is unnecessary, but may be required in # background jobs. If you are not receiving auto_sync data on your client, diff --git a/app/models/in_use_point.rb b/app/models/in_use_point.rb index 3c2b3720f..820684516 100644 --- a/app/models/in_use_point.rb +++ b/app/models/in_use_point.rb @@ -4,10 +4,10 @@ class InUsePoint < ApplicationRecord belongs_to :device DEFAULT_NAME = "point" - FANCY_NAMES = { + FANCY_NAMES = { GenericPointer.name => DEFAULT_NAME, - ToolSlot.name => "tool slot", - Plant.name => "plant", + ToolSlot.name => "tool slot", + Plant.name => "plant", } def readonly? @@ -15,6 +15,6 @@ class InUsePoint < ApplicationRecord end def fancy_name - "#{FANCY_NAMES[pointer_type] || DEFAULT_NAME} at (#{x}, #{y}, #{z})" + "#{InUsePoint::FANCY_NAMES[pointer_type] || DEFAULT_NAME} at (#{x}, #{y}, #{z})" end end diff --git a/app/models/point.rb b/app/models/point.rb index 621fa4610..dc4daaf7a 100644 --- a/app/models/point.rb +++ b/app/models/point.rb @@ -4,7 +4,7 @@ class Point < ApplicationRecord # axis value > 21k right now - RC # Using real constants instead of strings results # in circular dep. errors. - POINTER_KINDS = ["GenericPointer", "Plant", "ToolSlot"] + POINTER_KINDS = ["GenericPointer", "Plant", "ToolSlot"] self.inheritance_column = "pointer_type" belongs_to :device @@ -19,4 +19,10 @@ class Point < ApplicationRecord def name_used_when_syncing "Point" end + + def fancy_name + n = InUsePoint::FANCY_NAMES[pointer_type] || + InUsePoint::DEFAULT_NAME + "#{n} at (#{x}, #{y}, #{z})" + end end diff --git a/app/models/resource_update_step.rb b/app/models/resource_update_step.rb new file mode 100644 index 000000000..a74fcaba0 --- /dev/null +++ b/app/models/resource_update_step.rb @@ -0,0 +1,21 @@ +# If you create a "Mark As.." (resource_update) step +# and accidentally delete the resource that it modifies, +# referential integrity issues will emerge. +# +# The Model below is a SQL VIEW. +# It is NOT A TABLE. +# +# It simplifies the process of finding Points that +# are in use by the `resource_update` step. +class ResourceUpdateStep < ApplicationRecord + belongs_to :point + + def readonly? + true + end + + # Make sure you preload `self.point` before calling this. + def fancy_name + @fancy_name ||= point.fancy_name + end +end diff --git a/app/mutations/points/destroy.rb b/app/mutations/points/destroy.rb index 87c624940..b6d5d1321 100644 --- a/app/mutations/points/destroy.rb +++ b/app/mutations/points/destroy.rb @@ -1,9 +1,9 @@ module Points class Destroy < Mutations::Command - STILL_IN_USE = "Could not delete the following item(s): %s. Item(s) are "\ - "in use by the following sequence(s): %s." - JUST_ONE = "Could not delete %s. Item is in use by the following "\ - "sequence(s): %s." + STILL_IN_USE = "Could not delete the following item(s): %s. Item(s) are " \ + "in use by the following sequence(s): %s." + JUST_ONE = "Could not delete %s. Item is in use by the following " \ + "sequence(s): %s." required do model :device, class: Device @@ -11,8 +11,8 @@ module Points optional do boolean :hard_delete, default: false - array :point_ids, class: Integer - model :point, class: Point + array :point_ids, class: Integer + model :point, class: Point end P = :point @@ -21,21 +21,21 @@ module Points def validate maybe_wrap_ids # Collect names of sequences that still use this point. - problems = (tool_seq + point_seq) + problems = (tool_seq + point_seq + resource_update_seq) .group_by(&:sequence_name) .to_a - .reduce({S => [], P => []}) do |total, (seq_name, data)| - total[S].push(seq_name) - total[P].push(*(data || []).map(&:fancy_name)) - total - end + .reduce({ S => [], P => [] }) do |total, (seq_name, data)| + total[S].push(seq_name) + total[P].push(*(data || []).map(&:fancy_name)) + total + end p = problems[P].sort.uniq.join(", ") if p.present? - sequences = problems[S].sort.uniq.join(", ") - message = (point_ids.count > 1) ? STILL_IN_USE : JUST_ONE - problems = message % [p, sequences] + sequences = problems[S].sort.uniq.join(", ") + message = (point_ids.count > 1) ? STILL_IN_USE : JUST_ONE + problems = message % [p, sequences] add_error :whoops, :in_use, problems end @@ -52,7 +52,7 @@ module Points end end - private + private def archive_points points @@ -87,13 +87,19 @@ module Points .to_a end + def resource_update_seq + @resource_update_seq ||= ResourceUpdateStep + .includes(:point) + .where(point_id: point_ids) + end + def tool_seq @tool_seq ||= InUseTool .where(tool_id: every_tool_id_as_json, device_id: device.id) .to_a end - def maybe_wrap_ids + def maybe_wrap_ids raise "NO" unless (point || point_ids) inputs[:point_ids] = [point.id] if point end diff --git a/db/migrate/20190709194037_create_resource_update_steps.rb b/db/migrate/20190709194037_create_resource_update_steps.rb new file mode 100644 index 000000000..7fcc7e027 --- /dev/null +++ b/db/migrate/20190709194037_create_resource_update_steps.rb @@ -0,0 +1,5 @@ +class CreateResourceUpdateSteps < ActiveRecord::Migration[5.2] + def change + create_view :resource_update_steps + end +end diff --git a/db/structure.sql b/db/structure.sql index 902b11279..5e45d2e3d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1224,6 +1224,38 @@ CREATE SEQUENCE public.regimens_id_seq ALTER SEQUENCE public.regimens_id_seq OWNED BY public.regimens.id; +-- +-- Name: resource_update_steps; Type: VIEW; Schema: public; Owner: - +-- + +CREATE VIEW public.resource_update_steps AS + WITH resource_type AS ( + SELECT edge_nodes.primary_node_id, + edge_nodes.kind, + edge_nodes.value + FROM public.edge_nodes + WHERE (((edge_nodes.kind)::text = 'resource_type'::text) AND ((edge_nodes.value)::text = ANY ((ARRAY['"GenericPointer"'::character varying, '"ToolSlot"'::character varying, '"Plant"'::character varying])::text[]))) + ), resource_id AS ( + SELECT edge_nodes.primary_node_id, + edge_nodes.kind, + edge_nodes.value, + edge_nodes.sequence_id + FROM public.edge_nodes + WHERE ((edge_nodes.kind)::text = 'resource_id'::text) + ), user_sequence AS ( + SELECT sequences.name, + sequences.id + FROM public.sequences + ) + SELECT j1.sequence_id, + j1.primary_node_id, + (j1.value)::bigint AS point_id, + j3.name AS sequence_name + FROM ((resource_id j1 + JOIN resource_type j2 ON ((j1.primary_node_id = j2.primary_node_id))) + JOIN user_sequence j3 ON ((j3.id = j1.sequence_id))); + + -- -- Name: saved_gardens; Type: TABLE; Schema: public; Owner: - -- @@ -2760,6 +2792,14 @@ ALTER TABLE ONLY public.points ADD CONSTRAINT fk_rails_a62cbb8aca FOREIGN KEY (tool_id) REFERENCES public.tools(id); +-- +-- Name: farmware_envs fk_rails_ab55c3a1d1; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.farmware_envs + ADD CONSTRAINT fk_rails_ab55c3a1d1 FOREIGN KEY (device_id) REFERENCES public.devices(id); + + -- -- Name: primary_nodes fk_rails_bca7fee3b9; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -2768,14 +2808,6 @@ ALTER TABLE ONLY public.primary_nodes ADD CONSTRAINT fk_rails_bca7fee3b9 FOREIGN KEY (sequence_id) REFERENCES public.sequences(id); --- --- Name: farmware_envs fk_rails_bdadc396eb; Type: FK CONSTRAINT; Schema: public; Owner: - --- - -ALTER TABLE ONLY public.farmware_envs - ADD CONSTRAINT fk_rails_bdadc396eb FOREIGN KEY (device_id) REFERENCES public.devices(id); - - -- -- Name: alerts fk_rails_c0132c78be; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -2984,6 +3016,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20190613190531'), ('20190613215319'), ('20190621202204'), -('20190701155706'); +('20190701155706'), +('20190709194037'); diff --git a/db/views/resource_update_steps_v01.sql b/db/views/resource_update_steps_v01.sql new file mode 100644 index 000000000..e7f2a84f3 --- /dev/null +++ b/db/views/resource_update_steps_v01.sql @@ -0,0 +1,31 @@ +WITH + resource_type + AS + ( + SELECT primary_node_id, kind, "value" + FROM edge_nodes + WHERE (kind = 'resource_type' AND "value" IN ('"GenericPointer"', '"ToolSlot"', '"Plant"')) + ), + resource_id + AS + ( + SELECT primary_node_id, kind, "value", sequence_id + FROM edge_nodes + WHERE (kind = 'resource_id') + ), + user_sequence + AS + ( + SELECT name, id + FROM sequences + ) +SELECT + j1.sequence_id, + j1.primary_node_id, + j1.value::bigint as point_id, + j3.name AS sequence_name +FROM resource_id AS j1 + INNER JOIN resource_type AS j2 + ON j1.primary_node_id = j2.primary_node_id + INNER JOIN user_sequence as j3 + ON j3.id = j1.sequence_id; diff --git a/spec/lib/celery_script/corpus_spec.rb b/spec/lib/celery_script/corpus_spec.rb index 76b0762c8..38ab8cce4 100644 --- a/spec/lib/celery_script/corpus_spec.rb +++ b/spec/lib/celery_script/corpus_spec.rb @@ -1,4 +1,4 @@ -require 'spec_helper' +require "spec_helper" describe CeleryScript::Corpus do let(:device) { FactoryBot.create(:device) } @@ -13,19 +13,19 @@ describe CeleryScript::Corpus do args: { x: 1, y: 2, - z: 3 - } + z: 3, + }, }, offset: { kind: "coordinate", args: { "x": 0, "y": 0, - "z": 0 - } + "z": 0, + }, }, - speed: 100 - } + speed: 100, + }, }) check1 = CeleryScript::Checker.new(ok1, corpus, device) expect(check1.valid?).to be_truthy @@ -35,18 +35,18 @@ describe CeleryScript::Corpus do args: { location: { kind: "tool", - args: { tool_id: FactoryBot.create(:tool).id } + args: { tool_id: FactoryBot.create(:tool).id }, }, offset: { kind: "coordinate", args: { "x": 0, "y": 0, - "z": 0 - } + "z": 0, + }, }, - speed: 100 - } + speed: 100, + }, }) check2 = CeleryScript::Checker.new(ok2, corpus, device) expect(check2.valid?).to be_truthy @@ -63,10 +63,10 @@ describe CeleryScript::Corpus do args: { "x": 0, "y": 0, - "z": 0 - } + "z": 0, + }, }, - } + }, }) check = CeleryScript::Checker.new(bad, corpus, device) expect(check.valid?).to be_falsey @@ -80,14 +80,14 @@ describe CeleryScript::Corpus do args: { location: { kind: "tool", - args: { tool_id: "PROBLEM!" } # <= Invalid: + args: { tool_id: "PROBLEM!" }, # <= Invalid: }, offset: { kind: "coordinate", - args: { "x": 0, "y": 0, "z": 0 } + args: { "x": 0, "y": 0, "z": 0 }, }, - speed: 100 - } + speed: 100, + }, }) check = CeleryScript::Checker.new(bad, corpus, device) expect(check.valid?).to be_falsey @@ -95,15 +95,15 @@ describe CeleryScript::Corpus do end it "serializes into JSON" do - result = JSON.parse(corpus.to_json) + result = JSON.parse(corpus.to_json) - expect(result["version"]).to eq(Sequence::LATEST_VERSION) - expect(result["args"]).to be_kind_of(Array) - expect(result["nodes"]).to be_kind_of(Array) - keys = result["nodes"].sample.keys.sort.map(&:to_sym) - expect(keys).to eq([:allowed_args, :allowed_body_types, :docs, :name, :tags]) - expect(result["args"].sample.keys.sort).to eq(["allowed_values", - "name"]) + expect(result["version"]).to eq(Sequence::LATEST_VERSION) + expect(result["args"]).to be_kind_of(Array) + expect(result["nodes"]).to be_kind_of(Array) + keys = result["nodes"].sample.keys.sort.map(&:to_sym) + expect(keys).to eq([:allowed_args, :allowed_body_types, :docs, :name, :tags]) + expect(result["args"].sample.keys.sort).to eq(["allowed_values", + "name"]) end it "Handles message_type validations for version 1" do @@ -113,9 +113,9 @@ describe CeleryScript::Corpus do "kind": "send_message", "args": { "message": "Hello, world!", - "message_type": "wrong" + "message_type": "wrong", }, - "body": [] + "body": [], }) checker = CeleryScript::Checker.new(tree, corpus, device) expect(checker.error.message).to include("not a valid message_type") @@ -126,21 +126,21 @@ describe CeleryScript::Corpus do "kind": "send_message", "args": { "message": "Hello, world!", - "message_type": "fun" + "message_type": "fun", }, "body": [ { "kind": "channel", - "args": { "channel_name": "wrong" } - } - ] + "args": { "channel_name": "wrong" }, + }, + ], }) checker = CeleryScript::Checker.new(tree, corpus, device) expect(checker.error.message).to include("not a valid channel_name") end it "validates tool_ids" do - ast = { "kind": "tool", "args": { "tool_id": 0 } }; + ast = { "kind": "tool", "args": { "tool_id": 0 } } checker = CeleryScript::Checker.new(CeleryScript::AstNode.new(ast), corpus, device) @@ -151,47 +151,45 @@ describe CeleryScript::Corpus do it "Validates resource_update nodes" do ast = { "kind": "resource_update", "args": { "resource_type" => "Device", - "resource_id" => 23, # Mutated to "0" later.. - "label" => "mounted_tool_id", - "value" => 1 } } + "resource_id" => 23, # Mutated to "0" later.. + "label" => "mounted_tool_id", + "value" => 1 } } checker = CeleryScript::Checker.new(CeleryScript::AstNode.new(ast), corpus, device) expect(checker.valid?).to be(true) - expect(checker.tree.args[:resource_id].value).to eq(0) + expect(checker.tree.args[:resource_id].value).to eq(device.id) end it "rejects bogus resource_updates" do - fake_id = FakeSequence.create().id + 1 - expect(Sequence.exists?(fake_id)).to be(false) + fake_id = FactoryBot.create(:plant).id + 1 + expect(Plant.exists?(fake_id)).to be(false) ast = { "kind": "resource_update", - "args": { "resource_type" => "Sequence", - "resource_id" => fake_id, - "label" => "foo", - "value" => "Should Fail" } } + "args": { "resource_type" => "Plant", + "resource_id" => fake_id, + "label" => "foo", + "value" => "Should Fail" } } hmm = CeleryScript::AstNode.new(ast) expect(hmm.args.fetch(:resource_id).value).to eq(fake_id) checker = CeleryScript::Checker.new(hmm, corpus, device) expect(checker.valid?).to be(false) - expect(checker.error.message) - .to eq("Can't find Sequence with id of #{fake_id}") + expect(checker.error.message).to eq("Can't find Plant with id of #{fake_id}") end it "rejects bogus resource_types" do ast = { "kind": "resource_update", "args": { "resource_type" => "CanOpener", - "resource_id" => 0, - "label" => "foo", - "value" => "Should Fail" } } + "resource_id" => 0, + "label" => "foo", + "value" => "Should Fail" } } checker = CeleryScript::Checker.new(CeleryScript::AstNode.new(ast), corpus, device) expect(checker.valid?).to be(false) - expect(checker.error.message) - .to include('"CanOpener" is not a valid resource_type.') + expect(checker.error.message).to include('"CanOpener" is not a valid resource_type.') end it "has enums" do args = [name = :foo, list = ["bar", "baz"], tpl = ["foo: %s bar: %s"]] - c = CeleryScript::Corpus.new.enum(*args) + c = CeleryScript::Corpus.new.enum(*args) json = c.as_json enums = json.fetch(:enums) expect(enums.length).to eq(1) @@ -200,9 +198,9 @@ describe CeleryScript::Corpus do end it "has values" do - args = [name = :whatever, list = [Symbol, Hash]] - c = CeleryScript::Corpus.new.value(*args) - json = c.as_json + args = [name = :whatever, list = [Symbol, Hash]] + c = CeleryScript::Corpus.new.value(*args) + json = c.as_json values = json.fetch(:values) expect(values.length).to eq(1) expect(values.first.fetch("name")).to eq(name) @@ -215,10 +213,10 @@ describe CeleryScript::Corpus do body: [], tags: ["great"], docs: "spectacular") - json = c.as_json + json = c.as_json values = json.fetch(:nodes) expect(values.length).to eq(1) - value = values.first + value = values.first expect(value.fetch("tags").first).to eq("great") expect(value.fetch("docs")).to eq("spectacular") end diff --git a/spec/models/resource_update_step_spec.rb b/spec/models/resource_update_step_spec.rb new file mode 100644 index 000000000..7f318d772 --- /dev/null +++ b/spec/models/resource_update_step_spec.rb @@ -0,0 +1,7 @@ +require "spec_helper" + +describe ResourceUpdateStep do + it "is readonly" do + expect(ResourceUpdateStep.new.readonly?).to be(true) + end +end diff --git a/spec/mutations/points/destroy_spec.rb b/spec/mutations/points/destroy_spec.rb index 04f18574a..4cda1a54b 100644 --- a/spec/mutations/points/destroy_spec.rb +++ b/spec/mutations/points/destroy_spec.rb @@ -1,4 +1,4 @@ -require 'spec_helper' +require "spec_helper" require_relative "scenario" describe Points::Destroy do @@ -9,7 +9,7 @@ describe Points::Destroy do points = FactoryBot.create_list(:generic_pointer, 3, device: device) # use one point in a sequence. params = { - name: "Test Case I", + name: "Test Case I", device: device, body: [ { @@ -20,44 +20,44 @@ describe Points::Destroy do kind: "point", args: { pointer_type: "GenericPointer", - pointer_id: points.first.id - } + pointer_id: points.first.id, + }, }, - offset: { kind: "coordinate", args:{ x: 0, y: 0, z: 0 } } + offset: { kind: "coordinate", args: { x: 0, y: 0, z: 0 } }, }, - } - ] + }, + ], } sequence = Sequences::Create.run!(params) - before = Point.count + before = Point.count # Attempt to delete - result = Points::Destroy.run(point_ids: points.pluck(:id), device: device) + result = Points::Destroy.run(point_ids: points.pluck(:id), device: device) # Expect error about point in use still. expect(result.success?).to be false expect(Point.count).to eq(before) expect(result.errors.message_list.count).to eq(1) expect(result.errors.message_list.first).to include(params[:name]) - coords = [:x, :y, :z].map{|c|points.first[c]}.join(", ") - expected = "Could not delete the following item(s): point at (#{coords})."\ - " Item(s) are in use by the following sequence(s): Test Case I." + coords = [:x, :y, :z].map { |c| points.first[c] }.join(", ") + expected = "Could not delete the following item(s): point at (#{coords})." \ + " Item(s) are in use by the following sequence(s): Test Case I." expect(result.errors.message_list.first).to include(expected) end it "prevents deletion of active tool slots" do - s = Points::Scenario.new + s = Points::Scenario.new point_ids = [s.tool_slot.id] result = Points::Destroy.run(point_ids: point_ids, device: s.device) expect(result.success?).to be(false) - expected = "Could not delete Scenario Tool. Item is in use by the "\ - "following sequence(s): Scenario Sequence." + expected = "Could not delete Scenario Tool. Item is in use by the " \ + "following sequence(s): Scenario Sequence." expect(result.errors.message_list).to include(expected) end it "handles multiple sequence dep tracking issues at deletion time" do - point = FactoryBot.create(:generic_pointer, device: device, x: 4, y: 5, z: 6) - plant = FactoryBot.create(:plant, device: device, x: 0, y: 1, z: 0) + point = FactoryBot.create(:generic_pointer, device: device, x: 4, y: 5, z: 6) + plant = FactoryBot.create(:plant, device: device, x: 0, y: 1, z: 0) empty_point = { kind: "coordinate", args: { x: 0, y: 0, z: 0 } } - sequence_a = Sequences::Create.run!(device: device, + sequence_a = Sequences::Create.run!(device: device, name: "Sequence A", body: [ { @@ -66,13 +66,13 @@ describe Points::Destroy do location: { kind: "point", args: { - pointer_id: plant.id, - pointer_type: "Plant" - } + pointer_id: plant.id, + pointer_type: "Plant", + }, }, speed: 100, - offset: empty_point - } + offset: empty_point, + }, }, { kind: "move_absolute", @@ -80,17 +80,17 @@ describe Points::Destroy do location: { kind: "point", args: { - pointer_id: plant.id, - pointer_type: "GenericPointer" - } + pointer_id: plant.id, + pointer_type: "GenericPointer", + }, }, speed: 100, - offset: empty_point - } + offset: empty_point, + }, }, ]) - sequence_a = Sequences::Create.run!(device: device, + sequence_a = Sequences::Create.run!(device: device, name: "Sequence B", body: [ { @@ -99,13 +99,13 @@ describe Points::Destroy do location: { kind: "point", args: { - pointer_id: plant.id, - pointer_type: "Plant" - } + pointer_id: plant.id, + pointer_type: "Plant", + }, }, speed: 100, - offset: empty_point - } + offset: empty_point, + }, }, { kind: "move_absolute", @@ -113,13 +113,13 @@ describe Points::Destroy do location: { kind: "point", args: { - pointer_id: plant.id, - pointer_type: "GenericPointer" - } + pointer_id: plant.id, + pointer_type: "GenericPointer", + }, }, speed: 100, - offset: empty_point - } + offset: empty_point, + }, }, ]) @@ -127,16 +127,47 @@ describe Points::Destroy do .run(point_ids: [point.id, plant.id], device: device) .errors .message - expected = "Could not delete the following item(s): plant at (0.0, 1.0,"\ - " 0.0). Item(s) are in use by the following sequence(s): "\ - "Sequence A, Sequence B." + expected = "Could not delete the following item(s): plant at (0.0, 1.0," \ + " 0.0). Item(s) are in use by the following sequence(s): " \ + "Sequence A, Sequence B." expect(result[:whoops]).to eq(expected) end it "performs a hard (real) delete" do points = FactoryBot.create_list(:generic_pointer, 3, device: device) - ids = points.pluck(:id) + ids = points.pluck(:id) Points::Destroy.run!(point_ids: ids, device: device, hard_delete: true) expect(Point.where(id: ids).length).to eq(0) end + + def mark_as(resource) + { + kind: "resource_update", + args: { + resource_type: resource.class.to_s, + resource_id: resource.id, + label: "foo", + value: "bar", + }, + } + end + + it "doesnt delete plants used by `resource_update`" do + points = [ + FactoryBot.create(:generic_pointer, device: device), + FactoryBot.create(:plant, device: device), + FactoryBot.create(:tool_slot, device: device), + ] + body = points.map { |x| mark_as(x) } + sequence = Sequences::Create.run!(device: device, name: "X", body: body) + real_stuff = body.map do |x| + [x.dig(:args, :resource_id), x.dig(:args, :resource_type)] + end.to_h + result = Points::Destroy.run(device: device, point_ids: points.map(&:id)) + expect(result.errors).to be + message = result.errors.message.fetch("whoops") + points.map do |p| + expect(message).to include(p.fancy_name) + end + end end