Skip to content

Commit

Permalink
[Refactor] Remove OpenStruct usage
Browse files Browse the repository at this point in the history
Also added more test cases for conditional attributes.
  • Loading branch information
okuramasafumi committed Jan 18, 2025
1 parent 523251d commit e9c9713
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 27 deletions.
10 changes: 0 additions & 10 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,6 @@ Minitest/NoTestCases:
Exclude:
- 'test/dependencies/test_dependencies.rb'

# We need to use `OpenStruct` to wrap object in ConfitionalAttribute
Performance/OpenStruct:
Exclude:
- 'lib/alba/conditional_attribute.rb'

# We need to eval resource code to test errors on resource classes
Security/Eval:
Exclude:
Expand Down Expand Up @@ -145,11 +140,6 @@ Style/MethodCallWithArgsParentheses:
Style/MissingElse:
EnforcedStyle: case

# We need to use `OpenStruct` to wrap object in ConfitionalAttribute
Style/OpenStructUse:
Exclude:
- 'lib/alba/conditional_attribute.rb'

# It's example code, please forgive us
Style/OptionalBooleanParameter:
Exclude:
Expand Down
2 changes: 0 additions & 2 deletions alba.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,4 @@ Gem::Specification.new do |spec|
spec.bindir = 'exe'
spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) }
spec.require_paths = ['lib']

spec.add_dependency "ostruct", "~> 0.6"
end
23 changes: 10 additions & 13 deletions lib/alba/conditional_attribute.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

require_relative 'association'
require_relative 'constants'
require 'ostruct'

module Alba
# Represents attribute with `if` option
Expand All @@ -26,7 +25,7 @@ def with_passing_condition(resource:, object: nil)
fetched_attribute = yield(@body)
return fetched_attribute unless with_two_arity_proc_condition

return Alba::REMOVE_KEY unless resource.instance_exec(object, objectize(fetched_attribute), &@condition)
return Alba::REMOVE_KEY unless resource.instance_exec(object, second_object(object), &@condition)

fetched_attribute
end
Expand All @@ -51,17 +50,15 @@ def with_two_arity_proc_condition
@condition.is_a?(Proc) && @condition.arity >= 2
end

# OpenStruct is used as a simple solution for converting Hash or Array of Hash into an object
# Using OpenStruct is not good in general, but in this case there's no other solution
def objectize(fetched_attribute)
return fetched_attribute unless @body.is_a?(Alba::Association)

if fetched_attribute.is_a?(Array)
fetched_attribute.map do |hash|
OpenStruct.new(hash)
end
else
OpenStruct.new(fetched_attribute)
def second_object(object)
case @body
when Symbol, Alba::Association, Alba::TypedAttribute
object.__send__(@body.name)
when Alba::NestedAttribute
nil
when Proc
@body.call(object)
else raise Alba::Error, "Unreachable code, @body is: #{@body.inspect}"
end
end
end
Expand Down
2 changes: 2 additions & 0 deletions lib/alba/typed_attribute.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ module Alba
# Representing typed attributes to encapsulate logic about types
# @api private
class TypedAttribute
attr_reader :name

# @param name [Symbol, String]
# @param type [Symbol, Class]
# @param converter [Proc]
Expand Down
71 changes: 69 additions & 2 deletions test/usecases/conditional_attributes_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class UserResource1 < UserResource
end

class UserResource2 < UserResource
attribute :username, if: proc { |user| user.id == 1 } do
attribute :username, if: proc { |user, _username| user.id == 1 } do
'username'
end
end
Expand Down Expand Up @@ -248,6 +248,43 @@ def test_conditional_attributes_with_if_and_resource_method
)
end

class UserResource14 < UserResource
one :profile, resource: ProfileResource, key: :my_profile, if: proc { |_user, profile| profile.email.end_with?('com') }
end

def test_conditional_one_with_key_option
assert_equal(
'{"id":1,"my_profile":{"email":"[email protected]"}}',
UserResource14.new(@user).serialize
)
user = User.new(2, 'Foo')
profile = Profile.new(2, '[email protected]')
user.profile = profile
assert_equal(
'{"id":2}',
UserResource14.new(user).serialize
)
end

class UserResource15 < UserResource
many :articles,
proc { |articles| articles }, # dummy
resource: ArticleResource,
if: proc { |_user, articles| !articles.empty? }
end

def test_conditional_many_with_condition_proc
assert_equal(
'{"id":1,"articles":[{"title":"Hello World!"}]}',
UserResource15.new(@user).serialize
)
user = User.new(2, 'Foo')
assert_equal(
'{"id":2}',
UserResource15.new(user).serialize
)
end

class Foo
attr_reader :id, :name

Expand All @@ -262,7 +299,7 @@ class FooResource

attributes :id

nested :bar, if: proc { params[:flag] } do
nested :bar, if: proc { |foo, _| params[:flag] || foo.id == 42 } do # dummy second parameter to test it doesn't raise an exception
attributes :name
attribute :baz do
'baz'
Expand All @@ -280,6 +317,11 @@ def test_conditional_attributes_with_nested_attributes
'{"id":1}',
FooResource.new(foo, params: {flag: false}).serialize
)
foo42 = Foo.new(42, 'name')
assert_equal(
'{"id":42,"bar":{"name":"name","baz":"baz"}}',
FooResource.new(foo42, params: {flag: false}).serialize
)
end

class FooTypedResource
Expand Down Expand Up @@ -323,4 +365,29 @@ def test_conditional_attributes_with_type_and_resource_method
FooTypedResource2.new(foo, params: {flag: false}).serialize
)
end

class FooTypedResource3
include Alba::Resource

attributes id: Integer
attributes name: :String, if: proc { |foo, name| foo.id == 1 || name == 'my name' }
end

def test_conditional_attributes_with_type_with_two_arity_condition
foo1 = Foo.new(1, 'name')
assert_equal(
'{"id":1,"name":"name"}',
FooTypedResource3.new(foo1).serialize
)
foo2 = Foo.new(2, 'name')
assert_equal(
'{"id":2}',
FooTypedResource3.new(foo2).serialize
)
foo3 = Foo.new(3, 'my name')
assert_equal(
'{"id":3,"name":"my name"}',
FooTypedResource3.new(foo3).serialize
)
end
end

0 comments on commit e9c9713

Please sign in to comment.