Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] Move component categories helper to services #19667

Merged
merged 30 commits into from
Mar 1, 2018
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
5e73d6b
Move component categories helper to services
laoneo Feb 13, 2018
7d7bc87
CS
laoneo Feb 13, 2018
fdcf139
Fix unit tests
laoneo Feb 13, 2018
1a119fd
Use a class for the categories service instead of a service provider
laoneo Feb 13, 2018
efd56db
Merge branch '4.0-dev' into j4/boot
laoneo Feb 13, 2018
166c511
Make the sniffs happy
laoneo Feb 13, 2018
44c1140
Move boot the extension helper
laoneo Feb 14, 2018
3282e31
Merge remote-tracking branch 'remotes/upstream/4.0-dev' into j4/boot
laoneo Feb 14, 2018
db25ff4
Introduce extension loader interface
laoneo Feb 14, 2018
5f126d3
Revert not needed interface
laoneo Feb 14, 2018
27f89f2
CS
laoneo Feb 14, 2018
ff0000f
Language fixes
laoneo Feb 14, 2018
80931ab
Make getConatiner abstract
laoneo Feb 14, 2018
9eadbb9
CS
laoneo Feb 14, 2018
09e4574
Distinguish between optional and required options
laoneo Feb 15, 2018
ca128f7
CS
laoneo Feb 15, 2018
91c907d
Run isolated
laoneo Feb 16, 2018
75b29ce
Merge branch '4.0-dev' into j4/boot
laoneo Feb 16, 2018
d5b6c8a
Load the container trough a class
laoneo Feb 19, 2018
82a0e73
Load the container trough a class
laoneo Feb 19, 2018
bf25d1c
Safe check
laoneo Feb 19, 2018
8d8d41b
Merge remote-tracking branch 'remotes/upstream/4.0-dev' into j4/boot2
laoneo Feb 19, 2018
16b7c71
versions
laoneo Feb 19, 2018
3a3328e
Merge remote-tracking branch 'origin/j4/boot' into j4/boot
laoneo Feb 19, 2018
5a9912c
doc
laoneo Feb 19, 2018
161e90a
Merge branch '4.0-dev' into j4/boot
laoneo Feb 22, 2018
8ee0093
Merge branch '4.0-dev' into j4/boot
laoneo Feb 28, 2018
dfb3a61
Merge branch '4.0-dev' into j4/boot
laoneo Mar 1, 2018
a2447d3
Use ComponentInterface as a typehint for now
wilsonge Mar 1, 2018
084e0a1
Use main function
wilsonge Mar 1, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions administrator/components/com_content/services/services.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php
/**
* @package Joomla.Administrator
* @subpackage com_content
*
* @copyright Copyright (C) 2005 - 2017 Open Source Matters, Inc. All rights reserved.
* @license GNU General Public License version 2 or later; see LICENSE.txt
*/

defined('_JEXEC') or die;

$container->share(
'ContentComponentContainer',
function (\Joomla\DI\Container $parent)
{
$container = new \Joomla\CMS\Extension\ComponentContainer($parent);
$container->set('categories', new \Joomla\Component\Content\Site\Service\Category);

return $container;
},
true
);
2 changes: 1 addition & 1 deletion administrator/components/com_fields/Model/FieldModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ protected function preprocessForm(\JForm $form, $data, $group = 'content')
}

// Setting the context for the category field
$cat = \JCategories::getInstance(str_replace('com_', '', $component));
$cat = $this->bootComponent($component)->getCategories();

if ($cat && $cat->get('root')->hasChildren())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,18 @@
* @copyright Copyright (C) 2005 - 2017 Open Source Matters, Inc. All rights reserved.
* @license GNU General Public License version 2 or later; see LICENSE.txt
*/
namespace Joomla\Component\Content\Site\Service;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Services should be application independent. So why ...\Site\...?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can even be removed, but for now George wants to have it in place, see the comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with the class itself so far - I just don't see, why it is in a namespace that semantically restricts it to 'site'.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have application agnostic component code. So we'd need a solution to that to get around that (mildly important in some ways) semantic detail.


defined('_JEXEC') or die;

use Joomla\CMS\Categories\Categories;

/**
* Content Component Category Tree
*
* @since 1.6
*/
class ContentCategories extends JCategories
class Category extends Categories
{
/**
* Class constructor
Expand All @@ -25,7 +28,7 @@ class ContentCategories extends JCategories
*/
public function __construct($options = array())
{
$options['table'] = '#__content';
$options['table'] = '#__content';
$options['extension'] = 'com_content';

parent::__construct($options);
Expand Down
3 changes: 2 additions & 1 deletion libraries/src/Application/CMSApplication.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Joomla\CMS\Authentication\Authentication;
use Joomla\CMS\Event\AbstractEvent;
use Joomla\CMS\Event\BeforeExecuteEvent;
use Joomla\CMS\Extension\ExtensionLoader;
use Joomla\CMS\Input\Input;
use Joomla\CMS\Language\Language;
use Joomla\CMS\Menu\AbstractMenu;
Expand All @@ -35,7 +36,7 @@
*/
abstract class CMSApplication extends WebApplication implements ContainerAwareInterface, CMSApplicationInterface
{
use ContainerAwareTrait, ExtensionNamespaceMapper;
use ContainerAwareTrait, ExtensionLoader, ExtensionNamespaceMapper;

/**
* Array of options for the \JDocument object
Expand Down
3 changes: 2 additions & 1 deletion libraries/src/Application/CMSApplicationInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace Joomla\CMS\Application;

use Joomla\CMS\Extension\ExtensionLoaderInterface;
use Joomla\CMS\User\User;
use Joomla\Session\SessionInterface;

Expand All @@ -16,7 +17,7 @@
*
* @since 4.0.0
*/
interface CMSApplicationInterface
interface CMSApplicationInterface extends ExtensionLoaderInterface
{
/**
* Constant defining an enqueued emergency message
Expand Down
3 changes: 2 additions & 1 deletion libraries/src/Application/CliApplication.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Joomla\CMS\Application\CLI\CliInput;
use Joomla\CMS\Application\CLI\CliOutput;
use Joomla\CMS\Application\CLI\Output\Stdout;
use Joomla\CMS\Extension\ExtensionLoader;
use Joomla\Input\Cli;
use Joomla\Input\Input;
use Joomla\DI\Container;
Expand All @@ -32,7 +33,7 @@
*/
abstract class CliApplication extends AbstractApplication implements DispatcherAwareInterface, CMSApplicationInterface
{
use DispatcherAwareTrait, EventAware, IdentityAware, ContainerAwareTrait;
use DispatcherAwareTrait, EventAware, IdentityAware, ContainerAwareTrait, ExtensionLoader;

/**
* Output object
Expand Down
3 changes: 2 additions & 1 deletion libraries/src/Application/ConsoleApplication.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
defined('JPATH_PLATFORM') or die;

use Joomla\CMS\Console;
use Joomla\CMS\Extension\ExtensionLoader;
use Joomla\CMS\Input\Cli;
use Joomla\CMS\Plugin\PluginHelper;
use Joomla\Console\Application;
Expand All @@ -30,7 +31,7 @@
*/
class ConsoleApplication extends Application implements DispatcherAwareInterface, CMSApplicationInterface
{
use DispatcherAwareTrait, EventAware, IdentityAware, ContainerAwareTrait;
use DispatcherAwareTrait, EventAware, IdentityAware, ContainerAwareTrait, ExtensionLoader;

/**
* The application message queue.
Expand Down
78 changes: 45 additions & 33 deletions libraries/src/Categories/Categories.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class Categories
* @var array
* @since 1.6
*/
protected $_options = null;
protected $_options = [];

/**
* Class constructor
Expand All @@ -101,20 +101,20 @@ class Categories
*/
public function __construct($options)
{
// Required options
$this->_extension = $options['extension'];
$this->_table = $options['table'];
$this->_field = isset($options['field']) && $options['field'] ? $options['field'] : 'catid';
$this->_key = isset($options['key']) && $options['key'] ? $options['key'] : 'id';
$this->_statefield = $options['statefield'] ?? 'state';

$options['access'] = $options['access'] ?? 'true';
$options['published'] = $options['published'] ?? 1;
$options['countItems'] = $options['countItems'] ?? 0;
$options['currentlang'] = Multilanguage::isEnabled() ? Factory::getLanguage()->getTag() : 0;
// Default some optional options
$this->_options['access'] = 'true';
$this->_options['published'] = 1;
$this->_options['countItems'] = 0;
$this->_options['currentlang'] = Multilanguage::isEnabled() ? Factory::getLanguage()->getTag() : 0;

$this->_options = $options;

return true;
$this->setOptions($options);
}

/**
Expand All @@ -125,7 +125,8 @@ public function __construct($options)
*
* @return Categories|boolean Categories object on success, boolean false if an object does not exist
*
* @since 1.6
* @since 1.6
* @deprecated 5.0 Use the ComponentContainerInterface to get the categories
*/
public static function getInstance($extension, $options = array())
{
Expand All @@ -136,32 +137,11 @@ public static function getInstance($extension, $options = array())
return self::$instances[$hash];
}

$parts = explode('.', $extension);
$component = 'com_' . strtolower($parts[0]);
$section = count($parts) > 1 ? $parts[1] : '';
$classname = ucfirst(substr($component, 4)) . ucfirst($section) . 'Categories';

if (!class_exists($classname))
{
$path = JPATH_SITE . '/components/' . $component . '/helpers/category.php';

if (!is_file($path))
{
return false;
}
$parts = explode('.', $extension, 2);

include_once $path;
}
$categories = Factory::getApplication()->bootComponent($parts[0])->getCategories($options, count($parts) > 1 ? $parts[1] : '');

// Check for a possible service from the container otherwise manually instantiate the class
if (\JFactory::getContainer()->exists($classname))
{
self::$instances[$hash] = \JFactory::getContainer()->get($classname);
}
else
{
self::$instances[$hash] = new $classname($options);
}
self::$instances[$hash] = $categories;

return self::$instances[$hash];
}
Expand Down Expand Up @@ -389,4 +369,36 @@ protected function _load($id)
$this->_nodes[$id] = null;
}
}

/**
* Allows to set some optional options, eg. if the access level should be considered.
* Also clears the internal children cache.
*
* @param array $options The new options
*
* @return void
*
* @since __DEPLOY_VERSION__
*/
public function setOptions(array $options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this back to the constructor please. I know this is a bit simpler to view but I risking people call setOptions and reprovisioning just seems vaguely boring. Let's do one risky thing at a time

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to do it that way as in the ComponentContainer here , it is not possible to pass the options. If there is a way to set the options, then I can change it back to the constructor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You make this an anonymous function like we do in all the service providers for the main DI Container https://github.com/joomla/joomla-cms/pull/19667/files#diff-a56b3bb3afcd610a7b1560ed05d8a146R17

{
if (isset($options['access']))
{
$this->_options['access'] = $options['access'];
}

if (isset($options['published']))
{
$this->_options['published'] = $options['published'];
}

if (isset($options['countItems']))
{
$this->_options['countItems'] = $options['countItems'];
}

// Reset the cache
$this->_nodes = [];
$this->_checkedCategories = [];
}
}
56 changes: 56 additions & 0 deletions libraries/src/Extension/ComponentContainer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php
/**
* Joomla! Content Management System
*
* @copyright Copyright (C) 2005 - 2017 Open Source Matters, Inc. All rights reserved.
* @license GNU General Public License version 2 or later; see LICENSE.txt
*/

namespace Joomla\CMS\Extension;

defined('JPATH_PLATFORM') or die;

use Joomla\CMS\Categories\Categories;
use Joomla\DI\Container;
use Psr\Container\ContainerInterface;

/**
* Access to component specific services.
*
* @since __DEPLOY_VERSION__
*/
class ComponentContainer extends Container implements ComponentContainerInterface
{
/**
* Returns the category service. If the service is not available
* null is returned.
*
* @param array $options The options
* @param string $section The section
*
* @return Categories|null
*
* @see Categories::setOptions()
*
* @since __DEPLOY_VERSION__
*/
public function getCategories(array $options = [], $section = '')
{
$serviceName = 'categories';

if ($section)
{
$serviceName .= '.' . strtolower($section);
}

if (!$this->has($serviceName))
{
return null;
}

$categories = $this->get($serviceName);
$categories->setOptions($options);

return $categories;
}
}
36 changes: 36 additions & 0 deletions libraries/src/Extension/ComponentContainerInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php
/**
* Joomla! Content Management System
*
* @copyright Copyright (C) 2005 - 2017 Open Source Matters, Inc. All rights reserved.
* @license GNU General Public License version 2 or later; see LICENSE.txt
*/

namespace Joomla\CMS\Extension;

defined('JPATH_PLATFORM') or die;

use Joomla\CMS\Categories\Categories;

/**
* Access to component specific services.
*
* @since __DEPLOY_VERSION__
*/
interface ComponentContainerInterface
{
/**
* Returns the category service. If the service is not available
* null is returned.
*
* @param array $options The options
* @param string $section The section
*
* @return Categories|null
*
* @see Categories::setOptions()
*
* @since __DEPLOY_VERSION__
*/
public function getCategories(array $options = [], $section = '');
}
Loading