Skip to content

Commit 867cb47

Browse files
committed
Warn on missing inverse relationships
1 parent bb00bbd commit 867cb47

8 files changed

+59
-32
lines changed

lib/jsonapi/active_relation_retrieval.rb

+13-12
Original file line numberDiff line numberDiff line change
@@ -304,10 +304,10 @@ def find_included_fragments(source_fragments, relationship, options)
304304
end
305305

306306
def find_related_fragments_from_inverse(source, source_relationship, options, connect_source_identity)
307-
relationship = source_relationship.resource_klass._relationship(source_relationship.inverse_relationship)
308-
raise "missing inverse relationship" unless relationship.present?
307+
inverse_relationship = source_relationship._inverse_relationship
308+
return {} if inverse_relationship.blank?
309309

310-
parent_resource_klass = relationship.resource_klass
310+
parent_resource_klass = inverse_relationship.resource_klass
311311

312312
include_directives = options.fetch(:include_directives, {})
313313

@@ -332,7 +332,7 @@ def find_related_fragments_from_inverse(source, source_relationship, options, co
332332
end
333333

334334
join_manager = ActiveRelation::JoinManager.new(resource_klass: self,
335-
source_relationship: relationship,
335+
source_relationship: inverse_relationship,
336336
relationships: linkage_relationships.collect(&:name),
337337
sort_criteria: sort_criteria,
338338
filters: filters)
@@ -352,7 +352,7 @@ def find_related_fragments_from_inverse(source, source_relationship, options, co
352352
if options[:cache]
353353
# This alias is going to be resolve down to the model's table name and will not actually be an alias
354354
resource_table_alias = self._table_name
355-
parent_table_alias = join_manager.join_details_by_relationship(relationship)[:alias]
355+
parent_table_alias = join_manager.join_details_by_relationship(inverse_relationship)[:alias]
356356

357357
pluck_fields = [
358358
sql_field_with_alias(resource_table_alias, self._primary_key),
@@ -400,7 +400,7 @@ def find_related_fragments_from_inverse(source, source_relationship, options, co
400400
fragments[rid].add_related_from(parent_rid)
401401

402402
if connect_source_identity
403-
fragments[rid].add_related_identity(relationship.name, parent_rid)
403+
fragments[rid].add_related_identity(inverse_relationship.name, parent_rid)
404404
end
405405

406406
attributes_offset = 2
@@ -452,7 +452,7 @@ def find_related_fragments_from_inverse(source, source_relationship, options, co
452452
end
453453
end
454454

455-
parent_table_alias = join_manager.join_details_by_relationship(relationship)[:alias]
455+
parent_table_alias = join_manager.join_details_by_relationship(inverse_relationship)[:alias]
456456
source_field = sql_field_with_fixed_alias(parent_table_alias, parent_resource_klass._primary_key, "jr_source_id")
457457

458458
records = records.select(concat_table_field(_table_name, Arel.star), source_field)
@@ -471,7 +471,7 @@ def find_related_fragments_from_inverse(source, source_relationship, options, co
471471
parent_rid = JSONAPI::ResourceIdentity.new(parent_resource_klass, resource._model.attributes['jr_source_id'])
472472

473473
if connect_source_identity
474-
fragments[rid].add_related_identity(relationship.name, parent_rid)
474+
fragments[rid].add_related_identity(inverse_relationship.name, parent_rid)
475475
end
476476

477477
fragments[rid].add_related_from(parent_rid)
@@ -503,15 +503,16 @@ def count_related(source, relationship, options = {})
503503
end
504504

505505
def count_related_from_inverse(source_resource, source_relationship, options = {})
506-
relationship = source_relationship.resource_klass._relationship(source_relationship.inverse_relationship)
506+
inverse_relationship = source_relationship._inverse_relationship
507+
return -1 if inverse_relationship.blank?
507508

508-
related_klass = relationship.resource_klass
509+
related_klass = inverse_relationship.resource_klass
509510

510511
filters = options.fetch(:filters, {})
511512

512513
# Joins in this case are related to the related_klass
513514
join_manager = ActiveRelation::JoinManager.new(resource_klass: self,
514-
source_relationship: relationship,
515+
source_relationship: inverse_relationship,
515516
filters: filters)
516517

517518
records = apply_request_settings_to_records(records: records(options),
@@ -521,7 +522,7 @@ def count_related_from_inverse(source_resource, source_relationship, options = {
521522
filters: filters,
522523
options: options)
523524

524-
related_alias = join_manager.join_details_by_relationship(relationship)[:alias]
525+
related_alias = join_manager.join_details_by_relationship(inverse_relationship)[:alias]
525526

526527
records = records.select(Arel.sql("#{concat_table_field(related_alias, related_klass._primary_key)}"))
527528

lib/jsonapi/active_relation_retrieval_v09.rb

+3-1
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,9 @@ def load_resources_to_fragments(fragments, related_resources, source_resource, s
287287
if source_resource
288288
source_resource.add_related_identity(source_relationship.name, related_resource.identity)
289289
fragment.add_related_from(source_resource.identity)
290-
fragment.add_related_identity(source_relationship.inverse_relationship, source_resource.identity)
290+
291+
inverse_relationship = source_relationship._inverse_relationship
292+
fragment.add_related_identity(inverse_relationship.name, source_resource.identity) if inverse_relationship.present?
291293
end
292294
end
293295
end

lib/jsonapi/active_relation_retrieval_v10.rb

+4-8
Original file line numberDiff line numberDiff line change
@@ -464,10 +464,8 @@ def find_related_monomorphic_fragments(source_fragments, relationship, options,
464464
end
465465

466466
if connect_source_identity
467-
related_relationship = resource_klass._relationship(relationship.inverse_relationship)
468-
if related_relationship
469-
fragments[rid].add_related_identity(related_relationship.name, source_rid)
470-
end
467+
inverse_relationship = relationship._inverse_relationship
468+
fragments[rid].add_related_identity(inverse_relationship.name, source_rid) if inverse_relationship``.present?
471469
end
472470
end
473471

@@ -598,10 +596,8 @@ def find_related_polymorphic_fragments(source_fragments, relationship, options,
598596
related_fragments[rid].add_related_from(source_rid)
599597

600598
if connect_source_identity
601-
related_relationship = related_klass._relationship(relationship.inverse_relationship)
602-
if related_relationship
603-
related_fragments[rid].add_related_identity(related_relationship.name, source_rid)
604-
end
599+
inverse_relationship = relationship._inverse_relationship
600+
related_fragments[rid].add_related_identity(inverse_relationship.name, source_rid) if inverse_relationship.present?
605601
end
606602

607603
relation_position = relation_positions[row[2].underscore.pluralize]

lib/jsonapi/configuration.rb

+11
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ class Configuration
1414
:warn_on_missing_routes,
1515
:warn_on_performance_issues,
1616
:warn_on_eager_loading_disabled,
17+
:warn_on_missing_relationships,
18+
:raise_on_missing_relationships,
1719
:default_allow_include_to_one,
1820
:default_allow_include_to_many,
1921
:allow_sort,
@@ -69,6 +71,11 @@ def initialize
6971
self.warn_on_missing_routes = true
7072
self.warn_on_performance_issues = true
7173
self.warn_on_eager_loading_disabled = true
74+
self.warn_on_missing_relationships = true
75+
# If this is set to true an error will be raised if a resource is found to be missing a relationship
76+
# If this is set to false a warning will be logged (see warn_on_missing_relationships) and related resouces for
77+
# this relationship will not be included in the response.
78+
self.raise_on_missing_relationships = false
7279

7380
# :none, :offset, :paged, or a custom paginator name
7481
self.default_paginator = :none
@@ -330,6 +337,10 @@ def allow_include=(allow_include)
330337

331338
attr_writer :warn_on_eager_loading_disabled
332339

340+
attr_writer :warn_on_missing_relationships
341+
342+
attr_writer :raise_on_missing_relationships
343+
333344
attr_writer :use_relationship_reflection
334345

335346
attr_writer :resource_cache

lib/jsonapi/link_builder.rb

+4-4
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def primary_resources_url
3434
@primary_resources_url_cached ||= "#{ base_url }#{ engine_mount_point }#{ primary_resources_path }"
3535
else
3636
if JSONAPI.configuration.warn_on_missing_routes && !@primary_resource_klass._warned_missing_route
37-
warn "primary_resources_url for #{@primary_resource_klass} could not be generated"
37+
warn "primary_resources_url for #{@primary_resource_klass.name} could not be generated"
3838
@primary_resource_klass._warned_missing_route = true
3939
end
4040
nil
@@ -54,7 +54,7 @@ def relationships_related_link(source, relationship, query_params = {})
5454
url
5555
else
5656
if JSONAPI.configuration.warn_on_missing_routes && !relationship._warned_missing_route
57-
warn "related_link for #{relationship} could not be generated"
57+
warn "related_link for #{relationship.display_name} could not be generated"
5858
relationship._warned_missing_route = true
5959
end
6060
nil
@@ -66,7 +66,7 @@ def relationships_self_link(source, relationship)
6666
"#{ self_link(source) }/relationships/#{ route_for_relationship(relationship) }"
6767
else
6868
if JSONAPI.configuration.warn_on_missing_routes && !relationship._warned_missing_route
69-
warn "self_link for #{relationship} could not be generated"
69+
warn "self_link for #{relationship.display_name} could not be generated"
7070
relationship._warned_missing_route = true
7171
end
7272
nil
@@ -78,7 +78,7 @@ def self_link(source)
7878
resource_url(source)
7979
else
8080
if JSONAPI.configuration.warn_on_missing_routes && !source.class._warned_missing_route
81-
warn "self_link for #{source.class} could not be generated"
81+
warn "self_link for #{source.class.name} could not be generated"
8282
source.class._warned_missing_route = true
8383
end
8484
nil

lib/jsonapi/relationship.rb

+22-5
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class Relationship
99

1010
attr_writer :allow_include
1111

12-
attr_accessor :_routed, :_warned_missing_route
12+
attr_accessor :_routed, :_warned_missing_route, :_warned_missing_inverse_relationship
1313

1414
def initialize(name, options = {})
1515
@name = name.to_s
@@ -47,6 +47,7 @@ def initialize(name, options = {})
4747

4848
@_routed = false
4949
@_warned_missing_route = false
50+
@_warned_missing_inverse_relationship = false
5051

5152
exclude_links(options.fetch(:exclude_links, JSONAPI.configuration.default_exclude_links))
5253

@@ -162,6 +163,22 @@ def _relation_name
162163
@relation_name || @name
163164
end
164165

166+
def to_s
167+
display_name
168+
end
169+
170+
def _inverse_relationship
171+
@inverse_relationship_klass ||= self.resource_klass._relationship(self.inverse_relationship)
172+
if @inverse_relationship_klass.blank?
173+
message = "Missing inverse relationship detected for: #{self}"
174+
warn message if JSONAPI.configuration.warn_on_missing_relationships && !_warned_missing_inverse_relationship
175+
_warned_missing_inverse_relationship = true
176+
177+
raise message if JSONAPI.configuration.raise_on_missing_relationships
178+
end
179+
@inverse_relationship_klass
180+
end
181+
165182
class ToOne < Relationship
166183
attr_reader :foreign_key_on
167184

@@ -182,9 +199,9 @@ def initialize(name, options = {})
182199
@polymorphic_type_relationship_for = options[:polymorphic_type_relationship_for]
183200
end
184201

185-
def to_s
202+
def display_name
186203
# :nocov: useful for debugging
187-
"#{parent_resource}.#{name}(#{belongs_to? ? 'BelongsToOne' : 'ToOne'})"
204+
"#{parent_resource.name}.#{name}(#{belongs_to? ? 'BelongsToOne' : 'ToOne'})"
188205
# :nocov:
189206
end
190207

@@ -247,9 +264,9 @@ def initialize(name, options = {})
247264
# end
248265
end
249266

250-
def to_s
267+
def display_name
251268
# :nocov: useful for debugging
252-
"#{parent_resource_klass}.#{name}(ToMany)"
269+
"#{parent_resource.name}.#{name}(ToMany)"
253270
# :nocov:
254271
end
255272

lib/jsonapi/resource_identity.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ def <=>(other_identity)
5858
# Creates a string representation of the identifier.
5959
def to_s
6060
# :nocov:
61-
"#{resource_klass}:#{id}"
61+
"#{resource_klass.name}:#{id}"
6262
# :nocov:
6363
end
6464
end

test/unit/serializer/link_builder_test.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ def test_relationships_related_link_not_routed
248248
link = builder.relationships_related_link(source, relationship)
249249
assert_nil link
250250
end
251-
assert_equal(err, "related_link for Api::Secret::PostResource.author(BelongsToOne) could not be generated\n")
251+
assert_equal("related_link for Api::Secret::PostResource.author(BelongsToOne) could not be generated\n", err)
252252

253253
# should only warn once
254254
builder = JSONAPI::LinkBuilder.new(config)

0 commit comments

Comments
 (0)