diff --git a/CHANGELOG.md b/CHANGELOG.md index cbf80a6..aea8420 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,85 @@ ## parse-stack-next Changelog +### 5.7.1 + +#### Cache-invalidation webhooks no longer break every application hook for the same trigger + +- **FIXED**: `Parse::Cache::Invalidation` raised `NoMethodError: undefined + method 'guard'` on every `_User`, `_Role`, and `_Session` trigger it + registered, and the failure was not contained. A webhook handler block is + bound to the payload before it runs, so the bare `guard` / `bump_subject` / + `subject_id` calls in the handler bodies resolved against + `Parse::Webhooks::Payload`, which defines none of them. `guard`'s own + `rescue` never ran, because it lives inside the body that was never entered, + so the error escaped into the route dispatcher. The dispatcher folds a + trigger's handlers with `Array#map`, which abandons the collection on the + first raise, and these triggers install during `Parse.setup` and therefore + sit ahead of every application handler for the same trigger. One + unresolvable method name silently prevented an application's own + `after_save "_User"` hook from running at all, so any work downstream of it + stopped without a visible cause. The handlers now call the module on an + explicit receiver captured in a local, which is independent of whatever + `self` the dispatcher binds. Applications that set + `cache_invalidation_hooks: false` to work around this can remove it. +- **FIXED**: The invalidation tests dispatched handlers with a plain + `Proc#call`, which leaves `self` bound to the module the block closed over, + so every one of them passed against handlers that could not run in + production. They now dispatch through the real handler invocation and fail + against the broken code. + +#### Whether one failing `after_*` handler stops the rest is now a setting + +- **NEW**: `Parse::Webhooks.abort_after_callbacks_on_error` decides whether an + exception raised by one `after_*` handler prevents the remaining handlers for + that trigger from running. It defaults to `true`, which is the existing + behavior, so nothing changes on upgrade. Set it to `false` to isolate + handlers from each other: each one runs regardless of what an earlier one + raised, and the failure is reported with a warning and a + `parse.webhooks.handler_error` notification instead of being swallowed + silently. This was previously not a decision at all but a side effect of + folding handlers with `Array#map`, which abandons the collection on the + first raise. Registration order is not fully under an application's control, + since the SDK's own cache-invalidation triggers install during `Parse.setup` + and therefore sit ahead of handlers registered by application files loaded + later, so a single raising handler could silently prevent an application's + own hooks from running with nothing logged to say they had been skipped. +- **BEHAVIOR**: The setting governs only whether later handlers still run. + Neither mode reverts anything: an `after_*` trigger fires once the write has + already committed, so there is no version of this setting that can undo a + save. `before_*` dispatch is untouched, and only the accumulating, + non-rejectable triggers (`after_save`, `after_delete`, `after_logout`) can + hold more than one handler in the first place. A rejectable `before_*` + trigger must deny if any handler denies, so its raise must continue to + abort. + +#### Failed transactions restore the object they rolled back + +- **FIXED**: A failed `Parse::Object.transaction` left the in-memory object + holding its modified values. The rollback snapshotted `Parse::Object#attributes` + and restored it, but that method returns a schema map of field name to type + symbol rather than values, so nothing was ever restored. Property values + live in `@` instance variables; those are now what the rollback + captures and restores, including values nested inside arrays and hashes, + relation operation queues, and properties whose ivar did not exist before + the transaction. State is captured when the object first enters the + transaction rather than at `batch.add`, so the documented pattern of + mutating an object and adding it afterwards rolls back correctly. +- **FIXED**: A failed transaction also left the object with broken change + tracking. Restoring the schema map defined `@attributes`, which is the + instance variable `ActiveModel::Dirty` keys its behavior on: once defined it + builds an `AttributeMutationTracker` over that hash instead of the + `ForcedMutationTracker` a `Parse::Object` needs, and `clear_changes!` then + called `forgetting_assignment` on the `[key, value]` pairs `Hash#map` + yields. Both call sites rescue and warn, so every rollback quietly + downgraded the object rather than failing. The rollback no longer defines + `@attributes`. +- **FIXED**: The mongo-direct role-graph integration test gated on + `ANALYTICS_DATABASE_URI`, a production variable name the test stack never + sets, so both of its traversal assertions skipped on every run while the + per-file reporter still reported the file as passing. It now reads + `PARSE_TEST_MONGO_URI` like every other mongo-direct integration file and + still honors `ANALYTICS_DATABASE_URI` as an override. + ### 5.7.0 #### Cache keys move into a reserved, app-scoped keyspace @@ -205,7 +285,7 @@ Server's self-access rules, and role-only checks cannot claim a concrete member's pointer or `_User` self permission. CLP cache entries are isolated by Parse application so identically named classes cannot leak policy across - clients. These helpers are advisory—the eventual Parse Server request is + clients. These helpers are advisory: the eventual Parse Server request is still authoritative. #### Test infrastructure diff --git a/Gemfile.lock b/Gemfile.lock index 0c6136a..2c82b08 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - parse-stack-next (5.7.0) + parse-stack-next (5.7.1) activemodel (>= 6.1, < 9) activesupport (>= 6.1, < 9) connection_pool (>= 2.2, < 4) diff --git a/lib/parse/cache/invalidation.rb b/lib/parse/cache/invalidation.rb index 5ef38e2..2f8b5a8 100644 --- a/lib/parse/cache/invalidation.rb +++ b/lib/parse/cache/invalidation.rb @@ -52,26 +52,111 @@ def install!(cache) registered end + # Run a role-trigger invalidation. + # + # Public because the registered webhook block calls it on an explicit + # receiver. See {handle_identity_trigger} for why that matters. + # + # @!visibility private + # @param cache [Parse::Cache::Redis] the keyspace-configured cache. + # @return [void] + def handle_role_trigger(cache) + guard do + # A role write does not say which users are affected: membership + # and hierarchy changes arrive as relation deltas on `users` and + # `roles`, and the cached value is a flattened transitive closure, + # so a parent-role change reaches the members of every child. + # Clearing the whole plane is both correct and cheap under a + # scoped SCAN. Parse Server does the same, for the same reason. + cache.roles.clear + # Stamp the epoch so a *foreign* role entry written before this + # moment is rejected on read. Parse Server does not clear its own + # role cache on a `_Role` delete, so without this the next read + # would take its stale entry back and our clear would shorten + # revocation by nothing. + cache.roles.touch_epoch + end + end + + # Run an identity-trigger invalidation. + # + # Public, and invoked on an explicit receiver, because a webhook + # handler block does NOT run with `self` bound to the module that + # created it. {Parse::Webhooks.invoke_handler} binds the block to the + # payload (`payload.define_singleton_method(name, &block)`), and the + # historical `payload.instance_exec(payload, &block)` did the same. + # A block body calling a bare `guard` / `bump_subject` / `subject_id` + # therefore resolved against {Parse::Webhooks::Payload}, which defines + # none of them, and raised `NoMethodError` on every `_User`, `_Role`, + # and `_Session` trigger. + # + # That failure was not contained. `guard`'s `rescue` lives inside + # `guard`'s own body, which was never entered, so the error escaped + # into `call_route`'s `registry.map`. `Array#map` abandons the whole + # collection on the first raise, and these triggers register at + # `Parse.setup` and therefore sit AHEAD of any application handler for + # the same trigger. One unresolvable method name silently prevented + # every application `after_save "_User"` hook from running at all. + # + # Capturing the module in a local and calling it explicitly is what + # makes the handler independent of whatever `self` the dispatcher + # binds. + # + # @!visibility private + # @param cache [Parse::Cache::Redis] the keyspace-configured cache. + # @param type [Symbol] the trigger being handled. + # @param payload [Parse::Webhooks::Payload] the incoming payload. + # @return [void] + def handle_identity_trigger(cache, type, payload) + guard do + case type + when :after_logout + # The only trigger Parse Server permits on `_Session`. The + # object's own sessionToken is scrubbed from the payload, but + # the token is captured from the requesting user before + # scrubbing, and for a logout that user *is* the session being + # ended. A master-key logout carries no user, so fall back to + # the generation bump. + # + # Pass the RAW token, not a pre-hashed digest. + # `Parse::Cache::SubCache#invalidate` hashes its `key` + # argument internally for the `:idn` family (see + # `SubCache#logical_key`), the same way `#get` / `#set` do. + # That is what makes a `set(raw_token, ...)` / + # `get(raw_token)` pair round-trip. Hashing here first and + # handing SubCache an already-hashed value made it hash the + # digest a second time, landing on a key nothing had ever + # written to, so logout silently failed to evict the entry. + token = payload.respond_to?(:session_token) ? payload.session_token : nil + if token && !token.to_s.empty? + cache.identity.invalidate(token.to_s) + else + bump_subject(cache, subject_id(payload)) + end + else + # A `_User` write gives a user id, but identity entries are + # keyed by session token and no reverse map exists. Bumping a + # per-user generation invalidates every one of that user's + # entries in O(1), including tokens this process has never + # resolved, and without Parse Server's master-key `_Session` + # query. + bump_subject(cache, subject_id(payload)) + end + end + end + private def install_role_triggers!(cache) + # `invalidator` is a captured LOCAL, not `self`. The registered block + # runs with `self` rebound to the payload, so a bare method call in + # its body would resolve against `Parse::Webhooks::Payload`. A local + # closes over correctly regardless of the receiver the dispatcher + # binds. + invalidator = self TRIGGERS[:role].map do |(type, class_name)| Parse::Webhooks.route(type, class_name) do |payload| - guard do - # A role write does not say which users are affected: membership - # and hierarchy changes arrive as relation deltas on `users` and - # `roles`, and the cached value is a flattened transitive closure, - # so a parent-role change reaches the members of every child. - # Clearing the whole plane is both correct and cheap under a - # scoped SCAN. Parse Server does the same, for the same reason. - cache.roles.clear - # Stamp the epoch so a *foreign* role entry written before this - # moment is rejected on read. Parse Server does not clear its own - # role cache on a `_Role` delete, so without this the next read - # would take its stale entry back and our clear would shorten - # revocation by nothing. - cache.roles.touch_epoch - end + invalidator.handle_role_trigger(cache) true end [type, class_name] @@ -79,43 +164,10 @@ def install_role_triggers!(cache) end def install_identity_triggers!(cache) + invalidator = self TRIGGERS[:identity].map do |(type, class_name)| Parse::Webhooks.route(type, class_name) do |payload| - guard do - case type - when :after_logout - # The only trigger Parse Server permits on `_Session`. The - # object's own sessionToken is scrubbed from the payload, but - # the token is captured from the requesting user before - # scrubbing, and for a logout that user *is* the session being - # ended. A master-key logout carries no user, so fall back to - # the generation bump. - # - # Pass the RAW token, not a pre-hashed digest. - # `Parse::Cache::SubCache#invalidate` hashes its `key` - # argument internally for the `:idn` family (see - # `SubCache#logical_key`), the same way `#get` / `#set` do — - # that is what makes a `set(raw_token, ...)` / - # `get(raw_token)` pair round-trip. Hashing here first and - # handing SubCache an already-hashed value made it hash the - # digest a second time, landing on a key nothing had ever - # written to, so logout silently failed to evict the entry. - token = payload.respond_to?(:session_token) ? payload.session_token : nil - if token && !token.to_s.empty? - cache.identity.invalidate(token.to_s) - else - bump_subject(cache, subject_id(payload)) - end - else - # A `_User` write gives a user id, but identity entries are - # keyed by session token and no reverse map exists. Bumping a - # per-user generation invalidates every one of that user's - # entries in O(1), including tokens this process has never - # resolved, and without Parse Server's master-key `_Session` - # query. - bump_subject(cache, subject_id(payload)) - end - end + invalidator.handle_identity_trigger(cache, type, payload) true end [type, class_name] diff --git a/lib/parse/model/associations/belongs_to.rb b/lib/parse/model/associations/belongs_to.rb index 0e1c306..d6d06db 100644 --- a/lib/parse/model/associations/belongs_to.rb +++ b/lib/parse/model/associations/belongs_to.rb @@ -243,6 +243,10 @@ def belongs_to(key, opts = {}) instance_variable_set ivar, val end + # Capture after lazy hydration, before a caller can mutate an + # object returned by this association getter. + send(:_capture_transaction_state!) if respond_to?(:_capture_transaction_state!, true) + # Track association source for N+1 detection when returning an unfetched pointer # Uses a registry instead of setting instance variables on the pointer object if val.is_a?(Parse::Pointer) && val.pointer? && Parse.warn_on_n_plus_one diff --git a/lib/parse/model/associations/collection_proxy.rb b/lib/parse/model/associations/collection_proxy.rb index 828ea65..16f12a9 100644 --- a/lib/parse/model/associations/collection_proxy.rb +++ b/lib/parse/model/associations/collection_proxy.rb @@ -359,6 +359,9 @@ def <<(*list) # Notifies the delegate that the collection changed. def notify_will_change! + if @delegate && @delegate.respond_to?(:_capture_transaction_state!, true) + @delegate.send(:_capture_transaction_state!) + end collection_will_change! forward "#{@key}_will_change!" end diff --git a/lib/parse/model/associations/has_many.rb b/lib/parse/model/associations/has_many.rb index 0e690c4..0e55e61 100644 --- a/lib/parse/model/associations/has_many.rb +++ b/lib/parse/model/associations/has_many.rb @@ -502,6 +502,10 @@ def has_many(key, scope = nil, **opts) val = instance_variable_get ivar end + # Capture before this getter materializes or returns a mutable + # proxy that can be changed in place. + send(:_capture_transaction_state!) if respond_to?(:_capture_transaction_state!, true) + # if the result is not a collection proxy, then create a new one. unless val.is_a?(Parse::PointerCollectionProxy) results = [] diff --git a/lib/parse/model/core/actions.rb b/lib/parse/model/core/actions.rb index 632c69b..0d82e35 100644 --- a/lib/parse/model/core/actions.rb +++ b/lib/parse/model/core/actions.rb @@ -94,6 +94,28 @@ def initialize(object) module Core # Defines some of the save, update and destroy operations for Parse objects. module Actions + # Fiber-local context used to capture an object's state before its first + # mutation inside a transaction block. `batch.add` is intentionally too + # late for this: the public API documents mutating an object and adding it + # afterwards. + TRANSACTION_CONTEXT_KEY = :__parse_transaction_context__ + + # Distinguishes a property whose ivar did not exist from one explicitly + # set to nil. Rollback removes the former instead of defining it as nil. + UNDEFINED_PROPERTY = Object.new.freeze + + # Non-property state that can change while building/submitting a batch + # and must be restored along with the `@` property ivars. + ROLLBACK_STATE_IVARS = %i[ + @changed_attributes + @id + @mutations_from_database + @mutations_before_last_save + @_acl_snapshot_before_change + @_acl_pristine + @_authorization_acl_state + ].freeze + # @!visibility private def self.included(base) base.extend(ClassMethods) @@ -133,10 +155,16 @@ def self.included(base) # cannot corrupt the saved copy. def self.snapshot_property_values(obj) fields = obj.class.respond_to?(:fields) ? obj.class.fields.keys : [] - fields.each_with_object({}) do |key, snapshot| + relations = obj.class.respond_to?(:relations) ? obj.class.relations.keys : [] + property_keys = (fields + relations).uniq + seen = {} + property_keys.each_with_object({}) do |key, snapshot| ivar = :"@#{key}" - next unless obj.instance_variable_defined?(ivar) - snapshot[ivar] = dup_for_snapshot(obj.instance_variable_get(ivar)) + snapshot[ivar] = if obj.instance_variable_defined?(ivar) + dup_for_snapshot(obj.instance_variable_get(ivar), seen) + else + UNDEFINED_PROPERTY + end end end @@ -147,25 +175,144 @@ def self.snapshot_property_values(obj) # `:array` and association properties hold a {Parse::CollectionProxy}, # and duplicating the proxy still shares the underlying `@collection` # array, so `widget.tags << "b"` would mutate the snapshot too. The - # proxy's inner array is duplicated as well. + # proxy's mutable backing arrays and nested values are duplicated as + # well. Parse objects nested inside values remain references: a pointer + # property should restore the same object, not manufacture a clone. # # @param value [Object] the live property value. # @return [Object] a copy safe to hold across the transaction. - def self.dup_for_snapshot(value) - copy = begin - value.dup - rescue TypeError - # Symbols, Integers, true/false/nil and other immediates are not - # duplicable on older rubies; they are also immutable, so sharing - # the reference is safe. - return value + def self.dup_for_snapshot(value, seen = {}) + return value if value.nil? || value == true || value == false || value.is_a?(Symbol) || value.is_a?(Numeric) + return value if defined?(Parse::Pointer) && value.is_a?(Parse::Pointer) + + object_id = value.object_id + return seen[object_id] if seen.key?(object_id) + + case value + when Array + copy = value.dup + copy.clear + seen[object_id] = copy + value.each { |item| copy << dup_for_snapshot(item, seen) } + copy + when Hash + copy = value.dup + copy.clear + seen[object_id] = copy + value.each do |key, item| + copy[dup_for_snapshot(key, seen)] = dup_for_snapshot(item, seen) + end + copy + when Parse::CollectionProxy + copy = value.dup + seen[object_id] = copy + %i[@collection @additions @removals @changed_attributes].each do |ivar| + next unless value.instance_variable_defined?(ivar) + copy.instance_variable_set(ivar, dup_for_snapshot(value.instance_variable_get(ivar), seen)) + end + %i[@mutations_from_database @mutations_before_last_save].each do |ivar| + next unless value.instance_variable_defined?(ivar) + tracker = value.instance_variable_get(ivar) + copy.instance_variable_set(ivar, dup_mutation_tracker(tracker, copy, seen)) end + copy + when Parse::ACL + copy = Parse::ACL.new(dup_for_snapshot(value.as_json, seen), owner: value.delegate) + seen[object_id] = copy + copy + else + copy = begin + value.dup + rescue TypeError + # Immutable values are safe to share with the snapshot. + return value + end + seen[object_id] = copy + copy + end + end - if copy.instance_variable_defined?(:@collection) - inner = copy.instance_variable_get(:@collection) - copy.instance_variable_set(:@collection, inner.dup) if inner.is_a?(Array) + # Duplicate ActiveModel's mutation tracker without sharing its mutable + # forced/finalized change hashes. Forced trackers retain their owning + # Parse object (or copied collection proxy) so dirty reads keep working. + # + # @param tracker [Object, nil] ActiveModel mutation tracker. + # @param owner [Object] object whose attributes the copy should read. + # @param seen [Hash] identity map used by {dup_for_snapshot}. + # @return [Object, nil] isolated tracker copy. + def self.dup_mutation_tracker(tracker, owner, seen = {}) + return nil if tracker.nil? + return tracker if defined?(ActiveModel::NullMutationTracker) && tracker.is_a?(ActiveModel::NullMutationTracker) + return seen[tracker.object_id] if seen.key?(tracker.object_id) + + copy = tracker.dup + seen[tracker.object_id] = copy + if defined?(ActiveModel::ForcedMutationTracker) && tracker.is_a?(ActiveModel::ForcedMutationTracker) + copy.instance_variable_set(:@attributes, owner) + end + %i[@forced_changes @finalized_changes].each do |ivar| + next unless tracker.instance_variable_defined?(ivar) + copy.instance_variable_set(ivar, dup_for_snapshot(tracker.instance_variable_get(ivar), seen)) end copy + rescue TypeError + tracker + end + + # Capture all local state needed to restore an object after a failed + # transaction. This is separate from `#attributes`, which is a schema. + # + # @param obj [Parse::Object] object being transacted. + # @return [Hash] rollback state. + def self.snapshot_object_state(obj) + seen = {} + instance_variables = ROLLBACK_STATE_IVARS.each_with_object({}) do |ivar, snapshot| + snapshot[ivar] = if obj.instance_variable_defined?(ivar) + value = obj.instance_variable_get(ivar) + if %i[@mutations_from_database @mutations_before_last_save].include?(ivar) + dup_mutation_tracker(value, obj, seen) + else + dup_for_snapshot(value, seen) + end + else + UNDEFINED_PROPERTY + end + end + + { + object: obj, + property_values: snapshot_property_values(obj), + instance_variables: instance_variables, + } + end + + # Record that an object was initialized inside the active transaction. + # Its first rollback snapshot is intentionally taken when it is added to + # the batch, after initialization, so a failed create stays usable as an + # initialized unsaved object. + # + # @param obj [Parse::Object] newly initialized object. + # @return [void] + def self.mark_transaction_object_created(obj) + context = Fiber[TRANSACTION_CONTEXT_KEY] + return unless context + context[:created_objects][obj.object_id] = true + context[:snapshots].delete(obj.object_id) + end + + # Capture an object's state once, before its first transaction mutation. + # Objects created inside the transaction defer capture until `batch.add`. + # + # @param obj [Parse::Object] object whose state should be captured. + # @param context [Hash, nil] transaction context; defaults to the current fiber. + # @param include_created [Boolean] capture a newly created object at add time. + # @return [Hash, nil] the object's rollback state. + def self.capture_transaction_state(obj, context = Fiber[TRANSACTION_CONTEXT_KEY], include_created: false) + return unless context && obj + + object_id = obj.object_id + return if context[:created_objects].key?(object_id) && !include_created + context[:snapshots][object_id] ||= snapshot_object_state(obj) end # Restore the values captured by {snapshot_property_values}. @@ -180,6 +327,25 @@ def self.dup_for_snapshot(value) def self.restore_property_values(obj, snapshot) return unless snapshot.is_a?(Hash) snapshot.each do |ivar, value| + if value.equal?(UNDEFINED_PROPERTY) + obj.remove_instance_variable(ivar) if obj.instance_variable_defined?(ivar) + else + obj.instance_variable_set(ivar, value) + end + end + end + + # Restore one instance variable while preserving whether it originally + # existed. Used for dirty/ACL bookkeeping outside the property schema. + # + # @param obj [Object] target object. + # @param ivar [Symbol] instance variable name. + # @param value [Object] snapshotted value or {UNDEFINED_PROPERTY}. + # @return [void] + def self.restore_instance_variable(obj, ivar, value) + if value.equal?(UNDEFINED_PROPERTY) + obj.remove_instance_variable(ivar) if obj.instance_variable_defined?(ivar) + else obj.instance_variable_set(ivar, value) end end @@ -193,16 +359,35 @@ def self.rollback_object_state(state) obj = state[:object] return if obj.nil? restore_property_values(obj, state[:property_values]) - obj.instance_variable_set(:@changed_attributes, state[:changed_attributes]) - obj.instance_variable_set(:@id, state[:id]) - # Restore change tracking state. Leaving `@mutations_from_database` - # nil is fine: `ActiveModel::Dirty` lazily rebuilds it, and with - # `@attributes` no longer defined on the object it correctly rebuilds - # a `ForcedMutationTracker`. - obj.instance_variable_set(:@mutations_from_database, state[:mutations_from_database]) - obj.instance_variable_set(:@mutations_before_last_save, state[:mutations_before_last_save]) + if state[:instance_variables] + state[:instance_variables].each do |ivar, value| + restore_instance_variable(obj, ivar, value) + end + else + # Compatibility with rollback states produced before the complete + # snapshot format was introduced. + obj.instance_variable_set(:@changed_attributes, state[:changed_attributes]) + obj.instance_variable_set(:@id, state[:id]) + obj.instance_variable_set(:@mutations_from_database, state[:mutations_from_database]) + obj.instance_variable_set(:@mutations_before_last_save, state[:mutations_before_last_save]) + end + end + + # Hook used by generated property accessors and collection proxies. + # @api private + def _capture_transaction_state! + Parse::Core::Actions.capture_transaction_state(self) + end + + # ActiveModel calls this immediately after `_will_change!`; the + # hook also covers callers that explicitly mark a mutable value dirty. + def _read_attribute(attr_name) + _capture_transaction_state! + super end + private :_capture_transaction_state!, :_read_attribute + # Class methods applied to Parse::Object subclasses. module ClassMethods @@ -238,123 +423,109 @@ module ClassMethods def transaction(retries: 5, &block) raise ArgumentError, "Block required for transaction" unless block_given? - batch = Parse::BatchOperation.new(nil, transaction: true) - - # Store original state of objects for rollback + previous_context = Fiber[TRANSACTION_CONTEXT_KEY] + transaction_context = { snapshots: {}, created_objects: {} } + Fiber[TRANSACTION_CONTEXT_KEY] = transaction_context original_states = {} tracked_objects = [] - # Wrap the batch to capture objects being added - batch_wrapper = Object.new - batch_wrapper.define_singleton_method(:is_a?) do |klass| - klass == Parse::BatchOperation || super(klass) - end - batch_wrapper.define_singleton_method(:kind_of?) do |klass| - klass == Parse::BatchOperation || super(klass) - end - batch_wrapper.define_singleton_method(:instance_of?) do |klass| - klass == Parse::BatchOperation - end - batch_wrapper.define_singleton_method(:add) do |obj| - # Store original state when object is first added to transaction. - # Use obj.object_id (Ruby identity) as the key because Parse::Object#hash - # and #eql? treat all unsaved objects (nil id) as equal, which would cause - # only the first unsaved object to be tracked. - if obj.respond_to?(:attributes) && obj.respond_to?(:id) && !original_states.key?(obj.object_id) - original_states[obj.object_id] = { - object: obj, - property_values: Parse::Core::Actions.snapshot_property_values(obj), - changed_attributes: obj.instance_variable_get(:@changed_attributes)&.dup || {}, - id: obj.id, - mutations_from_database: obj.instance_variable_get(:@mutations_from_database), - mutations_before_last_save: obj.instance_variable_get(:@mutations_before_last_save), - } - tracked_objects << obj + begin + batch = Parse::BatchOperation.new(nil, transaction: true) + + # Wrap the batch to associate the pre-mutation snapshot with each + # object that actually participates in the transaction. + batch_wrapper = Object.new + batch_wrapper.define_singleton_method(:is_a?) do |klass| + klass == Parse::BatchOperation || super(klass) + end + batch_wrapper.define_singleton_method(:kind_of?) do |klass| + klass == Parse::BatchOperation || super(klass) + end + batch_wrapper.define_singleton_method(:instance_of?) do |klass| + klass == Parse::BatchOperation + end + batch_wrapper.define_singleton_method(:add) do |obj| + # Ruby identity is required because all unsaved Parse objects + # compare equal while their ids are nil. + if obj.respond_to?(:attributes) && obj.respond_to?(:id) && !original_states.key?(obj.object_id) + original_states[obj.object_id] = Parse::Core::Actions.capture_transaction_state( + obj, + transaction_context, + include_created: true, + ) + tracked_objects << obj + end + batch.add(obj) end - batch.add(obj) - end - # Forward other methods to the real batch - batch_wrapper.define_singleton_method(:method_missing) do |method, *args, &block| - batch.send(method, *args, &block) - end + # Forward other methods to the real batch. + batch_wrapper.define_singleton_method(:method_missing) do |method, *args, &method_block| + batch.send(method, *args, &method_block) + end + batch_wrapper.define_singleton_method(:respond_to_missing?) do |method, include_private = false| + batch.respond_to?(method, include_private) + end - result = yield(batch_wrapper) + result = yield(batch_wrapper) - # If block returns objects, add them to batch - if result.respond_to?(:change_requests) - batch_wrapper.add(result) - elsif result.is_a?(Array) - result.each { |obj| batch_wrapper.add(obj) if obj.respond_to?(:change_requests) } - end + # If block returns objects, add them to batch. + if result.respond_to?(:change_requests) + batch_wrapper.add(result) + elsif result.is_a?(Array) + result.each { |obj| batch_wrapper.add(obj) if obj.respond_to?(:change_requests) } + end - # Submit with retry logic for transaction conflicts - attempts = 0 - begin - attempts += 1 - responses = batch.submit - - # Check for success - if responses.all?(&:success?) - # Update tracked objects with data from successful responses - # Match responses to objects using the request tag (Ruby object_id) - # Build hash lookup once for O(n) instead of O(n²) linear search - objects_by_id = tracked_objects.each_with_object({}) { |o, h| h[o.object_id] = o } - requests = batch.requests - requests.zip(responses).each do |request, response| - next unless request && response && response.success? - result = response.result - next unless result.is_a?(Hash) - - # Find the object matching this request's tag - obj = objects_by_id[request.tag] - next unless obj - - # Update object with response data (objectId, createdAt, updatedAt) - if result["objectId"] - obj.instance_variable_set(:@id, result["objectId"]) - end - if result["createdAt"] - obj.instance_variable_set(:@created_at, Parse::Date.parse(result["createdAt"])) - end - if result["updatedAt"] - obj.instance_variable_set(:@updated_at, Parse::Date.parse(result["updatedAt"])) - elsif result["createdAt"] - obj.instance_variable_set(:@updated_at, Parse::Date.parse(result["createdAt"])) + # Submit with retry logic for transaction conflicts. + attempts = 0 + begin + attempts += 1 + responses = batch.submit + + if responses.all?(&:success?) + # Match responses to objects using the request tag (Ruby object_id). + objects_by_id = tracked_objects.each_with_object({}) { |o, h| h[o.object_id] = o } + batch.requests.zip(responses).each do |request, response| + next unless request && response && response.success? + result = response.result + next unless result.is_a?(Hash) + + obj = objects_by_id[request.tag] + next unless obj + + obj.instance_variable_set(:@id, result["objectId"]) if result["objectId"] + if result["createdAt"] + obj.instance_variable_set(:@created_at, Parse::Date.parse(result["createdAt"])) + end + if result["updatedAt"] + obj.instance_variable_set(:@updated_at, Parse::Date.parse(result["updatedAt"])) + elsif result["createdAt"] + obj.instance_variable_set(:@updated_at, Parse::Date.parse(result["createdAt"])) + end + + # Apply any additional attributes returned by beforeSave hooks. + obj.set_attributes!(result) if obj.respond_to?(:set_attributes!) + obj.send(:clear_changes!) if obj.respond_to?(:clear_changes!, true) end - # Apply any additional attributes returned by beforeSave hooks - obj.set_attributes!(result) if obj.respond_to?(:set_attributes!) - - # Clear change tracking since save was successful - obj.send(:clear_changes!) if obj.respond_to?(:clear_changes!, true) - end - - return responses - else - # Find first error - error_response = responses.find { |r| !r.success? } - - # Rollback local object states - original_states.each_value do |state| - Parse::Core::Actions.rollback_object_state(state) + return responses end + error_response = responses.find { |response| !response.success? } raise Parse::Error, "Transaction failed: #{error_response.error}" + rescue Parse::Error => e + if e.message.include?("251") && attempts < retries + sleep(0.1 * attempts) + retry + end + raise end - rescue Parse::Error => e - # Retry on transaction conflict (error code 251) - if e.message.include?("251") && attempts < retries - sleep(0.1 * attempts) # Exponential backoff - retry - end - - # Rollback local object states on final failure + rescue StandardError original_states.each_value do |state| Parse::Core::Actions.rollback_object_state(state) end - - raise e + raise + ensure + Fiber[TRANSACTION_CONTEXT_KEY] = previous_context end end diff --git a/lib/parse/model/core/fetching.rb b/lib/parse/model/core/fetching.rb index 996b5a4..f1deaa5 100644 --- a/lib/parse/model/core/fetching.rb +++ b/lib/parse/model/core/fetching.rb @@ -512,6 +512,10 @@ def prepare_for_dirty_tracking!(key) @_fetched_keys ||= [] @_fetched_keys << key unless @_fetched_keys.include?(key) end + + # A transaction must capture the value before the setter marks or + # replaces it. `batch.add` happens after mutation in the public API. + send(:_capture_transaction_state!) if respond_to?(:_capture_transaction_state!, true) end end end diff --git a/lib/parse/model/core/properties.rb b/lib/parse/model/core/properties.rb index ca129b8..0d6d17f 100644 --- a/lib/parse/model/core/properties.rb +++ b/lib/parse/model/core/properties.rb @@ -573,6 +573,10 @@ def property(key, data_type = :string, **opts) value = instance_variable_get ivar end + # Capture after any implicit fetch, but before a default value or + # mutable collection is materialized by this getter. + send(:_capture_transaction_state!) if respond_to?(:_capture_transaction_state!, true) + # if value is nil (even after fetching), then lets see if the developer # set a default value for this attribute. if value.nil? && respond_to?("#{key}_default") diff --git a/lib/parse/model/object.rb b/lib/parse/model/object.rb index bb953dd..88f4b9a 100644 --- a/lib/parse/model/object.rb +++ b/lib/parse/model/object.rb @@ -1337,6 +1337,8 @@ def attribute_names_for_serialization # for trusted hydration from server JSON; it bypasses the filter. # @return [Parse::Object] a the corresponding Parse::Object or subclass. def initialize(opts = {}) + Parse::Core::Actions.mark_transaction_object_created(self) + # Trusted hydration is signalled by the `@_trusted_init` instance # variable rather than by a `trusted:` keyword argument. Using a # keyword would break subclasses that override `initialize(*args)` @@ -2042,6 +2044,8 @@ def _resolve_acl_owner_id(owner) # caller intent to override. # @api private def acl_will_change! + _capture_transaction_state! + # Only capture snapshot on the first change (before any modifications) unless defined?(@_acl_snapshot_before_change) && @_acl_snapshot_before_change # Deep copy the ACL by creating a new one from its JSON representation diff --git a/lib/parse/stack/version.rb b/lib/parse/stack/version.rb index b7c100c..735f6fb 100644 --- a/lib/parse/stack/version.rb +++ b/lib/parse/stack/version.rb @@ -6,6 +6,6 @@ module Parse # The Parse Server SDK for Ruby module Stack # The current version. - VERSION = "5.7.0" + VERSION = "5.7.1" end end diff --git a/lib/parse/webhooks.rb b/lib/parse/webhooks.rb index 79889f9..b49703e 100644 --- a/lib/parse/webhooks.rb +++ b/lib/parse/webhooks.rb @@ -132,6 +132,46 @@ def key class << self + # Whether an exception raised by one `after_*` handler prevents the + # remaining handlers for that same trigger from running. + # + # Only the accumulating, non-rejectable `after_*` triggers can have more + # than one handler (see {Parse::Webhooks::Registration#route}), so this + # governs `after_save`, `after_delete`, and `after_logout` and nothing + # else. `before_*` dispatch is untouched: a raise there is how a handler + # denies an operation, and it must continue to abort. + # + # Defaults to `true`, which is the historical behavior. Handlers are + # folded with `Array#map`, and `map` abandons the collection on the first + # raise, so a handler that raises silently prevents every handler + # registered after it from running. That ordering is not something an + # application fully controls: the SDK's own cache-invalidation triggers + # install during `Parse.setup` and therefore sit ahead of handlers + # registered by application files loaded later. + # + # Set to `false` to isolate handlers from each other, so that each one + # runs regardless of what an earlier one raised. The error is reported + # (a warning plus a `parse.webhooks.handler_error` notification) and + # dispatch continues. + # + # Either way, nothing is reverted. An `after_*` trigger fires once the + # write has already committed, so there is no version of this setting + # that can undo the save; the only question it answers is whether the + # remaining handlers still get to run. + # + # @example Keep one failing handler from starving the others + # Parse::Webhooks.abort_after_callbacks_on_error = false + # + # @return [Boolean] + attr_writer :abort_after_callbacks_on_error + + # (see #abort_after_callbacks_on_error=) + # @return [Boolean] + def abort_after_callbacks_on_error + return @abort_after_callbacks_on_error unless @abort_after_callbacks_on_error.nil? + true + end + # Allows support for web frameworks that support auto-reloading of source. # @!visibility private def reload!(args = {}) @@ -245,6 +285,60 @@ def run_function(name, params) # block that declares a parameter (`do |payload| ... end`) or a splat # receives the payload. # + # Run every handler registered for one accumulating `after_*` trigger. + # + # `.last` is preserved as the composed result because Parse Server + # ignores the response body for these triggers and {#call_route} + # normalizes it anyway, so which handler's value survives is not + # observable. + # + # When {abort_after_callbacks_on_error} is false, a handler that raises + # is reported and skipped rather than taking the rest of the trigger down + # with it. Nothing is reverted in either mode: the write these triggers + # fire on has already committed. + # + # @param payload [Parse::Webhooks::Payload] the request payload. + # @param registry [Array] the handlers, in registration order. + # @param type [Symbol] the trigger being dispatched. + # @return [Object] the last handler result. + def dispatch_composed(payload, registry, type) + return registry.map { |hook| invoke_handler(payload, hook) }.last if + abort_after_callbacks_on_error + + last = nil + registry.each do |hook| + begin + last = invoke_handler(payload, hook) + rescue StandardError => e + report_handler_error(type, e) + end + end + last + end + + # Report a handler failure that was isolated rather than propagated. + # + # The message is included because an application's own handler raised it + # and the application needs it to debug; this is not the SDK's internal + # `guard`, which deliberately omits messages that can carry a cache key. + # + # @param type [Symbol] the trigger being dispatched. + # @param error [StandardError] the raised error. + # @return [void] + def report_handler_error(type, error) + warn "[Parse::Webhooks] #{type} handler raised #{error.class}: #{error.message}; " \ + "continuing with the remaining handlers " \ + "(Parse::Webhooks.abort_after_callbacks_on_error is false)" + return unless defined?(ActiveSupport::Notifications) + begin + ActiveSupport::Notifications.instrument( + "parse.webhooks.handler_error", trigger: type, error: error.class.name, + ) + rescue StandardError + nil + end + end + # @param payload [Parse::Webhooks::Payload] the request payload (becomes `self`). # @param block [Proc] the registered handler block. # @return [Object] the handler's result value. @@ -378,7 +472,10 @@ def call_route(type, className, payload = nil) end if registry.is_a?(Array) - result = registry.map { |hook| invoke_handler(payload, hook) }.last + # An Array registry only ever exists for the accumulating, + # non-rejectable `after_*` triggers, so isolating handlers here + # cannot affect `before_*` rejection semantics. + result = dispatch_composed(payload, registry, type) else result = invoke_handler(payload, registry) end diff --git a/test/lib/parse/cache_invalidation_test.rb b/test/lib/parse/cache_invalidation_test.rb index dd86cde..f4a6fab 100644 --- a/test/lib/parse/cache_invalidation_test.rb +++ b/test/lib/parse/cache_invalidation_test.rb @@ -52,9 +52,21 @@ def teardown Parse::Webhooks.instance_variable_set(:@routes, nil) end + # Dispatch the way Parse::Webhooks actually does. + # + # This used to be `h.call(payload)`. A plain `Proc#call` leaves `self` bound + # to whatever the block closed over lexically, which for these handlers is + # the `Parse::Cache::Invalidation` module, so a bare `guard` resolved and + # every test here passed. The real dispatcher does NOT do that: it binds the + # block to the payload via `define_singleton_method`, exactly as the historical + # `payload.instance_exec(payload, &block)` did. Under that binding the same + # handlers raised `NoMethodError: undefined method 'guard'` on every trigger. + # + # Calling the real `invoke_handler` is what makes these tests capable of + # failing when the handlers are broken in production. def fire(type, class_name, payload) handlers = Parse::Webhooks.routes[type][class_name] - Array(handlers).each { |h| h.call(payload) } + Array(handlers).each { |h| Parse::Webhooks.send(:invoke_handler, payload, h) } end # Minimal payload doubles: only what the invalidation handlers read. @@ -200,4 +212,53 @@ def test_logout_with_neither_token_nor_user_is_inert fire(:after_logout, "_Session", FakePayload.new(nil, "_Session", nil)) assert_empty @store.data end + + # --- dispatch binding --------------------------------------------------- + + # The registered handlers must not depend on `self`. + # + # `Parse::Webhooks.invoke_handler` binds each block to the payload, so a + # handler body calling a bare private method of the module that created it + # resolves against `Parse::Webhooks::Payload` and raises `NoMethodError`. + # This asserts the property directly rather than through any one trigger. + def test_handlers_do_not_resolve_methods_against_the_module + handler = Parse::Webhooks.routes[:after_save]["_User"].first + receiver = Object.new + receiver.define_singleton_method(:parse_object) { FakeUser.new("bound123") } + receiver.define_singleton_method(:parse_class) { Parse::Model::CLASS_USER } + + # Bind the block to an object that defines none of Invalidation's helpers, + # which is precisely what the real dispatcher does. + Parse::Webhooks.send(:invoke_handler, receiver, handler) + + assert_equal 1, @cache.identity.generation("bound123"), + "the handler must work when self is not the Invalidation module" + end + + # End-to-end through the real dispatcher, with a real Payload rather than a + # double, and an application handler registered after the SDK's. + # + # `call_route` runs the registry through `Array#map`, which abandons the + # whole collection on the first raise. Because these triggers install at + # `Parse.setup` they sit AHEAD of every application handler, so one raising + # SDK handler silently prevented the application's own `after_save "_User"` + # hook from running at all. That is the failure this pins. + def test_a_failing_sdk_handler_cannot_starve_an_application_handler + Parse::Webhooks.instance_variable_set(:@routes, nil) + Parse::Cache::Invalidation.install!(@cache) # registers first, as at setup + + app_ran = false + Parse::Webhooks.route(:after_save, "_User") { |_p| app_ran = true } + + payload = Parse::Webhooks::Payload.new( + "triggerName" => "afterSave", + "object" => { "className" => "_User", "objectId" => "starve123" }, + ) + + Parse::Webhooks.call_route(:after_save, "_User", payload) + + assert app_ran, "the application's after_save _User handler must still run" + assert_equal 1, @cache.identity.generation("starve123"), + "the SDK's own invalidation must also have run" + end end diff --git a/test/lib/parse/transaction_rollback_state_test.rb b/test/lib/parse/transaction_rollback_state_test.rb index 5455706..685129d 100644 --- a/test/lib/parse/transaction_rollback_state_test.rb +++ b/test/lib/parse/transaction_rollback_state_test.rb @@ -1,5 +1,6 @@ require_relative "../../test_helper" require "minitest/autorun" +require "ostruct" # Unit coverage for the state a transaction rollback captures and restores. # @@ -21,14 +22,36 @@ class RollbackWidget < Parse::Object property :title, :string property :quantity, :integer property :tags, :array + property :metadata, :object + property :note, :string + has_many :related_widgets, as: :rollback_widget, through: :relation end def build_widget - widget = RollbackWidget.new(title: "original", quantity: 1, tags: ["a"]) + widget = RollbackWidget.new( + title: "original", + quantity: 1, + tags: ["a"], + metadata: { nested: { count: 1 } }, + ) widget.clear_changes! widget end + def with_failed_batch + original_new = Parse::BatchOperation.method(:new) + Parse::BatchOperation.define_singleton_method(:new) do |*args, **kwargs| + batch = original_new.call(*args, **kwargs) + batch.define_singleton_method(:submit) do + [OpenStruct.new(success?: false, error: "forced failure")] + end + batch + end + yield + ensure + Parse::BatchOperation.define_singleton_method(:new, &original_new) + end + def test_snapshot_captures_property_values_not_the_schema widget = build_widget snapshot = Parse::Core::Actions.snapshot_property_values(widget) @@ -54,6 +77,31 @@ def test_snapshot_dups_mutable_values_so_later_mutation_does_not_corrupt_it "in-place mutation after the snapshot must not reach the saved copy" end + def test_snapshot_deeply_dups_nested_values + widget = build_widget + snapshot = Parse::Core::Actions.snapshot_property_values(widget) + + widget.metadata[:nested][:count] = 2 + + assert_equal 1, snapshot[:@metadata][:nested][:count], + "nested mutation after the snapshot must not reach the saved copy" + end + + def test_snapshot_isolates_relation_operation_arrays + widget = build_widget + relation = widget.related_widgets + relation.loaded = true + snapshot = Parse::Core::Actions.snapshot_property_values(widget) + related = RollbackWidget.new(title: "related") + + relation.add(related) + + saved_relation = snapshot[:@related_widgets] + assert_empty saved_relation.additions + assert_empty saved_relation.removals + assert_empty saved_relation.to_a + end + def test_rollback_restores_property_values widget = build_widget state = { @@ -129,6 +177,101 @@ def test_rollback_does_not_define_the_activemodel_attributes_ivar "over Parse's schema hash" end + def test_rollback_removes_a_property_ivar_that_was_originally_undefined + widget = build_widget + refute widget.instance_variable_defined?(:@note) + state = Parse::Core::Actions.snapshot_object_state(widget) + + widget.note = "added" + Parse::Core::Actions.rollback_object_state(state) + + refute widget.instance_variable_defined?(:@note) + end + + def test_public_transaction_captures_state_before_mutate_then_add + widget = build_widget + + with_failed_batch do + assert_raises(Parse::Error) do + Parse::Object.transaction do |batch| + widget.title = "modified" + batch.add(widget) + end + end + end + + assert_equal "original", widget.title + refute widget.changed? + refute widget.instance_variable_defined?(:@attributes) + end + + def test_public_transaction_restores_nested_and_undefined_values + widget = build_widget + + with_failed_batch do + assert_raises(Parse::Error) do + Parse::Object.transaction do |batch| + metadata = widget.metadata + widget.metadata_will_change! + metadata[:nested][:count] = 2 + widget.note = "added" + batch.add(widget) + end + end + end + + assert_equal 1, widget.metadata[:nested][:count] + refute widget.instance_variable_defined?(:@note) + refute widget.changed? + end + + def test_public_transaction_preserves_preexisting_dirty_state + widget = build_widget + widget.title = "pending before transaction" + changes_before = widget.changes + + with_failed_batch do + assert_raises(Parse::Error) do + Parse::Object.transaction do |batch| + widget.quantity = 2 + batch.add(widget) + end + end + end + + assert_equal "pending before transaction", widget.title + assert_equal 1, widget.quantity + assert_equal changes_before, widget.changes + end + + def test_failed_create_keeps_values_but_not_a_server_id + widget = nil + + with_failed_batch do + assert_raises(Parse::Error) do + Parse::Object.transaction do |batch| + widget = RollbackWidget.new(title: "new widget", quantity: 3) + batch.add(widget) + end + end + end + + assert_equal "new widget", widget.title + assert_equal 3, widget.quantity + assert_nil widget.id + assert widget.changed? + end + + def test_transaction_context_is_restored_after_failure + with_failed_batch do + assert_raises(Parse::Error) do + Parse::Object.transaction { [] } + end + end + + assert_nil Fiber[Parse::Core::Actions::TRANSACTION_CONTEXT_KEY] + end + # Pins the premise the bug rested on, so a future change to `#attributes` # that made it return values would surface here rather than silently. def test_attributes_returns_the_schema_map diff --git a/test/lib/parse/webhook_handler_isolation_test.rb b/test/lib/parse/webhook_handler_isolation_test.rb new file mode 100644 index 0000000..5726fb8 --- /dev/null +++ b/test/lib/parse/webhook_handler_isolation_test.rb @@ -0,0 +1,128 @@ +# encoding: UTF-8 +# frozen_string_literal: true + +require_relative "../../test_helper" + +# Whether one raising `after_*` handler prevents the rest from running. +# +# Handlers for the accumulating `after_*` triggers are folded with `Array#map`, +# and `map` abandons the collection on the first raise. That made "a raise +# starves every handler registered after it" an emergent property of the fold +# rather than a decision, and registration order is not fully under an +# application's control: the SDK's own cache-invalidation triggers install +# during `Parse.setup`, ahead of handlers registered by application files. +# +# `Parse::Webhooks.abort_after_callbacks_on_error` makes it a decision. +class WebhookHandlerIsolationTest < Minitest::Test + def setup + @previous = Parse::Webhooks.abort_after_callbacks_on_error + Parse::Webhooks.instance_variable_set(:@routes, nil) + end + + def teardown + Parse::Webhooks.abort_after_callbacks_on_error = @previous + Parse::Webhooks.instance_variable_set(:@routes, nil) + end + + # Dispatch the composed registry directly. `call_route` does a great deal of + # payload work around the fold that is irrelevant here and needs a configured + # client; this exercises the fold itself. + def dispatch(type, class_name, payload = nil) + registry = Parse::Webhooks.routes[type][class_name] + Parse::Webhooks.send(:dispatch_composed, payload || Object.new, registry, type) + end + + def register_three(type: :after_save, class_name: "Widget") + ran = [] + Parse::Webhooks.route(type, class_name) { |_p| ran << :first } + Parse::Webhooks.route(type, class_name) { |_p| raise IOError, "backend down" } + Parse::Webhooks.route(type, class_name) { |_p| ran << :third } + ran + end + + def test_default_is_to_abort + assert_equal true, Parse::Webhooks.abort_after_callbacks_on_error, + "the default must preserve the historical behavior" + end + + def test_aborting_stops_at_the_first_raise + ran = register_three + assert_raises(IOError) { dispatch(:after_save, "Widget") } + assert_equal [:first], ran, + "handlers after the raising one must not run when aborting" + end + + def test_isolating_runs_every_handler + Parse::Webhooks.abort_after_callbacks_on_error = false + ran = register_three + dispatch(:after_save, "Widget") + assert_equal [:first, :third], ran, + "a raising handler must not starve the ones registered after it" + end + + def test_isolating_swallows_the_error + Parse::Webhooks.abort_after_callbacks_on_error = false + register_three + dispatch(:after_save, "Widget") # must not raise + end + + def test_isolating_reports_the_failure + Parse::Webhooks.abort_after_callbacks_on_error = false + register_three + events = [] + subscriber = ActiveSupport::Notifications.subscribe("parse.webhooks.handler_error") do |*args| + events << ActiveSupport::Notifications::Event.new(*args) + end + begin + capture_io { dispatch(:after_save, "Widget") } + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) + end + + assert_equal 1, events.size, "an isolated failure must still be reported" + assert_equal "IOError", events.first.payload[:error] + assert_equal :after_save, events.first.payload[:trigger] + end + + # A silent skip would be worse than the starvation it replaces. + def test_isolating_warns_on_stderr + Parse::Webhooks.abort_after_callbacks_on_error = false + register_three + _out, err = capture_io { dispatch(:after_save, "Widget") } + assert_match(/IOError/, err) + assert_match(/backend down/, err) + end + + def test_isolation_applies_to_after_delete_and_after_logout + Parse::Webhooks.abort_after_callbacks_on_error = false + [[:after_delete, "Widget"], [:after_logout, "_Session"]].each do |type, class_name| + Parse::Webhooks.instance_variable_set(:@routes, nil) + ran = register_three(type: type, class_name: class_name) + dispatch(type, class_name) + assert_equal [:first, :third], ran, "#{type} must isolate handlers too" + end + end + + # `before_*` triggers store a single block rather than an array, so they never + # reach the composed fold at all. A raise there is how a handler denies the + # operation and must keep propagating regardless of this setting. + def test_before_triggers_store_a_single_handler_and_are_unaffected + Parse::Webhooks.abort_after_callbacks_on_error = false + Parse::Webhooks.route(:before_save, "Widget") { |_p| :first } + Parse::Webhooks.route(:before_save, "Widget") { |_p| :second } + + registry = Parse::Webhooks.routes[:before_save]["Widget"] + refute_kind_of Array, registry, + "before_save must not accumulate, so isolation cannot apply to it" + end + + def test_rejectable_non_object_triggers_are_not_composed + Parse::Webhooks.route(:before_login, "_User") { |_p| :first } + Parse::Webhooks.route(:before_login, "_User") { |_p| :second } + + registry = Parse::Webhooks.routes[:before_login]["_User"] + refute_kind_of Array, registry, + "a rejectable trigger must deny if ANY handler denies, so it " \ + "cannot be folded" + end +end