Re: code coverage report for ofbiz run-tests call
Scott Gray wrote: > On 11/12/2009, at 6:41 AM, Adam Heath wrote: > >> Scott Gray wrote: >>> Hi Adam, >>> >>> Looking at the results my first impression is that the coverage is >>> under-reported. For example, the accounting component has quite a few >>> tests but no coverage is shown at all (except for the test package >>> itself). Possibly because there is lot of logic in simple methods but >>> I'm 100% sure java code is also run during the tests. >>> >>> But still a great start and something that will be immensely useful if >>> we can up the accuracy a bit. >> >> Well, it doesn't, really. If you click thru to accounting.test, >> you'll see that there aren't really that many tests. And, upon >> further investigation, the lines after the runSync calls aren't run, >> due to some exception most likely. I'm not certian if this is do to >> my changes, or if the tests themselves are broken. I'm running a >> plain test run now to check that. Plus, there actually *is* line hits >> in accounting.invoice. > > The tests seem to be running fine on buildbot > (http://ci.apache.org/waterfall?show=ofbiz-trunk), I'm guessing it's the > test run problem that's causing the under reporting. There may not be > that many explicit accounting tests (even though it is a lot compared to > other components) but a lot of tests also touch accounting indirectly. > There is just no way that only 53 lines of java code are being executed > in accounting during the full test run. I know for a fact that code is > executed from PaymentGatewayServices, FinAccountPaymentServices, > PaymentWorker, UtilAccounting and a few others during the tests. I had some other changes in that tree that were causing tests to fail. I've rerun it now, all current tests pass, and I've uploaded a new report to http://www.brainfood.com/ofbiz-coverage Note that framework/base has almost 100% coverage. But that's a bad thing, because it's not explicitly testing it; all that code just happens to be utilized during the rest of the test run. Total coverage increased from 7% to 14%. > > Regards > Scott
Re: code coverage report for ofbiz run-tests call
Adrian Crum wrote: > Adam Heath wrote: >> Scott Gray wrote: >>> On 11/12/2009, at 6:41 AM, Adam Heath wrote: >>> >>>> Scott Gray wrote: >>>>> Hi Adam, >>>>> >>>>> Looking at the results my first impression is that the coverage is >>>>> under-reported. For example, the accounting component has quite a few >>>>> tests but no coverage is shown at all (except for the test package >>>>> itself). Possibly because there is lot of logic in simple methods but >>>>> I'm 100% sure java code is also run during the tests. >>>>> >>>>> But still a great start and something that will be immensely useful if >>>>> we can up the accuracy a bit. >>>> Well, it doesn't, really. If you click thru to accounting.test, >>>> you'll see that there aren't really that many tests. And, upon >>>> further investigation, the lines after the runSync calls aren't run, >>>> due to some exception most likely. I'm not certian if this is do to >>>> my changes, or if the tests themselves are broken. I'm running a >>>> plain test run now to check that. Plus, there actually *is* line hits >>>> in accounting.invoice. >>> The tests seem to be running fine on buildbot >>> (http://ci.apache.org/waterfall?show=ofbiz-trunk), I'm guessing it's the >>> test run problem that's causing the under reporting. There may not be >>> that many explicit accounting tests (even though it is a lot compared to >>> other components) but a lot of tests also touch accounting >>> indirectly. There is just no way that only 53 lines of java code are >>> being executed >>> in accounting during the full test run. I know for a fact that code is >>> executed from PaymentGatewayServices, FinAccountPaymentServices, >>> PaymentWorker, UtilAccounting and a few others during the tests. >> >> I had some other changes in that tree that were causing tests to fail. >> I've rerun it now, all current tests pass, and I've uploaded a new >> report to http://www.brainfood.com/ofbiz-coverage > > Oops, link doesn't work. http://www.brainfood.com/ofbiz-coverage/
Re: code coverage report for ofbiz run-tests call
Adrian Crum wrote: >> http://www.brainfood.com/ofbiz-coverage/ > > I like the holiday-themed colors. It would be cool if incomplete > coverage could be displayed as partially eaten sugar cookies. That's standard cobertura scheme.
Re: code coverage report for ofbiz run-tests call
Scott Gray wrote: >> Note that framework/base has almost 100% coverage. But that's a bad >> thing, because it's not explicitly testing it; all that code just >> happens to be utilized during the rest of the test run. > > Of course explicitly testing framework/base would be much better but why > is the current 100% coverage a bad thing? I mean implied testing is > better than no testing right? If I was to go in and incorrectly modify > some of those base methods then there's a good chance some of the higher > level tests would fail. In theory such explicit base testing could be run from the command line, very quickly, using the newly added 'tests' target defined in common.xml. Additionally, just because a line has been noted in cobertura, doesn't mean all variations have been tested. Consider the case that some condition is doing some kind of pattern match, or looking at Collection.contains or Map.containsKey. It's much simpler to verify that everything is tested when it is done explicitly.
Re: code coverage report for ofbiz run-tests call
Scott Gray wrote: >> Additionally, just because a line has been noted in cobertura, doesn't >> mean all variations have been tested. Consider the case that some >> condition is doing some kind of pattern match, or looking at >> Collection.contains or Map.containsKey. It's much simpler to verify >> that everything is tested when it is done explicitly. > > Okay I see what you mean now, it's a bad thing that coverage is reported > without explicit thorough testing. Even though the indirect coverage is > still better than no coverage whatsoever. As a better example, let's say that there is only 10% coverage on the entire ofbiz code base. But base has 100% coverage. That other 90% of untested code may test parts of base that may not work, and would break the higher-level code. It's easier to write tests that are close to the code being tested. Trying to tweak a high-level test to make certain all low-level code is wrong is very very difficult. Plus, if a typo gets introduced in one of those map keys, it might not be detected until much much later in time, when explicit tests are not used. In my opinion, as each new component is activated in the ofbiz system, it should have it's own set of tests that move it close to 100% coverage. So, I can test just base, and get 100%, then base+sql, and get 100% on base+sql, then base+sql+entity, and get 100% on base+sql+entity, and so on. You want to make certain that earlier components are correct before utilizing later ones, or the entire test run may fail spectactulary.
Re: code coverage report for ofbiz run-tests call
Jacques Le Roux wrote: > What about groovy scripts, are they handled by Cobertura? > And actions in Screens, is it worth do something? I guess checking > "static structural" files like web.xml, ofbiz-component.xml files (and > all xml like them, controllers, menus, commonsScreens and such) is not > necessary? This requires groovy including the line number metadata info in the compiled bytecode, then a groovy parser that cobertura can use, so no. Now, if there was a fancy program that supported plugins for other languages, then that would be cool, but I don't know of one.
Re: code coverage report for ofbiz run-tests call
Erwan de FERRIERES wrote: > Hi Jacques, > inline > > Le 11/12/2009 08:47, Jacques Le Roux a écrit : >> What about groovy scripts, are they handled by Cobertura? > From what I know Cobertura needs .class files, and for the groovy files, > it would mean to compile them first > http://docs.codehaus.org/display/GROOVY/Code+Coverage+with+Cobertura No, it needs a byte array that contains class data. I haven't looked inside groovy internals in a bit, but I'm fairly certain that at some point it creates said byte array, seeing as how the jvm requires it to do so.
Re: dropping crumbs styling from brainfood/erik schuessler
Jacques Le Roux wrote: > I wonder if the "sleeping giant" is not using Git and will flood us > sooner or later. > That's one of the dark aspect of git usage. You are able to commit a lot > in a single shoot, hard to review. > Nothing is perfect is this world He's a windows guy. For all the website content and design work he does, we do git for him on the server. For code repositories, I tend to do small, incremental commits. But for customer content type sites, I do an everything commit, just get it done, who cares style of work flow.
Re: dropping crumbs styling from brainfood/erik schuessler
Bruno Busco wrote: > Hi, > Any step from the "sleeping giant" ? ;-) Erik is not subscribed to this list. I'm forwarding this email to him, please keep him and this list cc'd
Re: dropping crumbs styling from brainfood/erik schuessler
Jacques Le Roux wrote: > First, I must say that maybe my view is biased because I have no time > to look at git and I'm a bit jealous :/ > My point was that it allows you to work a long time alone on a (possibly > large) task. > And even if you break it after in several svn commits it's still a lot > of commit to review in a single shoot. I agree it's easier than a large > svn commit though. I understand that having a big work to do it's > certainly better to use git. The only point which concern me is that > needs, at least, some collaboration/exchanges before diving in lonesome > work. > Maybe practice will show how to do it better. But as I said I'm still > far from being ready to switch from svn to git. > Hope I have passed my feeling Git also allows you to publish your local cloned repository, *before* you commit it back upstream into svn, so people can comment on it earlier. You can then rewrite your local history, taking into consideration all suggestions, and when it's finally right, the final commit to svn is clean and has no cruft. Git has plugins to commit an entire new feature as a series of patches in jira. I haven't yet used this plugin tho.
java dependency hell: was Re: code coverage report for ofbiz run-tests call
The coverage stuff I have done for ofbiz is based on the method I did it for webslinger. I wrote a shim to allow for other coverage tools to be used, then wrote a classpath/jar walker to find all classes matching a particular pattern, a dynamic temporary classloader to load the instrumenter, then a concrete implementation that uses Cobertura. I can check everything in, including a build.xml tweak, but can't add the cobertura library itself. So, with that out of the way, here's my current headache. cobertura-1.9.1 does not handle annotation definitions(@interface) in java files. 1.9.3 does. However, that requires upgrading from asm-2.2.3 to asm-3.2. But then groovy fails, 1.6.0-1.6.7 still use asm-2.2.3. Groovy has to be upgraded to 1.7-rc-2. But then webslinger fails, 'cuz it has direct links to asm, cobertura, and groovy. I've done the webslinger side, and my test cases pass, still have to upgrade the ofbiz stuff. ps: I love java :|
Re: svn commit: r889851 [1/17] - in /ofbiz/branches/addbirt: ./ applications/accounting/config/ applications/accounting/data/ applications/accounting/data/helpdata/ applications/accounting/entitydef/
doo...@apache.org wrote: > Author: doogie > Date: Fri Dec 11 23:11:39 2009 > New Revision: 889851 > > URL: http://svn.apache.org/viewvc?rev=889851&view=rev > Log: > Merged with trunk 889845. As evident by this commit, I've started looking at birt now. The first concrete real issue I notice, is that there are references to brainfood-type labels and reports. Those need to either be renamed to be more generic, or removed from this branch. ps: I haven't actually run it yet, just looking at diffs.
Re: proposal: ESME
Hans Bakker wrote: > On Sat, 2009-12-12 at 13:59 +0100, Jeroen van der Wal wrote: >> Hi Hans, >> >> I understand what it can do. Can you tell me why this feature should >> be integrated into Ofbiz? >> > > Then we are getting into the discussion if ESME is useful or not. That > is why i copied the first page in a previous message. Please check > earlier messages where i understood that in general the community agreed > it was useful. > > This proposal is not that IF we should use ESME but only how It is first if we can, then it is how. Things that go into ofbiz trunk must be useful for everyone, and you need to explain why this would be useful, and in which situations.
Re: svn commit: r890175 [1/2] - in /ofbiz/trunk: ./ applications/content/ applications/content/data/ framework/ framework/base/config/ framework/birt/ framework/birt/config/ framework/birt/data/ frame
hans...@apache.org wrote: > Author: hansbak > Date: Mon Dec 14 04:41:28 2009 > New Revision: 890175 > > URL: http://svn.apache.org/viewvc?rev=890175&view=rev > Log: > Merge birt branch 831204:886087 and 831209:885099. A contribution by > Antwebsystems employee Chattree It's not apparent from the diff sent to this list, but this commit has license issues; namely, thet jsps are back. Additionally, .classpath hasn't been updated.
Re: svn commit: r889836 - in /ofbiz/trunk: ./ framework/base/lib/ framework/base/lib/scripting/ framework/webslinger/lib/
Scott Gray wrote: > Hi Adam, > > I haven't confirmed it myself yet but I'm getting reports from our > developers that the groovy script cache isn't clearing anymore, you have > to restart OFBiz before any changes will take effect. I'll confirm it > shortly and update with any new info. I'm guessing that groovy has its > own built-in caching mechanism now. Hmm, then we need a test case. This could be done with the short test target in common.xml.
Re: Request for Comments: Board Report Dec 2009
David E Jones wrote: > I have a draft for the Dec 2009 OFBiz Board Report ready here: > > http://cwiki.apache.org/confluence/display/OFBADMIN/ASF+Board+Report+2009-12 > > If there is anything anyone would like to add, or correct, or whatever, > please let me know! I'll be submitting it later today. Hmm, I wasn't aware that I was now a PMC member. Was an email sent to me, and I just missed it?
Re: Request for Comments: Board Report Dec 2009
David E Jones wrote: > On Dec 14, 2009, at 11:46 AM, Adam Heath wrote: > >> David E Jones wrote: >>> I have a draft for the Dec 2009 OFBiz Board Report ready here: >>> >>> http://cwiki.apache.org/confluence/display/OFBADMIN/ASF+Board+Report+2009-12 >>> >>> If there is anything anyone would like to add, or correct, or whatever, >>> please let me know! I'll be submitting it later today. >> Hmm, I wasn't aware that I was now a PMC member. Was an email sent to >> me, and I just missed it? > > The vote was finalized last week, but there is a waiting period for board > response. That ended yesterday and earlier this morning I sent an email > (looks like about 2 hours ago). > > If you didn't get it, please let me know! Found it, thanks!
Re: svn commit: r889836 - in /ofbiz/trunk: ./ framework/base/lib/ framework/base/lib/scripting/ framework/webslinger/lib/
Scott Gray wrote: > Any progress on this? If not I think we should consider reverting, not > being able to work on groovy scripts at runtime is a bit of pain for > development. Does that mean you've confirmed it then?
Re: svn commit: r889836 - in /ofbiz/trunk: ./ framework/base/lib/ framework/base/lib/scripting/ framework/webslinger/lib/
Adam Heath wrote: > Scott Gray wrote: >> Any progress on this? If not I think we should consider reverting, not >> being able to work on groovy scripts at runtime is a bit of pain for >> development. > > Does that mean you've confirmed it then? In other words, what steps do you take to make this happen, exactly?
Re: svn commit: r889836 - in /ofbiz/trunk: ./ framework/base/lib/ framework/base/lib/scripting/ framework/webslinger/lib/
Scott Gray wrote: > On 16/12/2009, at 11:26 AM, Adam Heath wrote: > >> Scott Gray wrote: >>> Any progress on this? If not I think we should consider reverting, not >>> being able to work on groovy scripts at runtime is a bit of pain for >>> development. >> >> Does that mean you've confirmed it then? > > Oh yeah sorry, confirmed. It doesn't appear to be a problem with our > cache but with the GroovyClassLoader. Well, it appears that ofbiz is using groovy wrongly. A new GroovyClassLoader should be created for each parsed class. Then, when UtilCache clears out each entry, it'll get recompiled as appropriate. FlexibleStringExpander reuses the public GroovyUtil.groovyClassLoader; that needs to be fixed. Then, GroovyUtil can be fixed simply, along with seleniumxml.GroovyRunner.
Re: svn commit: r889836 - in /ofbiz/trunk: ./ framework/base/lib/ framework/base/lib/scripting/ framework/webslinger/lib/
Scott Gray wrote: > On 16/12/2009, at 11:28 AM, Adam Heath wrote: > >> Adam Heath wrote: >>> Scott Gray wrote: >>>> Any progress on this? If not I think we should consider reverting, not >>>> being able to work on groovy scripts at runtime is a bit of pain for >>>> development. >>> >>> Does that mean you've confirmed it then? >> >> In other words, what steps do you take to make this happen, exactly? > > Load any page that contains a groovy script, make a change to that > script and then reload the page. The original script is used and any > changes are not reflected even though the OFBiz cache no longer holds an > entry. Those aren't exact steps. By exact, I mean the list of urls you hit, then the script you edit.
Re: svn commit: r891180 - in /ofbiz/trunk: applications/accounting/webapp/accounting/WEB-INF/ applications/order/webapp/ordermgr/WEB-INF/ applications/product/webapp/facility/WEB-INF/ framework/birt/s
hans...@apache.org wrote: > Author: hansbak > Date: Wed Dec 16 10:44:27 2009 > New Revision: 891180 > > URL: http://svn.apache.org/viewvc?rev=891180&view=rev > Log: > Although we do not experience compile problems here, there were some problems > as reported https://issues.apache.org/jira/browse/OFBIZ-3353. which should be > fixed now, contribution by chattree > Modified: ofbiz/trunk/specialpurpose/component-load.xml > URL: > http://svn.apache.org/viewvc/ofbiz/trunk/specialpurpose/component-load.xml?rev=891180&r1=891179&r2=891180&view=diff > == > --- ofbiz/trunk/specialpurpose/component-load.xml (original) > +++ ofbiz/trunk/specialpurpose/component-load.xml Wed Dec 16 10:44:27 2009 > @@ -34,6 +34,7 @@ > > > > + > > > This doesn't belong. Please run svn diff before committing changes.
Re: svn commit: r890175 [1/2] - in /ofbiz/trunk: ./ applications/content/ applications/content/data/ framework/ framework/base/config/ framework/birt/ framework/birt/config/ framework/birt/data/ frame
David E Jones wrote: > Yes, I agree with you there Scott. Hans or Adam should have researched > and resolved legal questions before committing. The person committing the code is responsible for making certain it follows licensing guidelines. Please refer back to the SCA that we all had to sign before becomming a committer. If you aren't willing to follow those guidelines, then you shouldn't be committing code.
Re: svn commit: r890175 [1/2] - in /ofbiz/trunk: ./ applications/content/ applications/content/data/ framework/ framework/base/config/ framework/birt/ framework/birt/config/ framework/birt/data/ frame
Adam Heath wrote: > David E Jones wrote: >> Yes, I agree with you there Scott. Hans or Adam should have researched >> and resolved legal questions before committing. > > The person committing the code is responsible for making certain it > follows licensing guidelines. Please refer back to the SCA that we > all had to sign before becomming a committer. s/SCA/CLA/ > If you aren't willing to follow those guidelines, then you shouldn't > be committing code.
Re: run tests in ofbiz and installinmg cobertura.
Hans Bakker wrote: > I used ./ant run-tests and then i see coberura is missing, i see that > need to be installed but could not find any info on it related to ofbiz Everything should still work without the cobertura jar installed. What error did you get?
Re: run tests in ofbiz and installinmg cobertura.
Adam Heath wrote: > Hans Bakker wrote: >> I used ./ant run-tests and then i see coberura is missing, i see that >> need to be installed but could not find any info on it related to ofbiz > > Everything should still work without the cobertura jar installed. > What error did you get? This error has nothing to do with cobertura. Those exceptions you see early are ignored by the code. I designed the instrumentation system to deal with the error. The problem you are actually seeing has to do with there being no ant-trax.jar in framework/base/lib; this is needed by . Scott, could you get an ant-trax.jar for version 1.7.0? I only have 1.7.1 installed locally. Hans, next time, actually report the error(s) you get, instead of just saying "it doesn't work." If someone reported an error in something you had done, wouldn't you want to have as much information as possible?
Re: run tests in ofbiz and installinmg cobertura.
Scott Gray wrote: > On 21/12/2009, at 4:59 PM, Adam Heath wrote: > >> Adam Heath wrote: >>> Hans Bakker wrote: >>>> I used ./ant run-tests and then i see coberura is missing, i see that >>>> need to be installed but could not find any info on it related to ofbiz >>> >>> Everything should still work without the cobertura jar installed. >>> What error did you get? >> >> This error has nothing to do with cobertura. Those exceptions you see >> early are ignored by the code. I designed the instrumentation system >> to deal with the error. >> >> The problem you are actually seeing has to do with there being no >> ant-trax.jar in framework/base/lib; this is needed by . >> >> Scott, could you get an ant-trax.jar for version 1.7.0? I only have >> 1.7.1 installed locally. > > I'm on 1.7.1 as well, I'm sure we could get hold of a copy but I don't > have time to do so right now. I'm upgrading the internal ant to 1.7.1, and will fix this trax problem as well.
Re: svn commit: r892712 - in /ofbiz/trunk: ./ framework/common/servicedef/ framework/common/src/org/ofbiz/common/ framework/service/lib/ framework/service/src/org/ofbiz/service/ framework/service/src/
hans...@apache.org wrote: > Author: hansbak > Date: Mon Dec 21 07:31:58 2009 > New Revision: 892712 > > URL: http://svn.apache.org/viewvc?rev=892712&view=rev > Log: > Upgrade Axis1 to Axis2. Ofbiz now supports complex parameters in webservices > including WSDL generation. see OFBIZ-3363 for more info. A contribution of > Antwebsystems employee Chatree > > Added: > ofbiz/trunk/framework/service/lib/XmlSchema-1.4.3.jar (with props) > ofbiz/trunk/framework/service/lib/axiom-api-1.2.8.jar (with props) > ofbiz/trunk/framework/service/lib/axiom-impl-1.2.8.jar (with props) > ofbiz/trunk/framework/service/lib/axis2-kernel-1.5.1.jar (with props) > ofbiz/trunk/framework/service/lib/axis2-transport-http-1.5.1.jar (with > props) > ofbiz/trunk/framework/service/lib/axis2-transport-local-1.5.1.jar (with > props) > ofbiz/trunk/framework/service/lib/commons-httpclient-3.1.jar (with > props) > ofbiz/trunk/framework/service/lib/neethi-2.0.4.jar (with props) > > ofbiz/trunk/framework/service/src/org/ofbiz/service/test/ServiceSOAPTests.java >(with props) > Modified: > ofbiz/trunk/LICENSE > ofbiz/trunk/framework/common/servicedef/services_test.xml > ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonServices.java > ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java > ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelService.java > > ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/SOAPClientEngine.java > ofbiz/trunk/framework/service/testdef/servicetests.xml > > ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/SOAPEventHandler.java What about .classpath?
Re: svn commit: r892712 - in /ofbiz/trunk: ./ framework/common/servicedef/ framework/common/src/org/ofbiz/common/ framework/service/lib/ framework/service/src/org/ofbiz/service/ framework/service/src/
David E Jones wrote: > On Dec 21, 2009, at 1:35 AM, Adam Heath wrote: > >> hans...@apache.org wrote: >>> Author: hansbak >>> Date: Mon Dec 21 07:31:58 2009 >>> New Revision: 892712 >>> >>> URL: http://svn.apache.org/viewvc?rev=892712&view=rev >>> Log: >>> Upgrade Axis1 to Axis2. Ofbiz now supports complex parameters in >>> webservices including WSDL generation. see OFBIZ-3363 for more info. A >>> contribution of Antwebsystems employee Chatree >>> >>> Added: >>>ofbiz/trunk/framework/service/lib/XmlSchema-1.4.3.jar (with props) >>>ofbiz/trunk/framework/service/lib/axiom-api-1.2.8.jar (with props) >>>ofbiz/trunk/framework/service/lib/axiom-impl-1.2.8.jar (with props) >>>ofbiz/trunk/framework/service/lib/axis2-kernel-1.5.1.jar (with props) >>>ofbiz/trunk/framework/service/lib/axis2-transport-http-1.5.1.jar (with >>> props) >>>ofbiz/trunk/framework/service/lib/axis2-transport-local-1.5.1.jar >>> (with props) >>>ofbiz/trunk/framework/service/lib/commons-httpclient-3.1.jar (with >>> props) >>>ofbiz/trunk/framework/service/lib/neethi-2.0.4.jar (with props) >>> >>> ofbiz/trunk/framework/service/src/org/ofbiz/service/test/ServiceSOAPTests.java >>>(with props) >>> Modified: >>>ofbiz/trunk/LICENSE >>>ofbiz/trunk/framework/common/servicedef/services_test.xml >>>ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonServices.java >>>ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java >>>ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelService.java >>> >>> ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/SOAPClientEngine.java >>>ofbiz/trunk/framework/service/testdef/servicetests.xml >>> >>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/SOAPEventHandler.java >> What about .classpath? > > What about it? We can't assume everyone uses Eclipse... What does using eclipse have to do with updating that file? I don't use eclipse, but I still update .classpath. I also don't use ant.bat, yet I just updated that. If you know that you have to keep .classpath updated to make eclipse work, and you knowingly check in new libraries, or remove old ones, then why make eclipse broken? It's really not that hard to change. Honestly, I don't understand why you would think not keeping it uptodate is a good thing. ... perplexed ...
Re: svn commit: r892712 - in /ofbiz/trunk: ./ framework/common/servicedef/ framework/common/src/org/ofbiz/common/ framework/service/lib/ framework/service/src/org/ofbiz/service/ framework/service/src/
David E Jones wrote: > On Dec 21, 2009, at 1:49 AM, Adam Heath wrote: > >> David E Jones wrote: >>> On Dec 21, 2009, at 1:35 AM, Adam Heath wrote: >>> >>>> hans...@apache.org wrote: >>>>> Author: hansbak >>>>> Date: Mon Dec 21 07:31:58 2009 >>>>> New Revision: 892712 >>>>> >>>>> URL: http://svn.apache.org/viewvc?rev=892712&view=rev >>>>> Log: >>>>> Upgrade Axis1 to Axis2. Ofbiz now supports complex parameters in >>>>> webservices including WSDL generation. see OFBIZ-3363 for more info. A >>>>> contribution of Antwebsystems employee Chatree >>>>> >>>>> Added: >>>>> ofbiz/trunk/framework/service/lib/XmlSchema-1.4.3.jar (with props) >>>>> ofbiz/trunk/framework/service/lib/axiom-api-1.2.8.jar (with props) >>>>> ofbiz/trunk/framework/service/lib/axiom-impl-1.2.8.jar (with props) >>>>> ofbiz/trunk/framework/service/lib/axis2-kernel-1.5.1.jar (with props) >>>>> ofbiz/trunk/framework/service/lib/axis2-transport-http-1.5.1.jar >>>>> (with props) >>>>> ofbiz/trunk/framework/service/lib/axis2-transport-local-1.5.1.jar >>>>> (with props) >>>>> ofbiz/trunk/framework/service/lib/commons-httpclient-3.1.jar (with >>>>> props) >>>>> ofbiz/trunk/framework/service/lib/neethi-2.0.4.jar (with props) >>>>> >>>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/test/ServiceSOAPTests.java >>>>>(with props) >>>>> Modified: >>>>> ofbiz/trunk/LICENSE >>>>> ofbiz/trunk/framework/common/servicedef/services_test.xml >>>>> ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonServices.java >>>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java >>>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelService.java >>>>> >>>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/SOAPClientEngine.java >>>>> ofbiz/trunk/framework/service/testdef/servicetests.xml >>>>> >>>>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/SOAPEventHandler.java >>>> What about .classpath? >>> What about it? We can't assume everyone uses Eclipse... >> What does using eclipse have to do with updating that file? I don't >> use eclipse, but I still update .classpath. I also don't use ant.bat, >> yet I just updated that. >> >> If you know that you have to keep .classpath updated to make eclipse >> work, and you knowingly check in new libraries, or remove old ones, >> then why make eclipse broken? It's really not that hard to change. >> >> Honestly, I don't understand why you would think not keeping it >> uptodate is a good thing. >> >> ... perplexed ... > > Where did I write "not keeping it uptodate is a good thing?" And if I didn't > write it, how can you assert I was thinking it? How else could what you wrote be interpreted? Because you responded, it seemed to imply that you disagreed that it wasn't important to keep it uptodate. If you actually meant something else, then you should have said so. My 3 words meant that it should have been updated. What did you actually mean? I still can't see how your response could be interpreted in any other way. And I'm rather perplexed at having to explain my 3 simple words in such detail, esp. to you, David.
Re: svn commit: r892712 - in /ofbiz/trunk: ./ framework/common/servicedef/ framework/common/src/org/ofbiz/common/ framework/service/lib/ framework/service/src/org/ofbiz/service/ framework/service/src/
David E Jones wrote: > On Dec 21, 2009, at 2:04 AM, Adam Heath wrote: > >> David E Jones wrote: >>> On Dec 21, 2009, at 1:49 AM, Adam Heath wrote: >>> >>>> David E Jones wrote: >>>>> On Dec 21, 2009, at 1:35 AM, Adam Heath wrote: >>>>> >>>>>> hans...@apache.org wrote: >>>>>>> Author: hansbak >>>>>>> Date: Mon Dec 21 07:31:58 2009 >>>>>>> New Revision: 892712 >>>>>>> >>>>>>> URL: http://svn.apache.org/viewvc?rev=892712&view=rev >>>>>>> Log: >>>>>>> Upgrade Axis1 to Axis2. Ofbiz now supports complex parameters in >>>>>>> webservices including WSDL generation. see OFBIZ-3363 for more info. A >>>>>>> contribution of Antwebsystems employee Chatree >>>>>>> >>>>>>> Added: >>>>>>> ofbiz/trunk/framework/service/lib/XmlSchema-1.4.3.jar (with props) >>>>>>> ofbiz/trunk/framework/service/lib/axiom-api-1.2.8.jar (with props) >>>>>>> ofbiz/trunk/framework/service/lib/axiom-impl-1.2.8.jar (with props) >>>>>>> ofbiz/trunk/framework/service/lib/axis2-kernel-1.5.1.jar (with props) >>>>>>> ofbiz/trunk/framework/service/lib/axis2-transport-http-1.5.1.jar >>>>>>> (with props) >>>>>>> ofbiz/trunk/framework/service/lib/axis2-transport-local-1.5.1.jar >>>>>>> (with props) >>>>>>> ofbiz/trunk/framework/service/lib/commons-httpclient-3.1.jar (with >>>>>>> props) >>>>>>> ofbiz/trunk/framework/service/lib/neethi-2.0.4.jar (with props) >>>>>>> >>>>>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/test/ServiceSOAPTests.java >>>>>>>(with props) >>>>>>> Modified: >>>>>>> ofbiz/trunk/LICENSE >>>>>>> ofbiz/trunk/framework/common/servicedef/services_test.xml >>>>>>> ofbiz/trunk/framework/common/src/org/ofbiz/common/CommonServices.java >>>>>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelParam.java >>>>>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ModelService.java >>>>>>> >>>>>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/engine/SOAPClientEngine.java >>>>>>> ofbiz/trunk/framework/service/testdef/servicetests.xml >>>>>>> >>>>>>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/SOAPEventHandler.java >>>>>> What about .classpath? >>>>> What about it? We can't assume everyone uses Eclipse... >>>> What does using eclipse have to do with updating that file? I don't >>>> use eclipse, but I still update .classpath. I also don't use ant.bat, >>>> yet I just updated that. >>>> >>>> If you know that you have to keep .classpath updated to make eclipse >>>> work, and you knowingly check in new libraries, or remove old ones, >>>> then why make eclipse broken? It's really not that hard to change. >>>> >>>> Honestly, I don't understand why you would think not keeping it >>>> uptodate is a good thing. >>>> >>>> ... perplexed ... >>> Where did I write "not keeping it uptodate is a good thing?" And if I >>> didn't write it, how can you assert I was thinking it? >> How else could what you wrote be interpreted? Because you responded, >> it seemed to imply that you disagreed that it wasn't important to keep >> it uptodate. >> >> If you actually meant something else, then you should have said so. >> >> My 3 words meant that it should have been updated. What did you >> actually mean? >> >> I still can't see how your response could be interpreted in any other way. >> >> And I'm rather perplexed at having to explain my 3 simple words in >> such detail, esp. to you, David. > > What does it have to do with your 3 words? > > What did I mean? I meant: > > What about .classpath? If Hans doesn't use Eclipse, how can you expect him to > keep that Eclipse-specific file updated, especially since it has been > historically maintained by Eclipse users. Some things, like the LICENSE and > NOTICE files that Jacques pointed out, really are responsibilities of committers. > I wouldn't
Re: svn commit: r893313 - in /ofbiz/trunk/framework/base/src/org/ofbiz/base: conversion/DateTimeConverters.java test/BaseUnitTests.java
adri...@apache.org wrote:
> Author: adrianc
> Date: Tue Dec 22 20:54:55 2009
> New Revision: 893313
> +String target = dateToString.convert(utilDate);
> +assertTrue(buildConversionMessage("DateToString",
> utilDate.toString(), target), utilDate.toString().equals(target));
No, do not use assertTrue. Use assertEquals, so that you get "got
foo, expected bar" message from junit.
Re: getNextSeqId and multiple sequence on entity
Brett Palmer wrote: > I noticed this same behavior as Nicolas recently. It was convenient to use > the delegator.getNextSeqId() for a unique ID when you needed one. Is it > now a requirements that all Sequence Ids have an entity attached to it? Yeah, I too have used plain, unattached sequences in my own code. Please remove this restriction.
Re: Delegator rollback per test case?
Scott Gray wrote: > I'd really rather not have this idea die with a single response, I've > done my best to describe the perceived benefits and it would be good to > get some additional feedback from anyone who is interested. I don't care either way, however, I do have some suggestions on implementation. Create an ofbiz test runner, if one doesn't exist. Then, decorate the test suite with an annotation, that says that each test should be rolled back. junit 4 may even have some support for doing this automatically.
Re: Ant and Java 1.6 in trunk
Jacques Le Roux wrote: > Hi, > > In trunk main build.xml file, should we not change > destdir="tools/build/classes" compiler="javac1.5" target="1.5" > source="1.5"/> > to > destdir="tools/build/classes" compiler="javac1.6" target="1.6" > source="1.6"/> > and also all in all build.xml files? Sure, remove all the old-style javac presetdefs. I'm the one who did this originally.
Re: Ant and Java 1.6 in trunk
Adam Heath wrote: > Jacques Le Roux wrote: >> Hi, >> >> In trunk main build.xml file, should we not change >> > destdir="tools/build/classes" compiler="javac1.5" target="1.5" >> source="1.5"/> >> to >> > destdir="tools/build/classes" compiler="javac1.6" target="1.6" >> source="1.6"/> >> and also all in all build.xml files? > > Sure, remove all the old-style javac presetdefs. I'm the one who did > this originally. btw, I'm working on this, and a few other minor build tweaks.
Re: svn commit: r893961 - in /ofbiz/trunk/applications/party: servicedef/ src/org/ofbiz/party/party/
Adrian Crum wrote: > David E Jones wrote: >> On Dec 28, 2009, at 4:32 PM, Jacques Le Roux wrote: >> >>> From: "Ean Schuessler" Jacques Le Roux wrote: > Thanks, I saw that but was not sure how to use it. I remember now > why I > created Java services. I needed to create a PartyGroup. So initially I > looked for a service and found createPartyGroup wich is implemented > by a > method in PartyServices.java. Then I continued in Java :/. > Now I wonder what we should do regarding your sentence < thought a > few times that maybe it's not the best idea to do so, and instead > whenever a party is implied to be in a role if they are not already > then > they should be added automatically.>> in > http://markmail.org/message/j5yprwdv3fgz3rb6 I always took this to be a security thing, that a role must be authorized before it can be used in relationships and other structures. >>> If the aplication needs to create a PartyRole, I can't see any >>> reasons to not let it do it automatically >> >> The original idea was to use this as an extra protection, ie so users >> don't describe a party's relationship with something else using a >> certain role if the party isn't really in that role. >> >> The result, however, is that it is cumbersome, a real pain, and a >> common complaint... which is why I'm for changing the default behavior. > > Maybe have it configurable in a properties file, like > requirePartyRole=true or enforcePartyRole=true. Not the best, as existing code in ofbiz which uses this service may break if that property setting is changed. However, maybe adding a boolean flag to the service definition, defaulted to false, would help. Adding the single flag to all the callers would still be a win, rather than keeping the role creation stuff everywhere.
Re: Moving securityext to the framework
Bruno Busco wrote:
> One thing we need in the framework is the possibility to create a
> userLogin with an associated email address and have the possibility to
> have the password emailed if forgotten.
> This is actually done in
> public static String emailPassword(HttpServletRequest request,
> HttpServletResponse response) {
> that is located in LoginEvents.java in securityext.
>
> To get the email address, emailPassword(...) checks if the userLoginId
> exists, then find the related party, then find a related ContactMech
> with PRIMARY_MAIL purpose.
> To get the email body and other details, emailPassword(...) starts
> from a ProductStore and gets the related ProductStoreEmailSetting.
Why not move ContactMech(and children, but not PartyContactMech*) to
framework, then have a join table between UserLogin and ContactMech?
Re: Naming pattern of test definition files
David E Jones wrote: > If a test is written as a simple-method it should be names like other files, > ie as *Services.xml or *SimpleMethods.xml (an older form, not used much). Shouldn't test definitions(classes, simple methods, entity defs, etc) only be active if tests are being run? This would reduce the memory load during normal runs.
buildbot messages
So, every success of buildbot will send a message? I would prefer just when an error is detected, then when the build is fixed after such an error. Seeing one for every commit seems to be worthless.
Re: buildbot messages
Adam Heath wrote: > So, every success of buildbot will send a message? I would prefer > just when an error is detected, then when the build is fixed after > such an error. Seeing one for every commit seems to be worthless. Ah, I see others agree with this too.
Re: Discussion: BuildBot component independence check
Bruno Busco wrote: > How could we define new ant targets to deploy partial OFBiz configurations? > > I mean, an OFBiz configuration I would like to build and get working > is the one with the following applications: > > - application/commonext > - application/content > - application/party > - application/securityext > - specialpurpose/myportal > - specialpurpose/ofbizwebsite > > Does make sense to define a new ant target that removes all other > directories and then build what remains? > This new ant target could be executed in the BuildBot also. For reference, the debian packages have a -framework/-applications/-specialpurpose split. I'd like to be able to split each application and specialpurpose component into it's own package, tho, and use dependencies/etc to bring them all back together. However, the components still aren't 100% separated yet.
Re: Conditional seed data loading
David E Jones wrote: > Bring confusion in the future? That's like saying you shouldn't use > the term object-relational mapping unless you're talking about JPA. > > Just like object-relational mapping is a general term with many > variations and implementations, and supporting the JPA interfaces is > simply a feature of some of those implementations, portal and > portlet are also general terms with many variations and > implementations, and only some of them support the JSR 168 > specification. > > Should we then say that portal/portlet software not written in Java > should not use those terms? > > If so, which terms should those unfortunate souls use to describe > their creations? You should go work for sun(soon to be oracle) marketing. They'd love to have you on board.
Re: Discussion: BuildBot component independence check
Bruno Busco wrote: > Hi Adam, > thank you for the hint but I need some other (basic) information. > How should I use the debian package? > What is intended for? > > I see a .pl script. Can I use it to get a partial (framework-only) > deploy directory from a checked out OFBiz working directory ? > > Sorry for this basic questions. Update the debian/changelog, to at least contain the current svn version. Update the date. Do this by making a new stanza. debian/rules clean debian/rules binary Install debs that get created in .. There may be a few things that don't work, I've got a local git branch that has a few very small improvements in debian/* that need to be added. The packages do *not* support upgrades between different ofbiz versions, because ofbiz doesn't support that itself.
Re: svn commit: r895149 - in /ofbiz/trunk/applications/accounting/script/org/ofbiz/accounting/test: AutoInvoiceTests.xml AutoPaymentTests.xml
er...@apache.org wrote: > Author: erwan > Date: Sat Jan 2 09:01:28 2010 > New Revision: 895149 > > URL: http://svn.apache.org/viewvc?rev=895149&view=rev > Log: > Changing the year as we are now in 2010. This corrects buildbot errors > reported on the dev mailing list. > Some trailing spaces have been also removed. This is a bad commit, do not do space changes alongwith code-type changes.
Re: Discussion: BuildBot component independence check
Bruno Busco wrote: > The script should be able to delete some directories and to apply some > SVN patches (tipically to change component-load.xml and build.xml > files). Actually, that's not the best approach. It would be better to modify the build.xml and java classes to be more extensible, and not hard-code things into framework/base/config/ofbiz-components.xml, that reference hard-coded classes located in other components.(for example). I'd love to be able to drop a component-into hot-deploy, and have a new container automatically started. This would make switching between jetty and catalina simpler, and would allow for an asterisk component(which requires constantly running code).
Re: Moving securityext to the framework
Ean Schuessler wrote: > Adrian Crum wrote: >> I don't agree that emailing forgotten passwords is like the Webtools >> application. As you have discovered, emailing forgotten passwords >> entails some decision making, looking up information in various >> entities, selecting and rendering an email body template, etc. From my >> perspective, all of those things are outside the scope of the framework. > > I agree. It is easy to imagine that some applications would not allow a > password to be reset via email. It might be that the application uses > biometrics, cryptographic signatures or who knows what. The framework > authentication stubs should accommodate a diversity of approaches. > > One major question is whether framework, on its own, should even be > runnable as an application. In my opinion, it is a library, not an app > and doesn't need to be operational on its own. What is your definition of operational? A servlet container that is listing for requests on 8009, 8080? Ready to process rmi requests? Is framework a *pure* library, where the application that runs on top of it is responsible for starting any long-term, background services, or should framework be the application wrapper?
Re: buildbot failure in ASF Buildbot on ofbiz-trunk
build...@apache.org wrote: > The Buildbot has detected a new failure of ofbiz-trunk on ASF Buildbot. > Full details are available at: > http://ci.apache.org/builders/ofbiz-trunk/builds/2245 > > Buildbot URL: http://ci.apache.org/ > > Buildslave for this Build: isis_ubuntu > > Build Reason: > Build Source Stamp: [branch ofbiz/trunk] 895940 > Blamelist: jonesde Scott has said in the past that the buildbot doesn't run on every checkin. This means that multiple commits might be tested. Would it be possible to have failure messages list all of the revisions that are being covered, based on the last successful commit? Additionally, it'd be nice if once a failure is detected, it'd either do an auto binary disection, to find the commit that actually failed in the list of covered revisions, or it could just try them all. For instance, if 891234 is good, and then 892765 fails. 892765 would get recorded as a state change, putting ofbiz into fail mode. It would then try 891562, 891840, 892496, and 892599, reporting that it tried these other revisions, and noticed the error first occurred at 892496, then ofbiz would stay in fail mode, and the buildbot would keep quiet until it detects that ofbiz has been fixed. Or, if that is too complicated, at least list all the covered revisions, and the user that committed each revision.
Re: buildbot failure in ASF Buildbot on ofbiz-trunk
Scott Gray wrote: > On 6/01/2010, at 10:19 AM, Adam Heath wrote: > >> Scott has said in the past that the buildbot doesn't run on every >> checkin. This means that multiple commits might be tested. Would it >> be possible to have failure messages list all of the revisions that >> are being covered, based on the last successful commit? > > This is what happens, for example take a look at the build after the one > above which covered a few commits: > http://ci.apache.org/builders/ofbiz-trunk/builds/2246 Wasn't certain, thought it might, but it's nice to have it confirmed. >> Additionally, it'd be nice if once a failure is detected, it'd either >> do an auto binary disection, to find the commit that actually failed >> in the list of covered revisions, or it could just try them all. >> >> For instance, if 891234 is good, and then 892765 fails. 892765 would >> get recorded as a state change, putting ofbiz into fail mode. It >> would then try 891562, 891840, 892496, and 892599, reporting that it >> tried these other revisions, and noticed the error first occurred at >> 892496, then ofbiz would stay in fail mode, and the buildbot would >> keep quiet until it detects that ofbiz has been fixed. > > I have no idea if buildbot is capable of this but you're more than > welcome to investigate it and request changes from infra. Standard open-source response, kinda expected it. It'd still be nice if the request was forwarded on, if it wasn't too much of a hassle. If it is, I respect that, and I'll just add it to my infinite todo list squared. > In general though I think it's usually fairly obvious what the cause of > a problem is based on a quick look at the stdio from the failed build > task. I don't think it's a huge ask for each committer on the blamelist > to take a minute or two to figure out if it's their problem or not. Computer time is free, human time is not. Computers are continually getting faster, while humans get slower. Have you had a chance to play with git bisect yet? It makes me feel squishy in my happy place.
omg, more git magics
God, I just love git. Just ran into another *wonderful* feature. Here at brainfood, we use debian. As such, we make use of the debian packaging to deploy ofbiz to our servers. To provide for that, I have a branch of ofbiz that I made against head back in september, then I cherry-pick patches from trunk as we see fit. Yesterday, I was going thru all the changes in december, cherry-picking several patches. After I was all done, I then ran a compile. Wouldn't you know it, one of the changes didn't work. This was because it added a UtilValidate call, but not an import of UtilValidate(most likely because some earlier commit that I didn't cherry-pick already had added the import). So, now I needed to fix this problem. The issue, is that the commit was *not* the last one, but 9 commits old. So, ... git rebase -i HEAD~10, launches vim with: == pick 3373962 Remove duplicate webslinger-base-invoker libs ... pick 7aee368 I noticed during test runs that the same eca ... pick d3a6532 Merge birt branch 831204:886087 and 831209:885099. ... pick ac2d9b6 Ignore birt/build pick 897ffb2 Fix groovy class parsing/caching. pick f64071b Although we do not experience compile problems ... pick b7dc3d7 Applied patch from OFBIZ-3358, form ... pick 181a272 too many files copied over from the addbirt ... pick 35d9560 Utility methods for reading strings from streams ... == I then changed the second line to say 'edit' instead of 'pick', then saved the file. Git replayed all those changes to the 7aee368 commit, then told me to edit any files I needed to. I then ran git add to add them to the git index. git commit --amend altered the commit, allowing me to add the missing import line. Finally, git rebase --continue altered all the *other* commits that sat on top of the broken one, optionally updating any patches that may have merge conflicts(it's *very* smart at this). So now I have a working tree, without this compile error, and the proper fix is actually in the right commit, instead of being yet another commit at the head. This just makes me oh so happy.
Re: buildbot failure in ASF Buildbot on ofbiz-trunk
Scott Gray wrote: > On 6/01/2010, at 10:50 AM, Adam Heath wrote: > >> Scott Gray wrote: >>> On 6/01/2010, at 10:19 AM, Adam Heath wrote: >>> >>>> Additionally, it'd be nice if once a failure is detected, it'd either >>>> do an auto binary disection, to find the commit that actually failed >>>> in the list of covered revisions, or it could just try them all. >>>> >>>> For instance, if 891234 is good, and then 892765 fails. 892765 would >>>> get recorded as a state change, putting ofbiz into fail mode. It >>>> would then try 891562, 891840, 892496, and 892599, reporting that it >>>> tried these other revisions, and noticed the error first occurred at >>>> 892496, then ofbiz would stay in fail mode, and the buildbot would >>>> keep quiet until it detects that ofbiz has been fixed. >>> >>> I have no idea if buildbot is capable of this but you're more than >>> welcome to investigate it and request changes from infra. >> >> Standard open-source response, kinda expected it. It'd still be nice >> if the request was forwarded on, if it wasn't too much of a hassle. >> If it is, I respect that, and I'll just add it to my infinite todo >> list squared. > > The idea is to ask infra to do no more than is necessary (they're unpaid > volunteers), so at the very least we (you) should find out if buildbot > is capable of doing what you desire. > As below human time isn't free and I'd prefer it if people deal with > infra directly if the change is important enough to them. > >>> In general though I think it's usually fairly obvious what the cause of >>> a problem is based on a quick look at the stdio from the failed build >>> task. I don't think it's a huge ask for each committer on the blamelist >>> to take a minute or two to figure out if it's their problem or not. >> >> Computer time is free, human time is not. Computers are continually >> getting faster, while humans get slower. > > I don't disagree that knowing exactly what revision caused the failure > would be useful, it's just that I don't think it's useful enough for me > to spend any time worrying about :-) > >> Have you had a chance to play with git bisect yet? It makes me feel >> squishy in my happy place. > > Not yet, I don't usually bother tracking down an offending commit unless > I'm particularly interested in what the hell they were thinking or if I > need to discuss the whys and hows. > > I'm mostly still using git as I was using svn except for heavy use of > stash, branch and reset. I had to learn it quick. We were attempting to manage a website, on multiple internal machines, a staging server, a customer-hosted production server, and a customer-hosted development server. The size of the website was originally 22G, but grew during development to 40G(it had lots and lots of videos). Originally it was all managed in svn, but multi-server development sucks. We then moved to mercurial. However, mercurial is buggy with large systems, because it has a built-in limit due to its use of python-strings internally. In essence, a changeset can't be larger than 2G. If you don't update the site for a couple of days, it could be entirely possible that the whole set of changes would be more than 2G, and it would just stop working. Not to mention when files get renamed, that mercurial would end up storing multiple copies of the same blob of bytes(svn is better in that regard). We finally settled on git. During this time, we really only used it like svn, but committing locally, then made use of it as an advanced rsync deployment system. The switch to git for this large website happened last March. Since then, I've become ecstatic with git. It's not been a completely new workflow; I've used svk ages ago(but not really that much, just kinda like an offline commit), and mercurial 1.5 years ago. >
Re: SVN Access Problems
Adrian Crum wrote: > Is anyone else having problems accessing the repository? For the last > few days my SVN client keeps giving me this error when I attempt to > update my local copy: > > Error: REPORT of '/repos/asf/!svn/vcc/default': Could not read status > line: An > Error: existing connection was forcibly closed by the remote host. > Error: (https://svn.apache.org) > > I wanted to ask here before contacting ASF Infra. hint hint -- offline git-svn -- hint hint
Re: svn commit: r896213 - in /ofbiz/trunk/framework/common: servicedef/services_email.xml src/org/ofbiz/common/email/EmailServices.java
lekt...@apache.org wrote: > Author: lektran > Date: Tue Jan 5 20:48:15 2010 > New Revision: 896213 > > URL: http://svn.apache.org/viewvc?rev=896213&view=rev > Log: > Emails will now be sent to any valid recipients even if the SMTP server > rejected any invalid ones. This can be turned off if desired in > general.properties and also on a per service call basis. > A failure notification will be sent to the email's "from" address, listing > the failed recipients and the reason for each failure. The notification can > be turned off by setting the sendFailureNotification parameter to false in > the s > > These changes fix a problem where emails are not sent to valid recipients and > users receive no feedback when emails fail to be sent, generally because of > the smtp server rejecting one or more recipients. > > OFBIZ-3379, thanks to Pranay Pandey for the report and testing, also thanks > to Tim Ruppert and Ruth Hoffman for their input into the issue. > > Modified: > ofbiz/trunk/framework/common/servicedef/services_email.xml > ofbiz/trunk/framework/common/src/org/ofbiz/common/email/EmailServices.java > > Modified: > ofbiz/trunk/framework/common/src/org/ofbiz/common/email/EmailServices.java > URL: > http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/src/org/ofbiz/common/email/EmailServices.java?rev=896213&r1=896212&r2=896213&view=diff > == > --- > ofbiz/trunk/framework/common/src/org/ofbiz/common/email/EmailServices.java > (original) > +++ > ofbiz/trunk/framework/common/src/org/ofbiz/common/email/EmailServices.java > Tue Jan 5 20:48:15 2010 > @@ -76,6 +78,8 @@ > import org.ofbiz.widget.screen.ScreenRenderer; > import org.xml.sax.SAXException; > > +import com.sun.mail.smtp.SMTPAddressFailedException; > + Ick, bad, please don't do this.
Re: svn commit: r896213 - in /ofbiz/trunk/framework/common: servicedef/services_email.xml src/org/ofbiz/common/email/EmailServices.java
Scott Gray wrote: > On 8/01/2010, at 8:50 AM, Adam Heath wrote: > >> lekt...@apache.org wrote: >>> Author: lektran >>> Date: Tue Jan 5 20:48:15 2010 >>> New Revision: 896213 >>> >>> URL: http://svn.apache.org/viewvc?rev=896213&view=rev >>> Log: >>> Emails will now be sent to any valid recipients even if the SMTP >>> server rejected any invalid ones. This can be turned off if desired >>> in general.properties and also on a per service call basis. >>> A failure notification will be sent to the email's "from" address, >>> listing the failed recipients and the reason for each failure. The >>> notification can be turned off by setting the sendFailureNotification >>> parameter to false in the s >>> >>> These changes fix a problem where emails are not sent to valid >>> recipients and users receive no feedback when emails fail to be sent, >>> generally because of the smtp server rejecting one or more recipients. >>> >>> OFBIZ-3379, thanks to Pranay Pandey for the report and testing, also >>> thanks to Tim Ruppert and Ruth Hoffman for their input into the issue. >>> >>> Modified: >>>ofbiz/trunk/framework/common/servicedef/services_email.xml >>> >>> ofbiz/trunk/framework/common/src/org/ofbiz/common/email/EmailServices.java >>> >>> >>> Modified: >>> ofbiz/trunk/framework/common/src/org/ofbiz/common/email/EmailServices.java >>> >>> URL: >>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/src/org/ofbiz/common/email/EmailServices.java?rev=896213&r1=896212&r2=896213&view=diff >>> >>> == >>> >>> --- >>> ofbiz/trunk/framework/common/src/org/ofbiz/common/email/EmailServices.java >>> (original) >>> +++ >>> ofbiz/trunk/framework/common/src/org/ofbiz/common/email/EmailServices.java >>> Tue Jan 5 20:48:15 2010 >>> @@ -76,6 +78,8 @@ >>> import org.ofbiz.widget.screen.ScreenRenderer; >>> import org.xml.sax.SAXException; >>> >>> +import com.sun.mail.smtp.SMTPAddressFailedException; >>> + >> >> Ick, bad, please don't do this. > > I'm sorry, don't do what? Use internal sun libraries.
Re: Message in (current) default theme
Adrian Crum wrote: > +1 > > Bilgin Ibryam wrote: >> What about having both versions together: the message will disappear >> after sometime (as it was before) but you can still click on it for >> faster closing? >> >> An improvement would be to add a [x] sign at the message corner, so >> users guess that it can be clicked and closed. While we're on the subject, please add a way to see the message after it goes away. Consider the case of a request taking a long time, so the user changes focus to a new window. Then, when they come back, the window gets clicked, and the message goes away, never to be seen again.
Re: buildbot failure in ASF Buildbot on ofbiz-trunk
Adrian Crum wrote: > Most likely due to the slow response time of the SVN server. It would be nice if the buildbot was smart in that case.
Re: buildbot failure in ASF Buildbot on ofbiz-trunk
Adrian Crum wrote: > Bah. Chris dropped the baton. We could have had a good running joke there. You mean the ofbiz spirit stick.
Re: LRUMap.java
Adrian Crum wrote: > What is the purpose of LRUMap.java? It extends LinkedHashMap but it doesn't > add any functionality. The class seems pointless. We could use LinkedHashMap > instead. Backwards compatiblity, where external projects used LRUMap in the past. Check the history, the class used to do a lot more.
Re: svn commit: r897392 [1/4] - in /ofbiz/trunk/applications/accounting: data/ data/helpdata/ documents/
er...@apache.org wrote: > Author: erwan > Date: Sat Jan 9 09:09:27 2010 > New Revision: 897392 This is not a mail to erwan, but did anyone else get mail 4/4? I only got the first three.
Re: svn commit: r898338 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
adri...@apache.org wrote:
> Author: adrianc
> Date: Tue Jan 12 14:24:36 2010
> New Revision: 898338
>
> URL: http://svn.apache.org/viewvc?rev=898338&view=rev
> Log:
> First pass at converting UtilProperties.java to Java 6, plus some small code
> cleanups.
Where exactly is the 1.6 syntax you are using? It's not in making
more class fields static. It's not in removing the public methods(you
may have broken external code, please add the public static methods
back). It's not in removing the DCL pattern.
>
> Modified:
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>
> Modified:
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java?rev=898338&r1=898337&r2=898338&view=diff
> ==
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
> (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
> Tue Jan 12 14:24:36 2010
> @@ -57,26 +57,20 @@
> * (see the href="#xmlToProperties(java.io.InputStream,%20java.util.Locale,%20java.util.Properties)">xmlToProperties
> * method).
> */
> -...@suppresswarnings("serial")
> -public class UtilProperties implements java.io.Serializable {
> +public class UtilProperties {
>
> public static final String module = UtilProperties.class.getName();
> -
> /** An instance of the generic cache for storing the non-locale-specific
> properties.
> * Each Properties instance is keyed by the resource String.
> */
> -protected static UtilCache resourceCache =
> UtilCache.createUtilCache("properties.UtilPropertiesResourceCache");
> -
> +protected static final UtilCache resourceCache =
> UtilCache.createUtilCache("properties.UtilPropertiesResourceCache");
> /** An instance of the generic cache for storing the non-locale-specific
> properties.
> * Each Properties instance is keyed by the file's URL.
> */
> -protected static UtilCache urlCache =
> UtilCache.createUtilCache("properties.UtilPropertiesUrlCache");
> -
> -public static final Locale LOCALE_ROOT = new Locale("", "", "");
> -
> -protected static Locale fallbackLocale = null;
> -protected static Set defaultCandidateLocales = null;
> -protected static Set propertiesNotFound = FastSet.newInstance();
> +protected static final UtilCache urlCache =
> UtilCache.createUtilCache("properties.UtilPropertiesUrlCache");
> +protected static final Set propertiesNotFound =
> FastSet.newInstance();
> +protected static final Locale fallbackLocale = createFallbackLocale();
> +protected static final Set defaultCandidateLocales =
> createDefaultCandidateLocales();
>
> /** Compares the specified property to the compareString, returns true
> if they are the same, false otherwise
> * @param resource The name of the resource - if the properties file is
> 'webevent.properties', the resource name is 'webevent'
> @@ -606,21 +600,16 @@
> * general.properties.
> * @return The configured fallback locale
> */
> -public static Locale getFallbackLocale() {
> -if (fallbackLocale == null) {
> -synchronized (UtilProperties.class) {
> -if (fallbackLocale == null) {
> -String locale = getPropertyValue("general",
> "locale.properties.fallback");
> -if (UtilValidate.isNotEmpty(locale)) {
> -fallbackLocale = UtilMisc.parseLocale(locale);
> -}
> -if (fallbackLocale == null) {
> -fallbackLocale = UtilMisc.parseLocale("en");
> -}
> -}
> -}
> +protected static Locale createFallbackLocale() {
> +String locale = getPropertyValue("general",
> "locale.properties.fallback");
> +Locale result = null;
> +if (UtilValidate.isNotEmpty(locale)) {
> +result = UtilMisc.parseLocale(locale);
> +}
> +if (result == null) {
> +result = UtilMisc.parseLocale("en");
> }
> -return fallbackLocale;
> +return result;
> }
>
> /** Converts a Locale instance to a candidate Locale list. The list
> @@ -647,19 +636,12 @@
> * the LOCALE_ROOT (empty) locale - in that order.
> * @return A list of default candidate locales.
> */
> -public static Set getDefaultCandidateLocales() {
> -if (defaultCandidateLocales == null) {
> -synchronized (UtilProperties.class) {
> -if (defaultCandidateLocales == null) {
> -defaultCandidateLocales = FastSet.newInstance();
> -
> defaultCandidateLocales.addAll(localeToCandidateList(Locale.getDefault()));
> -// Change to Locale.ROOT in
Re: svn commit: r898338 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
Adrian Crum wrote: > Those public methods were created in anticipation of the Java 6 classes that > will replace them. I can put them back and deprecate them instead. If they were never in an official release of ofbiz, then go ahead and remove them.
Re: svn commit: r898338 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
Adam Heath wrote: > Adrian Crum wrote: >> Those public methods were created in anticipation of the Java 6 classes that >> will replace them. I can put them back and deprecate them instead. > > If they were never in an official release of ofbiz, then go ahead and > remove them. Heh, based on your last commit, you're gonna love me.
Re: svn commit: r898965 - in /ofbiz/branches/executioncontext20091231: ./ framework/api/src/org/ofbiz/api/context/ framework/context/src/org/ofbiz/context/ framework/example/data/ themes/bizznesstime/
adri...@apache.org wrote:
> Author: adrianc
> Date: Wed Jan 13 22:06:46 2010
> New Revision: 898965
>
> URL: http://svn.apache.org/viewvc?rev=898965&view=rev
> Log:
> Implemented permission filters. Added a user group to the Example component.
> Main navigation is controlled by the new security design.
>
> Added:
>
> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>(with props)
> Modified:
> ofbiz/branches/executioncontext20091231/BranchReadMe.txt
>
> ofbiz/branches/executioncontext20091231/framework/api/src/org/ofbiz/api/context/ThreadContext.java
>
> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/AccessControllerImpl.java
>
> ofbiz/branches/executioncontext20091231/framework/example/data/ExampleSecurityData.xml
>
> ofbiz/branches/executioncontext20091231/themes/bizznesstime/includes/appbar.ftl
>
> ofbiz/branches/executioncontext20091231/themes/bizznesstime/includes/secondary-appbar.ftl
>
> ofbiz/branches/executioncontext20091231/themes/bluelight/includes/appbarOpen.ftl
>
> ofbiz/branches/executioncontext20091231/themes/droppingcrumbs/includes/appbarOpen.ftl
>
> ofbiz/branches/executioncontext20091231/themes/flatgrey/includes/appbar.ftl
>
> ofbiz/branches/executioncontext20091231/themes/flatgrey/includes/footer.ftl
> Added:
> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java?rev=898965&view=auto
> ==
> ---
> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
> (added)
> +++
> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
> Wed Jan 13 22:06:46 2010
> @@ -0,0 +1,57 @@
> +
> ***/
> +package org.ofbiz.context;
> +
> +import static org.ofbiz.api.authorization.BasicPermissions.Access;
> +
> +import java.util.List;
> +
> +import javolution.util.FastList;
> +
> +import org.ofbiz.api.authorization.AccessController;
> +import org.ofbiz.api.context.ArtifactPath;
> +import org.ofbiz.api.context.ThreadContext;
> +import org.ofbiz.base.component.ComponentConfig;
> +import org.ofbiz.base.component.ComponentConfig.WebappInfo;
> +
> +/**
> + * ExecutionContext utility methods.
> + *
> + */
> +public class ContextUtil {
> +
> +public static List getAppBarWebInfos(String serverName,
> String menuName) {
> +List webInfos =
> ComponentConfig.getAppBarWebInfos(serverName, menuName);
> +String [] pathArray = {ArtifactPath.PATH_ROOT_NODE_NAME, null};
> +ArtifactPath artifactPath = new ArtifactPath(pathArray);
> +AccessController accessController =
> ThreadContext.getAccessController();
> +List resultList = FastList.newInstance();
> +for (WebappInfo webAppInfo : webInfos) {
> +pathArray[1] = webAppInfo.getContextRoot().replace("/", "");
> +artifactPath.saveState();
> +try {
> +accessController.checkPermission(Access, artifactPath);
> +resultList.add(webAppInfo);
> +} catch (Exception e) {}
> +artifactPath.restoreState();
> +}
> +return resultList;
> +}
> +
> +}
restoreState should be in finally. You don't handle runtime
exception. If it was in finally, you wouldn't need the catch. It's
also bad that you don't log the exception, or rethrow it.
Re: svn commit: r898965 - in /ofbiz/branches/executioncontext20091231: ./ framework/api/src/org/ofbiz/api/context/ framework/context/src/org/ofbiz/context/ framework/example/data/ themes/bizznesstime/
Adrian Crum wrote:
> --- On Wed, 1/13/10, Adam Heath wrote:
>
>> From: Adam Heath
>> Subject: Re: svn commit: r898965 - in
>> /ofbiz/branches/executioncontext20091231: ./
>> framework/api/src/org/ofbiz/api/context/
>> framework/context/src/org/ofbiz/context/ framework/example/data/
>> themes/bizznesstime/includes/ themes/bluelight/includes/
>> themes/droppingcrum...
>> To: dev@ofbiz.apache.org
>> Date: Wednesday, January 13, 2010, 6:31 PM
>> adri...@apache.org
>> wrote:
>>> Author: adrianc
>>> Date: Wed Jan 13 22:06:46 2010
>>> New Revision: 898965
>>>
>>> URL: http://svn.apache.org/viewvc?rev=898965&view=rev
>>> Log:
>>> Implemented permission filters. Added a user group to
>> the Example component. Main navigation is controlled by the
>> new security design.
>>> Added:
>>>
>>
>> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>>(with
>> props)
>>> Modified:
>>>
>>ofbiz/branches/executioncontext20091231/BranchReadMe.txt
>>>
>>
>> ofbiz/branches/executioncontext20091231/framework/api/src/org/ofbiz/api/context/ThreadContext.java
>>>
>>
>> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/AccessControllerImpl.java
>>>
>>
>> ofbiz/branches/executioncontext20091231/framework/example/data/ExampleSecurityData.xml
>>>
>>
>> ofbiz/branches/executioncontext20091231/themes/bizznesstime/includes/appbar.ftl
>>>
>>
>> ofbiz/branches/executioncontext20091231/themes/bizznesstime/includes/secondary-appbar.ftl
>>>
>>
>> ofbiz/branches/executioncontext20091231/themes/bluelight/includes/appbarOpen.ftl
>>>
>>
>> ofbiz/branches/executioncontext20091231/themes/droppingcrumbs/includes/appbarOpen.ftl
>>>
>>
>> ofbiz/branches/executioncontext20091231/themes/flatgrey/includes/appbar.ftl
>>>
>>
>> ofbiz/branches/executioncontext20091231/themes/flatgrey/includes/footer.ftl
>>
>>> Added:
>> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java?rev=898965&view=auto
>>>
>> ==
>>> ---
>> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>> (added)
>>> +++
>> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>> Wed Jan 13 22:06:46 2010
>>> @@ -0,0 +1,57 @@
>>> +
>> ***/
>>> +package org.ofbiz.context;
>>> +
>>> +import static
>> org.ofbiz.api.authorization.BasicPermissions.Access;
>>> +
>>> +import java.util.List;
>>> +
>>> +import javolution.util.FastList;
>>> +
>>> +import org.ofbiz.api.authorization.AccessController;
>>> +import org.ofbiz.api.context.ArtifactPath;
>>> +import org.ofbiz.api.context.ThreadContext;
>>> +import org.ofbiz.base.component.ComponentConfig;
>>> +import
>> org.ofbiz.base.component.ComponentConfig.WebappInfo;
>>> +
>>> +/**
>>> + * ExecutionContext utility methods.
>>> + *
>>> + */
>>> +public class ContextUtil {
>>> +
>>> +public static List
>> getAppBarWebInfos(String serverName, String menuName) {
>>> +List
>> webInfos = ComponentConfig.getAppBarWebInfos(serverName,
>> menuName);
>>> +String [] pathArray =
>> {ArtifactPath.PATH_ROOT_NODE_NAME, null};
>>> +ArtifactPath artifactPath
>> = new ArtifactPath(pathArray);
>>> +AccessController
>> accessController = ThreadContext.getAccessController();
>>> +List
>> resultList = FastList.newInstance();
>>> +for (WebappInfo
>> webAppInfo : webInfos) {
>>> +
>> pathArray[1] = webAppInfo.getContextRoot().replace("/",
>> "");
>>> +
>> artifactPath.saveState();
>>> +try {
>>> +
>> accessController.checkPermission(Access,
>> artifactPath);
>>> +
>> resultList.add(webAppInfo);
>>> +} catch
>> (Exception e) {}
>>> +
>> artifactPath.restoreState();
>>> +}
>>> +return resultList;
>>> +}
>>> +
>>> +}
>>
>> restoreState should be in finally. You don't handle
>> runtime
>> exception. If it was in finally, you wouldn't need
>> the catch. It's
>> also bad that you don't log the exception, or rethrow it.
>
> I think you're not understanding the application. This might help:
>
> http://java.sun.com/javase/6/docs/api/java/security/AccessController.html#checkPermission%28java.security.Permission%29
That has no bearing whatsoever. RuntimeException and Error can happen
at any point.
Re: svn commit: r899053 - /ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
adri...@apache.org wrote:
> Author: adrianc
> Date: Thu Jan 14 03:54:23 2010
> New Revision: 899053
>
> URL: http://svn.apache.org/viewvc?rev=899053&view=rev
> Log:
> Better exception handling - suggested by Adam Heath.
>
> Modified:
>
> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>
> Modified:
> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java?rev=899053&r1=899052&r2=899053&view=diff
> ==
> ---
> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
> (original)
> +++
> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
> Thu Jan 14 03:54:23 2010
> @@ -20,6 +20,7 @@
>
> import static org.ofbiz.api.authorization.BasicPermissions.Access;
>
> +import java.security.AccessControlException;
> import java.util.List;
>
> import javolution.util.FastList;
> @@ -48,7 +49,9 @@
> try {
> accessController.checkPermission(Access, artifactPath);
> resultList.add(webAppInfo);
> -} catch (Exception e) {}
> +} catch (AccessControlException e) {
> +// This exception is expected - do nothing
> +}
> artifactPath.restoreState();
And what about the OutOfMemoryException that could be thrown by
resultList.add()?
Anytime you have a push/pop kind of pattern, you must use push before
a try, and pop inside the finally.
foo.push();
try {
code();
} finally {
foo.pop();
}
Re: svn commit: r899053 - /ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
Adrian Crum wrote:
> --- On Wed, 1/13/10, Adam Heath wrote:
>
>> From: Adam Heath
>> Subject: Re: svn commit: r899053 -
>> /ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>> To: dev@ofbiz.apache.org
>> Cc: comm...@ofbiz.apache.org
>> Date: Wednesday, January 13, 2010, 8:17 PM
>> adri...@apache.org
>> wrote:
>>> Author: adrianc
>>> Date: Thu Jan 14 03:54:23 2010
>>> New Revision: 899053
>>>
>>> URL: http://svn.apache.org/viewvc?rev=899053&view=rev
>>> Log:
>>> Better exception handling - suggested by Adam Heath.
>>>
>>> Modified:
>>>
>>
>> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>>> Modified:
>> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java?rev=899053&r1=899052&r2=899053&view=diff
>>>
>> ==
>>> ---
>> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>> (original)
>>> +++
>> ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
>> Thu Jan 14 03:54:23 2010
>>> @@ -20,6 +20,7 @@
>>>
>>> import static
>> org.ofbiz.api.authorization.BasicPermissions.Access;
>>>
>>> +import java.security.AccessControlException;
>>> import java.util.List;
>>>
>>> import javolution.util.FastList;
>>> @@ -48,7 +49,9 @@
>>> try {
>>>
>> accessController.checkPermission(Access,
>> artifactPath);
>>>
>> resultList.add(webAppInfo);
>>> -} catch
>> (Exception e) {}
>>> +} catch
>> (AccessControlException e) {
>>> +
>> // This exception is expected - do nothing
>>> +}
>>>
>> artifactPath.restoreState();
>>
>> And what about the OutOfMemoryException that could be
>> thrown by
>> resultList.add()?
>>
>> Anytime you have a push/pop kind of pattern, you must use
>> push before
>> a try, and pop inside the finally.
>>
>> foo.push();
>> try {
>> code();
>> } finally {
>> foo.pop();
>> }
>
> I really appreciate you taking the time to review my code. In this case, I
> think you're making much ado about nothing. The object you are concerned
> about is on the stack, so if any exception is thrown it will be discarded.
Hmm. I just switched my checkout to your branch. I'm seeing some
things that need to be reviewed. I'm going to need to make some time
for this.
Initially, ArtifactPath implements Iterator. I would have
done that as Iterable, which would then allow ArtifactPath to
be used in for(type name: var) constructs.
Also, in the same class, getPathAsString has inconsistent used of
'this' when referencing the internal stringBuilder object.
Actually, what is the prefered way of making use of 'this'? Should it
always be used on local method access? local variable access?
Personally, I think it should *not* be used, as the compiler will
figure it out for you, and the computer should be used for what it is
good for, automation.
The static PATH_ROOT variable could get corrupted, if calling code
doesn't properly do a saveState/restoreState pair. This is based on
my earlier emails in this thread.
Aie, this one is bad. getPathAsString uses an instance variable,
stringBuilder, then manipulates it's state. This could break in
multi-threaded situations, esp. when considering that PATH_ROOT is a
shared static.
Based on ContextUtil and ArtifactPath, it looks like
saveState/restoreState are *always* called. This means that
ArtifactPath.stack will *always* be set to a FastList instance. It
would be better to remove the null checks, and make stack a final
instance variable.
The array based ArtifactPath constructor should use ... instead of [],
to allow varargs.
It's generally bad form have an object take a parameter, then not
internalize said parameter. Ie, the array constructor doesn't take
ownership of the pathElementArray, thereby allowing calling code to
manipulate it. If this is by design, it should be listed as such in
the javadocs.
Does ArtifactPath support federation? Ie, why not use jndi for this,
as it has path/name support stuff.
Re: svn commit: r899116 - /ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java
jaco...@apache.org wrote:
> Modified:
> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java?rev=899116&r1=899115&r2=899116&view=diff
> ==
> ---
> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java
> (original)
> +++
> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java
> Thu Jan 14 08:46:04 2010
> @@ -73,8 +73,8 @@
> }
>
> if (count < 1) {
> -Debug.logWarning("Count is less than one, not doing nothing: " +
> rawString(), module);
> -return false;
> +Debug.logWarning("Count is less than one, not doing anything: "
> + rawString(), module);
> +return true;
Do we *really* need to have this warning here? How often will the
loop count be zero?
I could see having a warning or error if it is negative.
build.xml ${lib.dir}
I'm currently looking at the executioncontext branch, and noticed
something in it's api/build.xml. It makes use of a ${lib.dir} to
include local jars into it's classpath. The assumption being you
might want to change the value of that.
However, it then hard-codes the lib name when accessing other
components jars. That seems quite wrong to me.
And, this same pattern is used thru all build.xml. I'm thinking we
should remove lib.dir, build.dir, etc, and just hard-code them.
ps: this really has nothing to do with executioncontext, and is a
general-purpose observation.
Re: build.xml ${lib.dir}
Adrian Crum wrote: > I noticed that too. If a component doesn't have a lib folder, then the > build fails. Actually, not completely correct. If a component does *not* have a lib folder, but *does* include a reference in it's local build.xml, then it fails. When creating a new component, if it doesn't have any libs, then don't add a path element for the lib folder. Just don't copy an existing build.xml without looking it over. It's just not that complex.
Re: svn commit: r899053 - /ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
Adrian Crum wrote: > Adam Heath wrote: >> It's generally bad form have an object take a parameter, then not >> internalize said parameter. Ie, the array constructor doesn't take >> ownership of the pathElementArray, thereby allowing calling code to >> manipulate it. If this is by design, it should be listed as such in >> the javadocs. > > That was by design. I didn't put a great deal of thought into that class > initially - it started out as a convenience. As development progressed, > it evolved. Writing JavaDocs is a LONG way off. > > A lot of the improvements you suggested (and are committing) I would > have gotten around to eventually. My focus was to get it operational so > it can be evaluated. > > By evaluated I mean on a higher level than what you're doing here. I > need the *concept* evaluated and commented on. Will this approach to > security work? Can we agree on its advantages? Try converting a > component over to the new security - was it easier or harder than > expected? How will this fit in with your projects? How can it be > expanded/improved/made easier? To do that kind of high-level evaluation, I need quick scans, and let my gut respond. It has a hard time doing that when there are littly nit-picky style issues. Plus, going thru all files like I have done allows me to see all the files quickly, and try to get a feel for them. I haven't read the design docs, haven't read the readme file, just trying to read the code to see if it makes sense. Initially, I like the idea of what is occuring here. There is more than I thought, based on just reading the list. Some of the things you are doing we've wanted to do with webslinger, so it's good that there is an implementation in ofbiz now. I'll continue to review, but it'll be a bit before I can actually sit down for an extended period.
Re: svn commit: r899053 - /ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
Adam Heath wrote:
> Adrian Crum wrote:
>> By evaluated I mean on a higher level than what you're doing here. I
>> need the *concept* evaluated and commented on. Will this approach to
>> security work? Can we agree on its advantages? Try converting a
>> component over to the new security - was it easier or harder than
>> expected? How will this fit in with your projects? How can it be
>> expanded/improved/made easier?
So, I see a general pattern in these classes, separate push()/pop()
methods, that have to be called correctly. Due to developer error, or
runtime error, it's possible that a pop() might not be called.
I would instead prefer to see this:
T result = Controller.runWith(data, new Callable() {
public T call() throws Exception {
// code
return null;
}
});
runWith can then encapsulate the correct try/catch/finally/push/pop
handling.
ps: OfbizSecurityTransform.execute() should call pushExecutionArtifact
outside of the try, not inside.
Re: svn commit: r899053 - /ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
Adam Heath wrote:
> T result = Controller.runWith(data, new Callable() {
> public T call() throws Exception {
> // code
> return null;
> }
> });
I've actually attempted this, and while I think the implementation of
this pattern is simple, actually *using* it in higher-level code ends
up making things rather verbose.
/me goes to think more
Re: svn commit: r899053 - /ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
Adrian Crum wrote:
> Adam Heath wrote:
>> Adam Heath wrote:
>>> T result = Controller.runWith(data, new Callable() {
>>> public T call() throws Exception {
>>> // code
>>> return null;
>>> }
>>> });
>>
>> I've actually attempted this, and while I think the implementation of
>> this pattern is simple, actually *using* it in higher-level code ends
>> up making things rather verbose.
>>
>> /me goes to think more
>
> I like the idea of encapsulating it all, but it seems to me at first
> glance that it will require a lot of code rewriting. I was trying to
> "inject" the new design into existing code without altering the existing
> code. New code could certainly follow a better pattern.
>
Why wasn't java.security.AccessController(and friends) used for this?
Re: svn commit: r899557 - /ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java
jaco...@apache.org wrote:
> Author: jacopoc
> Date: Fri Jan 15 08:14:39 2010
> New Revision: 899557
>
> URL: http://svn.apache.org/viewvc?rev=899557&view=rev
> Log:
> Based on suggestion by Adam Heath, the warning message is logged only if the
> count is negative; if the count is zero the next for loop will be skipped and
> the method will return true as well, but without warning message.
>
> Modified:
>
> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java
>
> Modified:
> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java?rev=899557&r1=899556&r2=899557&view=diff
> ==
> ---
> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java
> (original)
> +++
> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java
> Fri Jan 15 08:14:39 2010
> @@ -72,8 +72,8 @@
> return false;
> }
>
> -if (count < 1) {
> -Debug.logWarning("Count is less than one, not doing anything: "
> + rawString(), module);
> +if (count < 0) {
> +Debug.logWarning("Count is less than zero, not doing anything: "
> + rawString(), module);
> return true;
> }
Technically, this is a poor wording; negative counts are more an
error, and probably shouldn't be happening.
Re: svn commit: r899557 - /ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java
>>> ==
>>>
>>> ---
>>> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java
>>> (original)
>>> +++
>>> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Loop.java
>>> Fri Jan 15 08:14:39 2010
>>> @@ -72,8 +72,8 @@
>>> return false;
>>> }
>>>
>>> -if (count < 1) {
>>> -Debug.logWarning("Count is less than one, not doing
>>> anything: " + rawString(), module);
>>> +if (count < 0) {
>>> +Debug.logWarning("Count is less than zero, not doing
>>> anything: " + rawString(), module);
>>> return true;
>>> }
>>
>> Technically, this is a poor wording; negative counts are more an
>> error, and probably shouldn't be happening.
>
> I like the idea of making things blow up if there is a programming
> error. It forces the programmer to fix it.
Cool then. I'll get right on integrating a dynamite library.
Re: svn commit: r899053 - /ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
Adrian Crum wrote:
> Adam Heath wrote:
>> Adam Heath wrote:
>>> T result = Controller.runWith(data, new Callable() {
>>> public T call() throws Exception {
>>> // code
>>> return null;
>>> }
>>> });
>>
>> I've actually attempted this, and while I think the implementation of
>> this pattern is simple, actually *using* it in higher-level code ends
>> up making things rather verbose.
>
> I just added your idea to the branch, and it *is* simple. Let's say we
> want to make incoming requests a security-aware artifact:
>
> org.ofbiz.webapp.control.ControlServlet
>
> public void doGet(HttpServletRequest request, HttpServletResponse
> response) throws ServletException, IOException {
> try {
> ThreadContext.runExecutionArtifact(new RequestArtifact(request,
> response));
> } catch (Throwable t) {
> // Do something with t
> }
> }
If you were *really* hot stuff, you'd use APT, annotations, and
directly modify the code at compile time.
@ThreadContext(RequestArtifact.class)
public void doGet(HttpServletRequest request, HttpServletResponse
response) {
if (request.getAttribute()) {
// ...
}
}
Re: svn commit: r899053 - /ofbiz/branches/executioncontext20091231/framework/context/src/org/ofbiz/context/ContextUtil.java
David E Jones wrote:
> On Jan 15, 2010, at 2:12 PM, Adam Heath wrote:
>
>> Adrian Crum wrote:
>>> Adam Heath wrote:
>>>> Adam Heath wrote:
>>>>> T result = Controller.runWith(data, new Callable() {
>>>>>public T call() throws Exception {
>>>>>// code
>>>>>return null;
>>>>>}
>>>>> });
>>>> I've actually attempted this, and while I think the implementation of
>>>> this pattern is simple, actually *using* it in higher-level code ends
>>>> up making things rather verbose.
>>> I just added your idea to the branch, and it *is* simple. Let's say we
>>> want to make incoming requests a security-aware artifact:
>>>
>>> org.ofbiz.webapp.control.ControlServlet
>>>
>>> public void doGet(HttpServletRequest request, HttpServletResponse
>>> response) throws ServletException, IOException {
>>>try {
>>>ThreadContext.runExecutionArtifact(new RequestArtifact(request,
>>> response));
>>>} catch (Throwable t) {
>>>// Do something with t
>>>}
>>> }
>> If you were *really* hot stuff, you'd use APT, annotations, and
>> directly modify the code at compile time.
>>
>> @ThreadContext(RequestArtifact.class)
>> public void doGet(HttpServletRequest request, HttpServletResponse
>> response) {
>>if (request.getAttribute()) {
>>// ...
>>}
>> }
>
> Yes, please... let's make this as obfuscated and impossible to maintain as
> possible.
>
> This may be a community driven project, but job security should still be our
> #1 priority. On the other hand, I think it was Adam who said that after a
> couple of years he couldn't even follow his own code in parts of the Entity
> Engine (yes, stuff I've been tempted to rewrite since it's so tedious to
> trace through, though I think the ShoppingCart and related objects are still
> worse!).
Yeah, that was me, and it is the caching stuff. I agree about the
shopping cart and order read header too.
In all honesty, I would use dynamic ecas to do the caching stuff now.
Re: svn commit: r900050 - /ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
adri...@apache.org wrote:
> Author: adrianc
> Date: Sun Jan 17 03:13:19 2010
> New Revision: 900050
>
> URL: http://svn.apache.org/viewvc?rev=900050&view=rev
> Log:
> Reorganized code in TemporalExpressions.java. No functional change.
>
> Modified:
>
> ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
>
> Modified:
> ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java?rev=900050&r1=900049&r2=900050&view=diff
> ==
> ---
> ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
> (original)
> +++
> ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
> Sun Jan 17 03:13:19 2010
> @@ -37,224 +37,323 @@
> public class TemporalExpressions implements Serializable {
> public static final String module = TemporalExpressions.class.getName();
> public static final TemporalExpression NullExpression = new Null();
> +// Expressions are evaluated from largest unit of time to smallest.
> +// When unit of time is the same, then they are evaluated from
> +// least ambiguous to most. Frequency should always be first -
> +// since it is the most specific. Date range should always be last.
> +// The idea is to evaluate all other expressions, then check to see
> +// if the result falls within the date range.
> +public static final int SEQUENCE_DATE_RANGE = 1;
> +public static final int SEQUENCE_DAY_IN_MONTH = 400;
> +public static final int SEQUENCE_DOM_RANGE = 300;
> +public static final int SEQUENCE_DOW_RANGE = 500;
> +public static final int SEQUENCE_FREQ = 100;
> +public static final int SEQUENCE_HOUR_RANGE = 700;
> +public static final int SEQUENCE_MINUTE_RANGE = 800;
> +public static final int SEQUENCE_MONTH_RANGE = 200;
> +public static final int SEQUENCE_TOD_RANGE = 600;
I'd prefer to see this as an enum instead, it offers better type safety.
public enum SEQUENCE {
DATE_RANGE { public int value() { return 1; },
DAY_IN_MONTH { public int value() { return 400; },
...
public abstract int value();
}
If you don't want to have each enum contain a value method, then an
enum map could be used(which is not a real map), but then you'd have
Integer objects, instead of int values.
Re: Webslinger demo screen in example application
Bruno Busco wrote: > Hi, > I would like to see webslinger in action to better understand what it > can be used for. I think this would be useful to all developers. > > Could a webslinger expert add a demo screen or something in the > Example application that shows it? > > Thank you, > Bruno that would be me. I was thinking about how to do more webslinger stuff in ofbiz. For that, I've been looking at the widget subsystems, seeing how to better integrate them. I'll try and play around with something tomorrow.
Re: svn commit: r900497 - in /ofbiz/trunk/framework: bi/webapp/bi/WEB-INF/controller.xml example/webapp/birt/WEB-INF/controller.xml example/webapp/example/WEB-INF/controller.xml webtools/webapp/webtoo
Hans Bakker wrote:
> Hi Scott,
>
> i have a solution which is already used a couple of times in the system:
>
> value="${groovy:org.ofbiz.base.component.ComponentConfig.componentExists("projectmgr")}"/>
>
> this statement will check if the projectmgr component exists in a form.
>
> would that help?
No.
Create an abstract class or interface in framework, use the java
services api to load it dynamically(or read from a database table, and
populate it with seed data).
Any reference in framework to anything outside of framework, even if
surrounded by a condition, is not allowed.
Re: Discussion: Spin Off OFBiz Technology
Chris Snow wrote: > The entity engine would be good as a library/standalone framework! That's a little more difficult; the entity codebase uses a lot of code from base(in addition to my sql parser, which *is* generic). All that code would need to be changed to use other libraries, which is more work than I would be willing to do at this time.
Re: Supporting "null" in data load update
Adrian Crum wrote: > Plus, I like being specific - using fieldName="null" makes it clear what > you intend to do. ="null" is a string, who's contents are null. There can be no other way to interpet that. Better would be to do null_$field="", or some such. You can definately tell when an attribute exists, vs. just being an empty value. You have to used the NamedNodeMap interface, looping over the attributes.
Re: Compiler warnings in trunk
Adrian Crum wrote: > Scott Gray wrote: >> On 22/01/2010, at 8:03 AM, Erwan de FERRIERES wrote: >> >>> Hi all, >>> >>> there is still some compiler warnings in the trunk. >>> >>> I've list those files with calls to deprecated methods : >>> * OFBizBirtViewerReportService.java (call to a >>> org.eclipse.birt.report.service.ReportEngineService.DummyRemoteException >>> which is deprecated) >>> * UtilXml.java (this import : org.apache.xml.serialize.OutputFormat >>> is deprecated) >>> * JSONSimpleEventHandler.java (the AbstractJSONEventHandler has been >>> deprecated). >> >> I couldn't seem to disable this warning regardless of where I put the >> suppress warnings annotation, if anyone has some pointers I'll be glad >> to hear them. > > I think it is safe to remove the deprecated class from UtilXml now. The > method that uses it has been deprecated for nearly a year now. Is it in a release?
Re: svn commit: r902041 - /ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/ContentList.ftl
hans...@apache.org wrote:
> Author: hansbak
> Date: Fri Jan 22 09:09:36 2010
> New Revision: 902041
>
> URL: http://svn.apache.org/viewvc?rev=902041&view=rev
> Log:
> avoid error on screen if the status of the content item is not set
>
> Modified:
>
> ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/ContentList.ftl
>
> Modified:
> ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/ContentList.ftl
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/ContentList.ftl?rev=902041&r1=902040&r2=902041&view=diff
> ==
> ---
> ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/ContentList.ftl
> (original)
> +++
> ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/ContentList.ftl
> Fri Jan 22 09:09:36 2010
> @@ -24,7 +24,7 @@
> <#assign content = pContent.getRelatedOne("Content")>
> <#assign contentType = content.getRelatedOneCache("ContentType")>
> <#assign mimeType =
> content.getRelatedOneCache("MimeType")?if_exists>
> -<#assign status = content.getRelatedOneCache("StatusItem")>
> +<#assign status =
> content.getRelatedOneCache("StatusItem")?if_exists>
> <#assign pcType = pContent.getRelatedOne("PartyContentType")>
>
> href="<@ofbizUrl>EditPartyContents?contentId=${pContent.contentId}&partyId=${pContent.partyId}&partyContentTypeId=${pContent.partyContentTypeId}&fromDate=${pContent.fromDate}">${content.contentId}
Huh? Based on your log message, you're testing if the 'content' item
doesn't exist. But if that is so, then won't contentType also throw a
NPE?
Re: Compiler warnings in trunk
Adrian Crum wrote: > Adam Heath wrote: >> Adrian Crum wrote: >>> Scott Gray wrote: >>>> On 22/01/2010, at 8:03 AM, Erwan de FERRIERES wrote: >>>> >>>>> Hi all, >>>>> >>>>> there is still some compiler warnings in the trunk. >>>>> >>>>> I've list those files with calls to deprecated methods : >>>>> * OFBizBirtViewerReportService.java (call to a >>>>> org.eclipse.birt.report.service.ReportEngineService.DummyRemoteException >>>>> >>>>> which is deprecated) >>>>> * UtilXml.java (this import : org.apache.xml.serialize.OutputFormat >>>>> is deprecated) >>>>> * JSONSimpleEventHandler.java (the AbstractJSONEventHandler has been >>>>> deprecated). >>>> I couldn't seem to disable this warning regardless of where I put the >>>> suppress warnings annotation, if anyone has some pointers I'll be glad >>>> to hear them. >>> I think it is safe to remove the deprecated class from UtilXml now. The >>> method that uses it has been deprecated for nearly a year now. >> >> Is it in a release? > > Yes. Was it deprecated in the *last* release, or was deprecated added during *this* development cycle? If the former, then you can remove it. If the latter, it has to stay for the next release. If I'm causing you(or whoever goes about fixing these) too much work, then I'm sorry, but this is what is required for proper release management.
Re: svn commit: r902041 - /ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/ContentList.ftl
Jacopo Cappellato wrote:
> On Jan 22, 2010, at 6:00 PM, Adam Heath wrote:
>
>> hans...@apache.org wrote:
>>> Author: hansbak
>>> Date: Fri Jan 22 09:09:36 2010
>>> New Revision: 902041
>>>
>>> URL: http://svn.apache.org/viewvc?rev=902041&view=rev
>>> Log:
>>> avoid error on screen if the status of the content item is not set
>>>
>>> Modified:
>>>
>>> ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/ContentList.ftl
>>>
>>> Modified:
>>> ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/ContentList.ftl
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/ContentList.ftl?rev=902041&r1=902040&r2=902041&view=diff
>>> ==
>>> ---
>>> ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/ContentList.ftl
>>> (original)
>>> +++
>>> ofbiz/trunk/applications/party/webapp/partymgr/party/profileblocks/ContentList.ftl
>>> Fri Jan 22 09:09:36 2010
>>> @@ -24,7 +24,7 @@
>>> <#assign content = pContent.getRelatedOne("Content")>
>>> <#assign contentType =
>>> content.getRelatedOneCache("ContentType")>
>>> <#assign mimeType =
>>> content.getRelatedOneCache("MimeType")?if_exists>
>>> -<#assign status = content.getRelatedOneCache("StatusItem")>
>>> +<#assign status =
>>> content.getRelatedOneCache("StatusItem")?if_exists>
>>> <#assign pcType = pContent.getRelatedOne("PartyContentType")>
>>>
>>> >> href="<@ofbizUrl>EditPartyContents?contentId=${pContent.contentId}&partyId=${pContent.partyId}&partyContentTypeId=${pContent.partyContentTypeId}&fromDate=${pContent.fromDate}">${content.contentId}
>>
>> Huh? Based on your log message, you're testing if the 'content' item
>> doesn't exist. But if that is so, then won't contentType also throw a
>> NPE?
>
> Well, the log message says "if the status of the content", not the content
> itself.
Oh, right. So, with that cleared up, if I do this:
<#assign someItem = map.get("key")>
freemarker will throw an error if the key does not exist in the map?
Re: Supporting "null" in data load update
Bob Morley wrote:
> Actually I just did a quick test and the SAX parser returns all attributes
> that are defined in the document (even if they have empty string as their
> value). The code as originally written ...
>
> // treat empty strings as nulls
> if (value != null && value.length() > 0) {
>
> Exclude if the value is null (not sure how that would happen) but also if it
> is empty string. So it would be technically feasible to treat explicit
> empty string values as "null" for the database and having non-specified
> field values ignored (as they are now).
Again, no. There is a very big difference between an actual empty
string, and a null value. Do not overload the meaning.
> Having said that, if we feel it is more reasonable to be explicit here
> (fieldname="null" or null_fieldname="") then I am all for it.
>
> It seems to me personally, that it is more intuitive to have fieldname=""
> imply an explicit set to null in the database which would be consist with
> what you get on the initial create and would be a force back to "blank" on
> an update. Not having the field clearly implies ignore (which is what it
> does now), etc etc etc.
>
> Anyone buying that? :)
Not me.
Re: Compiler warnings in trunk
Scott Gray wrote: > A good majority of the deprecated methods prior to the last release were > deprecated using the javadoc style: /** @deprecated */ > I updated them post release to use the actual @Deprecated annotation, any > opinions on whether we're okay to remove them now or would we be better to > wait? Does javac warn if only javadoc deprecation is used?
Re: Compiler warnings in trunk
Scott Gray wrote: > On 22/01/2010, at 11:14 AM, Adam Heath wrote: > >> Scott Gray wrote: >>> A good majority of the deprecated methods prior to the last release were >>> deprecated using the javadoc style: /** @deprecated */ >>> I updated them post release to use the actual @Deprecated annotation, any >>> opinions on whether we're okay to remove them now or would we be better to >>> wait? >> Does javac warn if only javadoc deprecation is used? > > After a quick test, the answer is yes. Leaves me wondering what the point of > the annotation is. Don't use a modern compiler that actually understands the annotation. Use an older compiler that only understands the javadoc.
Re: Compiler warnings in trunk
Scott Gray wrote: > On 22/01/2010, at 11:32 AM, Adam Heath wrote: > >> Scott Gray wrote: >>> On 22/01/2010, at 11:14 AM, Adam Heath wrote: >>> >>>> Scott Gray wrote: >>>>> A good majority of the deprecated methods prior to the last release were >>>>> deprecated using the javadoc style: /** @deprecated */ >>>>> I updated them post release to use the actual @Deprecated annotation, any >>>>> opinions on whether we're okay to remove them now or would we be better >>>>> to wait? >>>> Does javac warn if only javadoc deprecation is used? >>> After a quick test, the answer is yes. Leaves me wondering what the point >>> of the annotation is. >> Don't use a modern compiler that actually understands the annotation. >> Use an older compiler that only understands the javadoc. > > You've lost me, I see two potential ways to understand what you're saying: > - Do what you're suggesting to properly test if javac warns on javadoc > deprecations > Given that prior to annotations being available this was the only way to > deprecate something then I'm not sure if that would be necessary? > - Do what you're suggesting to see what the point of the Deprecated > annotation is. > I'm not sure how using an older compiler would answer my question? The annotation allows you to query it at runtime, instead of it just being some magical bitflag in the bytecode .class file, that only java compilers(and other bytecode readers) understand. I guess that having @Deprecated allows things to detect at runtime, thru reflection, whether something is deprecated.
Re: svn commit: r902226 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/ConnectionFactory.java
lekt...@apache.org wrote:
> Author: lektran
> Date: Fri Jan 22 19:11:18 2010
> New Revision: 902226
>
> URL: http://svn.apache.org/viewvc?rev=902226&view=rev
> Log:
> Removed deprecated method
> ConnectionFactory.tryGenericConnectionSources(String, Element)
>
> Modified:
>
> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/ConnectionFactory.java
>
> Modified:
> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/ConnectionFactory.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/ConnectionFactory.java?rev=902226&r1=902225&r2=902226&view=diff
> ==
> ---
> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/ConnectionFactory.java
> (original)
> +++
> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/jdbc/ConnectionFactory.java
> Fri Jan 22 19:11:18 2010
> @@ -78,11 +78,6 @@
> return con;
> }
>
> -@Deprecated
> -public static Connection tryGenericConnectionSources(String helperName,
> Element inlineJdbcElement) throws SQLException, GenericEntityException {
> -return getManagedConnectionFactory().getConnection(helperName,
> inlineJdbcElement);
> -}
> -
> public static ConnectionFactoryInterface getManagedConnectionFactory() {
> if (_factory == null) { // don't want to block here
> synchronized (TransactionFactory.class) {
>
If something is deprecated, then it should be in the release notes(for
major releases). Then, when it finally *is* removed, it should again
be listed in release notes, saying such.
Re: svn commit: r902347 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/test/BaseUnitTests.java
adri...@apache.org wrote:
> Author: adrianc
> Date: Sat Jan 23 06:19:47 2010
> New Revision: 902347
>
> URL: http://svn.apache.org/viewvc?rev=902347&view=rev
> Log:
> Unit test for previous commit.
>
> Modified:
> ofbiz/trunk/framework/base/src/org/ofbiz/base/test/BaseUnitTests.java
>
> Modified:
> ofbiz/trunk/framework/base/src/org/ofbiz/base/test/BaseUnitTests.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/test/BaseUnitTests.java?rev=902347&r1=902346&r2=902347&view=diff
> ==
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/test/BaseUnitTests.java
> (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/test/BaseUnitTests.java Sat
> Jan 23 06:19:47 2010
> @@ -92,6 +92,10 @@
> assertTrue("overlaps range", range1.overlaps(overlapTest));
> assertTrue("overlaps range", range2.overlaps(overlapTest));
> assertFalse("does not overlap range", range1.overlaps(range2));
> +try {
> +ComparableRange range3 = new
> ComparableRange(new java.util.Date(), new
> java.sql.Timestamp(System.currentTimeMillis()));
> +assertTrue("mismatched classes", range3 != null);
assertFail("message");
> +} catch (IllegalArgumentException e) {}
> }
>
> public void testDateTimeConverters() {
>
>
Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ spec
hans...@apache.org wrote: > Author: hansbak > Date: Mon Jan 25 06:26:23 2010 > New Revision: 902716 > > URL: http://svn.apache.org/viewvc?rev=902716&view=rev > Log: > > function leave feedback and screen > - function auto relist item and screen > - Get feedback data from ebay site and save into ofbiz. > - Add rate pictures into framework/images/webapp/images > - screen for show awaiting feedback and recent feedback from buyer > - help screens are provided this was a bad commit, too many things done at once. Please try to be considerate of long-term maintainability, having small commits makes it easier to verify and debug.
Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ spec
David E Jones wrote: > This seems to be a little bit condescending. Trying to pick another fight? > > I'm just glad to have ring-side seats... ;) I agree with you, David.
Re: svn commit: r902885 - in /ofbiz/trunk: applications/manufacturing/widget/manufacturing/ applications/product/widget/catalog/ applications/workeffort/webapp/workeffort/task/ applications/workeffort
Jacques Le Roux wrote: > From: "Bruno Busco" >> Thank you Adrian for sharing this. >> So this confirms to me that a better style naming should be used (i.e. >> something like button-intraapp and button-interapp) >> >> But this also makes me think why the user should be informed through a >> different button style if the screen he is going to in implemented >> here or there. >> He should only be interested in what information he is going to >> access. So that all the UI is more consistent and seems like a whole >> thing. > > This is pertiment, IMO Actually, the backend html generation should use separate classes for this, then the css theme can choose to style them the same.
Re: svn commit: r903197 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
jler...@apache.org wrote:
> Author: jleroux
> Date: Tue Jan 26 12:30:02 2010
> New Revision: 903197
>
> URL: http://svn.apache.org/viewvc?rev=903197&view=rev
> Log:
> A patch from Sascha Rodekamp "Extend getPropertyNumber, Default Value"
> (https://issues.apache.org/jira/browse/OFBIZ-3425) - OFBIZ-3425
>
> Adds a method to have a default value to getPropertyNumber methods, can't
> hurt and maybe useful in some cases.
>
>
>
> Modified:
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>
> Modified:
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java?rev=903197&r1=903196&r2=903197&view=diff
> ==
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
> (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
> Tue Jan 26 12:30:02 2010
> @@ -118,6 +118,15 @@
> return value;
> }
>
> +public static double getPropertyNumber(String resource, String name,
> Double defaultValue) {
Why is this Double instead of double?
> +double value = getPropertyNumber(resource, name);
> + if(value == 0.0){
> + return defaultValue;
> + }
> +
> +return value;
> +}
> +
> public static double getPropertyNumber(String resource, String name) {
> String str = getPropertyValue(resource, name);
> double strValue = 0.0;
>
>
Re: svn commit: r903197 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
jler...@apache.org wrote:
> Author: jleroux
> Date: Tue Jan 26 12:30:02 2010
> New Revision: 903197
>
> URL: http://svn.apache.org/viewvc?rev=903197&view=rev
> Log:
> A patch from Sascha Rodekamp "Extend getPropertyNumber, Default Value"
> (https://issues.apache.org/jira/browse/OFBIZ-3425) - OFBIZ-3425
>
> Adds a method to have a default value to getPropertyNumber methods, can't
> hurt and maybe useful in some cases.
>
>
>
> Modified:
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
>
> Modified:
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java?rev=903197&r1=903196&r2=903197&view=diff
> ==
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
> (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilProperties.java
> Tue Jan 26 12:30:02 2010
> @@ -118,6 +118,15 @@
> return value;
> }
>
> +public static double getPropertyNumber(String resource, String name,
> Double defaultValue) {
> +double value = getPropertyNumber(resource, name);
> + if(value == 0.0){
> + return defaultValue;
> + }
> +
> +return value;
The formatting is bad here, please be careful.
> +}
> +
> public static double getPropertyNumber(String resource, String name) {
> String str = getPropertyValue(resource, name);
> double strValue = 0.0;
>
>
Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ spec
Tim Ruppert wrote: > Not trying to - but sorry for the tone if it came across that way Hans. I'm > simply asking why it gets past the committer's desk as "I'm trying to get my > people to provide smaller pieces" is an ok way of committing code. Isn't > that the responsibility of the committer to tell the person that's providing > the code to go back to the drawing board and break it up? Isn't that our job > as committers? And as the rest of the committers, isn't it our job to remind > the offending committer to spend more time with the person who's providing > the code so that we don't have to dig thru this much mess? > > It's not always easy for any of us, but I just don't see this from other > people, so I wanted to remind Hans. If all the other committers think this > is ok (which obviously they didn't since Adam brought it up), then I'll > happily back off. Since that's not the case - and this obviously doesn't > follow the best practices of the project - please let me know how I should > encourage Hans to do what is clearly in our best practices. > > What should be done is that this mess of a commit should be reverted and put > back in in pieces - you have to start somewhere and sometimes it's just not > good enough when you have an entire community to just say you're trying hard. > My two cents. Reverting the commit is wrong. Just try to be more careful in the future.
Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ spec
Adrian Crum wrote: > Jacopo, > > No one is trying to make war. The details have been pointed out in > previous threads and in Jira issues. make: *** No rule to make target `war'. Stop.
Re: svn commit: r902716 [1/2] - in /ofbiz/trunk: framework/images/webapp/images/rate/ specialpurpose/ebay/ specialpurpose/ebay/config/ specialpurpose/ebay/data/ specialpurpose/ebay/data/helpdata/ spec
David E Jones wrote: > Thanks Jacopo this makes a good distinction. > > We need peer review, and LOTS of it. We can't "wait" for code to improve > because code doesn't just automatically improve over time, someone has to > improve it and specific feedback on code itself will help with that. > > I don't see any reason to talk about the people or guesses about the behavior > that caused code problems (which is silly, how do you know?). > > I'd argue that much of the feedback to Hans has not been specific or focused > on the code, but rather either personal things or guesses at behavior behind > the code or general trends that are of limited usefulness when it comes to > improving the code. What's the point of any of that? I haven't done any of this last paragraph. In fact, when I generally reply to a commit mail, I tend to have blinders on, as to who actually did it, what their background is, where they are located, what timezone, etc. If I suddenly start remembing who did what, then it might make me start writing emails in a more personal matter, which then could be misinterpeted, since we all have different backgrounds. It's much better to look at *just the code*; it's much harder to argue against that for argumentative sake.
