Skip to content

Commit 55a5d96

Browse files
fix: issue where we cannot mutate arrays on base model derivatives
array properties are now always recursively coerced into the desire type upon being set, instead of "almost always" hash key names are no longer unnecessarily translated when creating base models via hash coercion errors are now stored and re-thrown instead of being re-computed each property access fixed inconsistencies where sometimes `TypeError`s would be thrown instead of `ArgumentError`s, and vice versa
1 parent 91ac3d1 commit 55a5d96

File tree

18 files changed

+299
-108
lines changed

18 files changed

+299
-108
lines changed

lib/onebusaway_sdk/errors.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,28 @@ class Error < StandardError
99
end
1010

1111
class ConversionError < OnebusawaySDK::Errors::Error
12+
# @return [StandardError, nil]
13+
def cause = @cause.nil? ? super : @cause
14+
15+
# @api private
16+
#
17+
# @param on [Class<StandardError>]
18+
# @param method [Symbol]
19+
# @param target [Object]
20+
# @param value [Object]
21+
# @param cause [StandardError, nil]
22+
def initialize(on:, method:, target:, value:, cause: nil)
23+
cls = on.name.split("::").last
24+
25+
message = [
26+
"Failed to parse #{cls}.#{method} from #{value.class} to #{target.inspect}.",
27+
"To get the unparsed API response, use #{cls}[#{method.inspect}].",
28+
cause && "Cause: #{cause.message}"
29+
].filter(&:itself).join(" ")
30+
31+
@cause = cause
32+
super(message)
33+
end
1234
end
1335

1436
class APIError < OnebusawaySDK::Errors::Error

lib/onebusaway_sdk/internal/type/array_of.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,14 @@ def hash = [self.class, item_type].hash
6262
#
6363
# @param state [Hash{Symbol=>Object}] .
6464
#
65-
# @option state [Boolean, :strong] :strictness
65+
# @option state [Boolean] :translate_names
66+
#
67+
# @option state [Boolean] :strictness
6668
#
6769
# @option state [Hash{Symbol=>Object}] :exactness
6870
#
71+
# @option state [Class<StandardError>] :error
72+
#
6973
# @option state [Integer] :branched
7074
#
7175
# @return [Array<Object>, Object]
@@ -74,6 +78,7 @@ def coerce(value, state:)
7478

7579
unless value.is_a?(Array)
7680
exactness[:no] += 1
81+
state[:error] = TypeError.new("#{value.class} can't be coerced into #{Array}")
7782
return value
7883
end
7984

lib/onebusaway_sdk/internal/type/base_model.rb

Lines changed: 77 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def fields
6060
[OnebusawaySDK::Internal::Type::Converter.type_info(type_info), type_info]
6161
end
6262

63-
setter = "#{name_sym}="
63+
setter = :"#{name_sym}="
6464
api_name = info.fetch(:api_name, name_sym)
6565
nilable = info.fetch(:nil?, false)
6666
const = if required && !nilable
@@ -84,28 +84,61 @@ def fields
8484
type_fn: type_fn
8585
}
8686

87-
define_method(setter) { @data.store(name_sym, _1) }
87+
define_method(setter) do |value|
88+
target = type_fn.call
89+
state = OnebusawaySDK::Internal::Type::Converter.new_coerce_state(translate_names: false)
90+
coerced = OnebusawaySDK::Internal::Type::Converter.coerce(target, value, state: state)
91+
status = @coerced.store(name_sym, state.fetch(:error) || true)
92+
stored =
93+
case [target, status]
94+
in [OnebusawaySDK::Internal::Type::Converter | Symbol, true]
95+
coerced
96+
else
97+
value
98+
end
99+
@data.store(name_sym, stored)
100+
end
88101

102+
# rubocop:disable Style/CaseEquality
103+
# rubocop:disable Metrics/BlockLength
89104
define_method(name_sym) do
90105
target = type_fn.call
91-
value = @data.fetch(name_sym) { const == OnebusawaySDK::Internal::OMIT ? nil : const }
92-
state = {strictness: :strong, exactness: {yes: 0, no: 0, maybe: 0}, branched: 0}
93-
if (nilable || !required) && value.nil?
94-
nil
95-
else
96-
OnebusawaySDK::Internal::Type::Converter.coerce(
97-
target, value, state: state
106+
107+
case @coerced[name_sym]
108+
in true | false if OnebusawaySDK::Internal::Type::Converter === target
109+
@data.fetch(name_sym)
110+
in ::StandardError => e
111+
raise OnebusawaySDK::Errors::ConversionError.new(
112+
on: self.class,
113+
method: __method__,
114+
target: target,
115+
value: @data.fetch(name_sym),
116+
cause: e
98117
)
118+
else
119+
Kernel.then do
120+
value = @data.fetch(name_sym) { const == OnebusawaySDK::Internal::OMIT ? nil : const }
121+
state = OnebusawaySDK::Internal::Type::Converter.new_coerce_state(translate_names: false)
122+
if (nilable || !required) && value.nil?
123+
nil
124+
else
125+
OnebusawaySDK::Internal::Type::Converter.coerce(
126+
target, value, state: state
127+
)
128+
end
129+
rescue StandardError => e
130+
raise OnebusawaySDK::Errors::ConversionError.new(
131+
on: self.class,
132+
method: __method__,
133+
target: target,
134+
value: value,
135+
cause: e
136+
)
137+
end
99138
end
100-
rescue StandardError => e
101-
cls = self.class.name.split("::").last
102-
message = [
103-
"Failed to parse #{cls}.#{__method__} from #{value.class} to #{target.inspect}.",
104-
"To get the unparsed API response, use #{cls}[#{__method__.inspect}].",
105-
"Cause: #{e.message}"
106-
].join(" ")
107-
raise OnebusawaySDK::Errors::ConversionError.new(message)
108139
end
140+
# rubocop:enable Metrics/BlockLength
141+
# rubocop:enable Style/CaseEquality
109142
end
110143

111144
# @api private
@@ -205,37 +238,44 @@ class << self
205238
#
206239
# @param state [Hash{Symbol=>Object}] .
207240
#
208-
# @option state [Boolean, :strong] :strictness
241+
# @option state [Boolean] :translate_names
242+
#
243+
# @option state [Boolean] :strictness
209244
#
210245
# @option state [Hash{Symbol=>Object}] :exactness
211246
#
247+
# @option state [Class<StandardError>] :error
248+
#
212249
# @option state [Integer] :branched
213250
#
214251
# @return [self, Object]
215252
def coerce(value, state:)
216253
exactness = state.fetch(:exactness)
217254

218-
if value.is_a?(self.class)
255+
if value.is_a?(self)
219256
exactness[:yes] += 1
220257
return value
221258
end
222259

223260
unless (val = OnebusawaySDK::Internal::Util.coerce_hash(value)).is_a?(Hash)
224261
exactness[:no] += 1
262+
state[:error] = TypeError.new("#{value.class} can't be coerced into #{Hash}")
225263
return value
226264
end
227265
exactness[:yes] += 1
228266

229267
keys = val.keys.to_set
230268
instance = new
231269
data = instance.to_h
270+
status = instance.instance_variable_get(:@coerced)
232271

233272
# rubocop:disable Metrics/BlockLength
234273
fields.each do |name, field|
235274
mode, required, target = field.fetch_values(:mode, :required, :type)
236275
api_name, nilable, const = field.fetch_values(:api_name, :nilable, :const)
276+
src_name = state.fetch(:translate_names) ? api_name : name
237277

238-
unless val.key?(api_name)
278+
unless val.key?(src_name)
239279
if required && mode != :dump && const == OnebusawaySDK::Internal::OMIT
240280
exactness[nilable ? :maybe : :no] += 1
241281
else
@@ -244,9 +284,10 @@ def coerce(value, state:)
244284
next
245285
end
246286

247-
item = val.fetch(api_name)
248-
keys.delete(api_name)
287+
item = val.fetch(src_name)
288+
keys.delete(src_name)
249289

290+
state[:error] = nil
250291
converted =
251292
if item.nil? && (nilable || !required)
252293
exactness[nilable ? :yes : :maybe] += 1
@@ -260,6 +301,8 @@ def coerce(value, state:)
260301
item
261302
end
262303
end
304+
305+
status.store(name, state.fetch(:error) || true)
263306
data.store(name, converted)
264307
end
265308
# rubocop:enable Metrics/BlockLength
@@ -438,7 +481,18 @@ def to_yaml(*a) = OnebusawaySDK::Internal::Type::Converter.dump(self.class, self
438481
# Create a new instance of a model.
439482
#
440483
# @param data [Hash{Symbol=>Object}, self]
441-
def initialize(data = {}) = (@data = OnebusawaySDK::Internal::Util.coerce_hash!(data).to_h)
484+
def initialize(data = {})
485+
@data = {}
486+
@coerced = {}
487+
OnebusawaySDK::Internal::Util.coerce_hash!(data).each do
488+
if self.class.known_fields.key?(_1)
489+
public_send(:"#{_1}=", _2)
490+
else
491+
@data.store(_1, _2)
492+
@coerced.store(_1, false)
493+
end
494+
end
495+
end
442496

443497
class << self
444498
# @api private

lib/onebusaway_sdk/internal/type/boolean.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,20 @@ def self.==(other) = other.is_a?(Class) && other <= OnebusawaySDK::Internal::Typ
3131
class << self
3232
# @api private
3333
#
34+
# Coerce value to Boolean if possible, otherwise return the original value.
35+
#
3436
# @param value [Boolean, Object]
3537
#
3638
# @param state [Hash{Symbol=>Object}] .
3739
#
38-
# @option state [Boolean, :strong] :strictness
40+
# @option state [Boolean] :translate_names
41+
#
42+
# @option state [Boolean] :strictness
3943
#
4044
# @option state [Hash{Symbol=>Object}] :exactness
4145
#
46+
# @option state [Class<StandardError>] :error
47+
#
4248
# @option state [Integer] :branched
4349
#
4450
# @return [Boolean, Object]

0 commit comments

Comments
 (0)