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

3.x - Add locator #290

Merged
merged 11 commits into from
Jul 12, 2022
24 changes: 21 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
# Basic docker based environment
# Necessary to trick dokku into building the documentation
# using dockerfile instead of herokuish
FROM ubuntu:17.04
FROM ubuntu:21.10

ENV TZ="Etc/UTC"
RUN apt-get update && \
apt-get install -y tzdata && \
ln -fs /usr/share/zoneinfo/Etc/UTC /etc/localtime && \
dpkg-reconfigure -f noninteractive tzdata

# Add basic tools
RUN apt-get update && \
Expand All @@ -15,10 +21,22 @@ RUN apt-get update && \

RUN LC_ALL=C.UTF-8 add-apt-repository ppa:ondrej/php && \
apt-get update && \
apt-get install -y php7.2-cli php7.2-mbstring php7.2-xml php7.2-zip php7.2-intl php7.2-opcache php7.2-sqlite
apt-get install -y \
php8.1-cli \
php8.1-mbstring \
php8.1-xml \
php8.1-zip \
php8.1-intl \
php8.1-opcache \
php8.1-sqlite \
php8.1-curl \
composer

# This prevents permission errors with the mounted vendor directory.
RUN git config --global --add safe.directory /code/vendor/cakephp/cakephp

WORKDIR /code

VOLUME ["/code"]

CMD [ '/bin/bash' ]
CMD [ "/bin/bash" ]
21 changes: 21 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
version: "3.9"
services:
console:
build: "."
environment:
DB_URL: "Cake\\ElasticSearch\\Datasource\\Connection://elasticsearch:9200?driver=Cake\\ElasticSearch\\Datasource\\Connection"
volumes:
- .:/code
elasticsearch:
image: "elasticsearch:7.17.4"
ports:
- 9200/tcp
environment:
discovery.type: single-node
ES_JAVA_OPTS: -Xms500m -Xmx500m
healthcheck:
test: "curl -f http://127.0.0.1:9200/_cluster/health || exit 1"
interval: "10s"
timeout: "5s"
retries: 10
start_period: "30s"
105 changes: 105 additions & 0 deletions src/Datasource/IndexLocator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
<?php
declare(strict_types=1);
/**
* CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
* Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
*
* Licensed under The MIT License
* For full copyright and license information, please see the LICENSE.txt
* Redistributions of files must retain the above copyright notice.
*
* @copyright Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
* @link http://cakephp.org CakePHP(tm) Project
* @since 3.5.0
* @license http://www.opensource.org/licenses/mit-license.php MIT License
*/
namespace Cake\ElasticSearch\Datasource;

use Cake\Core\App;
use Cake\Datasource\ConnectionManager;
use Cake\Datasource\Locator\AbstractLocator;
use Cake\ElasticSearch\Index;
use Cake\Utility\Inflector;

/**
* Datasource FactoryLocator compatible locater implementation.
*/
class IndexLocator extends AbstractLocator
{
/**
* Fallback class to use
*
* @var string
* @psalm-var class-string<\Cake\Elasticsearch\Index>
*/
protected $fallbackClassName = Index::class;

/**
* Whether fallback class should be used if a Index class could not be found.
*
* @var bool
*/
protected $allowFallbackClass = true;

/**
* Set fallback class name.
*
* The class that should be used to create a table instance if a concrete
* class for alias used in `get()` could not be found. Defaults to
* `Cake\Elasticsearch\Index`.
*
* @param string $className Fallback class name
* @return $this
* @psalm-param class-string<\Cake\Elasticsearch\Index> $className
*/
public function setFallbackClassName($className)
markstory marked this conversation as resolved.
Show resolved Hide resolved
{
$this->fallbackClassName = $className;

return $this;
}

/**
* Set if fallback class should be used.
*
* Controls whether a fallback class should be used to create a index
* instance if a concrete class for alias used in `get()` could not be found.
*
* @param bool $allow Flag to enable or disable fallback
* @return $this
*/
public function allowFallbackClass(bool $allow)
{
$this->allowFallbackClass = $allow;

return $this;
}

protected function createInstance(string $alias, array $options)
{
[, $classAlias] = pluginSplit($alias);
$options += ['name' => Inflector::underscore($classAlias)];

if (empty($options['className'])) {
$options['className'] = Inflector::camelize($alias);
Copy link
Member

Choose a reason for hiding this comment

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

Why not add this to the += line above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea! this is mostly copy/paste from the ORM locator.

}
$className = App::className($options['className'], 'Model/Index', 'Index');
if ($className) {
$options['className'] = $className;
} else {
if (!isset($options['name']) && strpos($options['className'], '\\') === false) {
[, $name] = pluginSplit($options['className']);
$options['name'] = Inflector::underscore($name);
}
$options['className'] = $this->fallbackClassName;
Copy link
Member

Choose a reason for hiding this comment

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

if className is being overwritten here, why do we use it in the if statement above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because we only want to generate a fallback class if the inflected one does not exist. On line 86 we attempt to resolve the inflected/provided classname. If that class doesn't exist we end up here so we can use the fallbackClassName.

One potential design flaw is that allowFallbackClass isn't used at all in this logic. Should I add that behavior or remove the ability to disable fallbackClasses? I guess we could provide a fallback class that throws an error on construction if we wanted to provide a 'strict mode'.

I could be misunderstanding the question though.

Copy link
Member

Choose a reason for hiding this comment

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

Yea I was referring to the lack of the fallback in that logic

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'll get that fixed up.

}

if (empty($options['connection'])) {
$connectionName = $options['className']::defaultConnectionName();
Copy link
Member

Choose a reason for hiding this comment

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

This is using a class name to call a static function?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. This is also how the OrmLocator works as well.

$options['connection'] = ConnectionManager::get($connectionName);
}
$options['registryAlias'] = $alias;

return new $options['className']($options);
}
}
76 changes: 19 additions & 57 deletions src/IndexRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

use Cake\Core\App;
use Cake\Datasource\ConnectionManager;
use Cake\ElasticSearch\Datasource\IndexLocator;
use Cake\Utility\Inflector;
use RuntimeException;

Expand All @@ -28,29 +29,26 @@
* created and that the correct connection is injected in.
*
* Provides an interface similar to Cake\ORM\TableRegistry.
* @deprecated 3.4.3 Statically accesible registry is deprecated. Prefer using `IndexLocator`
* alongside the `LocatorTrait` in CakePHP.
*/
class IndexRegistry
{
/**
* The map of instances in the registry.
* The locator that the global registy is wrapping.
*
* @var array
* @var Cake\ElasticSearch\Datasource\IndexLocator
*/
protected static $instances = [];
protected static $locator;

/**
* List of options by alias passed to get.
*
* @var array
*/
protected static $options = [];
protected static function getLocator(): IndexLocator
{
if (static::$locator === null) {
static::$locator = new IndexLocator();
}

/**
* Fallback class to use
*
* @var string
*/
protected static $fallbackClassName = Index::class;
return static::$locator;
}

/**
* Set fallback class name.
Expand All @@ -64,7 +62,7 @@ class IndexRegistry
*/
public static function setFallbackClassName($className)
{
static::$fallbackClassName = $className;
static::getLocator()->setFallbackClassName($className);
}

/**
Expand All @@ -79,43 +77,7 @@ public static function setFallbackClassName($className)
*/
public static function get($alias, array $options = [])
{
if (isset(static::$instances[$alias])) {
if (!empty($options) && static::$options[$alias] !== $options) {
throw new RuntimeException(sprintf(
'You cannot configure "%s", it already exists in the registry.',
$alias
));
}

return static::$instances[$alias];
}

static::$options[$alias] = $options;
[, $classAlias] = pluginSplit($alias);
$options += ['name' => Inflector::underscore($classAlias)];

if (empty($options['className'])) {
$options['className'] = Inflector::camelize($alias);
}
$className = App::className($options['className'], 'Model/Index', 'Index');
if ($className) {
$options['className'] = $className;
} else {
if (!isset($options['name']) && strpos($options['className'], '\\') === false) {
[, $name] = pluginSplit($options['className']);
$options['name'] = Inflector::underscore($name);
}
$options['className'] = static::$fallbackClassName;
}

if (empty($options['connection'])) {
$connectionName = $options['className']::defaultConnectionName();
$options['connection'] = ConnectionManager::get($connectionName);
}
$options['registryAlias'] = $alias;
static::$instances[$alias] = new $options['className']($options);

return static::$instances[$alias];
return static::getLocator()->get($alias, $options);
}

/**
Expand All @@ -126,7 +88,7 @@ public static function get($alias, array $options = [])
*/
public static function exists($alias)
{
return isset(static::$instances[$alias]);
return static::getLocator()->exists($alias);
}

/**
Expand All @@ -138,7 +100,7 @@ public static function exists($alias)
*/
public static function set($alias, Index $object)
{
return static::$instances[$alias] = $object;
return static::getLocator()->set($alias, $object);
}

/**
Expand All @@ -148,7 +110,7 @@ public static function set($alias, Index $object)
*/
public static function clear()
{
static::$instances = [];
return static::getLocator()->clear();
}

/**
Expand All @@ -159,6 +121,6 @@ public static function clear()
*/
public static function remove($alias)
{
unset(static::$instances[$alias]);
return static::getLocator()->remove($alias);
}
}
7 changes: 4 additions & 3 deletions src/Plugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Cake\Core\BasePlugin;
use Cake\Core\PluginApplicationInterface;
use Cake\Datasource\FactoryLocator;
use Cake\ElasticSearch\Datasource\IndexLocator;
use Cake\ElasticSearch\View\Form\DocumentContext;
use Cake\Event\EventManager;
use Traversable;
Expand All @@ -34,9 +35,9 @@ class Plugin extends BasePlugin
*/
public function bootstrap(PluginApplicationInterface $app): void
{
$callback = ['Cake\ElasticSearch\IndexRegistry', 'get'];
FactoryLocator::add('Elastic', $callback);
FactoryLocator::add('ElasticSearch', $callback);
$locator = new IndexLocator();
FactoryLocator::add('Elastic', $locator);
FactoryLocator::add('ElasticSearch', $locator);

// Attach the document context into FormHelper.
EventManager::instance()->on('View.beforeRender', function ($event) {
Expand Down
Loading