EZP-21764: Don't instantiate eZINI outside of classes

[jeromegamez]

At the moment, eZINI instances for ezfind.ini, solr.ini and site.ini get created outside of the the classes ezfSolrDocumentFieldBase and ezfeZPSolrQueryBuilder, even when their containing PHP files just get included through the autoloader. This can cause problems, e.g. in the context of Unit Tests when this happens before the INI files get overriden by other processes.

This PR preserves the static class variables, but ensures that they get instantiated inside the classes only.

If have thought about converting them to local variables, but wanted to make sure that derived classes, that access them, don't get problems.

Cheers
Jérôme

PS: This PR is part of a process to make the eZ Find Unit Tests work again.

[lolautruche]

Hi and thanks again @jeromegamez !

Why don't you create a utility method instead of always repeating the same code ?

[jeromegamez]

I thought about this, but then, we could also just call e.g. self::$FindINI = eZINI::instance( 'ezfind.ini' ); which seems to be the appropriate helper method. What do you think?

[lolautruche]

Mmm, you can sometimes have surprises when dealing with eZINI and its multiton pattern. I really prefer that you create one utility method per INI file.

[jeromegamez]

Done!

... looks weird, though :)

[lolautruche]

It's not exactly what I suggested.
The problem in your change is that your utility method doesn't return the instance 😉

Should be something like this:

    public static function getEzFindIniInstance()
    {
        return self::$FindINI = ( self::$FindINI instanceof eZINI ) ? self::$FindINI : eZINI::instance( 'ezfind.ini' );
    }

And everywhere else you use this getter when you need it ;-)

[jeromegamez]

I'm not sure I understand fully. How would I use this method? Should I do a

$eZFindINI = self::getEzFindIniInstance();

and replace every usage of self::$FindINI with $eZFindIni? I tried it the other way to reduce the amount of code changes, but if this is the best approach, I'm happy to do it this way.

[lolautruche]

Yes, this is what I meant.

[jeromegamez]

This should look better now, I hope.

[lolautruche]

+1

[yannickroger]

+1

[guillaumelecerf]

+1

[jeromegamez]

Rebased into current master to resolve merge conflicts and ease merging.

[jeromegamez]

The change "causing" the merge conflict since last was 6d6292c, which is preserved in this PR, as you can see in https://github.com/jeromegamez/ezfind/blob/EZP-21764_unintentional_ezini_instances/classes/ezfsolrdocumentfieldbase.php#L470 .

[andrerom]

Sorry to jump in one year later, but unless I'm missing something you can skip the whole static property actually.

Instance cache is kept inside eZINI::instance, so now that a method call is added to get property, the micro micro wasted static property cache optimization is no longer even worth it.

[jeromegamez]

And another year later, I finally removed the static ini helper methods and replaced them with proper eZINI::instance() calls :).

[andrerom]

Did you check if anything is using this given it has been public for so long?
basically small bc break here.

[jeromegamez]

Yes, I checked - at least inside the eZ Find extension, the static variables are not used in other places.

[andrerom]

+1, also for 5.4 and 5.3 backport if it applies cleanly (for easier maintenance)

[paulborgermans]

did not find any use of it "outside", for example in other pending pull requests ;)

[andrerom]

🚢 🎉 :)

[jeromegamez]

YEAH! 🎊 🎉 Thanks, André!