Skip to content
This repository

Moving JResponse to its own package and adding JResponseJson #1596

Closed
wants to merge 1 commit into from

7 participants

Patrick Alt Piotr Andrew Eddie Louis Landry laoneo Don Gilbert elinw
Patrick Alt

Hi,

this pull request is a replacement for #1470.

It introduces a new class JResponseJson which is able to simplify things with Ajax requests.

  • Using the flag success the JavaScript code (whether Mootools JRequest.JSON or jQuery.getJSON) can check whether the task was successful or not and react accordingly (If the request itself failes the error event of the JavaScript API can be used).
  • Then, the actual response data (if any) can be retrieved from data in the JSON object and
  • optionally a main response message from message.
  • Additionally all gathered messages in the JApplication message queue are also automatically sent back in messages. I'm not sure what will happen with that part because the message queue is only in the legacy application class at the moment without any replacement, but it's very useful at this point. The dependencies are checked afore.

Another advantage is that no $app->close(); is necessary anymore if the Ajax request is done with 'format=json' because the already existing API handles the rest. Just echo the response object at the end of the task.

With this pull request the class can be added to the platform so that it can be used for any application.
For example, in the CMS it could replace the current code in the installer and the language override manager. I saw that Finder and the media manager could also be adapted for using this class. If this pull request is accepted I can standardize this in the CMS.

I'm not sure whether the unit tests are written as expected (it's the first time I coded some).

Patrick Alt

@eddieajau

Can you add a reference to where this is being used in the CMS?

As @piotr-cz already mentioned, there are several places where this strategy is used.
Since there are different developers using such a class I think it's a good reason to integrate it into the platform:

  • For seeing it in action please have a look at the strings.json.php-Controller of the language manager in the CMS and at the JavaScript file overrider.js.

  • In the installation application nearly the same class is used: InstallationJsonResponse (defined directly in the controller file).

  • In the finder component an extended and specialised version of the class is used: FinderIndexerResponse (defined directly in the controller file).

  • For example, the controller file.json.php can easily be adapted for simplifying its code together with JResponseJson.

t's probably worth looking at this GSOC project for how a JSON response is handled

The benefit of this class against the handling in that application is that not only data is encoded and returned but also a success flag is set automatically depending on the arguments. For example, the class can handle normal objects and arrays but it can also handle an exception with which success is set to false and the message is appended to the response.
For the various ways to use this class please have a look at the unit test file.
I just want to add an additional example here. Assuming that the following is a task of a controller:

  public function execute()
  {
    try
    {
      $model = $this->getModel('example');
      $result = $model->createSomething();

      // Some more code

      echo new JResponseJson($result);
    }
    catch(Exception $e)
    {
      echo new JResponseJson($e);
    }
  }

If everything goes right the success flag will be set to true and the requested data will be returned. If an exception is thrown the success flag will be set to false and the message of the exception will be returned.

for example, not every platform application will necessarily have a message queue

This is already covered by using is_callable. In applications without a message queue, it is treated as if $ignoreMessages is set to true.

I'm not sure this adds a lot to JResponse but I'd certainly like to see this class extending it if it's to be used at all.

At the moment I don't see any benefit of that or are there other reasons for that? This class actually doesn't use any code from JResponse.
For making it clear: This class just creates the JSON string for the response which in turn usually would then be inserted into the body of JResponse (like it's happening automatically in the CMS) or another response object like in JApplicationWeb, for example.

Piotr

I'll just add that the advantages of having such class shows up when you have more than one JSON controller or outputting JSON response in different places: one client-side parser will understand response from any extension/ application

Andrew Eddie
Owner

@Chraneco I think what you are describing is a stand alone helper that's been design very much with the CMS in mind (or in other words, it's application specific). There's nothing wrong with that, but in terms of the Platform, I'm just not seeing it as generically useful (and I go back to my comments that the message queue is a 'CMS thing'). Maybe I'm looking at the problem it's trying to solve the wrong way, but I just don't get how it give the controller any added value over $this->app->setBody(json_encode($data));. Whatever the case, I do feel strongly that if a class such as JResponseJson is to exist, it should extend JResponse and add value to that base class. I also think that errors should not be coupled to the response object - the error/exception handler should set the response (not the response set the error).

I'll leave this open for a while longer to see what others think.

Louis Landry

If you move the package into the legacy tree then I'd be supportive of merging it.

Louis Landry

Upon thinking this through I think it best if this pull request is sent to the CMS tree anyway. We plan to deprecate JResponse in the near future in favor of just using the methods built into JApplicationWeb. This is one of the major functions of the new web application class. In terms of the CMS landscape I do think the idea and implementation has merit, but it isn't really in line with where we are wanting to go with the platform for the reasons I've already discussed.

Because I think this should be submitted to the CMS instead, and because it is no longer mergeable I'm going to close this. Thanks for the time, thoughts, and work regardless!

Louis Landry LouisLandry closed this November 10, 2012
Piotr

@LouisLandry
You mean /libraries/cms/response/ ?

Louis Landry

Correct.

https://github.com/joomla/joomla-cms/tree/master/libraries/cms is where I would put this stuff. I don't disagree with the notion of having JResponseJson extend JResponse though, Andrew is on to something there.

laoneo

What happens now with this pull request? Is it merged?

Don Gilbert
Owner

It is not merged. @elinw would know (maybe?) if this is something the CMS would use.

In the meantime, and this is what I do, when sending JSON responses as the application output, you can use this in your view:

$app = JFactory::getApplication();
$items = $this->get('Items');

// do post processing if need be

$app->close(json_encode($items));
elinw
elinw commented March 22, 2013

I like the idea a lot and I think it makes sense in the CMS libraries where we are doing some interesting things and have a couple of gsoc ideas that could use this plus we are not yet using JApplicationWeb so Louis's point about that also makes me think it makes sense to do given that we are potentially looking at 2.5 years of support for JApplication. I pretty much always trust Andrew's instincts on what should extend what, but we have also added a folder /cms/helper that could work if it is a standalone helper.

Andrew Eddie
Owner

@laoneo given the changes between Platform and Framework I'd make this request against the CMS tree. I don't think it makes sense putting anything new in this Platform as we are about to freeze it.

laoneo

@eddieajau means what? Should there be made a new pull request on the CMS? Can this pull request be moved over to the CMS?

For others if you want to do it https://groups.google.com/forum/?hl=en-GB&fromgroups=#!topic/joomla-dev-general/LLosuO2w6LU

Andrew Eddie
Owner

Yes, make a new pull request against the CMS repo. No, this pull request can't be moved.

Piotr piotr-cz referenced this pull request in joomla/joomla-cms August 31, 2013
Closed

Add JResponseJson as a CMS library class #1383

Patrick Alt Chraneco deleted the branch September 13, 2013
Patrick Alt

The JResponseJson class was recently merged into the CMS libraries.

I added a documentation article about it here:
http://docs.joomla.org/JSON_Responses_with_JResponseJson
Any feedback is very welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Oct 11, 2012
Patrick Alt Moving JResponse to its own package and adding JResponseJson cdbf415
This page is out of date. Refresh to see the latest.
2  libraries/joomla/document/document.php
@@ -9,8 +9,6 @@
9 9
 
10 10
 defined('JPATH_PLATFORM') or die;
11 11
 
12  
-jimport('joomla.environment.response');
13  
-
14 12
 /**
15 13
  * Document class, provides an easy interface to parse and display a document
16 14
  *
121  libraries/joomla/response/json.php
... ...
@@ -0,0 +1,121 @@
  1
+<?php
  2
+/**
  3
+ * @package     Joomla.Platform
  4
+ * @subpackage  Response
  5
+ *
  6
+ * @copyright   Copyright (C) 2005 - 2012 Open Source Matters, Inc. All rights reserved.
  7
+ * @license     GNU General Public License version 2 or later; see LICENSE
  8
+ */
  9
+
  10
+defined('JPATH_PLATFORM') or die;
  11
+
  12
+/**
  13
+ * JSON Response class.
  14
+ *
  15
+ * This class serves to provide the Joomla Platform with a common interface to access
  16
+ * response variables for e.g. Ajax requests.
  17
+ *
  18
+ * @package     Joomla.Platform
  19
+ * @subpackage  Response
  20
+ * @since       12.2
  21
+ */
  22
+class JResponseJson
  23
+{
  24
+	/**
  25
+	 * Determines whether the request was successful
  26
+	 *
  27
+	 * @var		boolean
  28
+	 * @since	12.2
  29
+	 */
  30
+	public $success		= true;
  31
+
  32
+	/**
  33
+	 * The main response message
  34
+	 *
  35
+	 * @var		string
  36
+	 * @since	12.2
  37
+	 */
  38
+	public $message		= null;
  39
+
  40
+	/**
  41
+	 * Array of messages gathered in the JApplication object
  42
+	 *
  43
+	 * @var		array
  44
+	 * @since	12.2
  45
+	 */
  46
+	public $messages	= null;
  47
+
  48
+	/**
  49
+	 * The response data
  50
+	 *
  51
+	 * var		mixed
  52
+	 * @since	12.2
  53
+	 */
  54
+	public $data			= null;
  55
+
  56
+	/**
  57
+	 * Constructor
  58
+	 *
  59
+	 * @param   mixed    $response        The Response data
  60
+	 * @param   string   $message         The main response message
  61
+	 * @param   boolean  $error           True, if the success flag shall be set to false, defaults to false
  62
+	 * @param   boolean  $ignoreMessages  True, if the message queue shouldn't be included, defaults to false
  63
+	 *
  64
+	 * @since   12.2
  65
+	 */
  66
+	public function __construct($response = null, $message = null, $error = false, $ignoreMessages = false)
  67
+	{
  68
+		$this->message = $message;
  69
+
  70
+		// Get the message queue if requested and available
  71
+		$app = JFactory::$application;
  72
+		if (!$ignoreMessages && !is_null($app) && is_callable(array($app, 'getMessageQueue')))
  73
+		{
  74
+			$messages = $app->getMessageQueue();
  75
+
  76
+			// Build the sorted messages list
  77
+			if (is_array($messages) && count($messages))
  78
+			{
  79
+				foreach ($messages as $message)
  80
+				{
  81
+					if (isset($message['type']) && isset($message['message']))
  82
+					{
  83
+						$lists[$message['type']][] = $message['message'];
  84
+					}
  85
+				}
  86
+			}
  87
+
  88
+			// If messages exist add them to the output
  89
+			if (isset($lists) && is_array($lists))
  90
+			{
  91
+				$this->messages = $lists;
  92
+			}
  93
+		}
  94
+
  95
+		// Check if we are dealing with an error
  96
+		if ($response instanceof Exception)
  97
+		{
  98
+			// Prepare the error response
  99
+			$this->success	= false;
  100
+			$this->message	= $response->getMessage();
  101
+		}
  102
+		else
  103
+		{
  104
+			// Prepare the response data
  105
+			$this->success	= !$error;
  106
+			$this->data			= $response;
  107
+		}
  108
+	}
  109
+
  110
+	/**
  111
+	 * Magic toString method for sending the response in JSON format
  112
+	 *
  113
+	 * @return	string	The response in JSON format
  114
+	 *
  115
+	 * @since		12.2
  116
+	 */
  117
+	public function __toString()
  118
+	{
  119
+		return json_encode($this);
  120
+	}
  121
+}
4  libraries/joomla/environment/response.php → libraries/joomla/response/response.php
... ...
@@ -1,7 +1,7 @@
1 1
 <?php
2 2
 /**
3 3
  * @package     Joomla.Platform
4  
- * @subpackage  Environment
  4
+ * @subpackage  Response
5 5
  *
6 6
  * @copyright   Copyright (C) 2005 - 2012 Open Source Matters, Inc. All rights reserved.
7 7
  * @license     GNU General Public License version 2 or later; see LICENSE
@@ -16,7 +16,7 @@
16 16
  * response variables.  This includes header and body.
17 17
  *
18 18
  * @package     Joomla.Platform
19  
- * @subpackage  Environment
  19
+ * @subpackage  Response
20 20
  * @since       11.1
21 21
  */
22 22
 class JResponse
2  libraries/legacy/application/application.php
@@ -9,8 +9,6 @@
9 9
 
10 10
 defined('JPATH_PLATFORM') or die;
11 11
 
12  
-jimport('joomla.environment.response');
13  
-
14 12
 /**
15 13
  * Base class for a Joomla! application.
16 14
  *
2  tests/suites/unit/joomla/document/JDocumentTest.php
@@ -7,8 +7,6 @@
7 7
  * @license     GNU General Public License version 2 or later; see LICENSE
8 8
  */
9 9
 
10  
-require_once JPATH_PLATFORM . '/joomla/environment/response.php';
11  
-
12 10
 /**
13 11
  * Test class for JDocument.
14 12
  * Generated by PHPUnit on 2009-10-09 at 12:13:55.
2  tests/suites/unit/joomla/document/feed/renderer/JDocumentRendererAtomTest.php
@@ -40,7 +40,7 @@ protected function setUp()
40 40
 		require_once JPATH_PLATFORM . '/joomla/application/router.php';
41 41
 		require_once JPATH_PLATFORM . '/joomla/environment/request.php';
42 42
 		require_once JPATH_PLATFORM . '/joomla/document/feed/feed.php';
43  
-		require_once JPATH_PLATFORM . '/joomla/environment/response.php';
  43
+
44 44
 		$this->saveFactoryState();
45 45
 
46 46
 		JFactory::$application = $this->getMock(
1  tests/suites/unit/joomla/document/feed/renderer/JDocumentRendererRSSTest.php
@@ -39,7 +39,6 @@ protected function setUp()
39 39
 		require_once JPATH_PLATFORM . '/joomla/application/router.php';
40 40
 		require_once JPATH_PLATFORM . '/joomla/environment/request.php';
41 41
 		require_once JPATH_PLATFORM . '/joomla/document/feed/feed.php';
42  
-		require_once JPATH_PLATFORM . '/joomla/environment/response.php';
43 42
 
44 43
 		$this->saveFactoryState();
45 44
 
1  tests/suites/unit/joomla/document/opensearch/JDocumentOpensearchTest.php
@@ -7,7 +7,6 @@
7 7
  * @license     GNU General Public License version 2 or later; see LICENSE
8 8
  */
9 9
 
10  
-require_once JPATH_PLATFORM . '/joomla/environment/response.php';
11 10
 require_once JPATH_PLATFORM . '/joomla/document/opensearch/opensearch.php';
12 11
 
13 12
 /**
220  tests/suites/unit/joomla/response/JResponseJsonTest.php
... ...
@@ -0,0 +1,220 @@
  1
+<?php
  2
+/**
  3
+ * @package     Joomla.UnitTest
  4
+ * @subpackage  Response
  5
+ *
  6
+ * @copyright   Copyright (C) 2005 - 2012 Open Source Matters, Inc. All rights reserved.
  7
+ * @license     GNU General Public License version 2 or later; see LICENSE
  8
+ */
  9
+
  10
+require_once __DIR__ . '/stubs/mock.application.php';
  11
+
  12
+/**
  13
+ * Test class for JResponseJson.
  14
+ *
  15
+ * @package     Joomla.UnitTest
  16
+ * @subpackage  JResponseJson
  17
+ * @since       12.2
  18
+ */
  19
+class JResponseJsonTest extends TestCase
  20
+{
  21
+	/**
  22
+	 * Set up for testing
  23
+	 *
  24
+	 * @return void
  25
+	 *
  26
+	 * @since  12.2
  27
+	 */
  28
+	public function setUp()
  29
+	{
  30
+		$this->app = JFactory::$application;
  31
+		JFactory::$application = null;
  32
+	}
  33
+
  34
+	/**
  35
+	 * Tear down test
  36
+	 *
  37
+	 * @return void
  38
+	 *
  39
+	 * @since  12.2
  40
+	 */
  41
+	public function tearDown()
  42
+	{
  43
+		JFactory::$application = $this->app;
  44
+	}
  45
+
  46
+	/**
  47
+	 * Tests a simple success response where only the JResponseJson
  48
+	 * class is instantiated and send
  49
+	 *
  50
+	 * @return void
  51
+	 *
  52
+	 * @since  12.2
  53
+	 */
  54
+	public function testSimpleSuccess()
  55
+	{
  56
+		ob_start();
  57
+		echo new JResponseJson;
  58
+		$output = ob_get_clean();
  59
+
  60
+		$response = json_decode($output);
  61
+
  62
+		$this->assertEquals(true, $response->success);
  63
+	}
  64
+
  65
+	/**
  66
+	 * Tests a success response with data to send back
  67
+	 *
  68
+	 * @return void
  69
+	 *
  70
+	 * @since  12.2
  71
+	 */
  72
+	public function testSuccessWithData()
  73
+	{
  74
+		$data = new stdClass;
  75
+		$data->value 		= 5;
  76
+		$data->average	= 7.9;
  77
+
  78
+		ob_start();
  79
+		echo new JResponseJson($data);
  80
+		$output = ob_get_clean();
  81
+
  82
+		$response = json_decode($output);
  83
+
  84
+		$this->assertEquals(true, $response->success);
  85
+		$this->assertEquals(5, $response->data->value);
  86
+		$this->assertEquals(7.9, $response->data->average);
  87
+	}
  88
+
  89
+	/**
  90
+	 * Tests a response indicating an error where an exception
  91
+	 * is passed into the object in order to set 'success' to false.
  92
+	 *
  93
+	 * The message of the exception is automatically sent back in 'message'.
  94
+	 *
  95
+	 * @return void
  96
+	 *
  97
+	 * @since  12.2
  98
+	 */
  99
+	public function testFailureWithException()
  100
+	{
  101
+		ob_start();
  102
+		echo new JResponseJson(new Exception('This and that went wrong'));
  103
+		$output = ob_get_clean();
  104
+
  105
+		$response = json_decode($output);
  106
+
  107
+		$this->assertEquals(false, $response->success);
  108
+		$this->assertEquals('This and that went wrong', $response->message);
  109
+	}
  110
+
  111
+	/**
  112
+	 * Tests a response indicating an error where the third argument
  113
+	 * is used to set 'success' to false and the second to set the message
  114
+	 *
  115
+	 * This way data can also be send back using the first argument.
  116
+	 *
  117
+	 * @return void
  118
+	 *
  119
+	 * @since  12.2
  120
+	 */
  121
+	public function testFailureWithData()
  122
+	{
  123
+		$data = new stdClass;
  124
+		$data->value		= 6;
  125
+		$data->average	= 8.9;
  126
+
  127
+		ob_start();
  128
+		echo new JResponseJson($data, 'Something went wrong', true);
  129
+		$output = ob_get_clean();
  130
+
  131
+		$response = json_decode($output);
  132
+
  133
+		$this->assertEquals(false, $response->success);
  134
+		$this->assertEquals('Something went wrong', $response->message);
  135
+		$this->assertEquals(6, $response->data->value);
  136
+		$this->assertEquals(8.9, $response->data->average);
  137
+	}
  138
+
  139
+	/**
  140
+	 * Tests a response indicating an error where more messages
  141
+	 * are sent back besides the main response message of the exception
  142
+	 *
  143
+	 * @return void
  144
+	 *
  145
+	 * @since  12.2
  146
+	 */
  147
+	public function testFailureWithMessages()
  148
+	{
  149
+		$app = new JApplicationResponseJsonMock;
  150
+		$app->enqueueMessage('This part was successful');
  151
+		$app->enqueueMessage('You should not do that', 'warning');
  152
+		JFactory::$application = $app;
  153
+
  154
+		ob_start();
  155
+		echo new JResponseJson(new Exception('A major error occured'));
  156
+		$output = ob_get_clean();
  157
+
  158
+		$response = json_decode($output);
  159
+
  160
+		$this->assertEquals(false, $response->success);
  161
+		$this->assertEquals('A major error occured', $response->message);
  162
+		$this->assertEquals('This part was successful', $response->messages->message[0]);
  163
+		$this->assertEquals('You should not do that', $response->messages->warning[0]);
  164
+	}
  165
+
  166
+	/**
  167
+	 * Tests a response indicating an error where messages
  168
+	 * of the message queue should be ignored
  169
+	 *
  170
+	 * Note: The third parameter $error will be ignored
  171
+	 * if an exception is used for indicating an error
  172
+	 *
  173
+	 * @return void
  174
+	 *
  175
+	 * @since  12.2
  176
+	 */
  177
+	public function testFailureWithIgnoreMessages()
  178
+	{
  179
+		$app = new JApplicationResponseJsonMock;
  180
+		$app->enqueueMessage('This part was successful');
  181
+		$app->enqueueMessage('You should not do that', 'warning');
  182
+		JFactory::$application = $app;
  183
+
  184
+		ob_start();
  185
+		echo new JResponseJson(new Exception('A major error occured'), null, false, true);
  186
+		$output = ob_get_clean();
  187
+
  188
+		$response = json_decode($output);
  189
+
  190
+		$this->assertEquals(false, $response->success);
  191
+		$this->assertEquals('A major error occured', $response->message);
  192
+		$this->assertEquals(null, $response->messages);
  193
+	}
  194
+
  195
+	/**
  196
+	 * Tests a simple success response where only the JResponseJson
  197
+	 * class is instantiated and send, but this time with additional messages
  198
+	 *
  199
+	 * @return void
  200
+	 *
  201
+	 * @since  12.2
  202
+	 */
  203
+	public function testSuccessWithMessages()
  204
+	{
  205
+		$app = new JApplicationResponseJsonMock;
  206
+		$app->enqueueMessage('This part was successful');
  207
+		$app->enqueueMessage('This one was also successful');
  208
+		JFactory::$application = $app;
  209
+
  210
+		ob_start();
  211
+		echo new JResponseJson;
  212
+		$output = ob_get_clean();
  213
+
  214
+		$response = json_decode($output);
  215
+
  216
+		$this->assertEquals(true, $response->success);
  217
+		$this->assertEquals('This part was successful', $response->messages->message[0]);
  218
+		$this->assertEquals('This one was also successful', $response->messages->message[1]);
  219
+	}
  220
+}
2  .../suites/unit/joomla/environment/JResponseTest.php → tests/suites/unit/joomla/response/JResponseTest.php
@@ -7,8 +7,6 @@
7 7
  * @license     GNU General Public License version 2 or later; see LICENSE
8 8
  */
9 9
 
10  
-require_once JPATH_PLATFORM . '/joomla/environment/response.php';
11  
-
12 10
 /**
13 11
  * Test class for JResponse.
14 12
  * Generated by PHPUnit on 2011-03-25 at 00:12:25.
55  tests/suites/unit/joomla/response/stubs/mock.application.php
... ...
@@ -0,0 +1,55 @@
  1
+<?php
  2
+/**
  3
+ * @package     Joomla.UnitTest
  4
+ * @subpackage  Response
  5
+ *
  6
+ * @copyright   Copyright (C) 2005 - 2012 Open Source Matters, Inc. All rights reserved.
  7
+ * @license     GNU General Public License version 2 or later; see LICENSE
  8
+ */
  9
+
  10
+/**
  11
+ * Test mock class JResponseJson.
  12
+ *
  13
+ * @package     Joomla.UnitTest
  14
+ * @subpackage  Response
  15
+ * @since       12.2
  16
+ */
  17
+class JApplicationResponseJsonMock
  18
+{
  19
+	/**
  20
+	 * @var    array  The message queue.
  21
+	 * @since  12.2
  22
+	 */
  23
+	public $queue = array();
  24
+
  25
+	/**
  26
+	 * Enqueue a system message.
  27
+	 *
  28
+	 * @param   string  $msg   The message to enqueue.
  29
+	 * @param   string  $type  The message type. Default is message.
  30
+	 *
  31
+	 * @return  void
  32
+	 *
  33
+	 * @since   12.2
  34
+	 */
  35
+	public function enqueueMessage($msg, $type = 'message')
  36
+	{
  37
+		$this->queue[] = array('message' => $msg, 'type' => strtolower($type));
  38
+	}
  39
+
  40
+	/**
  41
+	 * Get the system message queue.
  42
+	 *
  43
+	 * @return  array  The system message queue.
  44
+	 *
  45
+	 * @since   12.2
  46
+	 */
  47
+	public function getMessageQueue()
  48
+	{
  49
+		$queue = $this->queue;
  50
+
  51
+		$this->queue = array();
  52
+
  53
+		return $queue;
  54
+	}
  55
+}
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.