From 5a6cf2b56ab6fe8764b1e39ffae8ae188fc57319 Mon Sep 17 00:00:00 2001 From: Tim Cowlishaw Date: Mon, 6 May 2024 07:59:33 +0200 Subject: [PATCH] forward rendered device represenatation (after save) rather than raw message --- app/controllers/v0/readings_controller.rb | 2 +- app/jobs/send_to_datastore_job.rb | 2 +- app/lib/mqtt_messages_handler.rb | 2 +- app/models/concerns/message_forwarding.rb | 18 +++++++++++++++++- app/models/raw_storer.rb | 7 ++++--- app/models/storer.rb | 7 ++++--- spec/models/raw_storer_spec.rb | 22 ++++++++++++++++++---- spec/models/storer_spec.rb | 19 +++++++++++++++---- 8 files changed, 61 insertions(+), 18 deletions(-) diff --git a/app/controllers/v0/readings_controller.rb b/app/controllers/v0/readings_controller.rb index 1d136b94..99d3e0b5 100644 --- a/app/controllers/v0/readings_controller.rb +++ b/app/controllers/v0/readings_controller.rb @@ -37,7 +37,7 @@ def legacy_create if request.headers['X-SmartCitizenData'] MQTTClientFactory.create_client({clean_session: true, client_id: nil}) do |mqtt_client| - storer = RawStorer.new(mqtt_client) + storer = RawStorer.new(mqtt_client, self) JSON.parse(request.headers['X-SmartCitizenData']).each do |raw_reading| mac = request.headers['X-SmartCitizenMacADDR'] version = request.headers['X-SmartCitizenVersion'] diff --git a/app/jobs/send_to_datastore_job.rb b/app/jobs/send_to_datastore_job.rb index 184289ec..bc3c5e6f 100644 --- a/app/jobs/send_to_datastore_job.rb +++ b/app/jobs/send_to_datastore_job.rb @@ -16,7 +16,7 @@ def perform(data_param, device_id) end def storer - @storer ||= Storer.new(mqtt_client) + @storer ||= Storer.new(mqtt_client, ActionController::Base.new.view_context) end def mqtt_client diff --git a/app/lib/mqtt_messages_handler.rb b/app/lib/mqtt_messages_handler.rb index e364397d..e9040640 100644 --- a/app/lib/mqtt_messages_handler.rb +++ b/app/lib/mqtt_messages_handler.rb @@ -141,6 +141,6 @@ def data(message) def storer - @storer ||= Storer.new(mqtt_client) + @storer ||= Storer.new(mqtt_client, ActionController::Base.new.view_context) end end diff --git a/app/models/concerns/message_forwarding.rb b/app/models/concerns/message_forwarding.rb index 420f3932..1b970e66 100644 --- a/app/models/concerns/message_forwarding.rb +++ b/app/models/concerns/message_forwarding.rb @@ -4,7 +4,19 @@ module MessageForwarding def forward_reading(device, reading) forwarder = MQTTForwarder.new(mqtt_client) - forwarder.forward_reading(device.forwarding_token, device.id, reading) if device.forward_readings? + payload = payload_for(device, reading) + forwarder.forward_reading(device.forwarding_token, device.id, payload) if device.forward_readings? + end + + def payload_for(device, reading) + renderer.render( + partial: "v0/devices/device", + locals: { + device: device.reload, + current_user: nil, + slim_owner: true + } + ) end private @@ -12,4 +24,8 @@ def forward_reading(device, reading) def mqtt_client raise NotImplementedError end + + def renderer + raise NotImplementedError + end end diff --git a/app/models/raw_storer.rb b/app/models/raw_storer.rb index 43116504..87d34879 100644 --- a/app/models/raw_storer.rb +++ b/app/models/raw_storer.rb @@ -5,8 +5,9 @@ class RawStorer include MessageForwarding - def initialize(mqtt_client) + def initialize(mqtt_client, renderer) @mqtt_client = mqtt_client + @renderer = renderer end def store data, mac, version, ip, raise_errors=false @@ -75,7 +76,7 @@ def store data, mac, version, ip, raise_errors=false if !Rails.env.test? and device begin - Redis.current.publish("data-received", ActionController::Base.new.view_context.render( partial: "v0/devices/device", locals: {device: @device, current_user: nil})) + Redis.current.publish("data-received", renderer.render( partial: "v0/devices/device", locals: {device: @device, current_user: nil})) rescue end end @@ -83,6 +84,6 @@ def store data, mac, version, ip, raise_errors=false private - attr_reader :mqtt_client + attr_reader :mqtt_client, :renderer end diff --git a/app/models/storer.rb b/app/models/storer.rb index c62a4359..2cf3d71b 100644 --- a/app/models/storer.rb +++ b/app/models/storer.rb @@ -2,8 +2,9 @@ class Storer include DataParser::Storer include MessageForwarding - def initialize(mqtt_client) + def initialize(mqtt_client, renderer) @mqtt_client = mqtt_client + @renderer = renderer end def store device, reading, do_update = true @@ -51,13 +52,13 @@ def kairos_publish(reading_data) def ws_publish(device) return if Rails.env.test? or device.blank? begin - Redis.current.publish("data-received", ActionController::Base.new.view_context.render( partial: "v0/devices/device", locals: {device: device, current_user: nil})) + Redis.current.publish("data-received", renderer.render( partial: "v0/devices/device", locals: {device: device, current_user: nil})) rescue end end private - attr_reader :mqtt_client + attr_reader :mqtt_client, :renderer end diff --git a/spec/models/raw_storer_spec.rb b/spec/models/raw_storer_spec.rb index a11cbcf5..d46c89ad 100644 --- a/spec/models/raw_storer_spec.rb +++ b/spec/models/raw_storer_spec.rb @@ -33,8 +33,14 @@ def to_ts(time) end } + let(:renderer) { + # TODO: refactor these tests so they don't depend on the actual rendering, + # then replace this with a mock, to reduce brittleness. + ActionController::Base.new.view_context + } + subject(:storer) { - RawStorer.new(mqtt_client) + RawStorer.new(mqtt_client, renderer) } let(:json) { @@ -96,15 +102,23 @@ def to_ts(time) end context "when the device allows forwarding" do - # TODO Tim Refactor this now you're passing in the MQTT client + let(:device_json) { + double(:device_json) + } + + let(:renderer) { + double(:renderer).tap do |renderer| + allow(renderer).to receive(:render).and_return(device_json) + end + } + it "forwards the message with the forwarding token and the device's id" do forwarding_token = double(:forwarding_token) allow_any_instance_of(Device).to receive(:forwarding_token).and_return(forwarding_token) allow_any_instance_of(Device).to receive(:forward_readings?).and_return(true) - forwarder = double(:mqtt_forwarder) allow(MQTTForwarder).to receive(:new).and_return(forwarder) - expect(forwarder).to receive(:forward_reading).with(forwarding_token, device.id, json) + expect(forwarder).to receive(:forward_reading).with(forwarding_token, device.id, device_json) storer.store(json, device.mac_address, "1.1-0.9.0-A", "127.0.0.1", true) end end diff --git a/spec/models/storer_spec.rb b/spec/models/storer_spec.rb index b0acd793..de708ddd 100644 --- a/spec/models/storer_spec.rb +++ b/spec/models/storer_spec.rb @@ -12,8 +12,14 @@ end } + let(:renderer) { + double(:renderer).tap do |renderer| + allow(renderer).to receive(:render) + end + } + subject(:storer) { - Storer.new(mqtt_client) + Storer.new(mqtt_client, renderer) } context 'when receiving good data' do @@ -80,14 +86,19 @@ end context "when the device allows forwarding" do - # TODO tim refactor this now you're injecting the MQTT client + + let(:device_json) { + double(:device_json) + } + it "forwards the message with the forwarding token and the device's id" do forwarding_token = double(:forwarding_token) + forwarder = double(:mqtt_forwarder) allow(device).to receive(:forwarding_token).and_return(forwarding_token) allow(device).to receive(:forward_readings?).and_return(true) - forwarder = double(:mqtt_forwarder) + allow(renderer).to receive(:render).and_return(device_json) allow(MQTTForwarder).to receive(:new).and_return(forwarder) - expect(forwarder).to receive(:forward_reading).with(forwarding_token, device.id, @data) + expect(forwarder).to receive(:forward_reading).with(forwarding_token, device.id, device_json) storer.store(device, @data) end end