FIX Singleton creation

[jonom]

Ensure DataObject instances are aware they are singletons so functions like populateDefaults() can be skipped. (fixes #4878)

[jonom]

@tractorcow one of the builds failed during setup, I'm guessing re-running test won't help here?

[tractorcow]

Hm, the logic looks sound... but maybe @chillu should look over this too.

Perhaps in 4.x we should override Object::singleton() in DataObject, in order to inject the new parameter, using the 3rd argument to Injector::get(). That way all of the DataObject specific stuff stays in DataObject.php.

@chillu is this specific fix ok for 3.x?

[tractorcow]

Don't worry about that; It's the fault of imagick no longer supporting php 5.3

[jonom]

Perhaps in 4.x we should override Object::singleton() in DataObject, in order to inject the new parameter, using the 3rd argument to Injector::get()

That might work but some refactoring would be required first. Object::singleton() isn't called at all during build currently, instead the global singleton($className) function as defined in Core.php is used. I'm guessing there's a good reason for this but outside my expertise :)

BTW the manipulation I'm doing of constructor args could probably be done in get() instead of instantiate(), not sure if either one is better?

[tractorcow]

Yeah, that's why I hate global functions. Itcan use $class::singleton() fine though.;)

[jonom]

So the question is then, is there a reason we need a global singleton method?

Unless we can remove it (or effectively remove it by passing through calls to $className::singleton()) we're going to get inconsistent results between singleton($className) and $className::singleton() if we take the DataObject::singleton() path, which I don't think would be acceptable would it?

Let's give it a try... #5154 (I've really got to get tests working locally)

[tractorcow]

So the question is then, is there a reason we need a global singleton method?

Not anymore, really, at least not in 4.0. Injector::inst()->get() should be used for non-object subclasses, and anything that extends Object (or trait Injectable in master) should just use ::singleton().

[jonom]

Closing this for now pending outcome of #5154

[jonom]

New version at #7850