Rails core - Sep 2009 - 'columns' in Active Resource - proposed patch

So, one of the things I wanted to add to Active Resource is the
ability to have knowledge of expected ''columns''. ie a set of
things
that ARes knows are attributes of the given resource.

Why do I want this?
First let me explain the current situation.

Right now - ARes sets attributes based on whatever comes back from the
remote system when you do a fetch. This is fine and dandy and need not
be changed...  but what if we''re creating a brand new resource and
want to run local validations over it?

Say I''m implementing a flickr-image-submitting service. The flickr API
is known and published. Here''s an example that shows up the problem:


  class FlickrPhoto << ActiveResource::Base
    validates_presence_of :photo
  end

  # this works
  my_photo = FlickrPhoto.new(:photo => ''photo_stream_here'',
:title =>
''eben'', :tags => [''cat''])
  my_photo.valid? # => true
  my_photo.save   # => true
  my_photo.photo  # ''photo_stream_here''

  # this doesn''t
  my_photo = FlickrPhoto.new(:title => ''snooch'', :tags
=> [''cat'']) #
note: no photo
  my_photo.photo  # throws NoMethodError exception
  my_photo.valid? # throws NoMethodError exception
  my_photo.save   # throws NoMethodError exception

All the latter three don''t work because we''re calling
''photo'' which
doesn''t exist as a method on FlickrPhoto - even though we know in
advance (due to the published API) that photo is a required column.
This is pretty annoying when the validates_presence_of method should
be responding to exactly this situation...

We *could* overload validates_presence_of to respond to a
NoMethodException... but we''d probably also have to update errors.on
and the view-methods and various other locations... when all we really
want is to add something into method_missing to return ''nil''
if we
know it''s a column... but don''t have a value yet.

Active Record uses a ''columns'' method that is populated by
reflecting
on the db table... clearly Active Resource doesn''t have that luxury.

We could get all fancy and do a remote lookup on the remote system...
or we could just set the columns ourselves given that we know what
they should contain. eg:


  class FlickrPhoto << ActiveResource::Base
    columns = [:photo, :title, :description, :tags]
    validates_presence_of :photo
  end

  # now this does...
  my_photo = FlickrPhoto.new(:title => ''snooch'', :tags
=> [''cat'']) #
note: no photo
  my_photo.photo  # ''nil''
  my_photo.valid? # returns ''false'' with errors.on(:photo) ==
[''can''t
be blank'']
  my_photo.save   # returns ''false'' with errors.on(:photo) ==
[''can''t
be blank'']


The latter seems like the simplest solution and AFAICS will not
interfere with how ARes currently runs.

It also has a nice side-effect of allowing plugins that assume the
existence of the ''columns'' method to work with ARes as well.
The
plugins shouldn''t care that the columns come from the db-table or
otherwise...

I''ve provided a working proposed patch (below) to implement this and
would value opinion on a) the patch as is and b) any other options.

Note - this follows from discussion (and my original proposal) in the
following thread:
http://groups.google.com/group/rubyonrails-core/browse_thread/thread/2afb6dcb7c555eb1


Cheers,
Taryn


From 1cb4d7178f716d3c865ab8c36819c0f772fbfa01 Mon Sep 17 00:00:00 2001
From: taryn <teast@globalpersonals.co.uk>
Date: Tue, 1 Sep 2009 11:51:38 +0100
Subject: [PATCH] Added "columns" to Active Resource to stop
NoMethodError

Basic proposed idea for "columns" for Active Resource. All it does is
make
certain that any known columns return ''nil'' on access instead
of
raising a
NoMethodError - when they aren''t yet set by the remote system.

Columns can be set manually by a user by passing in an array of either
strings or symbols (they are converted to symbols internally).
This simply feeds method_missing - which will repsond with a nil
if an attribute has not been set but is a known column.

I''ve added base_tests for setting this attribute, and also
validations_tests
(because validates_presence_of requires an attribute to return
''nil''
instead
of raising an exception).

''columns'' is also an expected interface feature for a lot of
Active
Record
plugins. Adding this functionality to Active Resource means that it
will be
more easily interchangeable with AR.

At a later date we can consider funky things like remote lookup of
known
columns via a hit to the /new action - but for now - this is a
simple-but-solid foundation.
---
 activeresource/lib/active_resource/base.rb    |   51 +++++++++++++++++
+++++---
 activeresource/test/cases/base_test.rb        |   37 +++++++++++++++++
+
 activeresource/test/cases/validations_test.rb |   29 +++++++++++++-
 activeresource/test/fixtures/project.rb       |   19 ++++-----
 4 files changed, 118 insertions(+), 18 deletions(-)

diff --git a/activeresource/lib/active_resource/base.rb b/
activeresource/lib/active_resource/base.rb
index e5b8589..c14c16e 100644
--- a/activeresource/lib/active_resource/base.rb
+++ b/activeresource/lib/active_resource/base.rb
@@ -239,6 +239,40 @@ module ActiveResource
     cattr_accessor :logger

     class << self
+      # Accepts an array of column names that this Resource
recognises as
+      # known attributes.
+      #
+      # If the value of a known column has not been set and the value
is
+      # accessed - nil is returned instead of the usual +NoMethodError
+
+      # exception from +method_missing+
+      #
+      # This is especially important when you have a
+      # <tt>validates_presence_of</tt> validation on the column -
as
this
+      # relies on the attribute returning ''nil'' if it
hasn''t been set
and
+      # will not work if an exception is raised.
+      #
+      # example:
+      #   me = Person.new(:name => ''me'')
+      #   me.name # => ''me''
+      #   me.age  # => raises NoMethodError
+      #
+      #   Person.columns = [:name, :age]
+      #   you = Person.new(:name => ''you'')
+      #   you.name # => ''you''
+      #   you.age  # => ''nil''
+      #
+      # All attributes returned by the remote system will still be
stored on
+      # the object regardless of what is in this column set.
+      def columns=(cols)
+        @columns = cols.blank? ? [] : cols.map(&:to_sym)
+      end
+      # Returns the current set of known columns/attributes of the
Resource
+      # as specified by <tt>columns=</tt>
+      def columns
+        defined?(@columns) ? @columns : []
+      end
+
+
       # Gets the URI of the REST resources to map for this class.
The site variable is required for
       # Active Resource''s mapping to work.
       def site
@@ -1207,12 +1241,17 @@ module ActiveResource
         method_name = method_symbol.to_s

         case method_name.last
-          when "="
-            attributes[method_name.first(-1)] = arguments.first
-          when "?"
-            attributes[method_name.first(-1)]
-          else
-            attributes.has_key?(method_name) ? attributes
[method_name] : super
+        when "="
+          attributes[method_name.first(-1)] = arguments.first
+        when "?"
+          attributes[method_name.first(-1)]
+        else
+          # if we have this attribute - return it
+          return attributes[method_name] if attributes.has_key?
(method_name)
+          # if this is a known column but hasn''t been set - return
nil
+          return nil if self.class.columns.include?(method_symbol)
+          # otherwise defer upwards
+          super
         end
       end
   end
diff --git a/activeresource/test/cases/base_test.rb b/activeresource/
test/cases/base_test.rb
index 8c0217a..50c6195 100644
--- a/activeresource/test/cases/base_test.rb
+++ b/activeresource/test/cases/base_test.rb
@@ -103,6 +103,43 @@ class BaseTest < Test::Unit::TestCase
   end


+  def test_columns_accessor_accepts_array_of_syms
+    column_set = [:age, :name]
+
+    assert_nothing_raised { Person.columns = column_set }
+    assert_equal column_set, Person.columns
+  end
+
+  def test_columns_accessor_accepts_array_of_strings
+    column_set = [''name'', ''age'']
+    column_set_syms = column_set.map(&:to_sym)
+
+    assert_nothing_raised { Person.columns = column_set }
+    # should sort and sybolise them
+    assert_equal column_set_syms, Person.columns
+  end
+
+
+  def test_non_attribute_access_and_assignment_should_fail
+    me = Person.new
+    assert !me.respond_to?("my_unknown_column")
+    assert_raises(NoMethodError) { me.my_unknown_column }
+  end
+
+  def test_known_column_on_new_instance_should_return_nil
+    me = Person.new
+    assert !me.respond_to?("my_known_column") # sanity check
+
+    Person.columns = [:my_known_column]
+    assert_nothing_raised { assert_nil me.my_known_column }
+    # just for good measure - test we can set it too
+    assert_nothing_raised {
+      new_val = ''blah''
+      me.my_known_column = new_val
+      assert_equal new_val, me.my_known_column
+    }
+  end
+
   def test_site_accessor_accepts_uri_or_string_argument
     site = URI.parse(''http://localhost'')

diff --git a/activeresource/test/cases/validations_test.rb b/
activeresource/test/cases/validations_test.rb
index a8ab7d6..255fc6c 100644
--- a/activeresource/test/cases/validations_test.rb
+++ b/activeresource/test/cases/validations_test.rb
@@ -5,7 +5,7 @@ require "fixtures/project"
 # This test case simply makes sur that they are all accessible by
 # Active Resource objects.
 class ValidationsTest < ActiveModel::TestCase
-  VALID_PROJECT_HASH = { :name => "My Project", :description =>
"A
project" }
+  VALID_PROJECT_HASH = { :name => "My Project", :owner =>
''Bob'', :description => "A project",
:remote_column => ''something'' }
   def setup
     @my_proj = VALID_PROJECT_HASH.to_xml(:root => "person")
     ActiveResource::HttpMock.respond_to do |mock|
@@ -13,7 +13,7 @@ class ValidationsTest < ActiveModel::TestCase
     end
   end

-  def test_validates_presence_of
+  def test_validates_presence_of_known_column
     p = new_project(:name => nil)
     assert !p.valid?, "should not be a valid record without name"
     assert !p.save, "should not have saved an invalid record"
@@ -23,6 +23,31 @@ class ValidationsTest < ActiveModel::TestCase

     assert p.save, "should have saved after fixing the validation,
but had: #{p.errors.inspect}"
   end
+
+  def test_validates_presence_of_accessor_backed_attribute
+    p = new_project(:owner => nil)
+    assert !p.valid?, "should not be a valid record without owner"
+    assert !p.save, "should not have saved an invalid record"
+    assert_equal ["can''t be blank"], p.errors[:owner],
"should have
an error on name"
+
+    p.owner = "somebody"
+
+    assert p.save, "should have saved after fixing the validation,
but had: #{p.errors.inspect}"
+  end
+
+  def
test_validates_presence_of_unknown_column_should_raise_exception_if_not_present
+    p = Project.new(VALID_PROJECT_HASH.delete_if{|k,v| k
== :remote_column})
+    assert_raises(NoMethodError) {
+      assert !p.valid?, "should not be a valid record without
remote_column"
+    }
+    p.remote_column = "something"
+    assert_nothing_raised {
+      assert p.valid?, "should be a valid record with the remote
column set"
+      assert p.save, "should have saved after fixing the validation,
but had: #{p.errors.inspect}"
+    }
+
+  end
+

   def test_fails_save!
     p = new_project(:name => nil)
diff --git a/activeresource/test/fixtures/project.rb b/activeresource/
test/fixtures/project.rb
index e15fa6f..e800d39 100644
--- a/activeresource/test/fixtures/project.rb
+++ b/activeresource/test/fixtures/project.rb
@@ -1,8 +1,9 @@
 # used to test validations
 class Project < ActiveResource::Base
   self.site = "http://37s.sunrise.i:3000"
+  self.columns = [:name, :description]

-  validates_presence_of :name
+  validates_presence_of :name, :owner, :remote_column
   validate :description_greater_than_three_letters

   # to test the validate *callback* works
@@ -11,15 +12,13 @@ class Project < ActiveResource::Base
   end


-  # stop-gap accessor to default this attribute to nil
-  # Otherwise the validations fail saying that the method does not
exist.
-  # In future, method_missing will be updated to not explode on a
known
-  # attribute.
-  def name
-    attributes[''name''] || nil
-  end
-  def description
-    attributes[''description''] || nil
+  # This attribute isn''t in columns - so we need to fake up this nil-
ifying
+  # accessor function so that validates_presence_of does not explode
when
+  # the attribute hasn''t yet been set. Compare with the :name column
+  def owner
+    # this is clearly just a test-function, but you could, say, use
this to
+    # do a db lookup on the User table for the owner
+    attributes[''owner''] || nil
   end
 end

--
1.6.0.4

> I''ve provided a working proposed patch (below) to implement this
and
> would value opinion on a) the patch as is and b) any other options.
>
> Note - this follows from discussion (and my original proposal) in the
> following thread:
>
http://groups.google.com/group/rubyonrails-core/browse_thread/thread/2afb6dcb7c555eb1
I can''t make the patch apply thanks to email clients inserting
newlines somewhere along the way.  However I really like the
implementation you have here.

My only thought is a simple one, should these be called
''columns''?
Why not attributes?

If you could give us a patch on a lighthouse ticket I think this could
be good to go.
>
>
> Cheers,
> Taryn
>
>
> From 1cb4d7178f716d3c865ab8c36819c0f772fbfa01 Mon Sep 17 00:00:00 2001
> From: taryn <teast@globalpersonals.co.uk>
> Date: Tue, 1 Sep 2009 11:51:38 +0100
> Subject: [PATCH] Added "columns" to Active Resource to stop
> NoMethodError
>
> Basic proposed idea for "columns" for Active Resource. All it
does is
> make
> certain that any known columns return ''nil'' on access
instead of
> raising a
> NoMethodError - when they aren''t yet set by the remote system.
>
> Columns can be set manually by a user by passing in an array of either
> strings or symbols (they are converted to symbols internally).
> This simply feeds method_missing - which will repsond with a nil
> if an attribute has not been set but is a known column.
>
> I''ve added base_tests for setting this attribute, and also
> validations_tests
> (because validates_presence_of requires an attribute to return
''nil''
> instead
> of raising an exception).
>
> ''columns'' is also an expected interface feature for a lot
of Active
> Record
> plugins. Adding this functionality to Active Resource means that it
> will be
> more easily interchangeable with AR.
>
> At a later date we can consider funky things like remote lookup of
> known
> columns via a hit to the /new action - but for now - this is a
> simple-but-solid foundation.
> ---
>  activeresource/lib/active_resource/base.rb    |   51 +++++++++++++++++
> +++++---
>  activeresource/test/cases/base_test.rb        |   37 +++++++++++++++++
> +
>  activeresource/test/cases/validations_test.rb |   29 +++++++++++++-
>  activeresource/test/fixtures/project.rb       |   19 ++++-----
>  4 files changed, 118 insertions(+), 18 deletions(-)
>
> diff --git a/activeresource/lib/active_resource/base.rb b/
> activeresource/lib/active_resource/base.rb
> index e5b8589..c14c16e 100644
> --- a/activeresource/lib/active_resource/base.rb
> +++ b/activeresource/lib/active_resource/base.rb
> @@ -239,6 +239,40 @@ module ActiveResource
>     cattr_accessor :logger
>
>     class << self
> +      # Accepts an array of column names that this Resource
> recognises as
> +      # known attributes.
> +      #
> +      # If the value of a known column has not been set and the value
> is
> +      # accessed - nil is returned instead of the usual +NoMethodError
> +
> +      # exception from +method_missing+
> +      #
> +      # This is especially important when you have a
> +      # <tt>validates_presence_of</tt> validation on the
column - as
> this
> +      # relies on the attribute returning ''nil'' if it
hasn''t been set
> and
> +      # will not work if an exception is raised.
> +      #
> +      # example:
> +      #   me = Person.new(:name => ''me'')
> +      #   me.name # => ''me''
> +      #   me.age  # => raises NoMethodError
> +      #
> +      #   Person.columns = [:name, :age]
> +      #   you = Person.new(:name => ''you'')
> +      #   you.name # => ''you''
> +      #   you.age  # => ''nil''
> +      #
> +      # All attributes returned by the remote system will still be
> stored on
> +      # the object regardless of what is in this column set.
> +      def columns=(cols)
> +        @columns = cols.blank? ? [] : cols.map(&:to_sym)
> +      end
> +      # Returns the current set of known columns/attributes of the
> Resource
> +      # as specified by <tt>columns=</tt>
> +      def columns
> +        defined?(@columns) ? @columns : []
> +      end
> +
> +
>       # Gets the URI of the REST resources to map for this class.
> The site variable is required for
>       # Active Resource''s mapping to work.
>       def site
> @@ -1207,12 +1241,17 @@ module ActiveResource
>         method_name = method_symbol.to_s
>
>         case method_name.last
> -          when "="
> -            attributes[method_name.first(-1)] = arguments.first
> -          when "?"
> -            attributes[method_name.first(-1)]
> -          else
> -            attributes.has_key?(method_name) ? attributes
> [method_name] : super
> +        when "="
> +          attributes[method_name.first(-1)] = arguments.first
> +        when "?"
> +          attributes[method_name.first(-1)]
> +        else
> +          # if we have this attribute - return it
> +          return attributes[method_name] if attributes.has_key?
> (method_name)
> +          # if this is a known column but hasn''t been set -
return
> nil
> +          return nil if self.class.columns.include?(method_symbol)
> +          # otherwise defer upwards
> +          super
>         end
>       end
>   end
> diff --git a/activeresource/test/cases/base_test.rb b/activeresource/
> test/cases/base_test.rb
> index 8c0217a..50c6195 100644
> --- a/activeresource/test/cases/base_test.rb
> +++ b/activeresource/test/cases/base_test.rb
> @@ -103,6 +103,43 @@ class BaseTest < Test::Unit::TestCase
>   end
>
>
> +  def test_columns_accessor_accepts_array_of_syms
> +    column_set = [:age, :name]
> +
> +    assert_nothing_raised { Person.columns = column_set }
> +    assert_equal column_set, Person.columns
> +  end
> +
> +  def test_columns_accessor_accepts_array_of_strings
> +    column_set = [''name'', ''age'']
> +    column_set_syms = column_set.map(&:to_sym)
> +
> +    assert_nothing_raised { Person.columns = column_set }
> +    # should sort and sybolise them
> +    assert_equal column_set_syms, Person.columns
> +  end
> +
> +
> +  def test_non_attribute_access_and_assignment_should_fail
> +    me = Person.new
> +    assert !me.respond_to?("my_unknown_column")
> +    assert_raises(NoMethodError) { me.my_unknown_column }
> +  end
> +
> +  def test_known_column_on_new_instance_should_return_nil
> +    me = Person.new
> +    assert !me.respond_to?("my_known_column") # sanity check
> +
> +    Person.columns = [:my_known_column]
> +    assert_nothing_raised { assert_nil me.my_known_column }
> +    # just for good measure - test we can set it too
> +    assert_nothing_raised {
> +      new_val = ''blah''
> +      me.my_known_column = new_val
> +      assert_equal new_val, me.my_known_column
> +    }
> +  end
> +
>   def test_site_accessor_accepts_uri_or_string_argument
>     site = URI.parse(''http://localhost'')
>
> diff --git a/activeresource/test/cases/validations_test.rb b/
> activeresource/test/cases/validations_test.rb
> index a8ab7d6..255fc6c 100644
> --- a/activeresource/test/cases/validations_test.rb
> +++ b/activeresource/test/cases/validations_test.rb
> @@ -5,7 +5,7 @@ require "fixtures/project"
>  # This test case simply makes sur that they are all accessible by
>  # Active Resource objects.
>  class ValidationsTest < ActiveModel::TestCase
> -  VALID_PROJECT_HASH = { :name => "My Project", :description
=> "A
> project" }
> +  VALID_PROJECT_HASH = { :name => "My Project", :owner =>
> ''Bob'', :description => "A project",
:remote_column => ''something'' }
>   def setup
>     @my_proj = VALID_PROJECT_HASH.to_xml(:root => "person")
>     ActiveResource::HttpMock.respond_to do |mock|
> @@ -13,7 +13,7 @@ class ValidationsTest < ActiveModel::TestCase
>     end
>   end
>
> -  def test_validates_presence_of
> +  def test_validates_presence_of_known_column
>     p = new_project(:name => nil)
>     assert !p.valid?, "should not be a valid record without name"
>     assert !p.save, "should not have saved an invalid record"
> @@ -23,6 +23,31 @@ class ValidationsTest < ActiveModel::TestCase
>
>     assert p.save, "should have saved after fixing the validation,
> but had: #{p.errors.inspect}"
>   end
> +
> +  def test_validates_presence_of_accessor_backed_attribute
> +    p = new_project(:owner => nil)
> +    assert !p.valid?, "should not be a valid record without
owner"
> +    assert !p.save, "should not have saved an invalid record"
> +    assert_equal ["can''t be blank"], p.errors[:owner],
"should have
> an error on name"
> +
> +    p.owner = "somebody"
> +
> +    assert p.save, "should have saved after fixing the validation,
> but had: #{p.errors.inspect}"
> +  end
> +
> +  def
>
test_validates_presence_of_unknown_column_should_raise_exception_if_not_present
> +    p = Project.new(VALID_PROJECT_HASH.delete_if{|k,v| k
> == :remote_column})
> +    assert_raises(NoMethodError) {
> +      assert !p.valid?, "should not be a valid record without
> remote_column"
> +    }
> +    p.remote_column = "something"
> +    assert_nothing_raised {
> +      assert p.valid?, "should be a valid record with the remote
> column set"
> +      assert p.save, "should have saved after fixing the validation,
> but had: #{p.errors.inspect}"
> +    }
> +
> +  end
> +
>
>   def test_fails_save!
>     p = new_project(:name => nil)
> diff --git a/activeresource/test/fixtures/project.rb b/activeresource/
> test/fixtures/project.rb
> index e15fa6f..e800d39 100644
> --- a/activeresource/test/fixtures/project.rb
> +++ b/activeresource/test/fixtures/project.rb
> @@ -1,8 +1,9 @@
>  # used to test validations
>  class Project < ActiveResource::Base
>   self.site = "http://37s.sunrise.i:3000"
> +  self.columns = [:name, :description]
>
> -  validates_presence_of :name
> +  validates_presence_of :name, :owner, :remote_column
>   validate :description_greater_than_three_letters
>
>   # to test the validate *callback* works
> @@ -11,15 +12,13 @@ class Project < ActiveResource::Base
>   end
>
>
> -  # stop-gap accessor to default this attribute to nil
> -  # Otherwise the validations fail saying that the method does not
> exist.
> -  # In future, method_missing will be updated to not explode on a
> known
> -  # attribute.
> -  def name
> -    attributes[''name''] || nil
> -  end
> -  def description
> -    attributes[''description''] || nil
> +  # This attribute isn''t in columns - so we need to fake up this
nil-
> ifying
> +  # accessor function so that validates_presence_of does not explode
> when
> +  # the attribute hasn''t yet been set. Compare with the :name
column
> +  def owner
> +    # this is clearly just a test-function, but you could, say, use
> this to
> +    # do a db lookup on the User table for the owner
> +    attributes[''owner''] || nil
>   end
>  end
>
> --
> 1.6.0.4
> >
>


-- 
Cheers

Koz

On Sep 8, 4:48 am, Michael Koziarski <mich...@koziarski.com>
wrote:
> > I''ve provided a working proposed patch (below) to implement
this and
> > would value opinion on a) the patch as is and b) any other options.
>
> > Note - this follows from discussion (and my original proposal) in the
> > following thread:
>
>http://groups.google.com/group/rubyonrails-core/browse_thread/thread/...
>
> I can''t make the patch apply thanks to email clients inserting
> newlines somewhere along the way.  However I really like the
> implementation you have here.
damn - how annoying!

I''ve sent the patch to Josh as well - so he has a working copy.
Because it''s a bit of a radical departure (in theory) from how ARes
works right now, he understandably wanted more comment on it from the
rails-core team before he was willing to apply this one. :)

> My only thought is a simple one, should these be called
''columns''?
> Why not attributes?
Attributes already exists on ARes - but is used in a different way. It
doesn''t represent a canonical list of expected attributes that you
will either find (or not) on any given instance. It''s more of a list
of ''what attributes have been set so far on this instance''.
Repurposing an existing, working accessor would be more of a change
than just adding a new one... thus a different name was necessary.
I used ''columns'' because that''s what AR uses... and
that has its
advantages when you want to use AR and ARes interchangeably (which we
do). Plugins that assume you have a ''columns'' function... will
then
Just Work.

There''s an argument for having a commonly-named accessor interface
that does not have a db-inspired name... but that''s another
argument ;)
> If you could give us a patch on a lighthouse ticket I think this could be
good to go.
see previous comment ;)
Josh has a copy - but I need the big guns to run their eyes over this
to make sure it''s ok for go.
Any chance you could poke the right people to come look?

Cheers,
Taryn

Hey Taryn, sorry it took so long for me to get back to you.

My only concern is the "column" name, as Koz mentioned. We really
should have a common term other than "column" that could apply to any
ActiveModel.

I''m trying to contact DHH or Rick Olson who drafted some of this stuff
already.

-- Josh

On Sep 28, 6:55 pm, Joshua Peek <j...@joshpeek.com>
wrote:
> Hey Taryn, sorry it took so long for me to get back to you.
>
> My only concern is the "column" name, as Koz mentioned. We really
> should have a common term other than "column" that could apply to
any
> ActiveModel.
>
> I''m trying to contact DHH or Rick Olson who drafted some of this
stuff
> already.

Fantastic. Obviously from a functionality POV - it doesnt matter what
it''s called, so yeah - it''s good that this is all
that''s missing.

Hmmm. attributes really is quite natural for this... but attributes
covers all the ephemeral ones as well.

How about ''persisted_attributes'' or something similar?

As a bit of a contrast to, say, ''attributes_before_typecast''
and other
similar stuff.

Cheers,
Taryn

So David and I decided we both like "schema" for containing the list
of attributes and types for the model. It seems like a good fit for
any "ActiveModel".

In ActiveResource''s case, it will have an "undefined schema"
by
default where it just uses any attribute it receives in the response.
This basically the current behavior and we still want to support it.

Its still half baked, but we''ve been talking for a while now about
controllers providing the model schema. So "GET /people/new.xml =>
PeopleController#schema" would return a xml or json schema that
ActiveResource could configure its "auto schema" with.

The third option is a "defined schema" where you can specify exactly
what attributes and types you want.

Here is DHH''s example, its similar to migration definitions.

  class Person < ActiveRecord::Base
    schema do
      string  :type, :description
      string  :name, :url_name, :limit => 50
      integer :project_id
      integer :elements_count, :default => 0, :null => false
    end
  end


On Oct 2, 8:26 am, taryneast <taryne...@gmail.com>
wrote:
> On Sep 28, 6:55 pm, Joshua Peek <j...@joshpeek.com> wrote:
>
> > Hey Taryn, sorry it took so long for me to get back to you.
>
> > My only concern is the "column" name, as Koz mentioned. We
really
> > should have a common term other than "column" that could
apply to any
> > ActiveModel.
>
> > I''m trying to contact DHH or Rick Olson who drafted some of
this stuff
> > already.
>
> Fantastic. Obviously from a functionality POV - it doesnt matter what
> it''s called, so yeah - it''s good that this is all
that''s missing.
>
> Hmmm. attributes really is quite natural for this... but attributes
> covers all the ephemeral ones as well.
>
> How about ''persisted_attributes'' or something similar?
>
> As a bit of a contrast to, say,
''attributes_before_typecast'' and other
> similar stuff.
>
> Cheers,
> Taryn

On Oct 2, 6:30 pm, Joshua Peek <j...@joshpeek.com>
wrote:
> So David and I decided we both like "schema" for containing the
list
> of attributes and types for the model. It seems like a good fit for
> any "ActiveModel".
>
> In ActiveResource''s case, it will have an "undefined
schema" by
> default where it just uses any attribute it receives in the response.
> This basically the current behavior and we still want to support it.
Yep - makes sense
> Its still half baked, but we''ve been talking for a while now about
> controllers providing the model schema. So "GET /people/new.xml =>
> PeopleController#schema" would return a xml or json schema that
> ActiveResource could configure its "auto schema" with.
Yeah, a few people have suggested something of this sort - good idea.
The idea people have is that they''d send back some sort of xml chunk
that defines all the columns in the schema... I''m not sure how
we''d
want that to be formatted to point out column-types and any other
restrictions etc.

As a bare minimum - just name-type key pairs could work quite well to
begin with eg:

http://my_remote_site/people/schema.xml

returns:

<person>
  <name type="string" />
  <description type="text" />
  <project_id type="integer" />
</person>
> The third option is a "defined schema" where you can specify
exactly
> what attributes and types you want.
>
> Here is DHH''s example, its similar to migration definitions.
>
>   class Person < ActiveRecord::Base
>     schema do
>       string  :type, :description
>       string  :name, :url_name, :limit => 50
>       integer :project_id
>       integer :elements_count, :default => 0, :null => false
>     end
>   end
Nice - yes, that also takes it all to the next level of allowing us to
know what the column *types* are too.
I had some similar thoughts along that line, but it was all hazy in my
head how we could do it.
This example firms it up nicely.

I''m a little uncertain about how to treat the limits/''not
null''
etc...
Are they relevant/necessary? They are pretty databasey and we don''t
necessarily have a database here...
I''m just a bit concerned they may interfere with validations, unless
you envisage some sort of composite validation thing springing from
them. (eg a limit becomes validates_length_of)

Defaults are probably pretty simple to implement - just add them to an
accessor if the attribute is null/blank. :)

The others are also do-able (of course), but I would like some
clarification on whether you reckon they would serve as a complement
to the validations, or can be *implemented* using validations.. or
whatever you have in mind. :)

I''ll go ahead and start to alter my existing code to match the above
(unless otherwise told).

Taryn

> As a bare minimum - just name-type key pairs could work quite well to
> begin with eg:
>
> http://my_remote_site/people/schema.xml
>
> returns:
>
> <person>
>  <name type="string" />
>  <description type="text" />
>  <project_id type="integer" />
> </person>
The problem with this is we''re reinventing a schema language and
people are going to (rightly) ask why not just use relaxng or xsd.
Supporting them will be way more work.

new.xml will give you the fields, the defaults, and it''s pretty
straight forward.  Fundamentally though I just don''t think
there''s
much utility to programmatic discovery like this, the rest of your
code makes assumptions about what attributes are there, and there''s no
harm in you codifying this in your local app.

Having said that I think it''d be a neat feature for simple lo-fi APIs
where you control both ends of the wire.
 I''m a little uncertain about how to treat the limits/''not
null''
> Are they relevant/necessary? They are pretty databasey and we
don''t
> necessarily have a database here...
> I''m just a bit concerned they may interfere with validations,
unless
> you envisage some sort of composite validation thing springing from
> them. (eg a limit becomes validates_length_of)
>
> The others are also do-able (of course), but I would like some
> clarification on whether you reckon they would serve as a complement
> to the validations, or can be *implemented* using validations.. or
> whatever you have in mind. :)
For now I''d suggest just relying on the user to provide validations to
pick them up.  At some stage in the future we can look at tying
auto-validations in from the schema definitions.



-- 
Cheers

Koz

On Oct 14, 10:52 pm, Michael Koziarski <mich...@koziarski.com>
wrote:
> > As a bare minimum - just name-type key pairs could work quite well to
> > begin with eg:
>
> >http://my_remote_site/people/schema.xml
>
> > returns:
>
> > <person>
> >  <name type="string" />
> >  <description type="text" />
> >  <project_id type="integer" />
> > </person>
>
> The problem with this is we''re reinventing a schema language and
> people are going to (rightly) ask why not just use relaxng or xsd.
> Supporting them will be way more work.
This was considered more of a ''future idea'' than something to
do right
away anyway, I can see the point, though, in supporting something that
already exists rather than re-inventing yet another language.
Possibly even supporting just an extremely stripped-down subset of
either of the above...
Is there a gem that tracks either of the above fairly well?
> new.xml will give you the fields, the defaults, and it''s pretty
> straight forward.  
Not if the remote API is not written in Rails...
I''m trying to steer clear of the ''remote API is
Rails'' assumption that
seems inherent in a lot of ARes design.
Our system is written to interface with a remote Cold Fusion site - so
I''ve butted up against a number of these assumptions. :)

I''ve definitely considered the idea of using a blank new command as
the schema-definition - it''s just as good an idea as schema.xml
I guess I consider that schema.xml is more explicit for what you want.

In either case, in any system other than Rails, this function has to
be written explicitly  so having a vague idea of the kind of thing
we''d expect to be returned would be a good thing to know.
Having it named something different to ''new'' means you steer
clear of
conflicts in systems that aren''t truly RESTful - it gives one more
layer of certainty that we don''t accidentally create a new, empty
widget every time we just want to fetch the schema ;)
> Fundamentally though I just don''t think there''s
> much utility to programmatic discovery like this, the rest of your
> code makes assumptions about what attributes are there, and
there''s no
> harm in you codifying this in your local app.
Actually I agree :)
I think it''s much more likely to be useful that the person that writes
the API call up the people that own the remote system and ask for a
copy of their API and then just codify it into the app... but there
seems to be a desire for - and who am I to argue ;)
That being said, it won''t be the first feature I write.
> Having said that I think it''d be a neat feature for simple lo-fi
APIs where you control both ends of the wire.
I think in this case it''s even easier to do the schema-definition in
the app. The only time I see it being more useful is if the remote
system is prone to frequent change, and we want to track that
change... and we *don''t* want to use the auto-discovery based on
attributes that is what the current system already does (eg because we
have validates_presence_of on some of the fields).

Still - I''d be looking for a sensible use-case that wasn''t
covered by
existing ideas...
>  I''m a little uncertain about how to treat the
limits/''not null''
<snippage>
>
> For now I''d suggest just relying on the user to provide
validations to
> pick them up.  At some stage in the future we can look at tying
> auto-validations in from the schema definitions.
Yeah, I figure that''s the simplest solution - start with the basics
and move on from there.

Cheers,
Taryn

Just to continue the thread... I''m back on this (now that NaNoWriMo is
over).

I''ve ported my columns over to schema and added tests. The current
patch is still using an array of names for the schema (so obviously
not the eventual requirement), but it''s getting there.

If anybody wants to check it to see if it makes sense, the patch is
available here: http://pastie.org/725959

Currently it:
- accepts/returns the schema if set
- if not set - will return the attributes of the instance
- will ''respond_to?'' the attributes in the schema
- will return nil instead of MethodNotFound for attributes in the
schema

Next work:
- change it to accept on a hash, not an array
- start doing funky things depending on the hash options (ie
attributes_before_typecast ).

Taryn

On Oct 16, 8:37 am, taryneast <taryne...@gmail.com>
wrote:
> On Oct 14, 10:52 pm, Michael Koziarski <mich...@koziarski.com> wrote:
>
>
>
> > > As a bare minimum - just name-type key pairs could work quite
well to
> > > begin with eg:
>
> > >http://my_remote_site/people/schema.xml
>
> > > returns:
>
> > > <person>
> > >  <name type="string" />
> > >  <description type="text" />
> > >  <project_id type="integer" />
> > > </person>
>
> > The problem with this is we''re reinventing a schema language
and
> > people are going to (rightly) ask why not just use relaxng or xsd.
> > Supporting them will be way more work.
>
> This was considered more of a ''future idea'' than
something to do right
> away anyway, I can see the point, though, in supporting something that
> already exists rather than re-inventing yet another language.
> Possibly even supporting just an extremely stripped-down subset of
> either of the above...
> Is there a gem that tracks either of the above fairly well?
>
> > new.xml will give you the fields, the defaults, and it''s
pretty
> > straight forward.  
>
> Not if the remote API is not written in Rails...
> I''m trying to steer clear of the ''remote API is
Rails'' assumption that
> seems inherent in a lot of ARes design.
> Our system is written to interface with a remote Cold Fusion site - so
> I''ve butted up against a number of these assumptions. :)
>
> I''ve definitely considered the idea of using a blank new command
as
> the schema-definition - it''s just as good an idea as schema.xml
> I guess I consider that schema.xml is more explicit for what you want.
>
> In either case, in any system other than Rails, this function has to
> be written explicitly  so having a vague idea of the kind of thing
> we''d expect to be returned would be a good thing to know.
> Having it named something different to ''new'' means you
steer clear of
> conflicts in systems that aren''t truly RESTful - it gives one more
> layer of certainty that we don''t accidentally create a new, empty
> widget every time we just want to fetch the schema ;)
>
> > Fundamentally though I just don''t think there''s
> > much utility to programmatic discovery like this, the rest of your
> > code makes assumptions about what attributes are there, and
there''s no
> > harm in you codifying this in your local app.
>
> Actually I agree :)
> I think it''s much more likely to be useful that the person that
writes
> the API call up the people that own the remote system and ask for a
> copy of their API and then just codify it into the app... but there
> seems to be a desire for - and who am I to argue ;)
> That being said, it won''t be the first feature I write.
>
> > Having said that I think it''d be a neat feature for simple
lo-fi APIs where you control both ends of the wire.
>
> I think in this case it''s even easier to do the schema-definition
in
> the app. The only time I see it being more useful is if the remote
> system is prone to frequent change, and we want to track that
> change... and we *don''t* want to use the auto-discovery based on
> attributes that is what the current system already does (eg because we
> have validates_presence_of on some of the fields).
>
> Still - I''d be looking for a sensible use-case that
wasn''t covered by
> existing ideas...
>
>
>
> >  I''m a little uncertain about how to treat the
limits/''not null''
> <snippage>
>
> > For now I''d suggest just relying on the user to provide
validations to
> > pick them up.  At some stage in the future we can look at tying
> > auto-validations in from the schema definitions.
>
> Yeah, I figure that''s the simplest solution - start with the
basics
> and move on from there.
>
> Cheers,
> Taryn
--

You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.

and now I''ve got some baseline code working the discussion has moved
to a new thread:
http://groups.google.com/group/rubyonrails-core/browse_thread/thread/6b9631841a4daea2

Taryn

On Dec 3, 6:05 pm, taryneast <taryne...@gmail.com>
wrote:
> Just to continue the thread... I''m back on this (now that
NaNoWriMo is
> over).
>
> I''ve ported my columns over to schema and added tests. The current
> patch is still using an array of names for the schema (so obviously
> not the eventual requirement), but it''s getting there.
>
> If anybody wants to check it to see if it makes sense, the patch is
> available here:http://pastie.org/725959
>
> Currently it:
> - accepts/returns the schema if set
> - if not set - will return the attributes of the instance
> - will ''respond_to?'' the attributes in the schema
> - will return nil instead of MethodNotFound for attributes in the
> schema
>
> Next work:
> - change it to accept on a hash, not an array
> - start doing funky things depending on the hash options (ie
> attributes_before_typecast ).
>
> Taryn
>
> On Oct 16, 8:37 am, taryneast <taryne...@gmail.com> wrote:
>
> > On Oct 14, 10:52 pm, Michael Koziarski <mich...@koziarski.com>
wrote:
>
> > > > As a bare minimum - just name-type key pairs could work
quite well to
> > > > begin with eg:
>
> > > >http://my_remote_site/people/schema.xml
>
> > > > returns:
>
> > > > <person>
> > > >  <name type="string" />
> > > >  <description type="text" />
> > > >  <project_id type="integer" />
> > > > </person>
>
> > > The problem with this is we''re reinventing a schema
language and
> > > people are going to (rightly) ask why not just use relaxng or
xsd.
> > > Supporting them will be way more work.
>
> > This was considered more of a ''future idea'' than
something to do right
> > away anyway, I can see the point, though, in supporting something that
> > already exists rather than re-inventing yet another language.
> > Possibly even supporting just an extremely stripped-down subset of
> > either of the above...
> > Is there a gem that tracks either of the above fairly well?
>
> > > new.xml will give you the fields, the defaults, and it''s
pretty
> > > straight forward.  
>
> > Not if the remote API is not written in Rails...
> > I''m trying to steer clear of the ''remote API is
Rails'' assumption that
> > seems inherent in a lot of ARes design.
> > Our system is written to interface with a remote Cold Fusion site - so
> > I''ve butted up against a number of these assumptions. :)
>
> > I''ve definitely considered the idea of using a blank new
command as
> > the schema-definition - it''s just as good an idea as
schema.xml
> > I guess I consider that schema.xml is more explicit for what you want.
>
> > In either case, in any system other than Rails, this function has to
> > be written explicitly  so having a vague idea of the kind of thing
> > we''d expect to be returned would be a good thing to know.
> > Having it named something different to ''new'' means
you steer clear of
> > conflicts in systems that aren''t truly RESTful - it gives one
more
> > layer of certainty that we don''t accidentally create a new,
empty
> > widget every time we just want to fetch the schema ;)
>
> > > Fundamentally though I just don''t think there''s
> > > much utility to programmatic discovery like this, the rest of
your
> > > code makes assumptions about what attributes are there, and
there''s no
> > > harm in you codifying this in your local app.
>
> > Actually I agree :)
> > I think it''s much more likely to be useful that the person
that writes
> > the API call up the people that own the remote system and ask for a
> > copy of their API and then just codify it into the app... but there
> > seems to be a desire for - and who am I to argue ;)
> > That being said, it won''t be the first feature I write.
>
> > > Having said that I think it''d be a neat feature for
simple lo-fi APIs where you control both ends of the wire.
>
> > I think in this case it''s even easier to do the
schema-definition in
> > the app. The only time I see it being more useful is if the remote
> > system is prone to frequent change, and we want to track that
> > change... and we *don''t* want to use the auto-discovery based
on
> > attributes that is what the current system already does (eg because we
> > have validates_presence_of on some of the fields).
>
> > Still - I''d be looking for a sensible use-case that
wasn''t covered by
> > existing ideas...
>
> > >  I''m a little uncertain about how to treat the
limits/''not null''
> > <snippage>
>
> > > For now I''d suggest just relying on the user to provide
validations to
> > > pick them up.  At some stage in the future we can look at tying
> > > auto-validations in from the schema definitions.
>
> > Yeah, I figure that''s the simplest solution - start with the
basics
> > and move on from there.
>
> > Cheers,
> > Taryn
>
>
--

You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com.
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en.