crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.42k stars 1.62k forks source link

Allow Proc without specifying return type stored in a Hash #5039

Closed ansarizafar closed 7 years ago

ansarizafar commented 7 years ago

Currently, It is not possible to store a proc without specifying return type in a Hash but sometimes return type is not known in advance

@@store = {} of String => (Hash(String, JSON::Type) -> _ )

In my use case, I am generating return value from fields returning from a database query. Depending on the query return value would be different for each proc.

asterite commented 7 years ago

No, sorry. Crystal is not dynamic.

I've seen the conversation in IRC/Gitter. You need to code in a less dynamic way. You can for example use case over the database return value and invoke different functions.

asterite commented 7 years ago

But I'll leave this open. Maybe you can share some code and we can help you achieve what you want.

ansarizafar commented 7 years ago

I am a Nodejs developer and really excited about amazing Crystal. I am trying to port my Graphql inspired data layer from Nodejs to Crystal (RPC over http) with a single endpoint, request batching, Role-based authorization and parameters validation support.

Ideally, I wanted to store a proc without specifying a return type in a Hash but since it's not possible I am using a different approach. Here is my current code.

 app.post "/api" do |context|
     Api.run(context)   
  end

 abstract class Method
 end

class Api
    @@store = {} of String => Method

    def self.register(name : String, method : Method)
      @@store[name] = method
    end

    def self.run(context : HTTP::Server::Context)
      begin
        data = Tools.json_param(context)
        finalResult = data.map do |key, value|
          {key => @@store[key].list(value.as(Hash))}
        end

        return finalResult.to_json
      rescue ex
        # context.response.respond_with_error("Internal error", 500)
        "Internal error in Method"
      end
    end
  end
end

class Users < Method
  def list(args)
    begin
      count = Db.scalar "select count(*) from restock.users"
      data = [] of {id: String, name: String}

      Db.query "select id, name from restock.users" do |rs|
        rs.each do
          id, name = rs.read(String, String)
          data << {id: id, name: name}
        end
      end

      {success: true, data: data}
    rescue ex
      {success: false, data: [] of {id: String, name: String}}
    end
  end
end

Api.register "user.list", Users.new

Here is how I am calling RPC from the client.

image

Now I want to add more methods to User class and use it like a controller. My question is How can I store individual methods with a different key in @@store hash and later retrieve and call the method in Api.run.

asterite commented 7 years ago

You can do:

case key
when "user.list"
  User.list(...)
when "other.thing"
  Other.thing(...)
else
  raise "Unknown method"
end

There's no need to store procs, or even define classes like User for this.

Also, you might want to implement something like a REST API, so you can GET /users/56456, which is much more intuitive and common/standarized.

ansarizafar commented 7 years ago

There can be many classes like User class one for each entity that is why case/when solution will not work. With standard rest Apis we need to create many different endpoints and no request batching support is available which results in multiple roundtrips that is why Graphql is becoming so popular especially for mobile apps check this amazing Nodejs project https://www.graphile.org/.

hugoabonizio commented 7 years ago

@ansarizafar have you already looked at graphql-crystal shard?

asterite commented 7 years ago

@ansarizafar I don't understand, if there are many classes in your code you will have many register methods. In my case I will have many when branches. Isn't it the same?

ansarizafar commented 7 years ago

Yes. It's slower than Facebook implementation. I am trying to switch from Nodejs to Crystal for better performance, memory and CPU usage.

ansarizafar commented 7 years ago

There will be only one API class and register method but There can be many classes like User (For each entity like products, Customers, Orders etc.) User class will group all methods related to Users.

asterite commented 7 years ago

What about something like this:

class Registry
  REGISTRY = [] of Nil

  macro register(method, klass)
    {% REGISTRY << {method, klass} %}
  end

  macro finished
    def self.process(name, args)
      case name
        {% for thing in REGISTRY %}
          when {{thing[0]}}
            {{thing[1]}}(args)
        {% end %}
      else
        raise "Unknown method: #{name}"
      end
    end
  end
end

class Users
  def self.list(args)
    puts "Users.list(#{args})"
  end
end

class Projects
  def self.list(args)
    puts "Projects.list(#{args})"
  end
end

Registry.register "user.list", Users.list
Registry.register "project.list", Projects.list

Registry.process("user.list", {"id" => 1})
Registry.process("project.list", {"id" => 2})
ansarizafar commented 7 years ago

@asterite Brilliant solution, macros are so powerful. I should try to learn macros first. Thanks a lot. What about this

Registry.registerAll User

and the regitserAll macro registers all User class methods automatically. Dynamic coding.

asterite commented 7 years ago

Cool!

I think the macro finished is not documented, but it's explained here: https://github.com/crystal-lang/crystal/pull/3612

I'll close this for now.

asterite commented 7 years ago

Yes, you can do something like that. But it's getting more and more obscure:

class Registry
  REGISTRY = [] of Nil

  macro register(path)
    {% REGISTRY << path.resolve %}
  end

  macro finished
    def self.process(name, args)
      case name
        {% for klass in REGISTRY %}
          {% for method in klass.class.methods %}
            {% if method.name != "allocate" %}
              when "{{klass.name.downcase}}.{{method.name}}"
                {{klass}}.{{method.name}}(args)
            {% end %}
          {% end %}
        {% end %}
      else
        raise "Unknown method: #{name}"
      end
    end
  end
end

class User
  def self.list(args)
    puts "Users.list(#{args})"
  end

  def self.other(args)
    puts "Users.other(#{args})"
  end
end

class Project
  def self.list(args)
    puts "Projects.list(#{args})"
  end
end

Registry.register User
Registry.register Project

Registry.process("user.list", {"id" => 1})
Registry.process("user.other", {"id" => 1})
Registry.process("project.list", {"id" => 2})
hugoabonizio commented 7 years ago
macro register_all(klass)
  {% for method in klass.resolve.class.methods.map(&.name).reject { |m| m == "allocate" } %}
    Registry.register("{{ klass }}.{{ method }}".downcase, {{ klass }}.{{ method }})
  {% end %}
end

https://carc.in/#/r/2suv :smile:

ansarizafar commented 7 years ago

Awesome, But Ideally, We should be able to code anything with the language only, without requiring a macro. It would be difficult to learn a new language and a DSL to code common scenarios for developers trying to switch from other languages. Is it possible to register a proc block with register macro?

Registry.register "user.list" do |args|
  begin
    puts args
    count = Db.scalar "select count(*) from restock.users"
    data = [] of {id: String, name: String}
    Db.query "select id, name from restock.users" do |rs|
      rs.each do
        id, name = rs.read(String, String)
        data << {id: id, name: name}
      end
    end
    {success: true, data: data}
  rescue ex
    {success: false, data: [] of {id: String, name: String}}
  end
end
asterite commented 7 years ago

You can probably do something like this:

class Registry
  REGISTRY = [] of Nil

  macro register(name)
    {% REGISTRY << name %}

    class Registry
      def self.{{name.tr(".", "").id}}(args)
        {{yield}}
      end
    end
  end

  macro finished
    def self.process(name, args)
      case name
        {% for name in REGISTRY %}
          when {{name}}
            {{name.tr(".", "").id}}(args)
        {% end %}
      else
        raise "Unknown method: #{name}"
      end
    end
  end
end

Registry.register "user.list" do |args|
  puts "user.list(#{args})"
end

Registry.register "user.other" do |args|
  puts "user.other(#{args})"
end

Registry.register "project.list" do |args|
  puts "project.list(#{args})"
end

Registry.process("user.list", {"id" => 1})
Registry.process("user.other", {"id" => 1})
Registry.process("project.list", {"id" => 2})

But there's currently no way to do it without macros. Procs need a type. And Object isn't supported yet. And even if it were supported there would be no way to turn any object to JSON.

ansarizafar commented 7 years ago

Getting this error

in macro 'finished' /home/zafar/crystal/server/src/webware/api.cr:16, line 5:

   1.       def self.process(name, args)
   2.         case name
   3.
   4.             when "user.list"
>  5.               userlist(args)
   6.
   7.         else
   8.           raise "Unknown method: #{name}"
   9.         end
  10.       end
  11.

undefined method 'userlist'
asterite commented 7 years ago

Works for me: https://carc.in/#/r/2t8r

ansarizafar commented 7 years ago

Here is my code.

module WebWare

class Registry
  REGISTRY = [] of Nil

    macro register(name)
      {% REGISTRY << name %}

      class Registry
        def self.{{name.tr(".", "").id}}(args)
          {{yield}}
        end
      end
    end

    macro finished
      def self.process(name, args)
        case name
          {% for name in REGISTRY %}
            when {{name}}
              {{name.tr(".", "").id}}(args)
          {% end %}
        else
          raise "Unknown method: #{name}"
        end
      end
    end

    def self.run(context : HTTP::Server::Context)
        begin
          data = Tools.json_param(context)
          finalResult = data.map do |key, value|
            {key => self.process(key, value.as(Hash)) }

          end

          return finalResult.to_json
        rescue ex
          # context.response.respond_with_error("Internal error", 500)
          "Internal error in Method"
        end
      end

  end

end

If I use

Registry.process("user.list", {"id" => 1})

Then got this error

in src/webware/methods/users.cr:34: undefined method 'process' for Registry:Class

Registry.process("user.list", {"id" => 1})
ansarizafar commented 7 years ago

@asterite Its working now. Thanks for your support.