diff --git a/README.md b/README.md index 38b7a3d..3d30fa8 100644 --- a/README.md +++ b/README.md @@ -399,6 +399,20 @@ Hosts that must support older engine versions can feature-detect with | `GET /api/download?path=` | Serves an artifact and logs a download record | | `GET /file/*path` | Serves static build output (web-playable games, docs) | +### `WarpEngine-Version` + +Every response above carries the engine's version in a `WarpEngine-Version` header, so a +client can branch on the engine's age without a round trip to ask: + +``` +$ curl -sI https://teletypegames.org/api/software | grep -i warpengine +WarpEngine-Version: 0.4.0 +``` + +Set before the action runs rather than after, which means an error response carries it +too — a client needs the version most when something came back wrong. The name is +`WarpEngine::VERSION_HEADER`, so nothing spells it out twice. + ## Admin integration The host owns the single ActiveAdmin instance — authentication (Devise), @@ -419,6 +433,15 @@ host: former Go backend (Go zero-time timestamps, camelCase keys, legacy flat path fields). - Model extension points: `ActiveSupport.on_load(:warp_engine_)` hooks. +- **A software has one pipeline, and the newest assignment wins.** `Software#pipeline` is + a `has_one`, so two pipelines pointing at the same software is not an error the database + catches — it is a link that silently does nothing, with the software still showing + whichever row came first. Assigning a software that another pipeline holds therefore + *moves* it: the previous holder is left without one, the admin says which one it took it + from, and `Pipeline#software_taken_from` carries that list for anything else that cares. + Deliberately a callback rather than a unique index: rows here are soft-deleted, and a + unique index counts deleted rows, so a pipeline removed last year would block its + software from ever being linked again. ## Tests diff --git a/app/admin/pipelines.rb b/app/admin/pipelines.rb index 7ee947d..df101f7 100644 --- a/app/admin/pipelines.rb +++ b/app/admin/pipelines.rb @@ -46,11 +46,26 @@ ActiveAdmin.register WarpEngine::Pipeline, as: "Pipeline" do f.input :platform, as: :select, collection: WarpEngine::PlatformLink::SUPPORTED_PLATFORMS f.input :software_id, as: :select, collection: WarpEngine::Software.order(:title).map { |s| [ s.title, s.id ] }, - include_blank: "- none -" + include_blank: "- none -", + hint: "One pipeline per software. Picking one that another pipeline already " \ + "has moves the link here rather than refusing it." end f.actions end + controller do + # The reassignment itself is the model's job; this only makes it visible. Without a + # word about it, the other pipeline loses its software with nothing on screen to say + # that it happened. + def update + super + taken = resource.software_taken_from + return if taken.blank? + + flash[:notice] = [ flash[:notice], "Software taken from #{taken.join(', ')}." ].compact.join(" ") + end + end + sidebar "Details", only: :show do attributes_table_for resource do row :id diff --git a/app/controllers/warp_engine/api_controller.rb b/app/controllers/warp_engine/api_controller.rb index 1a9dce6..97577fb 100644 --- a/app/controllers/warp_engine/api_controller.rb +++ b/app/controllers/warp_engine/api_controller.rb @@ -5,6 +5,12 @@ module WarpEngine formats [ "json" ] end + # Every response the engine serves names the version that served it, so a client can + # branch on the engine's age without a round trip to ask. Set *before* the action, + # not after: an error handled by `rescue_from` never reaches an after_action, and a + # client needs the version most when something came back wrong. + before_action :set_version_header + rescue_from StandardError do |e| Rails.logger.error("[#{self.class.name}] #{e.class}: #{e.message}") render json: { error: "Internal server error" }, status: :internal_server_error @@ -24,6 +30,10 @@ module WarpEngine private + def set_version_header + response.headers[WarpEngine::VERSION_HEADER] = WarpEngine::VERSION + end + def resolve_mime(path) ext = File.extname(path.to_s).delete_prefix(".") Mime::Type.lookup_by_extension(ext) || "application/octet-stream" diff --git a/app/models/warp_engine/pipeline.rb b/app/models/warp_engine/pipeline.rb index 5d29322..8f5013a 100644 --- a/app/models/warp_engine/pipeline.rb +++ b/app/models/warp_engine/pipeline.rb @@ -8,8 +8,25 @@ module WarpEngine belongs_to :software, class_name: "WarpEngine::Software", optional: true + # Pipelines this record took the software from during the last save, by full name. + # The admin says so out loud: a silent reassignment is what made the old behaviour + # confusing in the first place. + attr_reader :software_taken_from + default_scope { where(deleted_at: nil) } + # One pipeline per software, and the newest assignment wins. + # + # `Software#pipeline` is a `has_one`, so two pipelines pointing at the same software + # is not an error — it is worse than one: the software keeps showing whichever row + # comes first, and assigning it elsewhere looks like it did nothing. Rather than + # refusing the assignment, the link moves: whoever held that software lets go of it. + # + # Deliberately a callback and not a unique index. Rows here are soft-deleted, and a + # unique index counts deleted rows too, so a pipeline someone removed last year would + # block the software from ever being linked again. + before_save :claim_software_from_other_pipelines, if: :will_save_change_to_software_id? + validates :woodpecker_repo_id, presence: true, uniqueness: true validates :repo_owner, presence: true validates :repo_name, presence: true @@ -31,6 +48,16 @@ module WarpEngine %w[software] end + private + + def claim_software_from_other_pipelines + return if software_id.blank? + + others = Pipeline.where(software_id: software_id).where.not(id: id) + @software_taken_from = others.map(&:full_name) + others.update_all(software_id: nil, updated_at: Time.current) + end + ActiveSupport.run_load_hooks(:warp_engine_pipeline, self) end end diff --git a/lib/warp_engine/version.rb b/lib/warp_engine/version.rb index b66d2a7..02a8188 100644 --- a/lib/warp_engine/version.rb +++ b/lib/warp_engine/version.rb @@ -1,3 +1,8 @@ module WarpEngine - VERSION = "0.3.0" + VERSION = "0.4.0" + + # The header every API response carries. Named here rather than written out at the one + # place that sets it: clients read it, the README documents it, and a string in three + # places is a string that eventually differs in one of them. + VERSION_HEADER = "WarpEngine-Version".freeze end diff --git a/spec/models/pipeline_spec.rb b/spec/models/pipeline_spec.rb index 7fa8642..0900999 100644 --- a/spec/models/pipeline_spec.rb +++ b/spec/models/pipeline_spec.rb @@ -59,4 +59,42 @@ RSpec.describe WarpEngine::Pipeline do describe "associations" do it { is_expected.to belong_to(:software).optional } end + + # A software has one pipeline. Two pipelines pointing at the same one is not an error + # the database catches, it is a link that silently does nothing — so the newest + # assignment takes it, and says what it took it from. + describe "assigning a software another pipeline already has" do + it "moves the link and leaves the other pipeline without one" do + software = create(:software) + held_by = create(:pipeline, software: software, repo_owner: "games", repo_name: "old") + taking = create(:pipeline, repo_owner: "games", repo_name: "new") + + taking.update!(software: software) + + expect(taking.reload.software).to eq(software) + expect(held_by.reload.software).to be_nil + expect(software.reload.pipeline).to eq(taking) + end + + it "names the pipelines it took the software from" do + software = create(:software) + create(:pipeline, software: software, repo_owner: "games", repo_name: "old") + taking = create(:pipeline, repo_owner: "games", repo_name: "new") + + taking.update!(software: software) + + expect(taking.software_taken_from).to eq([ "games/old" ]) + end + + it "leaves other pipelines alone when the software is cleared" do + software = create(:software) + keeps = create(:pipeline, software: software, repo_owner: "games", repo_name: "keeps") + other = create(:pipeline, repo_owner: "games", repo_name: "other") + + other.update!(software: nil) + + expect(keeps.reload.software).to eq(software) + expect(other.software_taken_from).to be_nil + end + end end diff --git a/spec/requests/version_header_spec.rb b/spec/requests/version_header_spec.rb new file mode 100644 index 0000000..a2818d5 --- /dev/null +++ b/spec/requests/version_header_spec.rb @@ -0,0 +1,30 @@ +require "rails_helper" + +# Every response the engine serves carries the version that served it, so a client can +# branch on the engine's age without asking a separate endpoint for it. +RSpec.describe "the WarpEngine-Version header", type: :request do + it "is on a normal response" do + create(:software) + + get "/api/software" + + expect(response).to have_http_status(:ok) + expect(response.headers["WarpEngine-Version"]).to eq(WarpEngine::VERSION) + end + + # The one a client needs most: something came back wrong, and it wants to know whether + # the engine on the other end is old enough to explain it. `rescue_from` never reaches + # an after_action, which is why the header is set before the action runs. + it "is on an error response" do + get "/api/image/999999" + + expect(response).to have_http_status(:not_found) + expect(response.headers["WarpEngine-Version"]).to eq(WarpEngine::VERSION) + end + + it "is on a served file" do + get "/file/nothing-here.zip" + + expect(response.headers["WarpEngine-Version"]).to eq(WarpEngine::VERSION) + end +end