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

SPOD sys.db.Manager cacheObject Neko/PHP inconsistency #20

Open
filt3rek opened this issue Apr 18, 2018 · 2 comments
Open

SPOD sys.db.Manager cacheObject Neko/PHP inconsistency #20

filt3rek opened this issue Apr 18, 2018 · 2 comments

Comments

@filt3rek
Copy link

Hej,

I don't know if it has been solved yet, but as it's not longer maintened by Haxe, I repost it here (original post : HaxeFoundation/haxe#5636)

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.

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...

@jonasmalacofilho
Copy link
Member

Hi again @filt3rek ,

Can you share an example of what the real world inconsistency looks like?

I see that the behavior can be different between targets but, just from looking at the code, I'm actually more tempted to go the opposite route and remove the (potential) extra fields from Neko.

@filt3rek
Copy link
Author

Hej again Jonas ! :)

Well, It's one year ago, I'll try to retrieve my example and post it. What I remember roughly is that you see "null" for objects in dbadmin/php... But It's better I retrieve my example...

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

No branches or pull requests

2 participants