Adrian Crum wrote: > Adam Heath wrote: >> Moving the caching higher implies that the constructed delegator won't >> be saved until it is completely done being constructed. However, >> during construction, the delegator creates several helper entities. >> These include EntityCrypto, and the cache support classes. These >> classes also still have the same problem of not storing delegator >> instances, instead just storing a name. So, they try to load a >> delegator with the correct name, and don't find it, because the >> original delegator is not done being constructed. >> >> But, we still aren't done with this particular problem. >> >> When GenericPK and GenericValue get created, they try to set their >> fields. Setting any field on a GenericEntity requires that it knows >> it's delegator. However, creation doesn't actually set the delegator. >> Even the delegator name is null. So any GenericEntity created during >> delegator instantiation ends up trying to load a delegator named >> "default", *not* with whatever delegator is actually being created. > > I ran into that circular logic problem in the ExecutionContext branch. I > agree - there are a lot of problems with the delegator implementation > and they should be fixed. Let me know if there is anything I can do to > help.
Oh, heh, funny you should say that. Your DelegatorFactory change gave me many problems. It's not backwards compatible. Webslinger is a purely standalone framework. It's made to run in anything, with anything. It just happens to have a way to function alongside ofbiz. The issue here, is that I have to produce updates to the webslinger codebase, and make them work against multiple ofbiz versions. I support 595296 thru 902021. Making it work against the latest version, with the delegator interface/class changes, was rather complex. I had to copy the old GenericDelegator class, erasing all documentation, all code, make every method throw an UnsupportedOperationException. I did this so I would have an api-compatible version, that I could compile against. I then had to make my build system compile 2 versions of webslinger, one against the latest ofbiz, and one against an old ofbiz with this special dummy GenericDelegator class earlier in the classpath. When making changes to ofbiz, it's not just the internal project code that has to be made consistent. Ofbiz doesn't exist by itself. It has to deal with external addons, that aren't even known. As such, much care needs to be taken any time you change the abi(not api). This was not done with the DelegatorFactory change. I even mentioned this briefly during it's inception. But I never spoke up in detail about it at the time. Also, any time a class changes(I'm saying this again, but it bears repeating), you need to consider the use cases of it. This paragraph is talking about the moving of some classes from util to lang, with no attention paid to backwards compatiblity, nor deprecation. If an end developer has to maintain multiple versions of their product against multiple versions of ofbiz, then changing abi like this causes *more* work for end developers. Any time you change something, and someone finds a problem with it, don't take the defensive, and say "How dare you find faults with my work; I'm not going to fix this, as it's just more work for me to do." In actuallity, you have caused more work for the person who found the problem. You have broken their own software; you have made them dislike the project as a whole. Absolutely be defensive in your programming. Whenever you do something, say to yourself: "How could this be used? Is it perfect? Will it break things for other people, thereby causing them more work?" And, when someone *does* find something wrong, don't respond saying that the finder should fix the issue. *You* are the one who did the original change, and are the absolutely bestest person to understand what was being attempted. You are the perfect person to fix the actual problem. As for real things to deal with: This applies to anyone reading this. Read that Java Concurrency in Practice book *again(, cover to cover. All constructors must *only* set variables internal to their *direct* class. Do *not* construct other classes, passing 'this' to them, because they may register themselves in global static containers. This means that 'this' is available for use by other parts of the system, before 'this' is actually finished being constructed.
