kinyerakenneth / lastfm-java

Automatically exported from code.google.com/p/lastfm-java
BSD 2-Clause "Simplified" License
0 stars 0 forks source link

DatabaseCache implementation does not work with MySQL5 #33

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Set up a datasource to a MySQL5 innodb database (with correct privileges 
granted)
2. Set the last.fm api to use a db cache
   Caller.getInstance().setCache(new DatabaseCache(datasource.getConnection()));

3. When the DatabaseCache attempts to create the LASTFM_CACHE table it fails.

Analysis:

This isnt exactly a bug as the DatabaseCache comment states 'this class should 
work with all common databases (which support varchar, timestamp and 
longvarchar datatypes)', and MySQL5 doesnt for the following reasons:

The column name key is a reserved word in mysql5
The key column varchar needs a length
The response column needs to be response longtext CHARACTER SET utf8 COLLATE 
utf8_general_ci

(For some reason specifying just a utf8 table is not sufficient, the column 
itself needs to be utf8 in order to store unicode data from last.fm).

I've attached a patch that exposes the key column and renames it _key, and a 
MySqlCache that extends the DatabaseCache class and overrides the column 
definitions to work with MySql5.

Cheers,
David

Original issue reported on code.google.com by depa...@gmail.com on 11 Aug 2011 at 10:36

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks for the report, I'll try and test this myself when I find the time. 
Thanks for the patch!

Original comment by jannikov...@gmail.com on 12 Oct 2011 at 10:34

GoogleCodeExporter commented 9 years ago
I wouldn't apply the patch as it is. While I agree with the general intention 
I'd have a number of reason not to:
- Rename _key to key_.
- Why would you only want to expose the key column name? Why expose it all?
-> either have a protected method in DatabaseCache that returns a collection 
with column names (has defaults but is overridable) OR
-> don't expose any of the column names

Original comment by marcel@frightanic.com on 2 Dec 2011 at 8:50

GoogleCodeExporter commented 9 years ago
I've been testing various databases with this class and have come to the 
following conclusions and results:
- I've been testing with MySQL 5, H2, HSQLDB and Derby a.k.a. JavaDB. There is 
no single text datatype all of them support (tested for "text", "long varchar" 
and "longvarchar") in addition, Derby's long varchar type only supports up to 
32'000 characters, which is too little for some requests (e.g. artist.getInfo 
with long biographies)
- DatabaseCache now supports at least MySQL 5 and H2 1.3, as it uses the "text" 
datatype
- I have updated the documentation for the class and for the createTable method 
so if you need to support another database you should be able to implement a 
suitable subclass quickly
- There does not seem to be a need to explicitly set the columns to utf8 with 
MySQL as long as the database is set to use utf8. There was however another bug 
in the DatabaseCache class that might have caused problems with character 
encoding. That bug should be fixed now, too and I have not encountered any 
problems with the new version (tested on a Windows machine, no explicit 
encoding set for either the table columns or the database connection)
- I have changed the name of the key column to id, that name is not exposed 
publicly

I'm setting this issue to fixed, as all the problems in the bug report have 
been addressed. If anybody has anything to add, any more information on the 
problems I outlined or found something I was wrong about above please feel free 
to add this to this issue or create a new one.

Original comment by jannikov...@gmail.com on 21 Dec 2011 at 5:30