HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.15k stars 656 forks source link

SPOD sys.db.Manager cacheObject Neko/others targets inconsistency #5636

Closed filt3rek closed 6 years ago

filt3rek commented 8 years ago

Hej,

I'm sorry I've just tested that on Neko and PHP targets playing with DBAdmin first, and I get inconsistency here between the 2 targets : https://github.com/HaxeFoundation/haxe/blob/development/std/sys/db/Manager.hx#L390 It seems that when creating an empty instance on Neko like that : untyped __dollar__new(x); where x is an object with fields/values, it fills these fields automatically on the class instance. But on the others targets this one is used : Type.createEmptyInstance( My ClassName ); Here it doesn't fill the class fields, it's ok, but then in this cacheObject function, there miss this fill. So I added that : Reflect.setField( o, f, val ); just after https://github.com/HaxeFoundation/haxe/blob/development/std/sys/db/Manager.hx#L401

So at the end it gives that :

function cacheObject( x : T, lock : Bool ) {
        #if neko
        var o = untyped __dollar__new(x);
        untyped __dollar__objsetproto(o, class_proto.prototype);
        #else
        var o : T = Type.createEmptyInstance(cast class_proto);
        untyped o._manager = this;
        #end
        normalizeCache(x);
        for (f in Reflect.fields(x) )
        {
            var val:Dynamic = Reflect.field(x,f), info = table_infos.hfields.get(f);
                        Reflect.setField( o, f, val );
            if (info != null)
            {
                var fieldName = getFieldName(info);
                Reflect.setField(o, fieldName, val);
            }
        }
        Reflect.setField(o,cache_field,x);
        addToCache(o);
        untyped o._lock = lock;
        return o;
    }

Now all is working fine and DBAdmin gives me all the fields like on Neko target.

bablukid commented 8 years ago

Hi @filt3rek , I noticed these inconsistencies in dbadmin, and fixed them in a pull request, it seems we're targeting the same problem. https://github.com/ncannasse/dbadmin/pull/9/commits/20a278b6140f4ab9701311c95e9067249a309478#diff-d7eb2302b4a10fed10e69d1266864b26R424

filt3rek commented 8 years ago

Hi @bablukid , yes we probably encoured the same problem. But when I digged to see where the differences between Neko and PHP come from exactly, I've been until sys.db.Manager where in the cacheObject function, the returned created instance of the class is not filled correctly on others targets than Neko. Maybe it should be fixed there ? I don't know because like you, I've seen this bug only on dbadmin for the moment...

Simn commented 6 years ago

SPOD is no longer maintained here and has been moved to https://github.com/haxefoundation/record-macros