SQL Syntax Error Creating Book Object in Sample App

[djc-msb]

Peter,

I know you are in the throws of finals, but I thought I would bring this to your attention. First, a little background. I am hoping to use your MTI solution in a service request portal.

I downloaded your sample app--modifying the database.yml and Gemfile to use mysql2 instead of sqlite3 -- since I prefer that.

I was able to create a Product without any difficulty.

However, when I went to create a book, by going to localhost:3000/books/ and then clicking on New book, an exception was caught on the create action--invalid SQL syntax.

Here is the log

Started GET "/books" for 127.0.0.1 at 2011-05-02 16:38:59 -0400 Processing by BooksController#index as HTML Book Load (0.3ms) SELECT view_books.* FROM view_books WHERE view_books.type = 'Book' Rendered books/index.html.erb within layouts/application (1.9ms) Completed 200 OK in 20ms (Views: 5.3ms | ActiveRecord: 0.3ms) citier -> Root Class citier -> table_name -> products citier -> Non Root Class citier -> table_name -> books citier -> tablename (view) -> view_books

Started GET "/books/new" for 127.0.0.1 at 2011-05-02 16:39:06 -0400 Processing by BooksController#new as HTML Rendered books/_form.html.erb (31.4ms) Rendered books/new.html.erb within layouts/application (34.9ms) Completed 200 OK in 55ms (Views: 38.0ms | ActiveRecord: 0.0ms) citier -> Root Class citier -> table_name -> products citier -> Non Root Class citier -> table_name -> books citier -> tablename (view) -> view_books citier -> Attributes for Book: {"author"=>"John Smith", "title"=>"A New Book"} citier -> Class (Product) could not be saved citier -> SQL : UPDATE products SET type = 'Book' WHERE id =

Started POST "/books" for 127.0.0.1 at 2011-05-02 16:39:19 -0400 Processing by BooksController#create as HTML Parameters: {"utf8"=>"✓", "authenticity_token"=>"60LGRi8rWdDfIr4kaiIjLIyBYyWSN+7Wmsz4ykX2t4c=", "book"=>{"title"=>"A New Book", "author"=>"John Smith"}, "commit"=>"Create Book"} WARNING: Can't mass-assign protected attributes: type SQL (0.2ms) BEGIN SQL (0.4ms) ROLLBACK SQL (0.4ms) UPDATE products SET type = 'Book' WHERE id = Mysql2::Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1: UPDATE products SET type = 'Book' WHERE id = Completed in 26ms

ActiveRecord::StatementInvalid (Mysql2::Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1: UPDATE products SET type = 'Book' WHERE id = ): app/controllers/books_controller.rb:46:in block in create' app/controllers/books_controller.rb:45:increate'

Rendered vendor/ruby/1.9.1/gems/actionpack-3.0.7/lib/action_dispatch/middleware/templates/rescues/_trace.erb (1.1ms) Rendered vendor/ruby/1.9.1/gems/actionpack-3.0.7/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb (3.2ms) Rendered vendor/ruby/1.9.1/gems/actionpack-3.0.7/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb within rescues/layout (10.0ms)

Any thoughts? Am I using this wrong?

Dan

[petehamilton]

Hi Dan,

Glad you are hoping to use the system!

It sounds like you have done things right so far, the fact you can create products is good. I haven't used the plugin with mysql2 yet and it's possible I might need to change some of the settings for the sql adapters.

Would you mind posting your models so I could try to replicate? You can leave out any processing/functions or validations if you want to keep it confidential, although obviously the more information the better.

Just as an on the spot diagnosis, the system works by going back through the heirarchy creating models, until it reaches the root model (Product), where it creates the root model, returning the model (in your case product) with an ID, and passes it back down the chain, creating the child models as it goes and updating the 'type'. It looks like the program is getting to the root and failing to save, or maybe to update the model once saved, so when it passes the id around the chain to your "Book" instance, the query fails. I could be wrong but that's my guess!

Thanks for bringing it to my attention!

Note: I do remember seeing some errors like this a while back in developing this, so I will probably be able to find the root of the problem by looking over commits around the same time etc.

[djc-msb]

Peter,

Thanks for getting back to me so quickly.

They are your Sample App models, without any changes. I didn't change anything except as regards the database configuration. Before I started using them, or rolled my own, I wanted to see how things worked in the sample app you provide and make sure things are working as I think they are supposed to after reading your code.

In the Gemfile switched from SQLite3 to mySQL, and the database.yml I updated appropriately to connect to my database server.

Dan

[petehamilton]

Hi Dan,

No worries, was just checking what the situation was. Will check it out by changing my sample app settings and get back to you.

Pete

[djc-msb]

Peter,

For what it's worth, it looks like the the problem exists with the original ALTRABio version (CITIEsForRAILS) too. I copied your sample code (with the DB config modifications), changed the modes to use "acts_as_cities" instead of "acts_as_citier" and saw the same error.

Dan

[petehamilton]

In that case, I will look at the sql adapters. might be just a case of copying over similar code for a new mysql2 adapter, perhaps it doesn't use the mysql adapter. Trying it now...

And thanks, that was useful to know!

[petehamilton]

Which version of rails are you using? The mysql2 adapter works differently in different versions so would help me isolate.

[djc-msb]

Pete,

I just did a side by side with the child_instance.rb save() code an the database. It looks like the Product did not get saved, so self.id & parent.id are blank--hence the WHERE id= at the end of the SQL statement is blank. I'm adding some debug lines to test that theory.

Dan

[petehamilton]

That fits with my theory... Looks promising, my machine is playing up with mysql2, when its sorted I actually will be able to fix! sigh...

[petehamilton]

Right I have replication! Now to debug...

[petehamilton]

Hmmm, actually it happens for sqlite as well, how odd, I'm sure I had it working! It's not mysql2 specific at any rate. And for some reason my console version seems to be dodgy too... I have this working in my own projects so maybe my samples are out of date?

[djc-msb]

Okay, I would suggest two areas of concern:

1. nil is not the same a false, so testing if the result of a save() is false doesn't work. The nil object actually has a non-zero value in ruby. It is better to test for blank? or nil?
2. The warning message "WARNING: Can't mass-assign protected attributes: type" probably tells you the reason for the failure to save. Not sure if preventing mass assignment of the type field is intentional, seems like it probably would be. I'll look into that. In checking the db, when I create a Product, the type is set to "Product", when I create a Book, no Product record is created.

Dan

[petehamilton]

Right, seems to be an issue with validation, parent model fields don't seem to quite work correctly. As in it's the fact that the 'name' field is being left out. If this was part of the form, it would work, but actually that's not the root of the issue. You'r issue just helped uncover this deeper one.

[petehamilton]

Right, Issue isolated as follows:

Issue Scenarios (Non-Working & Working)

Scenario 1 (Non-Working) - Full validation

class Product < ActiveRecord::Base
  acts_as_citier
  validates_presence_of :name
end
class Book < Product
  acts_as_citier
  validates_presence_of :title
end

Commands which work:

>> Book.create(:title=>"A Title", :name=>"A Book")
>> Book.create(:name=>"A Book")

Commands which fail:

>> Book.create(:title=>"A Book",:name=>nil)
>> Book.create(:title=>"A Book")

Scenario 2 (Working) - Removing some validation

Altering the Product model to remove validation and running the same code:

class Product < ActiveRecord::Base
  acts_as_citier
  # validates_presence_of :name
end
class Book < Product
  acts_as_citier
  validates_presence_of :title
end

All previous commands work:

>> Book.create(:title=>"A Title", :name=>"A Book")
>> Book.create(:name=>"A Book")
>> Book.create(:title=>"A Book",:name=>nil)
>> Book.create(:title=>"A Book")

It seems that something is going wrong with validation & save on parents. The validation works fine itself as in the following works as expected:

Book.create(:title=>"A Book").valid?

However, on save it doesn't seem to catch it.

Diagnosis (Theoretical)

1. Book tries to save
2. Moves back up hierarchy
3. Product tries to save and fails
4. Instead of stopping the save process on validation failure and returning false, it continues back to Book.
5. Book unwittingly receives no ID from the parent product model, since it didn't save correctly. But doesn't register the failed save.
6. Book tries to update a non existent parent with it's type
7. Save bombs out

Suggested Fix

Will start based on what you pointed out with the differences in checking false, nil, blank etc and work from there.

Update: Im now sure it's the statements for updating the type, they were being run regardless of whether parents were successfully saved. Have just put a conditional around it and testing that.

I'm sure this will be an easy fix to check the validation status at each stage in the save hierarchy. I will try and update soon with a fix, or if you want to fork and try a fix and experiment yourself, feel free and I will pull it in.

In my examples http://peterhamilton.github.com/citier/ I see I didn't do a straight save with a dodgy model, I just validated. Hence why I hadn't caught it.

Also, to prevent this in future, some unit tests should definitely be made! I'm ashamed to say I don't have a wealth of experience with testing etc, I don't know if you could contribute something along those lines to the project? Or maybe just pointers? Seeing as how close it is to working, it might benefit you, especially since you were planning on using it?

Have tried to nail it down, what do you think?

Cheers, Pete

[djc-msb]

I went in and added "attr_accessible" macros to both Products and Books models:

attr_accessible :type, :name, :price # product.rb attr_accessible :author, :title # book.rb

and then got a warning that it couldn't do mass-assignment on protected attributes create_at & updated_at when it tried to do the save on the Product object. Of course, you're probably getting these from the read of the table fields--and not excluding the save hash (which should be done because these are privileged fields, like ID).

My debug code show that your record_saved is getting properly set to false, so that isn't the problem (although it is safer test for parent.id.blank? rather than assume save() will return false on failure--with some NOSQL dbs, save returns a non-false failure value)

Anyway, it looks like you just sent me a message, so I'll check that.

Dan

[petehamilton]

Fixed. Committing and updating gem to v0.1.10. Pushed to Rubygems.