Fixed firmware and screen attachments

This reverts commit 5e9ab0cf02 (Removed screen and firmware struct attachment replace behavior, 2026-08-03) because you can't reliably have link:https://shrinerb.com[Shrine] upload a replacable image. We still need to have distinct methods for `#replace` and `#upload`.

This also prevents duplicate images being created.

Milestone: patch
This commit is contained in:
Brooke Kuhlmann
2026-08-05 13:29:07 -06:00
parent 482cf004fb
commit c273ec2112
9 changed files with 123 additions and 41 deletions
+1 -1
View File
@@ -54,7 +54,7 @@ module Terminus
# :reek:FeatureEnvy
def replace record, content, response
Pathname.mktmpdir do |root|
root.join("#{record.version}.bin").write(content).open { record.upload it }
root.join("#{record.version}.bin").write(content).open { record.replace it }
end
update = repository.update record.id, attachment_data: record.attachment_attributes
+1 -1
View File
@@ -47,7 +47,7 @@ module Terminus
def attach record, attachment
return unless attachment
record.upload attachment[:tempfile], metadata: {"filename" => "#{record.version}.bin"}
record.replace attachment[:tempfile], metadata: {"filename" => "#{record.version}.bin"}
repository.update record.id, attachment_data: record.attachment_attributes
end
+1 -1
View File
@@ -42,7 +42,7 @@ module Terminus
tempfile = image[:tempfile]
extension = File.extname tempfile
record.upload tempfile, metadata: {"filename" => "#{record.name}#{extension}"}
record.replace tempfile, metadata: {"filename" => "#{record.name}#{extension}"}
repository.update record.id, image_data: record.image_attributes
end
+1 -1
View File
@@ -51,7 +51,7 @@ module Terminus
def with_associations = screen.combine :model
def update_with_image path, mold, record
path.open { |io| record.upload io, metadata: {"filename" => mold.file_name} }
path.open { |io| record.replace io, metadata: {"filename" => mold.file_name} }
update record.id, image_data: record.image_attributes, **mold.image_attributes
end
end
+7 -3
View File
@@ -25,7 +25,7 @@ module Terminus
def attachment_destroy
store.delete attachment_id if attachment_id
attributes[:attachment_data].clear if attributes.key? :attachment_data
attributes[:attachment_data].clear
end
def attachment_id = attachment_attributes[:id]
@@ -49,12 +49,16 @@ module Terminus
attacher.assign(io, **).tap { |file| attributes[:attachment_data] = file.data }
end
def upload(io, **)
def replace(io, **)
attachment_destroy
attacher.upload(io, **).tap { |file| attributes[:attachment_data] = file.data }
upload(io, **)
self
end
def upload(io, **)
attacher.upload(io, **).tap { |file| attributes[:attachment_data] = file.data }
end
def errors = attacher.errors
def valid? = errors.empty?
+7 -2
View File
@@ -30,7 +30,7 @@ module Terminus
def image_destroy
store.delete image_id if image_id
attributes[:image_data].clear if attributes.key? :image_data
attributes[:image_data].clear
end
def image_id = image_attributes[:id]
@@ -61,8 +61,13 @@ module Terminus
def mime_type = image_attributes.dig :metadata, :mime_type
def upload(io, **)
def replace(io, **)
image_destroy
upload(io, **)
self
end
def upload(io, **)
attacher.upload(io, **).tap { |file| attributes[:image_data] = file.data }
self
end
+2 -2
View File
@@ -28,7 +28,7 @@ RSpec.describe Terminus::Repositories::Firmware, :db do
it "deletes associated attachment" do
upload = firmware.upload StringIO.new([123].pack("N"))
repository.update firmware.id, attachment_data: upload.attachment_attributes
repository.update firmware.id, attachment_data: upload.data
repository.delete firmware.id
expect(Hanami.app[:shrine].storages[:store].store).to eq({})
@@ -50,7 +50,7 @@ RSpec.describe Terminus::Repositories::Firmware, :db do
it "deletes all attachments" do
upload = firmware.upload StringIO.new([123].pack("N"))
repository.update firmware.id, attachment_data: upload.attachment_attributes
repository.update firmware.id, attachment_data: upload.data
repository.delete_all
expect(Hanami.app[:shrine].storages[:store].store).to eq({})
+41 -23
View File
@@ -123,25 +123,50 @@ RSpec.describe Terminus::Structs::Firmware, :db do
end
end
describe "#upload" do
it "uploads file when valid" do
path = temp_dir.join "test.bin"
path.binwrite [123].pack("N")
instance = path.open { |io| firmware.upload io }
describe "#replace" do
it "replaces file" do
instance = path.open { |io| firmware.replace io }
expect(instance.attachment_attributes).to match(
id: /\h{32}\.bin/,
metadata: {
filename: "test.bin",
height: nil,
size: 4,
mime_type: "application/octet-stream",
width: nil
width: nil,
height: nil
},
storage: "store"
)
end
it "updates storage ID" do
id = firmware.attachment_id
instance = path.open { |io| firmware.replace io }
expect(id).not_to eq(instance.attachment_id)
end
end
describe "#upload" do
it "uploads file when valid" do
path = temp_dir.join "test.bin"
path.binwrite [123].pack("N")
upload = firmware.upload path.open
expect(upload.data).to match(
"id" => /\h{32}\.bin/,
"metadata" => {
"filename" => "test.bin",
"height" => nil,
"size" => 4,
"mime_type" => "application/octet-stream",
"width" => nil
},
"storage" => "store"
)
end
it "updates attributes when valid" do
path = temp_dir.join "test.bin"
path.binwrite [123].pack("N")
@@ -160,26 +185,19 @@ RSpec.describe Terminus::Structs::Firmware, :db do
)
end
it "updates storage ID" do
id = firmware.attachment_id
instance = path.open { |io| firmware.upload io }
expect(id).not_to eq(instance.attachment_id)
end
it "doesn't upload file when invalid" do
upload = firmware.upload StringIO.new
expect(upload.attachment_attributes).to match(
id: /\h{32}/,
metadata: {
filename: nil,
height: nil,
size: 0,
mime_type: nil,
width: nil
expect(upload.data).to match(
"id" => /\h{32}/,
"metadata" => {
"filename" => nil,
"height" => nil,
"size" => 0,
"mime_type" => nil,
"width" => nil
},
storage: "store"
"storage" => "store"
)
end
end
+62 -7
View File
@@ -147,6 +147,68 @@ RSpec.describe Terminus::Structs::Screen, :db do
end
end
describe "#replace" do
it "replaces file when valid" do
instance = path.open { |io| screen.replace io }
expect(instance.image_attributes).to match(
id: /\h{32}\.png/,
metadata: {
bit_depth: 1,
checksum: match_md5_checksum,
filename: "test.png",
height: 1,
size: 81,
mime_type: "image/png",
width: 1
},
storage: "store"
)
end
it "updates attributes when valid" do
instance = path.open { |io| screen.replace io }
expect(instance.image_attributes).to match(
id: /\h{32}\.png/,
storage: "store",
metadata: {
bit_depth: 1,
checksum: match_md5_checksum,
filename: "test.png",
size: 81,
mime_type: "image/png",
width: 1,
height: 1
}
)
end
it "updates storage ID" do
id = screen.image_id
instance = path.open { |io| screen.replace io }
expect(id).not_to eq(instance.image_id)
end
it "doesn't replace file when invalid" do
instance = screen.replace StringIO.new
expect(instance.image_attributes).to match(
id: /\h{32}/,
metadata: {
bit_depth: nil,
checksum: match_md5_checksum,
filename: nil,
height: nil,
size: 0,
mime_type: nil,
width: nil
},
storage: "store"
)
end
end
describe "#upload" do
it "uploads file when valid" do
instance = path.open { |io| screen.upload io }
@@ -184,13 +246,6 @@ RSpec.describe Terminus::Structs::Screen, :db do
)
end
it "updates storage ID" do
id = screen.image_id
instance = path.open { |io| screen.upload io }
expect(id).not_to eq(instance.image_id)
end
it "doesn't upload file when invalid" do
upload = screen.upload StringIO.new