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

Added caching in relationships (+ test cases) (enhanced) #141

Merged
merged 7 commits into from
Nov 3, 2012

Conversation

sdrdis
Copy link
Contributor

@sdrdis sdrdis commented Nov 3, 2012

I fixed my sentences + added usage examples in the comments.

Previous message :
Don't hesitate to comment since I might have done some mistakes :).

For the belongsTo relationship ;

I added a optional parameter refresh which allows to force refresh
If the relation was loaded before and refresh is false or not set, then relation is loaded from cache
For the hasMany relationship ;

the first parameters can be either condition or refresh
if the relation was loaded before AND refresh is false or not set AND condition is not set, then relation is loaded from cache

@sdrdis
Copy link
Contributor Author

sdrdis commented Nov 3, 2012

note: there seems to have a problem with sqlite which go back more than 20 days ago. All values sent to database seems to be replaced with "?". I just learned it via travis.

@@ -702,6 +709,23 @@ AbstractClass.hasMany = function hasMany(anotherClass, params) {
*
* @param {Class} anotherClass - class to belong
* @param {Object} params - configuration {as: 'propertyName', foreignKey: 'keyName'}
*
* {Usage examples}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In markdown source code examples indented with four spaces. Plus one space after *, so it should be indented with five. All other is fine. Thanks. And sorry for pain :) If you want to check how your documentation looks like, goto: http://jsdoc.info/sdrdis/jugglingdb/abstract-class.html#class/belongsTo

@1602
Copy link
Owner

1602 commented Nov 3, 2012

This is for escaping. Following api: query('select ? from ? where a = ?',
'field', 'table', 'value');

On Sat, Nov 3, 2012 at 4:48 AM, Sébastien Drouyer
notifications@github.comwrote:

note: there seems to have a problem with sqlite which go back more than 20
days ago. All values sent to database seems to be replaced with "?". I just
learned it via travis.


Reply to this email directly or view it on GitHubhttps://github.com//pull/141#issuecomment-10034656.

Thanks,
Anatoliy Chakkaev

@sdrdis
Copy link
Contributor Author

sdrdis commented Nov 3, 2012

Noted for the spaces ;)

But for sqlite there seems to be a problem though :

Here is what I got from travis (https://travis-ci.org/#!/1602/jugglingdb/jobs/3041384):

INSERT INTO posts (title,subject,content,date,published,likes,related,id,userId) VALUES (?,?,?,?,?,?,?,?,?)
849SELECT 1 FROM posts WHERE id = 1 LIMIT 1
850✔ sqlite3 - should create object
851INSERT INTO posts (title,subject,content,date,published,likes,related,id,userId) VALUES (?,?,?,?,?,?,?,?,?)
852SELECT * FROM posts WHERE title = ?
853✔ sqlite3 - should create object without callback
854INSERT INTO posts (title,subject,content,date,published,likes,related,id,userId) VALUES (?,?,?,?,?,?,?,?,?)
855UPDATE posts SET title = ?, subject = ?, content = ?, date = ?, published = ?, likes = ?, related = ?, id = ?, userId = ? WHERE id = 3
856INSERT INTO posts (title,subject,content,date,published,likes,related,id,userId) VALUES (?,?,?,?,?,?,?,?,?)

@1602
Copy link
Owner

1602 commented Nov 3, 2012

There's no problem is all tests passed for sqlite adapter. If you look to
adapter source code you will see that this is expected behaviour. Sqlite
driver can escape values and replace '?' with escaped values.

On Sat, Nov 3, 2012 at 4:53 AM, Sébastien Drouyer
notifications@github.comwrote:

Noted for the spaces ;)

But for sqlite there seems to be a problem though :

Here is what I got from travis (
https://travis-ci.org/#!/1602/jugglingdb/jobs/3041384):

INSERT INTO posts(title,subject,content,date,published,likes,related,id,userId) VALUES
(?,?,?,?,?,?,?,?,?)
849SELECT 1 FROM posts WHERE id = 1 LIMIT 1
850✔ sqlite3 - should create object
851INSERT INTO posts(title,subject,content,date,published,likes,related,id,userId) VALUES
(?,?,?,?,?,?,?,?,?)
852SELECT * FROM posts WHERE title = ?
853✔ sqlite3 - should create object without callback
854INSERT INTO posts(title,subject,content,date,published,likes,related,id,userId) VALUES
(?,?,?,?,?,?,?,?,?)
855UPDATE posts SET title = ?, subject = ?, content = ?, date = ?,
published = ?, likes = ?, related = ?, id = ?, userId = ? WHERE id = 3
856INSERT INTO posts(title,subject,content,date,published,likes,related,id,userId) VALUES
(?,?,?,?,?,?,?,?,?)


Reply to this email directly or view it on GitHubhttps://github.com//pull/141#issuecomment-10034717.

Thanks,
Anatoliy Chakkaev

@sdrdis
Copy link
Contributor Author

sdrdis commented Nov 3, 2012

Sorry I was just surprised by the sql debug logs :).

I fixed the indentation issues as adapted to the markdown syntax.

anatoliychakkaev pushed a commit that referenced this pull request Nov 3, 2012
Added caching in relationships (+ test cases) (enhanced)
@anatoliychakkaev anatoliychakkaev merged commit 0c24dfa into 1602:master Nov 3, 2012
@anatoliychakkaev
Copy link
Collaborator

Some tests broken. Please run npm test and ensure everything works before pulling. You also can run only specified adapter tests:

ONLY=mongodb nodeunit test/common_test.js
ONLY=sqlite3 nodeunit test/common_test.js

@sdrdis
Copy link
Contributor Author

sdrdis commented Nov 3, 2012

I ran the tests at home before pushing with mysql, sqlite3, and postgresql, and it works for me everywhere even on sqlite3 (where as on travis it bugs). I will take a look tomorrow at my packages versions to see if there are differences. Thanks and sorry :).

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

Successfully merging this pull request may close these issues.

3 participants