1602 / jugglingdb

Multi-database ORM for nodejs: redis, mongodb, mysql, sqlite3, postgresql, arango, in-memory...
http://1602.github.io/jugglingdb/
2.05k stars 241 forks source link

Is a bug in model.create method?“ inst._initProperties(data);”is missing! #419

Closed srisy closed 8 years ago

srisy commented 9 years ago

when create object like: var post = new Post({//some data}); then call save method to real create a record in db: post.save(function(err, obj){//do something}); jugglingdb is actually call Model.create to create a object:

if (!this.id) {
        // Pass options and this to create
        var data = {
            data: this,
            options: options
        };
        return Model.create(data, callback);//call Model.create here,since no id
    }

in Model.create method,inst._initProperties(data); is not found:

 if (rev) {
            rev = Model._fromDB(rev);
            //is "inst._initProperties(data);" should in here???
            obj._rev = rev
 }

how to synchronize post.data and post.dataWas when record is create in db successful?

1602 commented 9 years ago

Can you create test case please?

srisy commented 9 years ago

the case: var p = new Post({title: 1}); p.title = 2; p.save(function (err, obj) { if (err) {console.log(err);} console.log(p.propertyChanged('title')); //true!!!
});

a record is create in db with title=2,but the p.data and p.dataWas is different after call save! i can't upload image,so sad! in this case,p.data.title=2,and p.dataWas.title=1,i found other method in Model, such as upsert etc.,all them call "inst._initProperties(data);" after database io return, to synchronize post.data and post.dataWas,but "create" not! why??

srisy commented 9 years ago

image when i add "obj == obj._initProperties(rev);" in the "create" method callback,all test pass!only one failed! image this is only one i test failed: image the message is: image why?? so miracle?

srisy commented 9 years ago

i'm building ms sql server adapter and all tests pass,if the statement "obj == obj._initProperties(rev);" is added.i will use it in my projects for testing more fully.

srisy commented 9 years ago

image comment the "obj == obj._initProperties(rev);",p.data.title is different from p.dataWas.title in "save" callback: image

1602 commented 9 years ago

Let me know if this fix works for you.

srisy commented 9 years ago

error when u fix the create: image p.data.title = 3 and p.dataWas.title = 3, when “p.title = 3;”

anatoliychakkaev commented 9 years ago

It's not wrong. propertyChanged('anything') should always return false after save.

On 16 August 2014 15:25, srisy notifications@github.com wrote:

error when u fix the create: [image: image] https://cloud.githubusercontent.com/assets/8347867/3942037/b5e6c70e-2550-11e4-9970-cf3c46fc3641.png p.data.title = 3 and p.dataWas.title = 3, when “p.title = 3;”

— Reply to this email directly or view it on GitHub https://github.com/1602/jugglingdb/issues/419#issuecomment-52394572.

srisy commented 9 years ago

after statement “p.title = 3;”, p.propertyChanged('title') is false!!! it should be true! p.data.title = 3 and p.dataWas.title = 3 at the same time, when “p.title = 3;”, i think u should turn back the fix.

anatoliychakkaev commented 9 years ago

Sorry, I don't get. Can you post full test case?

On 18 August 2014 10:36, srisy notifications@github.com wrote:

after statement “p.title = 3;”, p.propertyChanged('title') is false!!! it should be true! p.data.title = 3 and p.dataWas.title = 3 at the same time, when “p.title = 3;”, i think u should turn back the fix.

— Reply to this email directly or view it on GitHub https://github.com/1602/jugglingdb/issues/419#issuecomment-52470512.

Thanks, Anatoliy Chakkaev

srisy commented 9 years ago

Ok, put down this problem, let's discuss other one,see the picture: 1 content = 'New content'; should be save to db also i think, actually it‘s save to db, see the picture: image

the test item 'test.equal(post.content, 'content', 'real value turned back');' is right??? i would update all 'data' parameter's value to db which AbstractClass pass to adapter: 2

srisy commented 9 years ago

I can't understand this case: image 'nonSchemaField' as a property be placed to Post instance 'post', not post.data or post.dataWas image

anatoliychakkaev commented 9 years ago

Non-schema fields are not saved to database.

On 18 August 2014 11:12, srisy notifications@github.com wrote:

I can't understand this case: [image: image] https://cloud.githubusercontent.com/assets/8347867/3950060/8c837bd0-26bf-11e4-8aa6-865dbc45fb7e.png 'nonSchemaField' as a property be placed to Post instance 'post', not post.data or post.dataWas [image: image] https://cloud.githubusercontent.com/assets/8347867/3950103/1e5269ea-26c0-11e4-8b7c-55aa9f1cc81d.png

— Reply to this email directly or view it on GitHub https://github.com/1602/jugglingdb/issues/419#issuecomment-52473727.

Thanks, Anatoliy Chakkaev

srisy commented 9 years ago

it should be deleted in callback!

srisy commented 9 years ago

I encounter a problem when i run ‘datatype.test.js’, Create a record to db with a array column 'list',it has only value:'test' Model.create({str: 'hello', date: d, num: '3', bool: 1, list: ['test']}, function(err, m) {}); [AbstractClass.create] convert the 'list'to a object and add 'id' key, such as '{"id":"test"}',then pass the

object to [adapter.create] method wrapped in data object. the sql:

DELETE FROM [dbo].[Model]
INSERT INTO [dbo].[Model] ([str],[date],[num],[bool],[list])
VALUES ('hello','2014-08-21 18:04:27.106',3,1,'[{"id":"test"}]');
SELECT SCOPE_IDENTITY() AS insertId;

@1602 , run the ‘datatype.test.js’ to see the problem.

the previous several problem i found when running "common_test.js", can't pass rather than mysql or my sqlserver adapter.

srisy commented 9 years ago

failed in "common_test.js":

should update single attribute (fail) ✖ 
Assertion Message: real value turned back
AssertionError: 'New content' == 'content'
    at Object.equal (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\node_modules\nodeunit\lib\types.js:83:39)
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\test\common_test.js:366:26
    at Function.<anonymous> (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:448:9)
    at MsSQL.<anonymous> (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\lib\mssql.js:481:3)
    at innerCB (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\lib\mssql.js:229:3)
    at d:\My Projects\Mssql\Mssql\node_modules\mssql\lib\main.js:1186:53
    at Request.userCallback (d:\My Projects\Mssql\Mssql\node_modules\mssql\lib\tedious.js:543:59)
    at Request.callback (d:\My Projects\Mssql\Mssql\node_modules\mssql\node_modules\tedious\lib\request.js:30:27)
    at Connection.STATE.SENT_CLIENT_REQUEST.events.message (d:\My Projects\Mssql\Mssql\node_modules\mssql\node_modules\tedious\lib\connection.js:253:29)
    at Connection.dispatchEvent (d:\My Projects\Mssql\Mssql\node_modules\mssql\node_modules\tedious\lib\connection.js:667:59)
should save only schema-defined field in database (fail) ✖ 
AssertionError: true == false
    at Object.ok (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\node_modules\nodeunit\lib\types.js:83:39)
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\test\common_test.js:267:18
    at ModelConstructor.<anonymous> (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:328:29)
    at ModelConstructor.next (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\hooks.js:61:18)
    at ModelConstructor.<anonymous> (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:327:36)
    at ModelConstructor.next (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\hooks.js:61:18)
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:326:30
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\lib\mssql.js:316:4
    at innerCB (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\lib\mssql.js:229:3)
    at d:\My Projects\Mssql\Mssql\node_modules\mssql\lib\main.js:1186:53
should save object (fail) ✖ 
AssertionError: true == false
    at Object.ok (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\node_modules\nodeunit\lib\types.js:83:39)
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\test\common_test.js:234:26
    at ModelConstructor.<anonymous> (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:328:29)
    at ModelConstructor.next (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\hooks.js:61:18)
    at ModelConstructor.<anonymous> (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:327:36)
    at ModelConstructor.next (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\hooks.js:61:18)
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:326:30
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\lib\mssql.js:316:4
    at innerCB (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\lib\mssql.js:229:3)
    at d:\My Projects\Mssql\Mssql\node_modules\mssql\lib\main.js:1186:53

AssertionError: '3' == 2
    at Object.equal (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\node_modules\nodeunit\lib\types.js:83:39)
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\test\common_test.js:235:26
    at ModelConstructor.<anonymous> (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:328:29)
    at ModelConstructor.next (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\hooks.js:61:18)
    at ModelConstructor.<anonymous> (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:327:36)
    at ModelConstructor.next (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\hooks.js:61:18)
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:326:30
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\lib\mssql.js:316:4
    at innerCB (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\lib\mssql.js:229:3)
    at d:\My Projects\Mssql\Mssql\node_modules\mssql\lib\main.js:1186:53
srisy commented 9 years ago

custom primary keys not quite working, when define column with primaryKey:true,can you post a example how to use custom primary key? thank u!

commonTest.it("should support custom primary key", function (test) {
    test.expect(8);
    var AppliesTo = schema.define("AppliesTo", {
        AppliesToID: {
            type: Number,
            primaryKey:true
        },
        Title: {
            type: String,
            limit: 100
        },
        Identifier: {
            type: String,
            limit: 100
        },
        Editable: {
            type: Number
        }
    });

    schema.automigrate(function (err) {
        test.ifError(err);

        AppliesTo.create({Title: "custom key", Identifier: "ck", Editable: false}, function (err, data) {
            test.ifError(err);
            test.notStrictEqual(typeof data.AppliesToID, 'undefined');
            data.Title = 'change the title';
            test.equal(data.propertyChanged('Title'), true);
            data.save(function (err, obj) {
                test.ifError(err);
                test.equal(data.propertyChanged('Title'), false);
                test.equal(obj.__data.Title, obj.__dataWas.Title);
                test.equal(data, obj);
                test.done();
            });
        });
    });
});
 should support custom primary key (fail) ✖ 
AssertionError: 'undefined' !== 'undefined'
    at Object.notStrictEqual (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\node_modules\nodeunit\lib\types.js:83:39)
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\test\commontests.js:241:9
    at ModelConstructor.<anonymous> (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:328:29)
    at ModelConstructor.next (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\hooks.js:61:18)
    at ModelConstructor.<anonymous> (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:327:36)
    at ModelConstructor.next (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\hooks.js:61:18)
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb\lib\model.js:326:30
    at d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\lib\mssql.js:316:4
    at innerCB (d:\My Projects\Mssql\Mssql\node_modules\jugglingdb-mssql\lib\mssql.js:229:3)
    at d:\My Projects\Mssql\Mssql\node_modules\mssql\lib\main.js:1186:53