From e0649469e6d72f949224599fb71107b0e3322ee0 Mon Sep 17 00:00:00 2001 From: Sang Date: Sat, 5 Sep 2026 04:23:51 +0700 Subject: [PATCH] Fix :datetime raising NameError without Rails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cast_datetime names ActiveSupport::TimeWithZone unguarded, and nothing in the gem loads it — activesupport does not load it by default: $ ruby -e 'require "permittable"; ...' lib/permittable.rb:336:in `cast_datetime': uninitialized constant ActiveSupport::TimeWithZone (NameError) A Rails app gets the constant from active_support/time at boot, which is exactly why the spec suite never saw this: spec_helper requires active_record, which pulls in every core extension and masks a require the gem itself forgot. A standalone Contract — a webhook payload, a job argument, the controller-free use the gem advertises — loads nothing that defines it, so every :datetime field raised instead of validating. This is the same shape as the 0.5.1 nested-hash bug, and the spec added for that one already warned about the masking in its own comment. So rather than add a new bare-subprocess spec, that one now exercises EVERY scalar type instead of only nested hashes, which is what would have caught this. A companion spec pins that a real TimeWithZone is still accepted and normalised to UTC, so the fix cannot regress into a guard that quietly stops handling zoned times. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 +++ lib/permittable.rb | 19 ++++++++++++++- spec/contract_spec.rb | 57 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2acbfad..0b7a9b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ - **A route answering several verbs was documented for only one of them.** `rails_routes` kept `verb.split("|").first`, so the `PATCH|PUT` pair `resources` generates — and any `match via: [:patch, :put]` — exported the PATCH operation and silently dropped PUT. Every verb a route answers now gets its own descriptor. - **The error-response schema had drifted from what the server renders.** `ERROR_SCHEMA` is the one hand-written part of the export, and it had fallen behind twice over: a violation on a field with `message:` (or with app I18n copy) carries a third key the schema didn't mention, so clients generating types from it dropped the human-readable copy; and the `code` enumeration never gained `depth`, which a `:json` field's `max_depth:` bound emits. `message` is now documented as an optional property — `required` stays `param` + `code`, since a violation without one keeps the bare shape — and `depth` is listed. A new spec renders a real violation and holds the schema to the envelope, so this half of the "docs cannot drift" claim is now guarded like the request-body half. - **Two controllers on one path and verb silently overwrote each other.** A document cannot carry two operations in one slot, and the second claim replaced the first with no indication anything had been lost. The loser now lands in `x-permittable-controllers`, which is where the exporter already puts an operation it cannot place. +- **Every `:datetime` cast raised `NameError` in a host that had not loaded ActiveSupport's time extensions.** A standalone `Permittable::Contract` — validating a webhook payload or a job argument — got `uninitialized constant ActiveSupport::TimeWithZone` instead of a validated param, because activesupport does not load that class by default and the gem never asked for it. A Rails app gets it via `active_support/time` at boot, which is why the spec suite (`require "active_record"`) masked it, the same shape as the 0.5.1 nested-hash bug. Fixed by requiring `active_support/core_ext/time/calculations`, which loads `TimeWithZone` **and** the `Time` extensions it needs: the class alone is not self-sufficient, and converting a real zoned time calls `Time#sec_fraction`, so requiring only `time_with_zone` would have traded `NameError` for `NoMethodError` on activesupport 8.1. The bare-subprocess spec now casts every scalar type, including a real `TimeWithZone`, in a process with no Rails. + +### Changed +- **A `Time` passed to a `:datetime` field is no longer converted in place.** Normalising to UTC went through `value.to_time.utc`; `Time#to_time` returns `self` and `Time#utc` converts its **receiver**, so validating a request quietly rewrote the caller's own object — after `call!(at: t)`, `t` had become UTC. The cast now returns a new instance and leaves the argument alone. An `ActiveSupport::TimeWithZone` is likewise no longer handed back by way of the UTC instance it caches internally. ## 0.7.0 (2026-09-16) diff --git a/lib/permittable.rb b/lib/permittable.rb index c2bc622..ac21409 100644 --- a/lib/permittable.rb +++ b/lib/permittable.rb @@ -7,6 +7,18 @@ require "active_support/core_ext/object/deep_dup" # authored default:/example: values are copied before freezing require "active_support/core_ext/string/inflections" require "active_support/core_ext/string/filters" +# cast_datetime names ActiveSupport::TimeWithZone, which activesupport does not +# load by default. A Rails app has it via active_support/time at boot; a +# standalone host (a Contract validating a webhook payload or a job argument) +# has nothing that loads it, and every :datetime cast raised NameError there. +# +# The Time core extensions come with it, and are not optional: TimeWithZone is +# present but not self-sufficient. Converting one goes through +# TimeZone#utc_to_local, which calls Time#sec_fraction — defined in +# core_ext/time/calculations, which time_with_zone.rb does not itself require. +# Without this line a real TimeWithZone raises NoMethodError in a bare host on +# activesupport 8.1, having merely traded one crash for another. +require "active_support/core_ext/time/calculations" require "bigdecimal" require "date" require "time" @@ -517,7 +529,12 @@ def complete_date?(found) def cast_datetime(value) case value # DateTime is listed here, ahead of Date, because it subclasses Date. - when ActiveSupport::TimeWithZone, Time, DateTime then [:ok, value.to_time.utc] + # `getutc` rather than `utc`: `Time#utc` converts the RECEIVER, and + # `Time#to_time` returns self, so `value.to_time.utc` silently rewrote + # the caller's own object. A TimeWithZone's `getutc` hands back the + # instance it caches internally, so that one is duped. + when ActiveSupport::TimeWithZone then [:ok, value.getutc.dup] + when Time, DateTime then [:ok, value.to_time.getutc] when Date then [:ok, Time.utc(value.year, value.month, value.day)] when String # Same rule as :date — the DATE part must be named in full, or it is diff --git a/spec/contract_spec.rb b/spec/contract_spec.rb index a73807f..aa3c952 100644 --- a/spec/contract_spec.rb +++ b/spec/contract_spec.rb @@ -179,5 +179,62 @@ expect(status).to be_success, err expect(out).to eq('{"user":{"test_key":1},"address_attributes":{"location":2}}') end + + it "casts every scalar type with only `require \"permittable\"`" do + # :datetime named ActiveSupport::TimeWithZone unguarded, and nothing in + # the gem loaded it — so a host that had not loaded ActiveSupport's time + # extensions got NameError instead of a validated param. + script = <<~RUBY + require "json" + require "permittable" + contract = Permittable::Contract.define do + required :s, :string + required :i, :integer + required :f, :float + required :d, :decimal + required :b, :boolean + required :on, :date + required :at, :datetime + required :zoned, :datetime + end + # A REAL TimeWithZone, cast here rather than in-process: the constant + # existing is not enough, it also needs the Time core extensions, and + # an in-process example cannot see that because spec_helper has + # already loaded them. + require "active_support/time_with_zone" + zoned = ActiveSupport::TimeZone["Asia/Bangkok"].local(2026, 9, 5, 17, 30) + out = contract.call!(s: "x", i: "1", f: "1.5", d: "2.50", b: "true", + on: "2026-09-05", at: "2026-09-05T10:30:00Z", zoned: zoned) + print JSON.generate(out.transform_values(&:to_s)) + RUBY + lib = File.expand_path("../lib", __dir__) + out, err, status = Open3.capture3(RbConfig.ruby, "-I", lib, "-e", script) + expect(status).to be_success, err + expect(JSON.parse(out)).to eq( + "s" => "x", "i" => "1", "f" => "1.5", "d" => "0.25e1", "b" => "true", + "on" => "2026-09-05", "at" => "2026-09-05 10:30:00 UTC", + "zoned" => "2026-09-05 10:30:00 UTC" + ) + end + + it "does not rewrite the caller's own Time while normalising it to UTC" do + moment = Time.new(2026, 9, 5, 17, 30, 0, "+07:00") + result = described_class.define { required :at, :datetime }.call!(at: moment) + expect(result[:at]).to eq(Time.utc(2026, 9, 5, 10, 30)) + expect(result[:at].utc?).to be(true) + # `Time#utc` converts its receiver, and `Time#to_time` returns self. + expect(moment.utc?).to be(false) + expect(moment.utc_offset).to eq(7 * 3600) + end + + it "accepts an ActiveSupport::TimeWithZone for a :datetime without touching its cached UTC instance" do + require "active_support/time" + zone = ActiveSupport::TimeZone["Asia/Bangkok"] + moment = zone.local(2026, 9, 5, 17, 30) + result = described_class.define { required :at, :datetime }.call!(at: moment) + expect(result[:at]).to eq(Time.utc(2026, 9, 5, 10, 30)) + expect(result[:at].utc?).to be(true) + expect(result[:at]).not_to be(moment.utc) + end end end