jaxon-php / jaxon-core

The Jaxon core library
https://www.jaxon-php.org
BSD 3-Clause "New" or "Revised" License
65 stars 28 forks source link

Can Jaxon\Response\Manager's append(Response $xResponse) be nullable? #35

Closed vita10gy closed 5 years ago

vita10gy commented 5 years ago

We're former xajax users that were able to basically drop jaxon in place with the proper config.

All went well until we found a few user functions that never returned a response object. (They're just saves that don't need to do anything response wise.) I guess Xajax never had that as an implicit requirement.

However that now results in the following error in the response to the browser, although the save is successful: Argument 1 passed to Jaxon\Response\Manager::append() must be an instance of Jaxon\Response\Response, null given

Is it possible to make it so user functions don't have to return a Response, and if none is given the code just assumes it should make a new/empty Response object, and continue?

feuzeu commented 5 years ago

Hi,

I apologize for the delay, and I hope this will still help.

You can define a callback which will add a default response if none is given in your user functions. https://www.jaxon-php.org/docs/responses/callbacks.html

jaxon()->register(Jaxon::PROCESSING_EVENT, Jaxon::PROCESSING_EVENT_AFTER, 'setDefaultResponse');
function setDefaultResponse()
{
    $response = jaxon()->newResponse();
    // You can use the global response provided by the library
    // $response = jaxon()->getResponse();

    // Set the default response content
    ...

    // Append your response to the request response
    jaxon()->getResponseManager()->append($response);
}
vita10gy commented 5 years ago

Thanks for the suggestion. I added that but the fatal error happens before it gets to running setDefaultResponse. If I try PROCESSING_EVENT_BEFORE it gets there, but seems to make no difference in the fatal error above.

Also just to be clear I don't know ahead of time if the function has a response returned since I'm trying to make a backward compatible replacement that just works without touching a bunch of sites.

Likewise, I wouldn't want to do something that just swapped not needing to specify a response object by default with an approach that overrode when there WAS a response returned, if that's where this is headed.

$this->ajax_class = new \Jaxon\Jaxon();
 $this->ajax_class->setOption("core.prefix.function","xajax_");
 $this->ajax_class->setOption("core.request.uri",$_SERVER["REQUEST_URI"]);
//the line below didn't exist before your suggestion
 $this->ajax_class->register(\Jaxon\Jaxon::PROCESSING_EVENT, \Jaxon\Jaxon::PROCESSING_EVENT_BEFORE, 'setDefaultResponse');
        foreach($this->functions as $f)
        {
          $this->ajax_class->register(\Jaxon\Jaxon::USER_FUNCTION, $f);  
        }

        $this->ajax_class->processRequest();

and then as an example of what $this->functions could contain, here would be one with no response

function save_sorting($array)
  {
    $index = 1;
    foreach($array as $id)
    {
      $temp = new Page($id);
      $menu = $temp->getMenu();
      if($menu->exists())
    {
        $menu->setSortOrder($index);
    }
      $index++;
    }
  }

This results in a fatal error. However when I return a new Jaxon\Response\Response in save_sorting all is well.

Of course I could be missing the boat completely on something.

feuzeu commented 5 years ago

Hi, You were right, until the latest https://github.com/jaxon-php/jaxon-core/releases/tag/v2.2.4 release I just shipped, it was not possible in a user function not to return a response. With this release, you can define your event handler as follow.

function setDefaultResponse()
{
    $manager = jaxon()->getResponseManager();
    if($manager->hasNoResponse())
    {
        $response = jaxon()->newResponse();
        // You can use the global response provided by the library
        // $response = jaxon()->getResponse();

        // Set the default response content
        ...

        // Append your response to the request response
        $manager->append($response);
}
vita10gy commented 5 years ago

Awesome, thanks. Do we have to set a default, or is just doing nothing nothing cool too?

feuzeu commented 5 years ago

You're welcome. You have to set a default, since the request actually needs a response to be returned to the client.

feuzeu commented 5 years ago

Do you think it is a good idea if Jaxon returns the global response object when no response is defined either in the user function or in the after processing callback?