-
Notifications
You must be signed in to change notification settings - Fork 18
Improving the Operation Syntax
This syntax seems ugly:
class Announcement < Hyperloop:ServerOp
param :acting_user
param :receiver
param :message
def validate
raise Hyperloop::AccessViolation unless params.acting_user.admin?
params.receiver = User.find_by_login(params.receiver)
end
dispatch_to { params.receiver }
end
why? Its inconsistent... we have a mix of macro declarations (param and dispatch_to) and methods (validate, execute). This is not only internally inconsistent, but also inconsistent with the Components class.
Its also makes validate
problematic. If I going to create a base class Operation (like AdminOperation) which has a validate method, then I really want my subclasses to also run my validate method (contrary to normal ruby inheritence rules)
class AdminOp < Hyperloop::ServerOp
param :acting_user
def validate
# I hope my subclasses remember to call super, maybe ServerOp baseclass
# will make sure this happens?
raise Hyperloop::AccessViolation unless params.acting_user.admin?
end
end
class Announcement < AdminOp
param :receiver
param :message
def validate
# i hope Hyperloop::ServerOp calls super for me????
params.receiver = User.find_by_login(params.receiver)
end
dispatch_to { params.receiver }
end
It is quite possible to make sure that all superclass validate methods will get called, but it seems in bad taste. As a developer I don't want to have to "remember" that this validate method works specially.
To solve these problems I am thinking of using some more stuff stolen from Trailblazer 2.0
class AdminOp < Hyperloop::ServerOp
param :acting_user
validate { raise Hyperloop::AccessViolation unless params.acting_user.admin? }
end
class Announcement < Hyperloop::ServerOp
param :receiver
param :message
validate { params.receiver = User.find_by_login(params.receiver) }
dispatch { params.receiver }
end
``
Likewise we use the trailblazer `step` in place of execute,
```ruby
class DeleteUser < AdminOp
param :user
validate { params.user = User.find_by_login(params.user) } # user exists
validate { params.user != params.acting_user } # can't delete myself
validate { !params.user.admin? || AdminUser.count == 1 } # can't delete last admin user
step { SendDeleteMailNotice(params) }
step { param.user.delete }
dispatch { params.user } # if user is logged in, we can log em off
end
which cleans up promises nicely on the client side:
Instead of
def execute
CreateCloudTempUrl(file_name: params.file_name).then do |url, uuid|
HTTP.put(url, params_for_put).then do
CopyFromTempUrl(file_name: uuid)
end
end
end
we can say
step { CreateCloudTempUrl(file_name: params.file_name) }
step { |url, uuid| HTTP.put(url, params_for_put).then { uuid } }
step { |uuid| CopyFromTempUrl(file_name: uuid) }
and now def self.execute
becomes step scope: :class
which is beautifully consistent with store syntax:
class GetRandomUserOperation < Hyperloop::Operation
def self.execute
return @users.delete_at(rand(@users.length)) unless @users.blank?
@promise = get_more_users.then do |users|
@users = users
end if @promise.nil? || @promise.resolved?
@promise.then { execute }
end
end
class GetRandomUserOperation < Hyperloop::Operation
step scope: :class do
exit @users.delete_at(rand(@users.length)) unless @users.blank?
@promise = get_more_users.then do |users|
@users = users
end if @promise.nil? || @promise.resolved?
@promise.then { execute }
end
end