Skip to content

Commit b2a7b90

Browse files
olivier-thatcholivier-thatch
authored
Make entity fields required by default (#81)
This PR changes how grape-swagger-entity determines if an entity field is required or not: 1. if `documentation: { required: true/false }` is set, use that 2. otherwise, if `if` or `unless` is not null or if `expose_nil` is `false`, assume the parameter is not required 3. otherwise, assume the parameter is required This is consistent with how grape-entity renders fields: fields are always exposed unless `if` or `unless` is provided (cf. https://github.com/ruby-grape/grape-entity?tab=readme-ov-file#conditional-exposure). Closes #80. This is probably a breaking change, so I understand if you don't want to merge this as is, but it would be nice to be able to opt into this behavior.
1 parent 40b1908 commit b2a7b90

File tree

4 files changed

+86
-41
lines changed

4 files changed

+86
-41
lines changed

‎.rubocop_todo.yml‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# This configuration was generated by
22
# `rubocop --auto-gen-config`
3-
# on 2024-09-09 22:35:29 UTC using RuboCop version 1.66.1.
3+
# on 2025-05-12 21:32:47 UTC using RuboCop version 1.75.5.
44
# The point is for the user to remove these configuration records
55
# one by one as the offenses are removed from the code base.
66
# Note that changes in the inspected code, or installation of new
@@ -44,7 +44,7 @@ Metrics/AbcSize:
4444
# Offense count: 2
4545
# Configuration parameters: CountComments, CountAsOne.
4646
Metrics/ClassLength:
47-
Max: 112
47+
Max: 117
4848

4949
# Offense count: 2
5050
# Configuration parameters: AllowedMethods, AllowedPatterns.
@@ -64,7 +64,7 @@ Metrics/PerceivedComplexity:
6464
# Offense count: 5
6565
# Configuration parameters: EnforcedStyle, CheckMethodNames, CheckSymbols, AllowedIdentifiers, AllowedPatterns.
6666
# SupportedStyles: snake_case, normalcase, non_integer
67-
# AllowedIdentifiers: capture3, iso8601, rfc1123_date, rfc822, rfc2822, rfc3339, x86_64
67+
# AllowedIdentifiers: TLS1_1, TLS1_2, capture3, iso8601, rfc1123_date, rfc822, rfc2822, rfc3339, x86_64
6868
Naming/VariableNumber:
6969
Exclude:
7070
- 'spec/grape-swagger/entities/response_model_spec.rb'
@@ -100,9 +100,9 @@ RSpec/DescribeClass:
100100
# Offense count: 4
101101
# Configuration parameters: CountAsOne.
102102
RSpec/ExampleLength:
103-
Max: 187
103+
Max: 213
104104

105-
# Offense count: 24
105+
# Offense count: 26
106106
RSpec/LeakyConstantDeclaration:
107107
Exclude:
108108
- 'spec/grape-swagger/entities/response_model_spec.rb'
@@ -118,14 +118,14 @@ RSpec/MultipleDescribes:
118118
RSpec/MultipleExpectations:
119119
Max: 11
120120

121-
# Offense count: 20
121+
# Offense count: 21
122122
# Configuration parameters: EnforcedStyle, IgnoreSharedExamples.
123123
# SupportedStyles: always, named_only
124124
RSpec/NamedSubject:
125125
Exclude:
126126
- 'spec/grape-swagger/entities/response_model_spec.rb'
127127

128-
# Offense count: 39
128+
# Offense count: 46
129129
# Configuration parameters: AllowedGroups.
130130
RSpec/NestedGroups:
131131
Max: 5

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#### Features
44

5+
* [#81](https://github.com/ruby-grape/grape-swagger-entity/pull/81): Make entity fields required by default - [@olivier-thatch](https://github.com/olivier-thatch).
56
* Your contribution here.
67

78
#### Fixes

‎lib/grape-swagger/entity/parser.rb‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,15 @@ def parse_nested(entity_name, entity_options, parent_model = nil)
135135
end
136136

137137
def required_params(params)
138-
params.select { |_, options| options.fetch(:documentation, {}).fetch(:required, false) }
139-
.map { |(key, options)| [options.fetch(:as, key), options] }
140-
.map(&:first)
138+
params.each_with_object(Set.new) do |(key, options), accum|
139+
required = if options.fetch(:documentation, {}).key?(:required)
140+
options.dig(:documentation, :required)
141+
else
142+
!options.key?(:if) && !options.key?(:unless) && options[:expose_nil] != false
143+
end
144+
145+
accum.add(options.fetch(:as, key)) if required
146+
end.to_a
141147
end
142148

143149
def with_required(hash, required)

‎spec/grape-swagger/entities/response_model_spec.rb‎

Lines changed: 69 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ def app
4444
'properties' => {
4545
'code' => { 'type' => 'string', 'description' => 'Error code' },
4646
'message' => { 'type' => 'string', 'description' => 'Error message' }
47-
}
47+
},
48+
'required' => %w[code message]
4849
)
4950

5051
expect(subject['definitions'].keys).to include 'ThisApi_Entities_Something'
@@ -66,24 +67,32 @@ def app
6667
'code' => { 'type' => 'string', 'description' => 'Error code' },
6768
'message' => { 'type' => 'string', 'description' => 'Error message' },
6869
'attr' => { 'type' => 'string', 'description' => 'Attribute' } },
69-
'required' => ['attr']
70+
'required' => %w[text colors hidden_attr created_at kind kind2 kind3 tags relation
71+
attr code message]
7072
)
7173

7274
expect(subject['definitions'].keys).to include 'ThisApi_Entities_Kind'
7375
expect(subject['definitions']['ThisApi_Entities_Kind']).to eq(
74-
'type' => 'object', 'properties' => { 'title' => { 'type' => 'string', 'description' => 'Title of the kind.' },
75-
'content' => { 'description' => 'Content', 'type' => 'string',
76-
'x-some' => 'stuff' } }
76+
'type' => 'object',
77+
'properties' => {
78+
'title' => { 'type' => 'string', 'description' => 'Title of the kind.' },
79+
'content' => { 'type' => 'string', 'description' => 'Content', 'x-some' => 'stuff' }
80+
},
81+
'required' => %w[title content]
7782
)
7883

7984
expect(subject['definitions'].keys).to include 'ThisApi_Entities_Relation'
8085
expect(subject['definitions']['ThisApi_Entities_Relation']).to eq(
81-
'type' => 'object', 'properties' => { 'name' => { 'type' => 'string', 'description' => 'Name' } }
86+
'type' => 'object',
87+
'properties' => { 'name' => { 'type' => 'string', 'description' => 'Name' } },
88+
'required' => %w[name]
8289
)
8390

8491
expect(subject['definitions'].keys).to include 'ThisApi_Entities_Tag'
8592
expect(subject['definitions']['ThisApi_Entities_Tag']).to eq(
86-
'type' => 'object', 'properties' => { 'name' => { 'type' => 'string', 'description' => 'Name' } }
93+
'type' => 'object',
94+
'properties' => { 'name' => { 'type' => 'string', 'description' => 'Name' } },
95+
'required' => %w[name]
8796
)
8897
end
8998
end
@@ -134,8 +143,11 @@ class Nested < Grape::Entity
134143
end
135144
expose :nested_required do
136145
expose :some1, documentation: { required: true, desc: 'Required some 1' }
137-
expose :attr, as: :some2, documentation: { required: true, desc: 'Required some 2' }
138-
expose :some3, documentation: { desc: 'Optional some 3' }
146+
expose :attr, as: :some2, documentation: { desc: 'Required some 2' }
147+
expose :some3, documentation: { required: false, desc: 'Optional some 3' }
148+
expose :some4, if: -> { true }, documentation: { desc: 'Optional some 4' }
149+
expose :some5, unless: -> { true }, documentation: { desc: 'Optional some 5' }
150+
expose :some6, expose_nil: false, documentation: { desc: 'Optional some 6' }
139151
end
140152

141153
expose :nested_array, documentation: { type: 'Array', desc: 'Nested array' } do
@@ -235,7 +247,8 @@ def app
235247
'uuid' => { 'type' => 'string', 'format' => 'own', 'description' => 'customer uuid',
236248
'example' => 'e3008fba-d53d-4bcc-a6ae-adc56dff8020' },
237249
'color' => { 'type' => 'string', 'enum' => %w[red blue], 'description' => 'Color' }
238-
}
250+
},
251+
'required' => %w[guid uuid color]
239252
)
240253
expect(subject['TheseApi_Entities_Kind']).to eql(
241254
'type' => 'object',
@@ -244,30 +257,37 @@ def app
244257
'readOnly' => true },
245258
'title' => { 'type' => 'string', 'description' => 'Title of the kind.', 'readOnly' => false },
246259
'type' => { 'type' => 'string', 'description' => 'Type of the kind.', 'readOnly' => true }
247-
}
260+
},
261+
'required' => %w[id title type]
248262
)
249263
expect(subject['TheseApi_Entities_Tag']).to eql(
250-
'type' => 'object', 'properties' => { 'name' => { 'type' => 'string', 'description' => 'Name',
251-
'example' => 'random_tag' } }
264+
'type' => 'object',
265+
'properties' => { 'name' => { 'type' => 'string', 'description' => 'Name', 'example' => 'random_tag' } },
266+
'required' => %w[name]
252267
)
253268
expect(subject['TheseApi_Entities_Relation']).to eql(
254-
'type' => 'object', 'properties' => { 'name' => { 'type' => 'string', 'description' => 'Name' } }
269+
'type' => 'object',
270+
'properties' => { 'name' => { 'type' => 'string', 'description' => 'Name' } },
271+
'required' => %w[name]
255272
)
256273
expect(subject['TheseApi_Entities_Nested']).to eq(
274+
'type' => 'object',
257275
'properties' => {
258276
'nested' => {
259277
'type' => 'object',
260278
'properties' => {
261279
'some1' => { 'type' => 'string', 'description' => 'Nested some 1' },
262280
'some2' => { 'type' => 'string', 'description' => 'Nested some 2' }
263281
},
264-
'description' => 'Nested entity'
282+
'description' => 'Nested entity',
283+
'required' => %w[some1 some2]
265284
},
266285
'aliased' => {
267286
'type' => 'object',
268287
'properties' => {
269288
'some1' => { 'type' => 'string', 'description' => 'Alias some 1' }
270-
}
289+
},
290+
'required' => %w[some1]
271291
},
272292
'deep_nested' => {
273293
'type' => 'object',
@@ -277,17 +297,22 @@ def app
277297
'properties' => {
278298
'level_2' => { 'type' => 'string', 'description' => 'Level 2' }
279299
},
280-
'description' => 'More deepest nested entity'
300+
'description' => 'More deepest nested entity',
301+
'required' => %w[level_2]
281302
}
282303
},
283-
'description' => 'Deep nested entity'
304+
'description' => 'Deep nested entity',
305+
'required' => %w[level_1]
284306
},
285307
'nested_required' => {
286308
'type' => 'object',
287309
'properties' => {
288310
'some1' => { 'type' => 'string', 'description' => 'Required some 1' },
289311
'some2' => { 'type' => 'string', 'description' => 'Required some 2' },
290-
'some3' => { 'type' => 'string', 'description' => 'Optional some 3' }
312+
'some3' => { 'type' => 'string', 'description' => 'Optional some 3' },
313+
'some4' => { 'type' => 'string', 'description' => 'Optional some 4' },
314+
'some5' => { 'type' => 'string', 'description' => 'Optional some 5' },
315+
'some6' => { 'type' => 'string', 'description' => 'Optional some 6' }
291316
},
292317
'required' => %w[some1 some2]
293318
},
@@ -298,14 +323,16 @@ def app
298323
'properties' => {
299324
'id' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'Collection element id' },
300325
'name' => { 'type' => 'string', 'description' => 'Collection element name' }
301-
}
326+
},
327+
'required' => %w[id name]
302328
},
303329
'description' => 'Nested array'
304330
}
305331
},
306-
'type' => 'object'
332+
'required' => %w[nested aliased deep_nested nested_required nested_array]
307333
)
308334
expect(subject['TheseApi_Entities_NestedChild']).to eq(
335+
'type' => 'object',
309336
'properties' => {
310337
'nested' => {
311338
'type' => 'object',
@@ -314,14 +341,16 @@ def app
314341
'some2' => { 'type' => 'string', 'description' => 'Nested some 2' },
315342
'some3' => { 'type' => 'string', 'description' => 'Nested some 3' }
316343
},
317-
'description' => 'Nested entity'
344+
'description' => 'Nested entity',
345+
'required' => %w[some1 some2 some3]
318346
},
319347
'aliased' => {
320348
'type' => 'object',
321349
'properties' => {
322350
'some1' => { 'type' => 'string', 'description' => 'Alias some 1' },
323351
'some2' => { 'type' => 'string', 'description' => 'Alias some 2' }
324-
}
352+
},
353+
'required' => %w[some1 some2]
325354
},
326355
'deep_nested' => {
327356
'type' => 'object',
@@ -337,20 +366,26 @@ def app
337366
'description' => 'Level 3'
338367
}
339368
},
340-
'description' => 'Level 2'
369+
'description' => 'Level 2',
370+
'required' => %w[level_3]
341371
}
342372
},
343-
'description' => 'More deepest nested entity'
373+
'description' => 'More deepest nested entity',
374+
'required' => %w[level_2]
344375
}
345376
},
346-
'description' => 'Deep nested entity'
377+
'description' => 'Deep nested entity',
378+
'required' => %w[level_1]
347379
},
348380
'nested_required' => {
349381
'type' => 'object',
350382
'properties' => {
351383
'some1' => { 'type' => 'string', 'description' => 'Required some 1' },
352384
'some2' => { 'type' => 'string', 'description' => 'Required some 2' },
353-
'some3' => { 'type' => 'string', 'description' => 'Optional some 3' }
385+
'some3' => { 'type' => 'string', 'description' => 'Optional some 3' },
386+
'some4' => { 'type' => 'string', 'description' => 'Optional some 4' },
387+
'some5' => { 'type' => 'string', 'description' => 'Optional some 5' },
388+
'some6' => { 'type' => 'string', 'description' => 'Optional some 6' }
354389
},
355390
'required' => %w[some1 some2]
356391
},
@@ -362,12 +397,13 @@ def app
362397
'id' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'Collection element id' },
363398
'name' => { 'type' => 'string', 'description' => 'Collection element name' },
364399
'category' => { 'type' => 'string', 'description' => 'Collection element category' }
365-
}
400+
},
401+
'required' => %w[id name category]
366402
},
367403
'description' => 'Nested array'
368404
}
369405
},
370-
'type' => 'object'
406+
'required' => %w[nested aliased deep_nested nested_required nested_array]
371407
)
372408
expect(subject['TheseApi_Entities_Polymorphic']).to eql(
373409
'type' => 'object',
@@ -380,14 +416,15 @@ def app
380416
)
381417

382418
expect(subject['TheseApi_Entities_MixedType']).to eql(
419+
'type' => 'object',
383420
'properties' => {
384421
'tags' => {
385422
'description' => 'Tags',
386423
'items' => { '$ref' => '#/definitions/TheseApi_Entities_TagType' },
387424
'type' => 'array'
388425
}
389426
},
390-
'type' => 'object'
427+
'required' => %w[tags]
391428
)
392429

393430
expect(subject['TheseApi_Entities_SomeEntity']).to eql(
@@ -414,7 +451,8 @@ def app
414451
},
415452
'attr' => { 'type' => 'string', 'description' => 'Attribute' }
416453
},
417-
'required' => %w[attr],
454+
'required' => %w[text kind kind2 kind3 tags relation values nested nested_child
455+
polymorphic mixed attr code message],
418456
'description' => 'TheseApi_Entities_SomeEntity model'
419457
)
420458
end

0 commit comments

Comments
 (0)