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

[Tree - Closure] Issue enabling APC cache #58

Closed
comfortablynumb opened this issue May 26, 2011 · 10 comments
Closed

[Tree - Closure] Issue enabling APC cache #58

comfortablynumb opened this issue May 26, 2011 · 10 comments
Labels
Bug A confirmed bug in Extensions that needs fixing.

Comments

@comfortablynumb
Copy link
Contributor

Hi,

Using extensions from master branch (updated a week ago or so) and enabling APC Cache I get the following exception, which is not thrown if I disable it:

[Semantical Error] line 0, col 103 near 'descendant WHERE': Error: Class Entity\EntityClosure has no association named descendant

Am I doing something wrong here? or maybe I'm missing something?

Thanks in advance.

@l3pp4rd
Copy link
Contributor

l3pp4rd commented May 26, 2011

I think it is really an issue, since I forgot that onMetadataLoad event is never triggered if cache is used. Users will have to map their closures manually. Today I have a bunch of new issues reported, it was so quite for a month or two :) now I don't think that I will be able to fix it before the weekend. For now you can map it manually using annotations or whatever.

@comfortablynumb
Copy link
Contributor Author

Hey, don't worry! there's no rush :) It's awesome to have these extensions already working. And they work really great! I'd like to help with this type of issues but I'm not familiarized a lot with Doctrine 2 internals yet, and my actual project is really eating my time. But as soon as I finish this app I'll try to help wherever I can, being the performance issue of the closure strategy you mentioned sometime ago the first topic I'll try to help with.

Again, thanks a lot!

@stof
Copy link
Contributor

stof commented May 26, 2011

As the user still need to define the Closure entity, I guess it could be an acceptable solution to require it to map the associations manually for now as long as it is well documented.

@l3pp4rd
Copy link
Contributor

l3pp4rd commented May 26, 2011

Yes, we will need to add some examples in docs. So far nested set also needs specifying cascade on delete for parent, and I see that users pretty much just copies the example and it works well. Also it will allow advanced users to add additional cascades if they need them, or specify whether to fetch lazily or eagerly. So in my opinion this is indeed less magic and more customizability

@comfortablynumb
Copy link
Contributor Author

I agree with you guys. I think making the mapping configuration explicit for the closure entity is the way to go. It's much more clear that way, it avoids confusion and it lets the user, as said before, customize the closure entity.

So, +1 for me.

@l3pp4rd
Copy link
Contributor

l3pp4rd commented May 30, 2011

the problem with manual mapping is that, listener must know which fields are ancestor and descendant in closure entity. I surely do not want to add additional annotations just for that, any ideas?

@l3pp4rd
Copy link
Contributor

l3pp4rd commented May 30, 2011

@comfortablynumb check if it works on master branch. I have not added a manual mapping since it introduces more complications :)

@comfortablynumb
Copy link
Contributor Author

Sorry for the delay. I'll give it a try as soon as I can update my app's vendors (including these extensions) and my app accordingly. I'm stuck with some other stuff right now, but I'm planning to do it in the next few days.

Thanks a lot for your help!

@l3pp4rd
Copy link
Contributor

l3pp4rd commented Jun 1, 2011

np, try it when you have time

@comfortablynumb
Copy link
Contributor Author

Hi,

Just wanted to say that today I've finally updated my app's vendors, including your extensions. Sorry for the delay to answer. It took me some days to finally have time to update everything. The issue is gone :) thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A confirmed bug in Extensions that needs fixing.
Projects
None yet
Development

No branches or pull requests

3 participants