Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

settings can not use all objects when having a subclass and . . . #21

Open
mkristian opened this issue Jan 18, 2013 · 4 comments
Open

Comments

@mkristian
Copy link

Cuba.settings[ :state ] = Proc.new { "ok" }
class A < Cuba; end

just does not work. some with singletons, lambdas, methods, etc

I solved the problem for myself with this plugin
https://github.com/mkristian/cuba-api/blob/master/lib/cuba_api/config.rb
which does implement a configuration with inheritance on class level:

Cuba.plugin CubaApi::Config
Cuba[ :state ] = Proc.new { "ok" }
class A < Cuba; end

and the value of can be changed later and A will see the change as well.

Cuba.plugin CubaApi::Config
Cuba[ :state ] = "ok"
Cuba.settings[ :state ] = "ok"
class A < Cuba; end
Cuba[ :state ] = "not-ok"
Cuba.settings[ :state ] = "not-ok"
p A[:state]
> "not-ok"
p A.settings[:state]
> "ok"

making a deep copy during class instantiation is also tricky since order of when the class gets defined does matter.

class A < Cuba; end
Cuba.settings[ :state ] = "ok"
class B < Cuba; end
p A.settings[:state]
> nil
p B.settings[:state]
> "ok"

although A and B are essentially the same class but were defined at different states of Cuba. having those A and B in their own files and require them makes it even harder to understand what is going on

require 'a'
Cuba.settings[ :state ] = "ok"
require 'b'
p A.settings[:state]
> nil
p B.settings[:state]
> "ok"

again using the cuby_api/config plugin makes the code more intuitive.

require 'a'
Cuba[ :state ] = "ok"
require 'b'
p A[:state]
> "ok"
p B[:state]
> "ok"

since intuition varies from person to person I would like to know other opinions.

BTW I would happy to prepare pull request to get something like cuby_api/config into Cuba and deprecate the current settings.

finally need to mention that I do like this little cuba framework a lot - I am about to switch my rails-api server to cuba/cuba_api and the new setup is so much easier to understand. I can understand the whole lot (including cuba) by reading half an hour code.

@tak1n
Copy link

tak1n commented Aug 12, 2014

Got the same "problem".

Consider following example:

require 'cuba'
require 'cuba/render'
require 'erb'

class TestController < Cuba
  define do
    on root do
      res.write view("home")
    end
  end
end

# TestController is in own file (controllers/test_controller.rb)
# Dir['controllers/*.rb'].each { |file| require |file|

Cuba.plugin Cuba::Render

Cuba.define do
  on root do
    run TestController
  end
end

run Cuba

This results in

NoMethodError: undefined method `[]' for nil:NilClass
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba/render.rb:19:in `view'
        /home/benny/Web/ruby/config.ru:8:in `block (2 levels) in <class:TestController>'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:182:in `block in on'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:201:in `try'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:163:in `on'
        /home/benny/Web/ruby/config.ru:7:in `block in <class:TestController>'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:129:in `instance_eval'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:129:in `block in call!'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:128:in `catch'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:128:in `call!'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:113:in `call'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:76:in `call'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:337:in `run'
        /home/benny/Web/ruby/config.ru:17:in `block (3 levels) in <main>'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:182:in `block in on'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:201:in `try'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:163:in `on'
        /home/benny/Web/ruby/config.ru:16:in `block (2 levels) in <main>'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:129:in `instance_eval'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:129:in `block in call!'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:128:in `catch'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:128:in `call!'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:113:in `call'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/cuba-3.2.0/lib/cuba.rb:76:in `call'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/rack-1.5.2/lib/rack/lint.rb:49:in `_call'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/rack-1.5.2/lib/rack/lint.rb:37:in `call'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/rack-1.5.2/lib/rack/showexceptions.rb:24:in `call'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/rack-1.5.2/lib/rack/commonlogger.rb:33:in `call'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/rack-1.5.2/lib/rack/chunked.rb:43:in `call'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/rack-1.5.2/lib/rack/content_length.rb:14:in `call'
        /home/benny/Web/ruby/cuba-content/.gem/ruby/2.1.2/gems/rack-1.5.2/lib/rack/handler/webrick.rb:60:in `service'
        /home/benny/.rubies/mri-2.1.2/lib/ruby/2.1.0/webrick/httpserver.rb:138:in `service'
        /home/benny/.rubies/mri-2.1.2/lib/ruby/2.1.0/webrick/httpserver.rb:94:in `run'
        /home/benny/.rubies/mri-2.1.2/lib/ruby/2.1.0/webrick/server.rb:295:in `block in start_thread'

If I require / define my controller after Cuba.plugin Cuba::Render everything works as expected.
This because in cuba/render https://github.com/soveran/cuba/blob/master/lib/cuba/render.rb#L6-L10 app.settings where app = Cuba are defined. So if I define a subclass of Cuba before the settings are set this will result in an error.

britishtea added a commit to britishtea/cuba that referenced this issue Dec 3, 2014
…an#21)

This fixes an issue where settings added to Cuba.settings *after* inheriting from
`Cuba` are not picked up by the child classes. The issue is described in issue

Settings are lookup up in the settings Hash of the parent class if a setting does
not exist using `Hash#default_proc`. The settings Hash is still deepcloned, so
they're still overridable.
britishtea added a commit to britishtea/cuba that referenced this issue Dec 3, 2014
…an#21)

This fixes an issue where settings are added to Cuba.settings *after* inheriting
from `Cuba` are not picked up by the child classes. The problem is described in
issue soveran#21.

If a setting does not exist, Settings are looked up in the settings Hash of the parent class using `Hash#default_proc`. The settings Hash is still deepcloned, so
settings are still overridable.
@soveran
Copy link
Owner

soveran commented Jan 15, 2016

I think we have two issues here: one is the fact that by using Marshal.dump, we lose the ability to copy procs, lambdas, and in general any object that can't be serialized. The second issue is the fact that the settings in the child class are a copy of the settings in the parent class at the time of subclassing.

I don't know a solution for the first problem. If the settings are not copied, the risk is that a child class could modify a setting in a parent class. By using Marshal, we make sure nested data structures are copied and no references are shared. If we lookup the keys in the parent's settings and expose the values directly, we are sharing a reference that could be altered for all instances, and that's something we need to avoid. That means we have to pick the lesser evil: or we share references, or we avoid the serialization of anything that Marshal.dump can't handle. I vote for the status quo in that regard, as using hashes, arrays, and other mutable data structures is a common use case, and using procs and lambdas in the settings is uncommon. If we don't find a better serializer for this use case (or, in fact, any proper way of deep cloning any values, including procs), the best we can do is to document this limitation.

About the second problem, I think we can do better if we determine that the order in which the settings are defined should not determine which settings are copied. I have already an implementation that solves that problem, so all we need to do is to think hard and decide which would be the proper behavior: do we want the settings to be a snapshot of the parent's settings at the moment of subclassing, or we want the settings to be copied at the time of reading them in the child class?

@mkristian @tak1n I'd love to read your comments about this.

@jmrepetti
Copy link

Just curious about this. Any update on how to inherit the settings? I see settings as a part of a booting or setup step where we set Cuba settings in the beginning.
In which situations is convenient to change Cuba settings somewhere after subclassing it? We should be able to take care of the loading order of the files.
Just to understand the requirement, If someone change Cuba(class) settings, subsclasses should be updated with the new parent settings and keep their own settings(if any)?

@britishtea
Copy link

@soveran I think taking a copy at the moment of subclassing is cleanest. It would avoid situations where a cuba app tries to use a setting that isn't guaranteed to be there.

Consider the following:

a.rb

class A < Cuba
  define do
    on(default) { a.settings[:foo].upcase }
  end
end

**b.rb**
```ruby
require "a"

class B < A
  define do
    on(default) { A.settings[:foo] }
  end
end

app.rb

require "b"

A.settings[:foo] = "only now defined"

Cuba.define do
  on("a") { run A }
  on("b") { run B }
end

B now breaks unless A.settings[:foo] is set. It isn't obvious that this is required when using B "stand alone".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants