Fix setter value type with prado tpl/Page

[zendre4]

This is for fix #836 .

My fix ensure value for bool, integer, and float.

I test this on PHP 7.4 and 8.0. If union type are used, the fix do noting.

[belisoful]

Why not the ensureString as well? As is a sanity check... setting a property with a <%= %> may not necessarily return a string.

[belisoful]

Should be updated to be behavior aware with #825

[ctrlaltca]

I guess this going to be $component->hasMethod($setter)

[belisoful]

between getting the "new reflectionclass()" and this type-name, caching the class/property/type into a static would greatly speed things up.

[belisoful]

The setter/property may not be in the component but in a behavior modifying the component. looping through each behavior would be needed too but that requires #826.

[ctrlaltca]

Looping all the behaviors from here sounds time expensive (but can be fixed with some caching) but also a bit incorrect from an OOP point of view.
Maybe adding a way to get the reflected method from inside TComponent itself could be an idea. This would also move the reflection cache inside the class itself, so that other callers can benefit from it.

[belisoful]

TPropertyValue::EnsureCode($github)

my apologies if my critiques sound harsh, they aren't. I like the idea of auto-ensuring typed setters. The full scope of PRADO is quite vast and each change, not matter how small, requires looking at the whole.

[zendre4]

Ok @belisoful I add the ensureString and I modify the code for stored the type in a variable, it's ok for you for this two part ?

Regarding issue #825 what do you expect from me, I can't use hasMethod because it doesn't exist yet. I can modify my if, but it will not merge until #825 is finished...

And for #826 I don't sure to understand the change

[belisoful]

Adding the ensureString is great.

When I said to store the type in a variable I mean that a "static $cache" variable needs to be created. The cache needs to store the reflection based on the get_class($component). Then the static cache should store the setter method type for easy and quick retrieval from the cache. We want to only call Reflection on a class once, and we only want to get the setter type once. Retrieving the setter type from cache is much faster than calling the same thing over and over and over. For an example of what I am talking about, see TApplicationComponent::getClassFxEvents. That caches the data in a static as well as retrieves the data from the PRADO cache. Both of those are things we can do to speed up this code.

#825 and #826 is urgent and that is what I am focused upon. I am hoping to have this committed in the next day or two and merged shortly there after. You'll be able to use these methods from the main branch there after. My humble apologies for not having these completed long ago.

As for why #826 is important... It is possible for a behavior to implement a setter that doesn't exist in the owner's methods. So you'll need to loop through all the behaviors for the setters and their types beyond the main component.

Behaviors were added in 3.2.3 as a way of extending and changing the behavior of a TComponent. Also to be up-to-date technically with Yii (in that regard, a go beyond with "dy" and "fx" events). Behaviors can add events, properties, and methods to a TComponent that it doesn't originally have. A behavior can even make one object "look" and act like another.

A great example I love to use for behaviors is the "student" behavior for users. A StudentUserBehavior can add school class scheduling and grades properties to the TUser without having to have a separate structure with separate reference to maintain for each user. The TUser simply automatically gets the Student methods, events, and properties. If you want an example, see the Permissions manager TUserPermissionsBehavior for an example of how Advanced Behaviors are used. Also see the Prado\Util\Behaviors directory.

[zendre4]

ok I will wait for you to finish #825 and #826. After that, I'll use the new "hasMethod", and I'll try to use static and cache (I never used that in Prado yet)

[belisoful]

Using PRADO cache may be overkill and can be implemented after the static variable is used to cache class-setter types.

What I am looking for is to only call the Reflection class once per class, and once per setter.

[belisoful]

Also, when the getBehaviors method get implemented, take that array, prepend the original object, put the ensure code in a loop on each object in the array and you'll get the object and behavior setter type, in the proper order, easy-peasy.

[ctrlaltca]

This looks promising to me, as it solves the long time problem of interpreting the correct type for data coming from templates.
The ReflectionClass calls definitely needs some type of caching to avoid creating possibly hundreds of identical objects for a single template

[ctrlaltca]

The ReflectionClass calls definitely needs some type of caching to avoid creating possibly hundreds of identical objects for a single template

From a few quick tests and some literature found on the web it seems that creating ReflectionClass objects is only expensive the very first time, while following object gets created in a negligible time.
Still, if this needs to be extended to methods introduced by behaviors, we need caching for our part.

[belisoful]

scrubbing data types for function parameters is also needed for setting command line data in the shell. when --variable=someData_1234 the variable is mapped to a setter handler. The setter handler could read the function parameter data type and ensure to int, bool, float, string, array as well.

This functionality shouldn't be kept only in the template engine.

Also, This function is needed in the xml config.