[5.2] Ajax component support of Stringable results

[Fedik]

Pull Request for https://github.com/joomla/joomla-cms/discussions/43471 .

Summary of Changes

~~Adding support of Stringable https://www.php.net/manual/en/class.stringable.php results to Ajax component.~~ Adding Joomla\CMS\String\StringableInterface results to Ajax component, this is transitioning interface to Stringable https://www.php.net/manual/en/class.stringable.php. This allows to customise the response much better, than it is currently.

Testing Instructions

Add following code in to plugins/system/skipto/src/Extension/Skipto.php:

    public function onAjaxSkiptoTest1(\Joomla\CMS\Event\Plugin\AjaxEvent $event)
    {
        $event->updateEventResult(new class ('test 1') extends \Joomla\CMS\Response\JsonResponse implements \Joomla\CMS\String\StringableInterface {});
    }

    public function onAjaxSkiptoTest2(\Joomla\CMS\Event\Plugin\AjaxEvent $event)
    {
        $event->addResult(['foo', 'bar']);
    }

    public function onAjaxSkiptoTestError(\Joomla\CMS\Event\Plugin\AjaxEvent $event)
    {
        throw new \Exception('test error');
    }

Then open following links:

1. /administrator/index.php?option=com_ajax&plugin=skiptoTest1&group=system&format=raw
2. /administrator/index.php?option=com_ajax&plugin=skiptoTest1&group=system&format=json
3. /administrator/index.php?option=com_ajax&plugin=skiptoTest2&group=system&format=json
4. /administrator/index.php?option=com_ajax&plugin=skiptoTestError&group=system&format=raw
5. /administrator/index.php?option=com_ajax&plugin=skiptoTestError&group=system&format=json

Expected result AFTER applying this Pull Request

Check response for each:

1. {"success":true,"message":null,"messages":null,"data":"test 1"}
2. {"success":true,"message":null,"messages":null,"data":"test 1"}
3. {"success":true,"message":null,"messages":null,"data":[["foo","bar"]]}
4. Exception: test error
5. {"success":false,"message":"test error","messages":null,"data":null}

Link to documentations

Please select:

• [ ] Documentation link for docs.joomla.org:
• [ ] No documentation changes for docs.joomla.org needed
• [x] Pull Request link for manual.joomla.org: https://github.com/joomla/Manual/pull/274
• [ ] No documentation changes for manual.joomla.org needed

[korenevskiy]

Thanks, I tested it. I spent many hours testing on my project. It can be seen that you spent just as many hours writing, testing, finding bugs and writing documentation for PR. I tested in JSON format, with error, without error, with data and with a JsonResponse object. I also set the parameters $input->set('ignoreMessages',false, 'bool'), and also tested in RAW mode, also in different configurations. I also made an error throw with a message. Now Joomla has become better. Now we can send customized messages as system messages in case of an error, and special notifications. Respect.

there were no problems during testing.

[Fedik]

@korenevskiy Please visit https://issues.joomla.org/tracker/joomla-cms/43530 and push the button, thanks

[carlitorweb]

/administrator/index.php?option=com_ajax&plugin=skiptoTest2&group=system&format=raw

Warning: Array to string conversion in [ROOT]\components\com_ajax\ajax.php on line 238

[Fedik]

@carlitorweb that is correct, you cannot run skiptoTest2 with format=raw, only with format=json. As in my instruction. Because format=raw cannot handle anything else than scalar or stringable objects, and skiptoTest2 produces an array.

[carlitorweb]

I have tested this item :white_check_mark: successfully on 66f35b7956067b5a31978a21f4948b07a1ad28ee

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43530.}

[korenevskiy]

I have tested this item :white_check_mark: successfully on 66f35b7956067b5a31978a21f4948b07a1ad28ee

there were no problems during testing.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43530.}

[Quy]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43530.}

[rdeutz]

We discussed this last week and we think this is a b/c break. It would be better to add a new format instead of changing an existing one and do "kind of magic" on this.

[Fedik]

we think this is a b/c break

What kind of b.c break? I do not see any

[korenevskiy]

We discussed this last week and we think this is a b/c break. It would be better to add a new format instead of changing an existing one and do "kind of magic" on this.

By creating a new format, we force each developer to refer to the documentation each time in order to understand the subtleties of the differences between each format. But ultimately the formats are the same, it's just a matter of processing JSON.

[Fedik]

p

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43530.}

[Fedik]

I have update PR, to be more backward compatible. Please test again.

[korenevskiy]

Can you tell me how the Joomla\CMS\String\StringableInterface interface is used here?

It can be seen that the minimum version of Joomla 5.1 is 8.1, Joomla 5.2 may have the same. The built-in Stringable interface in PHP has a minimum version of 8.0.

Using the Joomla\CMS\String\StringableInterface, you will oblige developers to repeatedly return to the documentation or to the AJAX component code to understand what needs to be inherited from the new interface. If you inherit from Joomla\CMS\String\StringableInterface, the extension will still depend on the latest version of Joomla 5.2. And also the main reason for this whole idea.

The object of the JsonResponse class will be passed as a result to the parameter of the new object of the JsonResponse class and will be nested JSON in the main JSON. Thus, it does not give me the opportunity to debug the code by passing additional messages.

[korenevskiy]

Let's replace this

if (!($results instanceof Throwable) && $results instanceof StringableInterface)

to it

if (!($results instanceof Throwable) && $results instanceof \Joomla\CMS\Response\JsonResponse)

Thus, we will allow those who want to additionally serialize a string in JSON to do so. And also independently transmit an object of class \JsonResponse. or do a json_validate() check

It seems that the new interface will cause confusion. After all, its purpose is only for JsonResponse, Maybe we'll call it \JsonResponseStringableInterface

[Fedik]

If you want extend JsonResponse, you can see it in the test:

new class ('test 1') extends \Joomla\CMS\Response\JsonResponse implements \Joomla\CMS\String\StringableInterface {};

Or make your own:

$response = new class () implements StringableInterface {
  public $potatos = [];

  public function __toString(): string
  {
    return json_encode($this->potatos);
  }
};
$response->potatos = $arrayOfPotatos;

the extension will still depend on the latest version of Joomla 5.2

yeap, it is new feature

[korenevskiy]

Why don't you allow the \JsonResponse object to be returned from the user's extension ?

[korenevskiy]

I understood your idea, and I see the potential, but there is no simplicity in this solution. And if you allow the \JsonResponse object to be returned, the result will be the same. Maybe?

if (!($results instanceof Throwable) 
&& ($results instanceof \Joomla\CMS\Response\JsonResponse || \Joomla\CMS\String\StringableInterface))

[korenevskiy]

Still, the new interface needs to be renamed so that it is clear that this applies to \JsonResponse, since the wrong interface will be inherited by mistake. And if suddenly the new year is intentionally inherited from the new interface in cases not related to \JsonResponse, then this will produce meaningless objects, as a result it will need to be inherited from 2 interfaces. You have created a good flexibility, deeper, but it will cause problems. Either allow \JsonResponse to be returned, or return an object of a special interface from which \JsonResponse will be inherited. i.e. inherit \JsonResponse also from the \Joomla\CMS\String\StringableInterface interface

[Fedik]

but there is no simplicity in this solution

It is transitioning to native PHP Stringable, which we cannot use now because some backward compatibility problem can happen in some edje cases.

The idea is the response can be anything, as long as it is Stringable, and so can be rendered in browser. Example for ?format=xml:

$response = new class () implements StringableInterface {
  public $potatoXml;

  public function __toString(): string
  {
    return $this->potatoXml->asXML();
  }
};
$response->potatoXml = $potatoXml;

[korenevskiy]

The idea is the response can be anything, as long as it is Stringable, and so can be rendered in browser. Example for ?format=xml:

Of course, the string can be XML. But we are discussing the return inside the JSON condition. If the condition is JSON, then we need to check for JSON, or for JsonRespone

switch ($format) {
    case 'json':

In this case, you need to have an interface for responses only. And the \JsonResponse and \XMLResponse classes inherit from the new interface. StringableResponseInterface so that it is enough to inherit only a choice either from the interface or only from the class. But at the same time, so that there was such an opportunity to use a choice of either that or that. And you suggest using both at once,

[korenevskiy]

I just realized that there is no point in introducing a new interface. Before, it seemed to me that $var instanceof Stringtable is equivalent to is_string($var). I thought that supposedly both variables to cast a string would give the same value. But it's obvious that the values are the same, but the condition is different. This means that there is no point in introducing a new interface. The condition is if the extension code returns an object $obj instanceof Stringable, which means the object will be cast to a string, but if a string is returned from the extension code, then obviously it needs to be passed as a parameter to the object of the JsonResponse class. There is no point for a new interface.

[Fedik]

But it's obvious that the values are the same

No. String is a string and Object is object (even when it implement Stringable). new JsonResponse($string) will produce: {"data":"the string value"} new JsonResponse($stringableObject) will produce : {"data":{ ... the object properties}}

[korenevskiy]

The answer can be anything, including XML. I agree, maybe, but it will be another condition of the format, are we discussing ?format=json, and json creation takes place inside the switch case 'Json' condition. Inside the JSON condition, the \JSONResponse object was created before, which means backward compatibility is quite simple, which is placed in the \JSONResponse object, the parameter should not be a \JSONResponse, this will already indicate that backward compatibility has been implemented. Suppose you want new features to use any object to generate JSON, such an object can inherit the Stringable interface. In any case, there was no such opportunity before. The custom object will be converted to a string instead of a non-\JSONResponse. But if the user wants to bring his object with the interface to a string and additionally serialize, then let his object itself lead to a string, and already return the string from the extension, this will allow the string to be serialized inside \JSONResponse. For the \JSONResponse class, add the parent \Stringable

But I think that in any case, the interface you offer should be renamed to \StringtableResponseInterface or don't use the new interface at all..

[Fedik]

which means backward compatibility is quite simple

Nope, when existing extension provides result as JsonResponse, then it will be wrapped in to another JsonResponse. So in result we will have: new JsonResponse($anotherJsonResponse) will produce : {"data":{"data":{ ... the object properties}}}.

Even if it does not make sense, it still changing the response that com_ajax will send. and that will break that extension.

[korenevskiy]

new JsonResponse($anotherJsonResponse) will produce : {"data":{"data":{ ... the object properties}}}.

if (!($results instanceof Throwable) && $results instanceof Stringable)

At the same time, it is necessary to add to the \JSONResponsee class libraries/src/Response/JsonResponse.php

class JsonResponse implements  Stringable{
}

If you do as you say, parents should do this interface for \JSONResponse.

[korenevskiy]

I like your idea, but it seems to me that it is possible to achieve an elegant solution without adding a new interface.

[korenevskiy]

OR

if (!($results instanceof Throwable) && $results instanceof StringableResponseInterface)

At the same time, it is necessary to add to the \JSONResponsee class libraries/src/Response/JsonResponse.php

class JsonResponse implements  StringableResponseInterface{
}

Where StringableResponseInterface it is joomla new interface

[Fedik]

No, JsonResponse already implements Stringable implicitly.

Unlike most interfaces, Stringable is implicitly present on any class that has the magic __toString() method defined

And currently we cannot add our StringableInterface to it, because it will change response format. In reasons I wrote earlier.

Currently developers who wants a custom response should implement StringableInterface on their own. And in future (~joomla 7), we change com_ajax to use PHP native Stringable, and all will continue to work without breaks.

[korenevskiy]

You right Maybe?

if (!($results instanceof Throwable) 
&& ($results instanceof \Joomla\CMS\Response\JsonResponse || \StringableResponseInterface))

Where StringableResponseInterface it is joomla new interface

[korenevskiy]

Is it worth checking the object for JsonSerializable interface?

https://www.php.net/manual/en/jsonserializable.jsonserialize.php