feat(datasource graphql hasura): add Hasura datasource with Rails polymorphism support - #343
Conversation
…ymorphism support Introspects a Hasura GraphQL API and exposes its tables as collections named after their Rails class names. Rails-style polymorphic associations (a `<base>_type`/`<base>_id` column pair) are detected from the Hasura metadata or from an explicit configuration, and emitted as PolymorphicManyToOne and PolymorphicOneToMany relations, so the Forest UI gets the native polymorphic widget and related data is always filtered by type. Only a `manual_configuration` relationship can be one branch of a polymorphic association: one backed by a foreign key constraint is monomorphic, and accepting it would absorb a legitimate belongs_to whenever an unrelated `<base>_type` enum column sits next to `<base>_id`. Also handles the cases a real instance surfaced: foreign keys referencing a non-id primary key, multi-column mappings, targets that are not exposed, table names tracked in several Postgres schemas, tables without a primary key, name collisions on reverse relations, Postgres enums and array columns, aggregates returned as JSON strings, and a blocked metadata endpoint. `validation/` holds a Postgres + Hasura stack and an end-to-end script covering those scenarios (31 checks), next to the RSpec suite.
24 new issues
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
- persist an explicit nil on insert instead of dropping the key, which let the column default win over a value the user cleared (the ActiveRecord datasource writes null here); create and update now behave the same way - require the local key of an array relationship to be the primary key the polymorphic association targets before treating it as the reverse side, so a relationship mapped on another column is no longer replaced by one querying the primary key - keep a physical column that shares its name with a polymorphic association, and skip the association with a warning rather than shadowing the column - set write_timeout alongside the read and open ones, and translate Net::WriteTimeout like the other transport failures Also splits the functions flagged as too complex (parse_table, parse_tables, polymorphic_targets, validate_aggregation_field) along their natural seams.
…sm detection out The collection carried the whole aggregation pipeline (validation, the parent-table detour Hasura forces on grouped aggregates, value coercion) and the introspector carried the polymorphism detection. Both now live in classes of their own, Query::Aggregator and Introspection::PolymorphismDetector, leaving the collection to its CRUD surface and the introspector to reading the schema. Configuration takes its options as a keyword hash validated against the known list, so an unknown option is reported by name.
|
The qlty comments were all metrics rather than defects, so here is where they stand after 62de6a3. Most disappeared with the splits already pushed for the correctness fixes. The two remaining file-complexity ones led to a change worth making on its own: Running Two categories I left alone on purpose:
Verified on 50 specs plus the 32-check run against a real Hasura instance, rubocop clean. |
- a condition tree that matches every row converts to nil instead of an empty `_and`, which Hasura reads as vacuously true and which slipped past the mutation guard, so an update could have touched a whole table - an object relationship is only taken for a polymorphic branch when its mapping uses the expected foreign key, including when its target is configured: a table can hold both an ordinary relationship and a branch towards the same target - primary keys come from `_by_pk` only; an `id` column on a view or a tracked function carries no uniqueness to address records by - an explicit allow-list wins over the built-in system-table prefixes - a scalar column named like a `<relation>_aggregate` companion field is kept - Postgres arrays take the type of their element (`_int4` reads as Number) - grouping on several fields is rejected rather than silently honouring the first - parent rows sharing a group value are merged, as SQL grouping would - a zero `count(columns: field)` is kept: rows exist, they all hold null - Max/Min over dates order by instant instead of collapsing to zero
Merging parent rows that share a group value, added in the previous commit, went through a float conversion: a Sum over bigint lost precision past 2^53, and a Max or Min over text compared two zeros and kept whichever row came first. Whole numbers are now added as Integers, which Ruby does not cap, and Max/Min compare through a tuple that orders numbers and instants together, then text lexically. Sorting reuses that same tuple, so the order and the merge agree.
|
Update on the qlty comments, now that the code has moved. Running What I did act on: the earlier findings led to real splits — What I am not acting on, and why:
Marking these threads resolved so the review stays readable — happy to reopen any of them if a reviewer disagrees. If we want the thresholds to match our rubocop conventions, that is worth a Current state: 63 specs, 32 checks against a real Hasura instance, rubocop clean. |
…onest at scale The parent-table detour read the first 1000 parents in arbitrary order, unfiltered, and logged a warning nobody charting sees: past 1000 parents the chart was silently wrong. Parents are now filtered by the chart's predicate through the relationship, ordered by their primary key and paginated; past 10 000 parent rows the chart fails with a clear error instead of returning a subset. Rows whose foreign key is NULL were invisible to the detour and fell out of every bucket, where SQL grouping gives them one of their own: they are now aggregated apart and merged in as the nil group. A foreign key was advertised as groupable whether or not Hasura declares the reverse array relationship the grouping query needs, so the UI could offer a group-by that the aggregator then rejects. The marking now happens once all collections are registered, and only where the reverse relationship exists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ra errors and harden the configuration Every failure surfaced as a 400 ValidationError, dressing a downed Hasura up as a client mistake and hiding it from 5xx-based monitoring. Errors Hasura itself returns keep the 400; an unreachable endpoint (timeout, DNS, TLS, non-2xx, invalid body) now raises TransportError, a ForestException carrying a 503 status, so the message stays actionable and the incident stays visible. The client gains its own spec, which the transport paths never had. Two configuration traps are closed. A polymorphic relation declared on a table missing its <base>_type/<base>_id column pair emitted a relation towards columns that do not exist; it is now skipped with a warning naming the missing columns. And when the uri carries no '/v1/graphql' segment, no metadata endpoint is derived anymore: substituting on such a uri silently posted metadata commands to the GraphQL endpoint itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- exclude validation/ from the built gem: the Docker stack, seed SQL and setup script were shipping to every client - add the LICENSE file the gemspec announces, like the other packages - raise minimum_coverage to the repo-wide 90 (actual coverage is 97%) - update the README to the new grouped-aggregation, error and metadata derivation behaviours Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| DEFAULTS.each { |option, default| instance_variable_set("@#{option}", options.fetch(option, default)) } | ||
| # Only derivable from the conventional endpoint path: substituting on any | ||
| # other uri would silently post metadata commands to the GraphQL endpoint. | ||
| @metadata_uri ||= uri.include?('/v1/graphql') ? uri.sub('/v1/graphql', '/v1/metadata') : nil |
| foreign_key = collection.schema[:fields][field.foreign_key] | ||
| foreign_key.is_groupable = true if foreign_key.respond_to?(:is_groupable=) | ||
| end | ||
| end |
| end | ||
|
|
||
| offset += PARENT_PAGE | ||
| end |
…d to end Seeds an orphan comment (membership_id NULL) and asserts that grouped charts give it the bucket SQL grouping would, on the foreign key, through a leaderboard relation path, and under a chart filter (which also exercises the parent-side relationship predicate against a real Hasura). Also realigns the transport-failure scenario with the previous commit: it still rescued GraphqlError where the client now raises TransportError, and it asserts the 503 status. Run against the Docker stack: 33 scenarios, 0 failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- an aggregation spanning exactly the 10 000-parent cap completed the walk in
theory but raised in practice: the strict comparison lets a final partial or
empty page close the pagination, and the error now fires only when a full
page lands past the cap
- Max/Min merging and result ordering compared numbers through Float, so two
bigints rounding to the same double tied and kept whichever row came first;
whole numbers now stay Integers, which Ruby compares with Floats exactly
- a projection selecting nothing (a valid toolkit input) generated `table { }`,
which is not valid GraphQL: the selection falls back to the primary key
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ll bucket
The orphan query only matched a NULL foreign key, so a child row whose key
references no parent — possible on a constraint-less relationship — escaped
both the parent walk and the null bucket and vanished from grouped charts.
The query now negates the relationship itself (`_not: { relation: {} }`),
which is how Hasura selects rows without a matching parent: NULL and dangling
keys land in the LEFT JOIN's NULL group. SQL would keep a dangling key as a
group of its own when grouping by the foreign key, but Hasura cannot
enumerate those keys; counted under nil beats dropped.
This also removes the non-nullable shortcut: a NOT NULL column can still
dangle without a constraint, so the orphan query always runs.
Validated against the Docker stack: 33 scenarios, 0 failure.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… and honestly capped
Found by an adversarial pass over the aggregation pipeline:
- a zero count(columns: x) could not tell "no rows" (SQL omits the group)
from "rows whose x is all NULL" (SQL keeps it at zero): a row_count alias
now rides along every aggregate selection, which also stops parents without
any child from surfacing as spurious zero groups, and keeps all-NULL
Sum/Max/Min groups instead of dropping them
- aggregate values of numeric columns are normalized to numbers at
extraction: one chart no longer mixes 1500 and "1500" depending on whether
a group was merged in Ruby, and text columns compare lexically again ("9"
beats "10", as SQL collates) since only genuine text reaches the tuple
- the 10 000-parent cap is enforced even when the overflowing page is
partial, matching what the README promises; exactly 10 000 still completes
- Sum/Avg/Max/Min without a field are rejected by name instead of emitting
an empty GraphQL selection set
- QueryBuilder.update refuses a filter that converts to no condition instead
of defaulting to {}, which Hasura reads as match-all — a backstop behind
the collection guard; delete keeps {} deliberately (bulk "select all")
- the orphan-bucket query is skipped when no orphan can exist (NOT NULL
foreign key backed by a real constraint)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…figuration Found by an adversarial pass over introspection and configuration: - with the metadata unreachable, a configured polymorphic target reachable through two object relationships absorbed one of them arbitrarily — it could be a plain belongs_to, silently deleted; the target is now skipped with a warning naming both relationships - a 200 introspection response with a null or partial __schema crashed boot with NoMethodError; it now raises IntrospectionError suggesting that introspection may be disabled, and a malformed metadata entry (legacy string table form, relationship without using) degrades to the naming conventions like an unreachable endpoint - non-public schema mappings no longer claim the bare table name: the bare GraphQL field can only be the public table, and the alias could invalidate a legitimate public mapping as ambiguous - two tables classifying to the same collection name (user_status and user_statuses) crashed boot deep in the toolkit; the first is kept and the warning names the tables and the type_values remedy - a table listed in both included_tables and excluded_tables was exposed; the exclusion now always wins, as the Configuration API says - a 200 GraphQL body that is not an object, or carries neither data nor errors, raises TransportError instead of leaking nil into the collection - Configuration instances no longer share the DEFAULTS objects (mutating one datasource's headers leaked into every other), and a misshapen polymorphic_relations raises ConfigurationError instead of crashing introspection Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
||
| data | ||
| rescue *TRANSPORT_ERRORS => e | ||
| raise TransportError, "Could not reach the GraphQL endpoint (#{e.class}): #{e.message}" |
| end | ||
| end | ||
| end | ||
| end |
| 'enabled on this endpoint?' | ||
| end | ||
|
|
||
| [types, query_fields] |
| { mapping: mapping, manual: false } | ||
| elsif manual | ||
| { mapping: manual['column_mapping'], manual: true } | ||
| end |
| hasura_field: relationship.name, | ||
| primary_key: relationship.mapping&.values&.first || target_table.primary_key.first || 'id' | ||
| } | ||
| end |
| # relationships towards the same configured target are indistinguishable: | ||
| # one may be a plain belongs_to, and absorbing it would silently delete a | ||
| # legitimate relation. Refuse to guess. | ||
| def ambiguous_branch?(table, base, remote_table, relationships) |
| end | ||
| end | ||
| end | ||
| end |
| data.dig(aggregation.operation.downcase, aggregation.field) | ||
| end | ||
|
|
||
| value.is_a?(String) && number_field?(aggregation) ? numeric(value) : value |
… collisions and composite keys
- crossed custom_root_fields renames could make one table's type name shadow
another table's root field in the shared lookup map, silently mixing their
class names, collections and polymorphic pairings: the converter now keeps
a root index and a type index and each lookup uses the spelling it holds
- the polymorphic discriminators were locked read-only even when they belong
to a composite primary key (Rails taggings), recreating the impossible-to-
create table the composite-key carve-out had just fixed — and their
Present validation now goes away with the lock, instead of demanding a
value the user cannot type in
- polymorphism detection runs after collection-name deduplication, so a
polymorphic target can never carry the primary key of a dropped table
- custom_root_fields.select in its object form ({ name:, comment: }) no
longer derails the metadata keying, a custom-named select_by_pk root is a
lookup rather than a second unlistable collection, and the graphql-default
naming convention is matched by registering each mapping under both the
Postgres and the camelized spellings, columns included
- polymorphic_relations accepts the type name like type_values does, and a
relationship whose name collides with an existing field logs a warning
instead of vanishing silently
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| end | ||
| end | ||
|
|
||
| def register_mapping(mappings, ambiguous, key, entry) |
| schema_name = table_info['schema'] | ||
| table_name = table_info['name'] | ||
|
|
||
| schema_name.nil? || schema_name == 'public' ? table_name : "#{schema_name}_#{table_name}" |
|
|
||
| field.is_read_only = true | ||
| field.validation = [] | ||
| end |
| foreign_collection: collection_name_of(remote.name), | ||
| origin_key: origin_key, | ||
| origin_key_target: relationship.mapping&.keys&.first || primary_key_of(table) | ||
| )] |
|
|
||
| # extra_where is a raw bool_exp and-combined with the converted filter | ||
| # (the null-bucket query adds `{ fk => { _is_null => true } }`). | ||
| def aggregate(names, filter, aggregation, extra_where: nil) |
| # offset pagination is stable, and filtered by the chart's predicate through | ||
| # the relationship, so the pages only walk parents owning at least one | ||
| # matching child row. | ||
| def grouped_aggregate(names, relation, filter, aggregation, page) |
| # Distinct values of `column` among rows without a matching parent — the | ||
| # dangling foreign keys a grouped chart must keep as groups of their own. | ||
| # distinct_on requires the matching order_by. | ||
| def orphan_keys(names, filter, column, relation_name, limit) |
|
|
||
| private | ||
|
|
||
| def add_sort(names, filter, args, var_defs, variables) |
…gnore a bare _type column build_selection walks relations recursively, but materialization stopped at the top level: a polymorphic reached through an ordinary relation (or through a PolymorphicOneToMany's rows) came back without the placeholder the serializer reads. Nested records now delegate to their own collection, mirroring the selection walk, hashes and arrays alike. A column literally named `_type` next to an `_id` column produced an empty detection base, emitting an unnamed polymorphic association that absorbed whatever relationship joins through `_id`. An empty base is no base. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e root fields The select root already followed custom_root_fields; the other operation roots were still derived from the type name, so a renamed insert, update, delete or select_aggregate root broke its operation with an unknown-field error. The metadata that declares those renames is already being read: every custom root field is now recorded and resolved onto the table, and each operation queries its own root — derived names remain the fallback when the metadata is unreachable. Type names (`<base>_bool_exp`, `<base>_insert_input`…) still derive from the type, which custom_root_fields does not touch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cept a false key value Avg raised whenever two parent rows shared a group value, failing a valid leaderboard chart. An average cannot be merged from averages, but its sum and non-null count can: both now ride along the aggregate selection, groups merge by adding them — SQL AVG over the union weights by count — and the division happens once the groups are final. The raise remains only for a response missing the aliases. materialize_placeholder gated the phantom on the truthiness of the foreign key, so a false value — legitimate on a boolean primary key — displayed as an empty reference. Only nil means "no reference". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…flattening - build_fields reads as the pipeline it is (columns, polymorphics, relationships, reverses), the shadow-warning loop moving to add_relationships - aggregation_selection splits its operation and Avg-merge parts - materialize_nested receives the nested value rather than digging it out - normalized_root_fields flattens then filters instead of accumulating The remaining qlty annotations are parameter-count and file-size metrics whose fix would be indirection for its own sake; they are addressed in the PR discussion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Update on the qlty comments, as of 1912077. Four hotspots were genuine readability wins and are refactored: The rest are threshold metrics, not defects, and stay as they are deliberately:
Behaviour is unchanged: 121 unit examples, rubocop clean, and the 34-scenario Hasura validation suite all pass on this commit. |
| end | ||
|
|
||
| fields[name] = schema | ||
| end |
…p guard The complexity annotation followed the loop into add_relationships; the warn-and-skip guard reads better as shadowed_relationship?, mirroring the detector's ambiguous_branch?. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The automated version bump sed targets VERSION = ".*" (with double quotes): 'sed -i 's/VERSION = ".*"/VERSION = "${nextRelease.version}"/g' packages/forest_admin_datasource_graphql_hasura/lib/forest_admin_datasource_graphql_hasura/version.rb; ' But this package's version.rb reads: (single quotes + .freeze, unlike every other package in the repo, e.g. VERSION = "1.35.2" without .freeze). The regex will never match this format : this package's version will stay frozen at 1.0.0 on every release, never bumped automatically. Two-part fix:
|
There was a problem hiding this comment.
Important (non-blocking) findings
A few things worth a follow-up, none of which need to hold up this merge:
-
client.rb:70andintrospector.rb:108— bothrescue StandardErrorare broad and, inclient.rb, under-logged (.infoinstead of.warn). A genuine bug further down the call chain (a typo, an unexpected metadata shape reaching aNoMethodError) would be silently relabeled as "Hasura metadata API not available" / "metadata could not be parsed" instead of failing loudly in dev/CI. Consider narrowing to the actually-expected exception types (Client::TRANSPORT_ERRORS+JSON::ParserErrorfor the client; shape-related errors for the introspector). -
introspector.rb:65(EXCLUDED_SUFFIXES) — a real table named e.g.bank_connectionordata_streamis silently dropped with zero logging, unlike every other skip path in this package. Three of the four suffixes (_aggregate,_by_pk,_connection) also look redundant with the structural list-type check already done inparse_tables; only_streamgenuinely needs a name-based filter, and even that could be gated on the base name being a known root field rather than a blanket suffix match. -
configuration.rb/introspector.rb—excluded_tables/included_tablesare matched against the exposed root field name, not the underlying table name. When Hasura'scustom_root_fields/custom_namerenames a table (explicitly supported elsewhere in this PR),excluded_tables: ['secrets']won't exclude asecretstable exposed asvault.polymorphic_relationsandtype_valuesboth correctly check both spellings — worth the same treatment here, since getting exclusion wrong exposes data rather than just skipping a relation. -
introspection/structures.rb—Relationship/Polymorphicare plainStructs with no validation at construction. A malformed polymorphic relation (emptytargets, a target table that doesn't exist) is representable as "valid" — only the discipline of the single call site inPolymorphismDetectorcurrently prevents it from reachingSchemaConvertersilently. A lightweight factory method with anArgumentErrorguard would make this impossible rather than "currently disciplined." -
structures.rb(Relationship#manual) — a nilable-boolean tri-state (true/false/nil) is a footgun for any futureif rel.manual/!rel.manualcheck, sincefalse("real FK") andnil("metadata unavailable") mean opposite things trust-wise. A 3-value symbol (:constraint/:manual/:unknown) would remove the ambiguity at no extra cost. -
introspector.rb:293vspolymorphism_detector.rb:16-26—Table#polymorphicsstarts empty at construction and is only ever populated by a later, mandatory pass ofPolymorphismDetector#detect. A "not yet processed" table and a "genuinely has no polymorphism" table are structurally identical. Nothing in the types enforces the current call order indatasource.rb; ifSchemaConverterwere ever invoked before detection ran, it would silently emit ordinaryobject/arrayrelationships instead ofPolymorphicManyToOne/PolymorphicOneToMany, with no error raised. -
configuration.rb(table_allowed?) —included_tables/excluded_tablesaren't validated asArrays. Passing aStringby mistake (included_tables: "users") silently becomes a substring check ("users".include?("us") #=> true) instead of a crash or a clear validation error.
None of these block the merge — the current pipeline works because the classes involved are disciplined with each other today. The risk is in future maintenance: nothing in the types enforces that discipline, and a violation would be silent (wrong schema) rather than loud (a named error), which cuts against the "never crash, but never silently mislead either" philosophy the rest of the package follows well.
| body = JSON.generate({ type: 'export_metadata', version: 2, args: {} }) | ||
| response = post(@configuration.metadata_uri, body) | ||
|
|
||
| return nil unless response.is_a?(Net::HTTPSuccess) |
There was a problem hiding this comment.
Non-2xx metadata responses are dropped with no logging at all, unlike the two neighboring failure branches (lines 53-59 and 70-76, which both log). A rejected/expired admin secret, a wrong metadata_uri, or a proxy blocking the metadata route all become an unexplained loss of polymorphism detection in production, with nothing to grep for.
return nil unless response.is_a?(Net::HTTPSuccess)Suggest logging the response status code here before falling back, at the same level as the other branches — and consider .warn rather than .info across all three fallback branches in this method, since the impact (polymorphism detection silently disabled) is the same in each case.
| payload = JSON.parse(response.body) | ||
| metadata = payload['metadata'] || payload | ||
|
|
||
| metadata['sources'] ? metadata : nil |
There was a problem hiding this comment.
Same silent-drop issue as the status check above, on the payload-shape check: a 200 response whose JSON doesn't carry a sources key (different Hasura metadata API version, a gateway returning an unrelated JSON body, a partial export) becomes nil with zero logging — indistinguishable from "endpoint unreachable".
metadata['sources'] ? metadata : nilSuggest logging the unexpected shape (e.g. payload.keys.first(5)) before returning nil, same as the status-code branch.
|
|
||
| # A relation towards a table the datasource does not expose (excluded, or | ||
| # dropped for want of a primary key) breaks schema generation at boot. | ||
| unless remote |
There was a problem hiding this comment.
The guard against relationships pointing at an unexposed table (excluded, dropped for lacking a PK, etc.) has no test coverage — neither here nor in the array-relationship equivalent (convert_array_relationship, lines 217-218):
def convert_object_relationship(table, relationship)
remote = resolve_table(relationship.remote_table)
...
unless remoteThe comment above this method says this exists specifically to prevent boot crashing on a real production schema. Worth a dedicated spec: a table with a relationship pointing at an excluded/PK-less table, asserting the relationship field is simply absent and the rest of the table still builds without raising.
| end | ||
|
|
||
| def register_mapping(mappings, ambiguous, key, entry) | ||
| ambiguous << key if mappings.key?(key) && mappings[key] != entry |
There was a problem hiding this comment.
register_mapping's collision-drop path is untested:
def register_mapping(mappings, ambiguous, key, entry)
ambiguous << key if mappings.key?(key) && mappings[key] != entryThe only related spec ("keeps the public mapping when another schema tracks the same table name") doesn't actually exercise this branch — exposed_root_field prefixes non-public schemas, so public.transfers and banking.transfers resolve to different keys and mappings[key] != entry never triggers there.
A genuine collision (two metadata entries landing on the identical exposed key with different relationship mappings) should have a dedicated spec asserting the mapping gets dropped and a warning is logged, rather than one entry silently winning.
| table.polymorphics.each do |polymorphic| | ||
| # A physical column of that name wins: replacing it would drop it from | ||
| # the schema, leaving it neither readable nor writable. | ||
| if fields.key?(polymorphic.name) |
There was a problem hiding this comment.
None of the three name-collision fallback paths for polymorphic associations have test coverage, despite being called out explicitly in the PR description ("name collisions on reverse relations"):
- here — a physical column sharing the polymorphic association's name (association dropped, column wins)
shadowed_relationship?(lines 170-178) — two relationships/fields sharing a namereverse_polymorphic_name(lines 296-313) — the 3-candidate naming fallback for a reversePolymorphicOneToMany, including the case where all three candidates are already taken
This is exactly the kind of multi-branch fallback logic that regresses silently — a future reordering of the candidates in reverse_polymorphic_name could start emitting a field under the wrong name with no test failing. Worth 2-3 dedicated specs, one per branch.
…e version version.rb used single quotes and .freeze, which the release sed (VERSION = ".*", double quotes) would never match: the package would have shipped frozen at 1.0.0 forever. Aligned on the repo convention (double quotes, no .freeze) and added the file to the same rubocop exclusions as every other version.rb. Spotted by @matthv. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
From @matthv's review: - every metadata fallback in Client#fetch_metadata now warns with its actual cause (HTTP status, missing sources with the top-level keys, transport failure, underivable uri): each one silently disables polymorphism detection in production and deserves something to grep for - the broad rescues are narrowed to what they are meant to absorb — transport errors in the client, shape errors (TypeError, NoMethodError, KeyError) around the metadata parsing — so a genuine bug fails loudly instead of being relabeled "metadata unavailable" - the name-based EXCLUDED_SUFFIXES are gone: a real table named data_stream or bank_connection was silently dropped. The structural list-shape check already rejects _aggregate/_by_pk/_connection roots, and _stream companions are recognized by their base root field existing - excluded_tables/included_tables now match the underlying table name as well as the exposed root field — an exclusion must hold under custom_root_fields renaming, or it silently re-exposes data — and both options are validated as arrays (a String would have become a substring check) - the untested guards get their specs: relationships towards unexposed tables, a metadata mapping genuinely claimed twice, and the three polymorphic name-collision fallbacks including the all-candidates-taken path Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks @matthv — everything actionable landed in 4d9d5fa and 81e518a. Fixed:
Deferred, deliberately (happy to do them in a follow-up if you feel strongly):
129 unit examples, rubocop clean, and the 34-scenario Hasura validation suite pass on the head. |
| private | ||
|
|
||
| def parse_metadata(body) | ||
| payload = JSON.parse(body) |
There was a problem hiding this comment.
Narrowing this rescue from StandardError to *TRANSPORT_ERRORS (great fix for the original "too broad" finding) removed the net that used to catch a TypeError here, and two realistic Hasura responses now crash Datasource#initialize at boot — exactly the failure mode fetch_metadata's own docstring says it prevents:
204 no-content metadata response
TypeError: no implicit conversion of nil into String
client.rb:74 (parse_metadata) -> JSON.parse(body)
JSON array metadata payload ('[]')
TypeError: no implicit conversion of String into Integer
client.rb:75 (parse_metadata) -> payload['metadata'] || payload
Net::HTTPNoContent < Net::HTTPSuccess is true, so a 204 sails through the response.is_a?(Net::HTTPSuccess) guard at line 63-64 with response.body == nil. execute already guards against exactly this a few lines up (line 29: "A 204 is a success with a nil body, which JSON.parse would turn into an unwrapped TypeError") — fetch_metadata/parse_metadata didn't get the same treatment.
Suggested fix — either add the empty-body guard parse_metadata is currently missing:
def parse_metadata(body)
return metadata_fallback('the metadata endpoint returned an empty body') if body.nil? || body.empty?
payload = JSON.parse(body)
return metadata_fallback("the metadata response is not a JSON object (#{payload.class})") unless payload.is_a?(Hash)
metadata = payload['metadata'] || payload
...or simply add TypeError, NoMethodError back to this method's own rescue clause. The explicit guard is preferable — it keeps the warning message accurate (a JSON::ParserError from garbage JSON currently reports "the metadata endpoint is not reachable", which is misleading since it did answer, just with something unparseable).
There was a problem hiding this comment.
Fixed in 1d7d99d, with your explicit-guard variant: empty-body and non-object guards mirroring Client#execute, plus JSON::ParserError caught locally so garbage JSON reports "not valid JSON" instead of the transport rescue's misleading "not reachable". Specs added for all three shapes (204, [], <html>).
…ed rescue no longer covers Narrowing fetch_metadata's rescue removed the net that absorbed a 204 no-content response (nil body into JSON.parse) and a JSON array payload (String index into an Array) — both crashed the boot again. parse_metadata now guards them explicitly, mirroring Client#execute, and catches its own JSON::ParserError so garbage JSON reports "not valid JSON" instead of the transport rescue's misleading "not reachable". Spotted by @matthv. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| shape = metadata.is_a?(Hash) ? "top-level keys: #{metadata.keys.first(5).join(", ")}" : metadata.class | ||
| metadata_fallback("the metadata response carries no sources (#{shape})") | ||
| rescue JSON::ParserError | ||
| metadata_fallback('the metadata response is not valid JSON') |
# [1.37.0](v1.36.3...v1.37.0) (2026-08-06) ### Features * **datasource graphql hasura:** add Hasura datasource with Rails polymorphism support ([#343](#343)) ([d0d6d9b](d0d6d9b))
|
🎉 This PR is included in version 1.37.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
3 participants
What
A new
forest_admin_datasource_graphql_hasurapackage: it introspects a Hasura GraphQL API and exposes its tables as Forest Admin collections, including Rails-style polymorphic associations (belongs_to :commentable, polymorphic: true).Collections are named after the Rails class name (
transfers→Transfer, overridable), because the Forest serializer resolves the target of aPolymorphicManyToOnefrom the raw value of the type column — both namings have to match.Why polymorphism needs handling
Hasura cannot express a polymorphic join: a
column_mappingcarries no type condition. The best a team can declare is one manual object relationship per target, all joining oncommentable_idalone — which resolves the wrong record whenever two targets share an id (a comment onCard#42would also "resolve"Transfer#42), and lists foreign records on the reverse side.This datasource detects the pattern and emits instead:
PolymorphicManyToOne(Comment.commentable), so the UI gets the native polymorphic widget;PolymorphicOneToManyon each target, filtered on the type value, so related data never leaks records of another type.Detection requires a
manual_configurationrelationship: one backed by a real foreign key constraint is monomorphic by definition, and accepting it would absorb a legitimatebelongs_towhenever an unrelated<base>_typeenum column happens to sit next to<base>_id. When the metadata API is unreachable (common in production), associations can be declared throughpolymorphic_relations.Also handled
Cases a real instance surfaced: foreign keys referencing a non-
idprimary key, multi-column mappings, relations towards tables that are not exposed, table names tracked in several Postgres schemas, tables without a primary key, name collisions on reverse relations, Postgres enums and array columns (no pattern operators — their comparison expressions have none), aggregates returned as JSON strings, null-awareIn/NotInsoPresent/Blankdon't overlap on text columns, escaped LIKE wildcards,jsonbvalues on writes, and errors surfaced as actionable messages rather than an opaque 500 (400 for errors Hasura returns, 503 for transport failures, so infrastructure incidents stay visible to monitoring). Grouped aggregations filter and paginate the parent rows (hard failure past 10 000 rather than a silently partial chart), give NULL foreign keys the bucket SQL grouping would, and foreign keys are only advertised as groupable when the reverse relationship the grouping query needs is declared.Limitations
Documented in the package README: grouping works on a foreign key or a
<relation>:<column>path (Hasura only exposes GROUP BY through nested<relation>_aggregate), no date truncation, no filtering/sorting through a polymorphic relation (a Forest limitation shared with the ActiveRecord datasource), no nested writes.Tests
79 RSpec examples, plus
validation/: a Postgres + Hasura stack seeded with a Rails-like banking schema and an end-to-end script running 34 checks against it (multi-target polymorphism, NULL-bucket grouping, two polymorphic associations on one table, namespaced models, uuid and composite primary keys, dangling and null references, blocked metadata, CRUD, charts).docker compose -f validation/docker-compose.yml up -d bash validation/setup_hasura.sh BUNDLE_GEMFILE=Gemfile-test bundle exec ruby validation/validate.rbLint, test and release pipelines are wired for the new package.
🤖 Generated with Claude Code
Note
Add Hasura GraphQL datasource with Rails polymorphic association support
forest_admin_datasource_graphql_hasuragem that connects Forest Admin to a Hasura GraphQL backend by introspecting the schema and optional Hasura metadata at startup.belongs_toassociations (<base>_type/<base>_idcolumns) viaPolymorphismDetector, buildingPolymorphicManyToOnefields and generating reversePolymorphicOneToManyrelations on targets.SchemaConverterconverts introspected tables into Forest Admin field schemas with typed filter operators, primary key detection, and composite key handling; ambiguous or invalid relationships are skipped with warnings.Collectionimplements list/create/update/delete/aggregate over Hasura GraphQL, materializing polymorphic placeholders from discriminator columns at read time; updates with an empty filter raiseForestExceptionto prevent full-table updates.FilterConvertertranslates Forest Admin condition trees to Hasura_bool_exphashes with NULL-awareIN/NOT INand case-insensitive pattern matching.Changes since #343 opened
included_tablesandexcluded_tablesconfiguration options are arrays [81e518a]ForestAdminDatasourceGraphqlHasura.VERSIONconstant from a frozen single-quoted string to a mutable double-quoted string and excluded version file from rubocop string literal and mutable constant rules [81e518a]ForestAdminDatasourceGraphqlHasura::Client.parse_metadata[1d7d99d]Macroscope summarized 8e20113.