[WIP] Workspace Node Update Command and fixes

[dantleech]

• This command updates multiple nodes in a workspace based on a given
  SQL query.

./bin/phpcr workspace:node:update "SELECT FROM nt:unstructred" --set-prop=foo=bar --add-mixin=some:mixin

Follows same options as node:touch. There is also room for adding a workspace:node:remove command for selectively removing nodes from a workspace.

Also some refactoring.

[dantleech]

Note that this only works in theory - havn't yet tried it on a real implementation.

[dbu]

i'd like to keep such typehints as they help spot coding errors and enable autocomplete in IDEs like phpstorm or netbeans and eclipse.

[dantleech]

Sorry, that was probably a bit over zealous of me. But I am still of the opinion that we shouldn't have these type hints. There are only 7 instances of this in the entire ./vendor/symfony directory which would seem to indicate that its not a generally held standard. But I can put this one back.

[dbu]

i add them whenever i see phpstorm indicating it can't know the class of something. most of the time its not needed: if we call a method of a known class that is properly documented, phpstorm knows what we get back and acts accordingly. or we just pass the unknown thing we get to a typehinted method, where again phpstorm knows what should go on. its only cases like here where it makes a difference.

[lsmith77]

the only alternative for this would be to write getter methods ..

[dbu]

i really like this, also the refactoring part to avoid code duplication. a few inputs, otherwise looks good.

[dantleech]

Yeah. I agree with your points, in addotion I thought of renaming to workspace:query:update then also have query:select and query:remove also.

Maybe we can drop "workspace"

David Buchmann notifications@github.com a écrit :

i really like this, also the refactoring part to avoid code
duplication. a few inputs, otherwise looks good.

---

Reply to this email directly or view it on GitHub:
#67 (comment)

Envoyé de mon téléphone Android avec K-9 Mail. Excusez la brièveté.

[dbu]

hm. from the implementation point of view, the renaming would make sense. but from the semantical point of view i think i care more about that i can update nodes and remove nodes. that i can specify the node to update or remove by query is more the means than the goal. so i would opt for calling the commands node:update and node:remove (which we have already, but only for single nodes). the query select thing exists already as well, its called workspace:query - and i think that name is ok.

btw, it would be cool to specify allow an absolute path as simple alternative to the query. then we can just improve node:remove and have one command to update one or several nodes. node:touch would become more or less unneeded at that point, if we say node:udpate also creates a node.

or we could separate the query based and path based commands by calling the query based things nodes:*.

[dantleech]

I think I prefer

  phpcr:node-type:list
  phpcr:node-type:register
  phpcr:node:dump
  phpcr:node:move
  phpcr:node:remove
  phpcr:node:touch
  phpcr:workspace:create 
  phpcr:workspace:export  
  phpcr:workspace:import 
  phpcr:workspace:list 
  phpcr:workspace:purge 
  phpcr:query:select "SELECT foo"
  phpcr:query:update "SELECT foo" --set-prop=foo=bar
  phpcr:query:delete "SELET foo"

When working on individual nodes you provide the "path" as the argument, when working with queries (many nodes) you provide the "query".

I think my point is that when updating many nodes you operate on the query, so the query is the subject.

I think its also nice to seperate single node manipulation and batch.

[dbu]

do we have the helper available at this point? then we could ask the repository for the available query languages. jackalope-doctrine-dbal does not support sql, only jcr_sql2. jackalope-jackrabbit additionally supports xpath...

[dbu]

alright, i am ok with this naming scheme. it will make people not miss the update / delete queries of normal sql databases too much :-)

musts: the copy-paste comment, re-add typehint
nice to haves: check available query languages of current impl, property rename, interactive mode

[dantleech]

ok just an update for today, have now opted for

nodes:update --type="nt:unstructured" --where="foo=\"bar\" AND bar=\"foo\"" --set-prop=foo=bar

However am currently stuck on the helper side of things: The "phpcr" helper used in the DoctrineBundle is not the same as the one used in "phpcr-utils". It is a DocumentManagerHelper instance, which is configured by a static call DocumentCommandHelper::setApplicationPhpcrSession. Seems that this could be refactored, but I don't want to get into that right now :)

So I think its best to add a new helper, "phpcr-cli" for example, all the phpcr commands can then extend a common base class that automatically adds this helper using the session obtained from the phpcr helper.

OK, I think I've sorted my problem there :) Will have another crack at it tomorrow if I have time.

[dbu]

i noticed yesterday that the phpcr-bundle was not up to date with the refactorings for the dumper-helper. i fixed that. we could aim to tag a new phpcr-utils + jackalope version end of this week and adjust phpcr-bundle to it and tag versions of both. now we can still change helpers and clean up things. note that while commands are not services, it is possible to fetch a service from the container to avoid hardcoded classes: doctrine/DoctrinePHPCRBundle@b63e629

[lsmith77]

the curly bracket should be on this line .. same goes for all the foreach statements below

[dantleech]

this is very strange. Looks like an editor glitch on my end. Looks fine in VIM but the raw file is like below.

[dantleech]

ok, this is ready to go.This is an example:

php app/console doctrine:phpcr:nodes:update --query="SELECT * FROM [nt:unstructured] WHERE [phpcr:class]=\"Symfony\\Cmf\\Bundle\\RoutingBundle\\Document\\Route\"" --set-prop=foo=bar --remove-prop=bar --add-mixin=mix:lockable --remove-mixin=mix:lockable

Not sure if "query" above should be an option or an argument.

Have addressed @dbu s points above except for the rename-prop feature, which can be easily added.

[dantleech]

This comment does not apply

[dbu]

thanks a lot, nice command!

[lsmith77]

was the plural here intentional? NodesUpdateCommand vs. NodeUpdateCommand

[lsmith77]

btw .. i am preparing an update of DoctrinePHPCRBundle ..

[dantleech]

Oops, forgot to make the corresponding bundle PR. Can do this evening.

Lukas Kahwe Smith notifications@github.com a écrit :

+namespace PHPCR\Util\Console\Command;
+
+use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Console\Helper\DialogHelper;
+use PHPCR\Util\Console\Command\BaseCommand;
+
+/**

• * Command which can update the properties of nodes found
• * using the given JCR query.
• * @author Daniel Leech daniel@dantleech.com
• */
  +class NodesUpdateCommand extends BaseCommand

btw .. i am preparing an update of DoctrinePHPCRBundle ..

---

Reply to this email directly or view it on GitHub:
https://github.com/phpcr/phpcr-utils/pull/67/files#r4658962

Envoyé de mon téléphone Android avec K-9 Mail. Excusez la brièveté.

[lsmith77]

no worries. I have it ready

[dantleech]

As an additional feature, what about adding closure support?:

./app/console doctrine:phpcr:nodes:update "SELECT * FROM nt:unstructured" --apply-closure="function ($node) { $node->setProperty('foobar', $node->getProperty('barfoo')); }"

[dbu]

well, at some point we have to stop :-) people can also write their own commands for complicated updates. using this one as a template how to do it. i think closure is a cool idea and not over the top yet, but not critical for 1.0.