From 07c6736c6af68b14c12526b2b6e0ccb8b30c9972 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Wed, 8 Jul 2026 09:13:27 +0200 Subject: [PATCH] Compare the base API, not to_s, when refreshing a mounted app `refresh_already_mounted` deduplicates endpoints for the "same" mounted app by comparing `endpoint.mounted_app.to_s == app.to_s`. That only works by accident: `mount` turns every mounted Grape API into a throwaway `mount_instance` (a fresh `Class.new(@base_parent)` per mount), so two mounts of the same API are distinct objects, and `Grape::API::Instance` delegates `to_s` to its `@base` (the original API). The comparison is really "same base API", done through a string proxy -- brittle (two APIs that happen to share a name collide) and unclear. Expose the base API via a reader and compare it by identity instead. Plain Rack apps have no base and are mounted as-is, so they fall back to object identity. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + lib/grape/api/instance.rb | 2 ++ lib/grape/dsl/routing.rb | 14 +++++++++++++- spec/grape/api_spec.rb | 12 ++++++++++++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b737a1ab0..64274f0cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ * [#2792](https://github.com/ruby-grape/grape/pull/2792): Move `Grape::Path` to `Grape::Router::Pattern::Path` and add a `Grape::Router::Pattern.build` factory (`Grape::Path` kept as a deprecated constant) - [@ericproulx](https://github.com/ericproulx). * [#2785](https://github.com/ruby-grape/grape/pull/2785): Make route `params` a first-class endpoint input and split `Grape::Router::Route#params` into `#params` (declared definitions) and `#params_for(input)` (extracted values) - [@ericproulx](https://github.com/ericproulx). * [#2786](https://github.com/ruby-grape/grape/pull/2786): Make route `requirements` and `anchor` explicit keyword arguments and first-class endpoint inputs instead of opaque `route_options` keys - [@ericproulx](https://github.com/ericproulx). +* [#2788](https://github.com/ruby-grape/grape/pull/2788): Compare the base API instead of `to_s` when refreshing a mounted app - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/lib/grape/api/instance.rb b/lib/grape/api/instance.rb index e9ba6ffe6..c9e39a3ff 100644 --- a/lib/grape/api/instance.rb +++ b/lib/grape/api/instance.rb @@ -27,6 +27,8 @@ class << self def_delegators :@base, :to_s + attr_reader :base + def base=(grape_api) @base = grape_api grape_api.instances << self diff --git a/lib/grape/dsl/routing.rb b/lib/grape/dsl/routing.rb index 4862d745d..20bbc7d07 100644 --- a/lib/grape/dsl/routing.rb +++ b/lib/grape/dsl/routing.rb @@ -152,7 +152,7 @@ def mount(mounts, *opts) refresh_already_mounted = opts.any? ? opts.first[:refresh_already_mounted] : false if refresh_already_mounted && !endpoints.empty? endpoints.delete_if do |endpoint| - endpoint.mounted_app.to_s == app.to_s + same_mounted_app?(endpoint.mounted_app, app) end end @@ -294,6 +294,18 @@ def refresh_mounted_api(mounts, *opts) mount(mounts, *opts) end + # Two mounts refer to the same app when they share the same base Grape + # API. +mount+ turns every mounted Grape API into a throwaway + # +mount_instance+ (a fresh +Class.new+ per mount), so object identity + # never holds across mounts; comparing the base is the real signal. + # Plain Rack apps have no base and are mounted as-is, so they fall back + # to object identity. + def same_mounted_app?(mounted, app) + return mounted.base.equal?(app.base) if mounted.respond_to?(:base) && app.respond_to?(:base) + + mounted.equal?(app) + end + # Execute first the provided block, then each of the # block passed in. Allows for simple 'before' setups # of settings stack pushes. diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 65bf63ccc..79005c7ea 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -3665,6 +3665,18 @@ def self.call(object, _env) expect { subject.mount app => '/thing' } .to change { subject.endpoints.count }.by(1) end + + it 'does not confuse two distinct apps that share a name' do + other = Class.new(described_class) { get('/y') { 'y' } } + # Force a #to_s collision to prove the refresh keys off the base API + # identity rather than the string representation. + allow(app).to receive(:to_s).and_return('SameName') + allow(other).to receive(:to_s).and_return('SameName') + + subject.mount app => '/thing' + expect { subject.mount({ other => '/thing' }, refresh_already_mounted: true) } + .to change { subject.endpoints.count }.by(1) + end end context 'mounting an API' do