+1 for me for becoming completely case-sensitive, I understand that this will break compatibility with 2.8 a bit more, but it would makes things simpler I think.
BTW: Is the same discussion true for camelCaseLinks and matchEnglishPlurals, and if so, should we drop that too ? /Harry 2009/10/31 Janne Jalkanen <[email protected]> > > Almost. JCR is by definition case-sensitive, and the underlying > implementation enforces that (the pages could very well be in a JDBC > database). So the exact details how Priha stores the data is unimportant. > > The problem is that JSPWiki as currently written is case-sensitive, which > means that using links like [this page] and [This page] and [This Page] > actually refer to different pages. We just do some magic where we normalize > the page names to some casing rules, but that is breaking down right now, > and is not really case-insensitive. It's just a guess and causes all sorts > of interesting problems like with ReferenceManager (e.g. when the page does > not exist, what exactly should the right spelling be?) > > So I think the correct solution for 3.0 should be just to define that page > names are case insensitive. > > (To convolute the thing slightly, in English a normalization where all > words are capitalized, e.g. "This Is A Page" is okay, since that's the way > you write headlines. However, in Finnish this kind of normalization is > simply wrong and looks really bad. Case insensitivity solves this problem > neatly.) > > /Janne > > > On Oct 31, 2009, at 20:29 , Harry Metske wrote: > > let me see if I understand this problem correctly: >> >> We currently save the name and the case of a PageName in the name of file >> (2.8) or the name of a directory (3.0 / priha), right ? >> If you run on a platform that is not (completely) case-sensitive with >> regard >> to file/dir names, this get's us into trouble, right ? >> If that's the case and we want the preserve platform neutrality, we should >> use something else to hold the name and case of pagenames, and wiki:title >> seems a proper solution for that. >> >> /Harry >> >> 2009/10/31 Janne Jalkanen <[email protected]> >> >> Yup, getting back on this... >>> >>> It seems to me that the following patch fixes the issue, BUT it does >>> expose >>> quite a lot of other problems, mostly related to the fact that JSPWiki >>> (and >>> our tests) assume that page names stay in the same case as it was >>> created. >>> However, if we lowercase all page names as they are stored, the case >>> information is lost. If we do not, then there's no way that we can >>> really >>> get the JCR name from an arbitrarily cased page name. >>> >>> One possibility is to add a new attribute, "wiki:title", which would >>> contain the page name as originally created, and would be the "display" >>> name. >>> >>> /Janne >>> >>> ### Eclipse Workspace Patch 1.0 >>> #P JSPWiki >>> Index: src/java/org/apache/wiki/content/WikiPath.java >>> =================================================================== >>> --- src/java/org/apache/wiki/content/WikiPath.java (revision 831483) >>> +++ src/java/org/apache/wiki/content/WikiPath.java (working copy) >>> @@ -193,22 +193,24 @@ >>> * String representation. This is to fulfil the general contract of >>> * equals(). >>> * >>> - * @return int >>> + * @return {...@inheritdoc} >>> */ >>> + // FIXME: Slow, since it creates a new String every time. >>> public int hashCode() >>> { >>> - return m_stringRepresentation.hashCode(); >>> + return m_stringRepresentation.toLowerCase().hashCode(); >>> } >>> >>> /** >>> * A WikiPath is compared using it's toString() method. >>> * >>> * @param o The Object to compare against. >>> - * @return int >>> + * @return {...@inheritdoc} >>> */ >>> + // FIXME: Slow, since it creates a new String every time. >>> public int compareTo( WikiPath o ) >>> { >>> - return toString().compareTo( o.toString() ); >>> + return toString().toLowerCase().compareTo( >>> o.toString().toLowerCase() ); >>> } >>> >>> /** >>> @@ -226,11 +228,11 @@ >>> { >>> WikiPath n = (WikiPath) o; >>> >>> - return m_space.equals( n.m_space ) && m_path.equals( >>> n.m_path >>> ); >>> + return m_space.equalsIgnoreCase( n.m_space ) && >>> m_path.equalsIgnoreCase( n.m_path ); >>> } >>> else if( o instanceof String ) >>> { >>> - return toString().equals( o ); >>> + return toString().equalsIgnoreCase( (String)o ); >>> } >>> return false; >>> } >>> Index: src/java/org/apache/wiki/content/ContentManager.java >>> =================================================================== >>> --- src/java/org/apache/wiki/content/ContentManager.java (revision >>> 831483) >>> +++ src/java/org/apache/wiki/content/ContentManager.java (working >>> copy) >>> @@ -113,7 +113,7 @@ >>> */ >>> public static final String DEFAULT_SPACE = "Main"; >>> >>> - private static final String JCR_DEFAULT_SPACE = >>> "pages/"+DEFAULT_SPACE; >>> + private static final String JCR_DEFAULT_SPACE = >>> "pages/"+DEFAULT_SPACE.toLowerCase(); >>> >>> private static final String JCR_PAGES_NODE = "pages"; >>> >>> @@ -1433,8 +1433,8 @@ >>> String spaceName; >>> String spacePath; >>> >>> - spaceName = wikiName.getSpace(); >>> - spacePath = wikiName.getPath(); >>> + spaceName = wikiName.getSpace().toLowerCase(); >>> + spacePath = wikiName.getPath().toLowerCase(); >>> >>> return "/"+JCR_PAGES_NODE+"/"+spaceName+"/"+spacePath; >>> >>> } >>> >>> >>> On Oct 28, 2009, at 22:25 , Janne Jalkanen wrote: >>> >>> >>> Hm. This seems to be a bug in JSPWiki, actually. JCR Names *are* case >>>> sensitive, so we should actually be normalizing the name to "this is a >>>> test". The fact that it's created with an upper-case name suggests that >>>> JSPWiki is not normalizing the name properly. >>>> >>>> /Janne >>>> >>>> On Oct 28, 2009, at 20:17 , Harry Metske wrote: >>>> >>>> frustrated by the one remaining failing unit test I took a closer look >>>> at >>>> >>>>> why WikiEngineTest.testSpacedNames1() fails. >>>>> I can only conclude that 2 of the 3 tests are wrong. >>>>> First a page is created with name "This is a test", this eventually >>>>> resulting in a directory being created somewhere deep down the priha >>>>> repo >>>>> : >>>>> >>>>> mets...@gneisenau >>>>> ~/workspace/JSPWiki/build/tests/priha/workspaces/jspwiki/pages/Main >>>>> $ ls -l >>>>> total 12 >>>>> -rw-r--r-- 1 metskem metskem 43 2009-10-28 17:55 jcr:primaryType.data >>>>> -rw-r--r-- 1 metskem metskem 82 2009-10-28 17:55 jcr:primaryType.info >>>>> drwxr-xr-x 2 metskem metskem 4096 2009-10-28 19:07 This is a test >>>>> >>>>> Then we do 3 tests: >>>>> >>>>> assertEquals( "normal", "puppaa", m_engine.getText("This is a >>>>> test").trim() ); >>>>> assertEquals( "lowercase", "puppaa", m_engine.getText("this is a >>>>> test").trim() ); >>>>> assertEquals( "randomcase", "puppaa", m_engine.getText("ThiS Is a >>>>> teSt").trim() ); >>>>> >>>>> Only the first one succeeds, the second and third one fail ( of course >>>>> I >>>>> would say), at least on Linux with priha, file/dir names are case >>>>> sensitive. >>>>> >>>>> What am I missing, and if nothing, can I remove the last two tests ? >>>>> >>>>> thanks, >>>>> Harry >>>>> >>>>> 2009/10/27 Harry Metske <[email protected]> >>>>> >>>>> I just did a fresh svn checkout, and ran "ant clean tests" from the >>>>> >>>>>> cmdline. >>>>>> This gives me 3 failures and 13 errors: >>>>>> http://www.computerhok.nl/tmp/junit-noframes.html >>>>>> >>>>>> The WikiEngineTest.testSpacedNames1() always fails (Linux versus >>>>>> Mac/Windows) >>>>>> >>>>>> Here's an overview of more tests : >>>>>> http://www.computerhok.nl/tmp/jspwiki-testresult.html >>>>>> >>>>>> regards, >>>>>> Harry >>>>>> >>>>>> 2009/10/27 Andrew Jaquith <[email protected]> >>>>>> >>>>>> Sounds like we have a few issues here: >>>>>> >>>>>> >>>>>>> 1) Guitests. I'll see what I can find. Probably something minor. I >>>>>>> know the "tests" target runs all test classes ending in "*Test" and >>>>>>> ignores "AllTests", while Eclipse (and probably guitests) just runs >>>>>>> the AllTests classes. It's likely that one or more of the AllTests >>>>>>> classes is failing to include, oh, about 34 tests. :) >>>>>>> >>>>>>> 2) Graceful LDAP fail (inside the tests themselves). Any ideas on how >>>>>>> to implement? The easy way would be to look for a localhost listener >>>>>>> on 4890 (where the OpenLDAP test fixture listens) and then not run >>>>>>> the >>>>>>> tests if it isn't found. Should they FAIL or PASS in that case? It >>>>>>> sounds like passing is the right thing to do. >>>>>>> >>>>>>> 3) Differences in your test pass rate versus mine. Not sure why your >>>>>>> "ant tests" run would produce different results than mine. I did try >>>>>>> running mine with a completely new, checked-out branch. Because I >>>>>>> can't know what changes you might have in your local branch, could >>>>>>> you >>>>>>> check out a clean copy and diff the tree versus yours? SOMETHING is >>>>>>> different. Also, I'd like to know what Harry and others are seeing. >>>>>>> Gents, any clues? >>>>>>> >>>>>>> I agree that all three methods should return the same number of test >>>>>>> cases, and pass/fail the same ways. I also agree that tests should be >>>>>>> self-contained. That was part of the rationale for the Ant script >>>>>>> tweaks I checked in recently. >>>>>>> >>>>>>> Eclipse, by the way, hasn't been reliable for me, for testing, for a >>>>>>> while. I tend to exhaust memory somewhere around JSPWikiMarkupParser. >>>>>>> But I haven't tried it in the last few months (i.e. before my massive >>>>>>> bug-hunting campaign). >>>>>>> >>>>>>> Andrew >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Tue, Oct 27, 2009 at 3:25 AM, Janne Jalkanen >>>>>>> <[email protected]> wrote: >>>>>>> >>>>>>> Interestingly, I applied your most recent checkins applied (and I >>>>>>>> have >>>>>>>> >>>>>>>>> small one patch to JSPWikiMarkupParserTest that I haven't checked >>>>>>>>> in). >>>>>>>>> I am running 100% clean, with no errors. Total number of tests: >>>>>>>>> 1024 >>>>>>>>> -- a nice round number. :) WikiEngineTest.testOldVersionVars has >>>>>>>>> been running fine for me for a while. >>>>>>>>> >>>>>>>>> >>>>>>>> There's no way it should've run, unless you have some old >>>>>>>> code/config >>>>>>>> >>>>>>>> files >>>>>>> >>>>>>> lying around. Can you check out a previous version to a clean >>>>>>>> directory >>>>>>>> >>>>>>>> and >>>>>>> >>>>>>> see if it runs? >>>>>>>> >>>>>>>> As a control case, I also checked out a new built from trunk, and >>>>>>>> >>>>>>>>> simply typed 'ant tests'. I used a vanilla build with absolutely no >>>>>>>>> customizations, even to build.properties. It ran completely clean >>>>>>>>> also >>>>>>>>> except for 1 JSPWikiMarkupParserTest test (because I haven't >>>>>>>>> checked >>>>>>>>> in that fix), 1024 tests total. >>>>>>>>> >>>>>>>>> >>>>>>>> Running the AllTests from Eclipse or with "ant guitests" results in >>>>>>>> 990 >>>>>>>> >>>>>>>> test >>>>>>> >>>>>>> cases. "ant tests" is the only one giving 1024 tests, and I get 12 >>>>>>>> >>>>>>>> failures >>>>>>> >>>>>>> and 14 errors for it. LdapAuthorizerTest, LdapUserDatabaseTest and >>>>>>>> XMLUserDatabaseTest all fail with all tests. >>>>>>>> >>>>>>>> What I find odd is that guitests and tests targets should give the >>>>>>>> same >>>>>>>> results, since they both are run from build.xml. >>>>>>>> >>>>>>>> The only other item causing the discrepancy would be if you don't >>>>>>>> >>>>>>>>> have >>>>>>>>> a local LDAP server running for the LDAP tests. Those should cause, >>>>>>>>> at >>>>>>>>> most, 14 failures or errors. I'll add in some code to build.xml to >>>>>>>>> set >>>>>>>>> up the LDAP fixtures and/or disable the tests if the OpenLDAP >>>>>>>>> executable isn't available. >>>>>>>>> >>>>>>>>> >>>>>>>> I think it's probably a better idea to do the test directly in the >>>>>>>> tests >>>>>>>> itself. The JCR TCK throws a NonExecutableException when the test >>>>>>>> case >>>>>>>> cannot be executed (and this shows up as a passed test). >>>>>>>> >>>>>>>> I think it's important that all three methods give the same number >>>>>>>> of >>>>>>>> >>>>>>>> test >>>>>>> >>>>>>> cases; if the number is not reliable, it's too easy to forget to run >>>>>>>> >>>>>>>> certain >>>>>>> >>>>>>> tests. >>>>>>>> >>>>>>>> Also, I sometimes run all tests for a given package from within >>>>>>>> Eclipse. >>>>>>>> >>>>>>>> I'd >>>>>>> >>>>>>> like the test cases to be self-contained enough so that I don't have >>>>>>>> to >>>>>>>> remember which tests are supposed to run under which conditions. >>>>>>>> >>>>>>>> /Janne >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>> >
