Allow selection of extending documents #218

Merged
merged 7 commits into from Jan 24, 2013

Conversation

Projects
None yet
4 participants
@dantleech
Contributor

dantleech commented Jan 17, 2013

To allow:

/** @Document */
class Foo
{
}

/** @Document */
class ExtendsFoo
{
}

$qb->from('Foo')->getQuery()->execute(); // will return Foo's and ExtendsFoo's
  • Adds new multivalue node type property phpcr:classparents
  • Popualtes this with the class names of ALL extending classes on
    persist
  • QueryBuilder selects on phpcr:class OR phpcr:classparents

This is just a quick WDYT, so no tests yet. But it works

If this is good:

  • perhaps it would be a good idea to add a command to "sync" the registered nodes with their documents and their extending classes?, e.g. if user decides to make an already persisted document type a child class
  • due to a bug in doctrine-dbal this won't work with multiple parent classes (can only select on one value with that transport)

doctrine-dbal needs jackalope/jackalope-doctrine-dbal#83 for this to work.

@stof

View changes

lib/Doctrine/ODM/PHPCR/DocumentClassMapper.php
+ $parentClasses = array();
+
+ while ($parent = $refl->getParentClass()) {
+ $parentRefl = $refl->getParentClass();

This comment has been minimized.

Show comment Hide comment
@stof

stof Jan 17, 2013

Member

you already have $parent for this``

@stof

stof Jan 17, 2013

Member

you already have $parent for this``

@stof

View changes

lib/Doctrine/ODM/PHPCR/DocumentClassMapper.php
+ $parentRefl = $refl->getParentClass();
+ $parentClasses[] = $parentRefl->name;
+ $refl = $parentRefl;
+ }

This comment has been minimized.

Show comment Hide comment
@stof

stof Jan 17, 2013

Member

why not just using class_parents ?

@stof

stof Jan 17, 2013

Member

why not just using class_parents ?

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 17, 2013

Contributor

Didn't know that that function existed. Thanks.

@dantleech

dantleech Jan 17, 2013

Contributor

Didn't know that that function existed. Thanks.

@dbu

This comment has been minimized.

Show comment Hide comment
@dbu

dbu Jan 17, 2013

Member

great stuff, i really think it makes a lot of sense to do this. the alternative to achieve this behaviour could be to have to manually define child classes on Document and MappedSuperclass classes and build the information when querying (and probably cache it somehow). then query for phpcr:class = x or phpcr:class = y ...

the plus of determining at runtime would be that we do not need to store extra information and that there would be no need for a sync tool. on the other hand it is more pain for the user as he has to update classes. and in some situations it will just be impossible to update base classes if they come from some 3rd party bundle. so overall +1 for how you implemented it.

one thing i wonder is if we store all parent classes or only those that are themselves a Document or a MappedSuperclass. the later would probably make more sense - at least it is like that when using the ORM.

regarding the sync tool: it would be great to have some sort of migrations support. there is quite a few operations i can think of:

  • renaming a class (for example move to different namespace)
  • rename or remove a field/property
  • changes to the inheritance structure now that we persist that structure

but the sync would be for a new PR

so what we have to decide here is: do we store all classes or only those mapped by PHPCR-ODM. opinions anybody?

Member

dbu commented Jan 17, 2013

great stuff, i really think it makes a lot of sense to do this. the alternative to achieve this behaviour could be to have to manually define child classes on Document and MappedSuperclass classes and build the information when querying (and probably cache it somehow). then query for phpcr:class = x or phpcr:class = y ...

the plus of determining at runtime would be that we do not need to store extra information and that there would be no need for a sync tool. on the other hand it is more pain for the user as he has to update classes. and in some situations it will just be impossible to update base classes if they come from some 3rd party bundle. so overall +1 for how you implemented it.

one thing i wonder is if we store all parent classes or only those that are themselves a Document or a MappedSuperclass. the later would probably make more sense - at least it is like that when using the ORM.

regarding the sync tool: it would be great to have some sort of migrations support. there is quite a few operations i can think of:

  • renaming a class (for example move to different namespace)
  • rename or remove a field/property
  • changes to the inheritance structure now that we persist that structure

but the sync would be for a new PR

so what we have to decide here is: do we store all classes or only those mapped by PHPCR-ODM. opinions anybody?

@dantleech

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 18, 2013

Contributor

Updated, have gone with only registering classes that the ODM is aware of - I think that makes the most sense.

Also, if there are, for example, 3 classes in the heirarchy and the second class is not mapped, and the third is, we will record the third and not the second - it was a choice between that or stopping the registration at the first non-mapped parent class.

Finally, am having real problems getting the QueryBuilder unit test to pass with this, the mock expectations have become a little unmanageable, will have another look at that tomorrow.

Contributor

dantleech commented Jan 18, 2013

Updated, have gone with only registering classes that the ODM is aware of - I think that makes the most sense.

Also, if there are, for example, 3 classes in the heirarchy and the second class is not mapped, and the third is, we will record the third and not the second - it was a choice between that or stopping the registration at the first non-mapped parent class.

Finally, am having real problems getting the QueryBuilder unit test to pass with this, the mock expectations have become a little unmanageable, will have another look at that tomorrow.

@stof

This comment has been minimized.

Show comment Hide comment
@stof

stof Jan 18, 2013

Member

The ORM stops as soon as it reaches an unmapped class (which is one of the reason for MappedSuperClass). I think you should behave the same for consistency.

Member

stof commented Jan 18, 2013

The ORM stops as soon as it reaches an unmapped class (which is one of the reason for MappedSuperClass). I think you should behave the same for consistency.

@dantleech

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 18, 2013

Contributor

ok, no problem.

Contributor

dantleech commented Jan 18, 2013

ok, no problem.

@lsmith77

View changes

lib/Doctrine/ODM/PHPCR/DocumentClassMapper.php
+
+ $parentClasses = array();
+
+ while ($parent = $refl->getParentClass()) {

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 18, 2013

Member

i think we should better compute this list as part of the ClassMetadata so that it doesnt need to be computed at run time.

@lsmith77

lsmith77 Jan 18, 2013

Member

i think we should better compute this list as part of the ClassMetadata so that it doesnt need to be computed at run time.

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 18, 2013

Contributor

ok, I think that makes sense, so effectively $class->getMappedParentClasses() ?

@dantleech

dantleech Jan 18, 2013

Contributor

ok, I think that makes sense, so effectively $class->getMappedParentClasses() ?

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 18, 2013

Member

yes .. though i would just called it getParentClasses()

@lsmith77

lsmith77 Jan 18, 2013

Member

yes .. though i would just called it getParentClasses()

dantleech added some commits Jan 17, 2013

Allow selection of extending documents
- Adds new multivalue node type property phpcr:classparents
- Popualtes this with the class names of ALL extending classes on
  persist
- QueryBuilder selects on phpcr:class OR phpcr:classparents
@dantleech

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 23, 2013

Contributor

Updated - The class metadata factory is now responsible for setting the mapped parent classes on the class metadata, which is then used by the DocumentMapper to set the phpcr:classparents attribute.

The factory reuses the existing getClassParents method in the self same factory -- which returns all mapped classes in the class heirarchy, so it is contrary to how to ORM functions (it stops at the first transient) but consistent with the existing logic. What do you think? Do we see any reasons why we should not be able to have an intermediate non-mapped class?

Contributor

dantleech commented Jan 23, 2013

Updated - The class metadata factory is now responsible for setting the mapped parent classes on the class metadata, which is then used by the DocumentMapper to set the phpcr:classparents attribute.

The factory reuses the existing getClassParents method in the self same factory -- which returns all mapped classes in the class heirarchy, so it is contrary to how to ORM functions (it stops at the first transient) but consistent with the existing logic. What do you think? Do we see any reasons why we should not be able to have an intermediate non-mapped class?

@dantleech

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 23, 2013

Contributor

Oops, transient classes are non-mapped classes. Better fix that.

Contributor

dantleech commented Jan 23, 2013

Oops, transient classes are non-mapped classes. Better fix that.

+ *
+ * @var array
+ */
+ public $mappedParentClasses = array();

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 23, 2013

Member

i am still not a big fan of the "mapped" prefix here ..

@lsmith77

lsmith77 Jan 23, 2013

Member

i am still not a big fan of the "mapped" prefix here ..

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 23, 2013

Contributor

Yeah, I thought it might be better to be explicit, although the ClassMetadataFactory uses getParentClasses for the same thing. Can change.

@dantleech

dantleech Jan 23, 2013

Contributor

Yeah, I thought it might be better to be explicit, although the ClassMetadataFactory uses getParentClasses for the same thing. Can change.

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 23, 2013

Member

i would prefer this .. the name of the class itself is after all stored in $this->name

@lsmith77

lsmith77 Jan 23, 2013

Member

i would prefer this .. the name of the class itself is after all stored in $this->name

@dantleech

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 24, 2013

Contributor

ok, have renamed getMappedParentClasses to getParentClasses.

Contributor

dantleech commented Jan 24, 2013

ok, have renamed getMappedParentClasses to getParentClasses.

@@ -25,6 +25,7 @@
use PHPCR\NodeInterface;
use PHPCR\PropertyType;
+use Doctrine\ODM\PHPCR\Mapping\MappingException;

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 24, 2013

Member

this can be removed, right?

@lsmith77

lsmith77 Jan 24, 2013

Member

this can be removed, right?

+ $props->offsetSet(1, $phpcrClassTpl);
+ $props->offsetSet(2, $phpcrClassParentsTpl);
+
+

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 24, 2013

Member

extra new line

@lsmith77

lsmith77 Jan 24, 2013

Member

extra new line

@dantleech

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 24, 2013

Contributor

Updated. Removed whitespace and unsused use statement.

Contributor

dantleech commented Jan 24, 2013

Updated. Removed whitespace and unsused use statement.

lsmith77 added a commit that referenced this pull request Jan 24, 2013

@lsmith77 lsmith77 merged commit 7edc3bf into doctrine:master Jan 24, 2013

1 check was pending

default The Travis build is in progress
Details
@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 24, 2013

Member

the merge leads to a tricky "issue" in the sonata admin. suddenly all the MultilangMenuNode instances show up in the MenuNode list. maybe we need to add an option somewhere so that this inheritance can be prevented?

Member

lsmith77 commented Jan 24, 2013

the merge leads to a tricky "issue" in the sonata admin. suddenly all the MultilangMenuNode instances show up in the MenuNode list. maybe we need to add an option somewhere so that this inheritance can be prevented?

@dantleech

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 24, 2013

Contributor

I'm not sure how the Multilang thing works, so I can't offer much advice there. Maybe make MenuNode into AbstractMenuNode (a mapped superclass) and have MenuNode and MultilangMenuNode extend that?

Contributor

dantleech commented Jan 24, 2013

I'm not sure how the Multilang thing works, so I can't offer much advice there. Maybe make MenuNode into AbstractMenuNode (a mapped superclass) and have MenuNode and MultilangMenuNode extend that?

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 24, 2013

Member

the issue is quite simple. MultilangMenuNode extends MenuNode ..
we have one admin ui for the multi lang menu nodes and another one for the others.

with this change all nodes on http://cmf.liip.ch/en/admin/bundle/menu/multilangmenuitem/list also show up in http://cmf.liip.ch/en/admin/bundle/menu/menuitem/list (note that site hasnt been updated yet, but this is what i see locally now with the master of the cmf-sandbox)

Member

lsmith77 commented Jan 24, 2013

the issue is quite simple. MultilangMenuNode extends MenuNode ..
we have one admin ui for the multi lang menu nodes and another one for the others.

with this change all nodes on http://cmf.liip.ch/en/admin/bundle/menu/multilangmenuitem/list also show up in http://cmf.liip.ch/en/admin/bundle/menu/menuitem/list (note that site hasnt been updated yet, but this is what i see locally now with the master of the cmf-sandbox)

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 24, 2013

Member

also i should note that i do see situations where i would want that behavior which is why i said maybe this needs to be an option inside SonataDoctrinePhpcrBundle .. but that bundle might need some support by PHPCR ODM to make this possible without a ton of effort

Member

lsmith77 commented Jan 24, 2013

also i should note that i do see situations where i would want that behavior which is why i said maybe this needs to be an option inside SonataDoctrinePhpcrBundle .. but that bundle might need some support by PHPCR ODM to make this possible without a ton of effort

@dantleech

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 24, 2013

Contributor

hmm, but regarding that specific problem

symfony-cmf/MenuBundle#30

would solve it no? having a concrete child class each for MenuNode and MultiLangMenuNode

Contributor

dantleech commented Jan 24, 2013

hmm, but regarding that specific problem

symfony-cmf/MenuBundle#30

would solve it no? having a concrete child class each for MenuNode and MultiLangMenuNode

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 24, 2013

Member

yes it would solve it .. but without giving the option to infact have both in the same list. plus we have the same situation in lots of other places .. ContentBundle, SimpleCmsBundle .. probably also eventually BlockBundle .. not sure if we really want to require having to define Abstract classes in all of those cases.

seems to me it would make more sense to be able to give control over if to only check the exact class or also the parents would be the more elegant solution.

Member

lsmith77 commented Jan 24, 2013

yes it would solve it .. but without giving the option to infact have both in the same list. plus we have the same situation in lots of other places .. ContentBundle, SimpleCmsBundle .. probably also eventually BlockBundle .. not sure if we really want to require having to define Abstract classes in all of those cases.

seems to me it would make more sense to be able to give control over if to only check the exact class or also the parents would be the more elegant solution.

@dantleech

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 24, 2013

Contributor

hmm, for me it makes sense to have the concrete instances extending an abstract class -- as MenuNode and MultiLangMenuNode are two cases for the same thing. Also I am used to the ORM, and have never needed that feature.

What do you have in mind? A document annotation?

Contributor

dantleech commented Jan 24, 2013

hmm, for me it makes sense to have the concrete instances extending an abstract class -- as MenuNode and MultiLangMenuNode are two cases for the same thing. Also I am used to the ORM, and have never needed that feature.

What do you have in mind? A document annotation?

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 24, 2013

Member

i was thinking about some way to set this option on the query object

Member

lsmith77 commented Jan 24, 2013

i was thinking about some way to set this option on the query object

@dantleech

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 24, 2013

Contributor

I'm just thinking, if in the future we switched to mapping classes to NodeTypes, would this feature still be possible? i.e. selecting only the named node types and not the types that extend it.

Contributor

dantleech commented Jan 24, 2013

I'm just thinking, if in the future we switched to mapping classes to NodeTypes, would this feature still be possible? i.e. selecting only the named node types and not the types that extend it.

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 24, 2013

Member

hmm you are right ... it would probably not. @dbu the node type inheritance when querying nodes via SQL2 cannot be disabled .. right?

Member

lsmith77 commented Jan 24, 2013

hmm you are right ... it would probably not. @dbu the node type inheritance when querying nodes via SQL2 cannot be disabled .. right?

@dbu

This comment has been minimized.

Show comment Hide comment
@dbu

dbu Jan 24, 2013

Member

from how i read that, no it would not allow to control that. but in the odm i totally see the usecase. to say i only want this class but no descendants. maybe we could query explicitly on not having the class we look for in the parent class field? or indeed have an option to from to say if its strict or with inheritance.

using node types, the only ways i can see would be indeed to use a common base type, or to filter the result on the exact node type. maybe we could also add a check on jcr:primaryType to the query. like select * from nt:unstructured where jcr:primaryType = nt:unstructured. might work, did not try it.

Member

dbu commented Jan 24, 2013

from how i read that, no it would not allow to control that. but in the odm i totally see the usecase. to say i only want this class but no descendants. maybe we could query explicitly on not having the class we look for in the parent class field? or indeed have an option to from to say if its strict or with inheritance.

using node types, the only ways i can see would be indeed to use a common base type, or to filter the result on the exact node type. maybe we could also add a check on jcr:primaryType to the query. like select * from nt:unstructured where jcr:primaryType = nt:unstructured. might work, did not try it.

@dbu

This comment has been minimized.

Show comment Hide comment
@dbu

dbu Jan 24, 2013

Member

and regarding #218 that would save us here, at the cost of adding another level of abstraction. but it won't save a user if he does something extending a SimpleCmsBundle Page or such.

actually for the menubundle i even wonder if we do not want/can mix them in sonata? i mean, as a user i probably don't care, i just want to edit the menu entry...

Member

dbu commented Jan 24, 2013

and regarding #218 that would save us here, at the cost of adding another level of abstraction. but it won't save a user if he does something extending a SimpleCmsBundle Page or such.

actually for the menubundle i even wonder if we do not want/can mix them in sonata? i mean, as a user i probably don't care, i just want to edit the menu entry...

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 25, 2013

Member

indeed having the ability to handle mixed collections in sonata admin would be quite useful.

Member

lsmith77 commented Jan 25, 2013

indeed having the ability to handle mixed collections in sonata admin would be quite useful.

@dbu

This comment has been minimized.

Show comment Hide comment
@dbu

dbu Jan 25, 2013

Member

yes, but i think nontheless we need a way to query phpcr-odm for strictly one class and no descendant classes.

Member

dbu commented Jan 25, 2013

yes, but i think nontheless we need a way to query phpcr-odm for strictly one class and no descendant classes.

@dantleech

This comment has been minimized.

Show comment Hide comment
@dantleech

dantleech Jan 25, 2013

Contributor

AFAIK the ORM doesn't support selecting just the parent instances - why should the ODM be any different? It forces you to subclass, which I think is cleaner architecturally. In the case oif the MenuBundle, MenuNode and MultiLangMenuNode are two types of menu, if you want them all you select the abstract type, if not the concrete.

In what cases would sub-classing not be appropriate?

Sonata already supports handling abstract classes if thats what you mean, its quite neat e.g. http://dcms.dantleech.com/admin/site/dantleech.com/endpoint/list (click add new)

Contributor

dantleech commented Jan 25, 2013

AFAIK the ORM doesn't support selecting just the parent instances - why should the ODM be any different? It forces you to subclass, which I think is cleaner architecturally. In the case oif the MenuBundle, MenuNode and MultiLangMenuNode are two types of menu, if you want them all you select the abstract type, if not the concrete.

In what cases would sub-classing not be appropriate?

Sonata already supports handling abstract classes if thats what you mean, its quite neat e.g. http://dcms.dantleech.com/admin/site/dantleech.com/endpoint/list (click add new)

@dbu

This comment has been minimized.

Show comment Hide comment
@dbu

dbu Jan 25, 2013

Member

but that is because ORM is much more strongly typed. we could be more flexible with phpcr-odm.

if we follow that logic we have to provide abstract documents for everything in our bundles and extend them as is for the concrete classes. not sure if that is really elegant or cleaner. but that way at least people could always extend our things in a consistent manner.

on the other hand, the more we discuss here i wonder if mixed lists really are such a problem. the most natural access to the content is the tree anyways. and inside a subtree i would want to see all kind of different document classes - the structure from a user point of view is the tree, not the class hierarchy. if sonata can handle the situation that some documents in its list actually need a different form to edit them then we would be fine. and maybe we could add a filter to only see the list of some subtree to avoid problems for example in the sandbox with a tree of SimpleCmsBundle page documents that extend the route documents.

maybe @beberlei has some input on this one?

Member

dbu commented Jan 25, 2013

but that is because ORM is much more strongly typed. we could be more flexible with phpcr-odm.

if we follow that logic we have to provide abstract documents for everything in our bundles and extend them as is for the concrete classes. not sure if that is really elegant or cleaner. but that way at least people could always extend our things in a consistent manner.

on the other hand, the more we discuss here i wonder if mixed lists really are such a problem. the most natural access to the content is the tree anyways. and inside a subtree i would want to see all kind of different document classes - the structure from a user point of view is the tree, not the class hierarchy. if sonata can handle the situation that some documents in its list actually need a different form to edit them then we would be fine. and maybe we could add a filter to only see the list of some subtree to avoid problems for example in the sandbox with a tree of SimpleCmsBundle page documents that extend the route documents.

maybe @beberlei has some input on this one?

@stof

This comment has been minimized.

Show comment Hide comment
@stof

stof Jan 26, 2013

Member

querying only for the parent class only does not make sense from an OO point of view. Instances of child classes are instances of the parent class too.

Member

stof commented Jan 26, 2013

querying only for the parent class only does not make sense from an OO point of view. Instances of child classes are instances of the parent class too.

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 26, 2013

Member

ok .. i agree. the issue i noted is an issue we need to solve in SonataAdminBundle .. ie. being able to handle mixed collections in the list and pointing them to the right form as needed.

@rande: does SonataAdminBundle provide anything here yet?

as for being able to query for a specific class only. this will then just require custom query construction.

Member

lsmith77 commented Jan 26, 2013

ok .. i agree. the issue i noted is an issue we need to solve in SonataAdminBundle .. ie. being able to handle mixed collections in the list and pointing them to the right form as needed.

@rande: does SonataAdminBundle provide anything here yet?

as for being able to query for a specific class only. this will then just require custom query construction.

@lsmith77 lsmith77 referenced this pull request in sonata-project/SonataDoctrinePhpcrAdminBundle Jan 26, 2013

Open

handling of inheritance (or more generally mixed collections) #80

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Jan 26, 2013

Member

ok i have created sonata-project/SonataDoctrinePhpcrAdminBundle#80 as the place to continue this discussion

Member

lsmith77 commented Jan 26, 2013

ok i have created sonata-project/SonataDoctrinePhpcrAdminBundle#80 as the place to continue this discussion

@lsmith77

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Mar 27, 2013

Member

it seems there is some issue in the validation code which causes a ContraintViolationException: " Node type definition does not allow to set the property with name 'phpcr:classparents' and value 'array'"

btw: i guess now that thanks to @dbu we have a working cnd parser we would no longer need this different handling in this command, but i guess it would just move these methods calls to a different place ..

it seems there is some issue in the validation code which causes a ContraintViolationException: " Node type definition does not allow to set the property with name 'phpcr:classparents' and value 'array'"

btw: i guess now that thanks to @dbu we have a working cnd parser we would no longer need this different handling in this command, but i guess it would just move these methods calls to a different place ..

This comment has been minimized.

Show comment Hide comment
@lsmith77

lsmith77 Mar 27, 2013

Member

btw it could be related to how we persist node type definitions in the Doctrine DBAL transport which causes some information to be lost.

Member

lsmith77 replied Mar 27, 2013

btw it could be related to how we persist node type definitions in the Doctrine DBAL transport which causes some information to be lost.

This comment has been minimized.

Show comment Hide comment
@dbu

dbu Mar 28, 2013

Member

it must be a bug in jackalope dbal with either node type definition storing or node type validation (jackalope jackrabbit does not validate at all, it trusts jackrabbit to validate).

the problem won't go away with the cnd parser as this will generate the same definitions. but we can simplify the code, which is a good thing.

Member

dbu replied Mar 28, 2013

it must be a bug in jackalope dbal with either node type definition storing or node type validation (jackalope jackrabbit does not validate at all, it trusts jackrabbit to validate).

the problem won't go away with the cnd parser as this will generate the same definitions. but we can simplify the code, which is a good thing.

This comment has been minimized.

Show comment Hide comment
@dbu

dbu Mar 28, 2013

Member

very weird. i tried to reproduce this in the phpcr-api-tests like this phpcr/phpcr-api-tests@5ce38c9#L4R13

but there is no fail with jackalope-doctrine-dbal. what am i doing different than we do here?

Member

dbu replied Mar 28, 2013

very weird. i tried to reproduce this in the phpcr-api-tests like this phpcr/phpcr-api-tests@5ce38c9#L4R13

but there is no fail with jackalope-doctrine-dbal. what am i doing different than we do here?

This comment has been minimized.

Show comment Hide comment
@dbu

dbu Mar 28, 2013

Member

so cnd import in jackalope-dbal now works as soon as we merge jackalope/jackalope-doctrine-dbal#95

Member

dbu replied Mar 28, 2013

so cnd import in jackalope-dbal now works as soon as we merge jackalope/jackalope-doctrine-dbal#95

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