Skip to content

Introduce expression language in UPDATE queries #141

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

Merged
merged 1 commit into from
Mar 1, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ dev-master
### Features

- [profile:show] Added command to display current profile
- [query:update] Introduced `expr()` function to allow setting poperty values using expression language.

### Bug fixes

Expand Down
7 changes: 4 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,22 @@
"name": "phpcr/phpcr-shell",
"description": "Shell for PHPCR",
"require": {
"symfony/console": "~2.3",
"symfony/console": "~2.4",
"jackalope/jackalope": "~1.1",
"phpcr/phpcr": "~2.1",
"phpcr/phpcr-utils": "~1.2",
"symfony/finder": "~2.3",
"symfony/serializer": "~2.3",
"symfony/yaml": "~2.3",
"symfony/dependency-injection": "~2.3",
"symfony/expression-language": "~2.4",
"dantleech/glob-finder": "~1.0"
},
"minimum-stability": "dev",
"require-dev": {
"symfony/symfony": "2.6",
"symfony/process": "~2.3",
"symfony/filesystem": "~2.3",
"symfony/process": "~2.4",
"symfony/filesystem": "~2.4",
"phpunit/phpunit": "~3.7.28",
"behat/behat": "~3.0.0",
"phpspec/phpspec": "2.0",
Expand Down
27 changes: 27 additions & 0 deletions features/all/phpcr_query_update.feature
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,30 @@ Feature: Execute a a raw UPDATE query in JCR_SQL2
| UPDATE [nt:unstructured] mixin_foo('bar') |
| UPDATE [nt:unstructured] APPLY mixin_foo('bar') |
| UPDATE [nt:unstructured] mixin_foo'bar') |

Scenario Outline: Execute update query with expressions
When I execute the "<query>" command
Then the command should not fail
And I save the session
Copy link

Choose a reason for hiding this comment

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

This scenario is really weird. I save the session is an actionable sentence. It should be in the WHEN part of the scenario, not the THEN part.

Copy link
Member Author

Choose a reason for hiding this comment

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

So what would be better?

When I execute the "<query>" command
Then the command should not fail
When I save the session
Then the node at "<path>" should ...
And I should see the following:
...

?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really sure how to best structure, this created an issue as all of the features need to be fixed: #145

Copy link

Choose a reason for hiding this comment

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

Having When parts after the Then parts looks weird. The recommendation is to stick to this workflow (you can have multiple steps in each section if needed):

  • GIVEN: initialize the state of the project (Given a node "foobar" has been created)
  • WHEN: perform an action (When I execute a command)
  • THEN: assert something (Then it should fail)

A good rule to identify where each step belongs is to read them:

  • When steps should always use an action word (otherwise you cannot perform an action)
  • Then steps will generally use should (but it may depend of your convention)
  • Given steps will either use past sentences with action words (describing what happened previously) or descriptive verbs (Given there is ...) depending on your convention

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 think the main issue is that it is necessary to execute 2 commands (the second one is to save the session). The first command should not fail - if it did fail the second command (session:save) would still succeed.

So I need to indicate that the first command should or should not fail before saving the session.

I suppose it would make sense to combine the session save in a single statement:

When I run the "UPDATE ..." command and save the session
Then the command should not fail
And the node at "/foo" should exist
And I should see the following:
"""
foo
"""

Copy link

Choose a reason for hiding this comment

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

Well, it is a matter of refactoring the context so that the command should not fail does not take the result of saving the session into account.

And btw, does the user need to save the session explicitly after running a command ? If no, it should not appear in the scenario at all.

Then the node at "<path>" should have the property "<property>" with value "<expectedValue>"
And I should see the following:
"""
1 row(s) affected
"""
Examples:
| query | path | property | expectedValue |
| UPDATE [nt:unstructured] AS a SET a.title = expr('row.getNode().getName()') WHERE localname() = 'article1' | /cms/articles/article1 | title | article1 |
| UPDATE [nt:unstructured] AS a SET a.title = expr('row.getPath()') WHERE localname() = 'article1' | /cms/articles/article1 | title | /cms/articles/article1 |

Scenario: Execute an update with a quoted expression (can't do this in Examples above)
When I execute the following command:
"""
UPDATE [nt:unstructured] AS a SET a.weight = expr('row.getNode().getPropertyValue("weight") * 2') WHERE a.name = 'Product One'
"""
Then the command should not fail
And I save the session
Then the node at "/cms/products/product1" should have the property "weight" with value "20"
And I should see the following:
"""
1 row(s) affected
"""
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public function execute(InputInterface $input, OutputInterface $output)
$result = $query->execute();
$rows = 0;

$updateProcessor = new UpdateProcessor();
$updateProcessor = $this->get('query.update.processor');

foreach ($result as $row) {
$rows++;
Expand Down
8 changes: 8 additions & 0 deletions src/PHPCR/Shell/DependencyInjection/Container.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public function __construct($mode = PhpcrShell::MODE_STANDALONE)
$this->registerPhpcr();
$this->registerEvent();
$this->registerConsole();
$this->registerQuery();
}

public function registerHelpers()
Expand Down Expand Up @@ -167,6 +168,13 @@ public function registerConsole()
->addArgument(new Reference('phpcr.session'));
}

public function registerQuery()
{
$this->register('query.update.expression_language', 'Symfony\Component\ExpressionLanguage\ExpressionLanguage');
$this->register('query.update.processor', 'PHPCR\Shell\Query\UpdateProcessor')
->addArgument(new Reference('query.update.expression_language'));
}

public function getMode()
{
return $this->mode;
Expand Down
11 changes: 10 additions & 1 deletion src/PHPCR/Shell/Query/UpdateProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace PHPCR\Shell\Query;

use PHPCR\Query\RowInterface;
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;

/**
* Processor for node updates
Expand All @@ -25,7 +26,7 @@ class UpdateProcessor
*/
private $functionMap = array();

public function __construct()
public function __construct(ExpressionLanguage $expressionLanguage)
{
$this->functionMapApply = array(
'mixin_add' => function ($operand, $row, $mixinName) {
Expand All @@ -42,6 +43,14 @@ public function __construct()
);

$this->functionMapSet = array(
'expr' => function ($operand, $row, $expression) use ($expressionLanguage) {
return $expressionLanguage->evaluate(
$expression,
array(
'row' => $row,
)
);
},
'array_replace' => function ($operand, $row, $v, $x, $y) {
$operand->validateScalarArray($v);
foreach ($v as $key => $value) {
Expand Down
8 changes: 8 additions & 0 deletions src/PHPCR/Shell/Test/ContextBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ public function iExecuteTheCommand($args)
$this->executeCommand($args);
}

/**
* @Given I execute the following command:
*/
public function iExecuteTheFollowingCommand(PyStringNode $command)
{
$this->executeCommand($command);
}

/**
* @Given /^I execute the following commands:$/
*/
Expand Down