I analysed and debugged a bit and found the following explanation: * depending on the constructor of ConcurrentSkipListMap the natural ordering is defined by the keys of the map (Feature-Ids) * Iterator of values() and entrySet() "returns the values in ascending order of the corresponding keys"
which brakes the code. For now I'm reverting the merge commit and looking for a better solution. - Frank 2015-11-02 21:53 GMT+01:00 Frank Gasdorf <[email protected]>: > Interessting and Sure, going to have a look at. Wondering because I > refactored to preserve ordering, tests in GeoTools are fine. > > 2015-11-02 21:17 GMT+01:00 Andrea Aime <[email protected]>: > >> Hi Jody and Frank, >> it looks like the changes to MemoryDataStore are breaking tests in >> GeoServer: >> >> Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.652 sec >> <<< FAILURE! >> testEscapes(org.geoserver.wfs.response.CSVOutputFormatTest) Time >> elapsed: 501 sec <<< FAILURE! >> org.junit.ComparisonFailure: expected:<A l[abel with "quotes"]> but >> was:<A l[ong label >> with windows >> newlines]> >> at org.junit.Assert.assertEquals(Assert.java:115) >> at org.junit.Assert.assertEquals(Assert.java:144) >> at >> org.geoserver.wfs.response.CSVOutputFormatTest.testEscapes(CSVOutputFormatTest.java:118) >> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) >> at >> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) >> at >> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) >> at java.lang.reflect.Method.invoke(Method.java:606) >> at >> org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47) >> at >> org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) >> at >> org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44) >> at >> org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) >> at >> org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26) >> at >> org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27) >> at org.junit.rules.RunRules.evaluate(RunRules.java:20) >> at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271) >> at >> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70) >> at >> org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50) >> at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238) >> at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63) >> at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236) >> at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53) >> at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229) >> at >> org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26) >> at >> org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27) >> at org.junit.runners.ParentRunner.run(ParentRunner.java:309) >> at >> org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252) >> at >> org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141) >> at >> org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112) >> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) >> at >> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) >> at >> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) >> at java.lang.reflect.Method.invoke(Method.java:606) >> at >> org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189) >> at >> org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165) >> at >> org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85) >> at >> org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115) >> at >> org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75) >> >> The test in question is indeed using a MemoryDataStore. I checked which >> other parts of GeoServer tests >> might be affected, came up with the following list (only including what's >> in supported land): >> >> > find . -name "*.java" | xargs grep MemoryDataStore >> ./wms/src/test/java/org/geoserver/wms/svg/SVGMapProducerTest.java:import >> org.geotools.data.memory.MemoryDataStore; >> ./wms/src/test/java/org/geoserver/wms/svg/SVGMapProducerTest.java: >> MemoryDataStore ds = new MemoryDataStore(); >> ./wms/src/test/java/org/geoserver/wms/WMSMockData.java:import >> org.geotools.data.memory.MemoryDataStore; >> ./wms/src/test/java/org/geoserver/wms/WMSMockData.java: private >> MemoryDataStore dataStore; >> ./wms/src/test/java/org/geoserver/wms/WMSMockData.java: dataStore >> = new MemoryDataStore(); >> ./wms/src/test/java/org/geoserver/wms/WMSMockData.java: >> dataStore.setNamespaceURI("http://geoserver.org";); // required for >> GeoTools 12 implemetnation of MemoryDataStore >> ./wms/src/test/java/org/geoserver/wms/WMSMockData.java: * Creates a >> vector layer with associated FeatureType in the internal MemoryDataStore >> with the >> ./wms/src/main/java/org/geoserver/wms/map/GetMapXmlReader.java:import >> org.geotools.data.memory.MemoryDataStore; >> ./wms/src/main/java/org/geoserver/wms/map/GetMapXmlReader.java: >> MemoryDataStore reTypedDS = new MemoryDataStore(reader); >> ./wms/src/main/java/org/geoserver/wms/map/GetMapKvpRequestReader.java:import >> org.geotools.data.memory.MemoryDataStore; >> ./wms/src/main/java/org/geoserver/wms/map/GetMapKvpRequestReader.java: >> MemoryDataStore reTypedDS = new MemoryDataStore(new >> ForceCoordinateSystemFeatureReader( >> ./web/demo/src/main/java/org/geoserver/web/crs/CRSAreaOfValidityMapBuilder.java:import >> org.geotools.data.memory.MemoryDataStore; >> ./web/demo/src/main/java/org/geoserver/web/crs/CRSAreaOfValidityMapBuilder.java: >> MemoryDataStore ds = new MemoryDataStore(); >> ./wfs/src/test/java/org/geoserver/wfs/response/CSVOutputFormatTest.java:import >> org.geotools.data.memory.MemoryDataStore; >> ./wfs/src/test/java/org/geoserver/wfs/response/CSVOutputFormatTest.java: >> MemoryDataStore data = new MemoryDataStore(); >> >> Could you have a look? >> >> Cheers >> Andrea >> >> >> On Sat, Oct 31, 2015 at 1:26 PM, Frank Gasdorf < >> [email protected]> wrote: >> >>> Hello folks, >>> >>> I updated the initial pull request. Additional to the internal >>> ConcurrentSkipListMap usage I synched the access to internal HashMaps >>> (memory >>> & schema objects) to avoid issues with concurrent access (createSchema, >>> removeSchema (see pull https://github.com/geotools/geotools/pull/1022) >>> and addFeatures and features() methodes) >>> >>> Do you have any comments or requests for change on the outstanding pull >>> request https://github.com/geotools/geotools/pull/1002 >>> >>> Hope anybody of you can have a look at while I start working on the pull >>> to backport it into 14.x >>> >>> Thanks a lot >>> >>> - Frank >>> >>> >>> 2015-09-13 9:09 GMT+02:00 Andrea Aime <[email protected]>: >>> >>>> On Thu, Sep 10, 2015 at 11:13 PM, Jody Garnett <[email protected]> >>>> wrote: >>>> >>>>> Since its not an "Ordered" DataStore that should preserve order I'd >>>>>> suggest to remove these tests. Opinions? >>>>>> >>>>> >>>>> If you change the functionality, change the test to match. There >>>>> should now be several feature collection implementations to choose from so >>>>> code that requires a TreeSet based functionality has a migration path. >>>>> >>>>> I'm wondering how you would think about this "proposal". However, >>>>>> STRtree internal creates a new List every type the Index is requested. I >>>>>> haven't investigated thread savety of STRtree yet. Maybe anybody of you >>>>>> has >>>>>> much more experiance than me to give feedback on that >>>>>> >>>>> >>>>> The index needs to get rebuilt each time a feature is modified, so >>>>> other than that overhead (which could be pushed off until "commit" ) it >>>>> should be workable. The List should not be a problem, as it offers you a >>>>> bit of independence (at the cost of possibly traversing through out of >>>>> date >>>>> information). >>>>> >>>>> >>>> One coud use a quadtree index instead, it can be modified without >>>> rebuilding it, but it's more prone to degeration to linked list, >>>> the STRTree is packed and optimized for balancing, but as you say, it's >>>> a pain if stuff gets modified. >>>> I don't think any of the JTS indexes is thread safe. >>>> >>>> Cheers >>>> Andrea >>>> >>>> -- >>>> == >>>> GeoServer Professional Services from the experts! Visit >>>> http://goo.gl/it488V for more information. >>>> == >>>> >>>> Ing. Andrea Aime >>>> @geowolf >>>> Technical Lead >>>> >>>> GeoSolutions S.A.S. >>>> Via Poggio alle Viti 1187 >>>> 55054 Massarosa (LU) >>>> Italy >>>> phone: +39 0584 962313 >>>> fax: +39 0584 1660272 >>>> mob: +39 339 8844549 >>>> >>>> http://www.geo-solutions.it >>>> http://twitter.com/geosolutions_it >>>> >>>> *AVVERTENZE AI SENSI DEL D.Lgs. 196/2003* >>>> >>>> Le informazioni contenute in questo messaggio di posta elettronica e/o >>>> nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il >>>> loro utilizzo è consentito esclusivamente al destinatario del messaggio, >>>> per le finalità indicate nel messaggio stesso. Qualora riceviate questo >>>> messaggio senza esserne il destinatario, Vi preghiamo cortesemente di >>>> darcene notizia via e-mail e di procedere alla distruzione del messaggio >>>> stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso, >>>> divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od >>>> utilizzarlo per finalità diverse, costituisce comportamento contrario ai >>>> principi dettati dal D.Lgs. 196/2003. >>>> >>>> >>>> >>>> The information in this message and/or attachments, is intended solely >>>> for the attention and use of the named addressee(s) and may be confidential >>>> or proprietary in nature or covered by the provisions of privacy act >>>> (Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection >>>> Code).Any use not in accord with its purpose, any disclosure, reproduction, >>>> copying, distribution, or either dissemination, either whole or partial, is >>>> strictly forbidden except previous formal approval of the named >>>> addressee(s). If you are not the intended recipient, please contact >>>> immediately the sender by telephone, fax or e-mail and delete the >>>> information in this message that has been received in error. The sender >>>> does not give any warranty or accept liability as the content, accuracy or >>>> completeness of sent messages and accepts no responsibility for changes >>>> made after they were sent or for other risks which arise as a result of >>>> e-mail transmission, viruses, etc. >>>> >>>> ------------------------------------------------------- >>>> >>> >>> >>> _______________________________________________ >>> udig-dev mailing list >>> [email protected] >>> To change your delivery options, retrieve your password, or unsubscribe >>> from this list, visit >>> https://locationtech.org/mailman/listinfo/udig-dev >>> >> >> >> >> -- >> == >> GeoServer Professional Services from the experts! Visit >> http://goo.gl/it488V for more information. >> == >> >> Ing. Andrea Aime >> @geowolf >> Technical Lead >> >> GeoSolutions S.A.S. >> Via Poggio alle Viti 1187 >> 55054 Massarosa (LU) >> Italy >> phone: +39 0584 962313 >> fax: +39 0584 1660272 >> mob: +39 339 8844549 >> >> http://www.geo-solutions.it >> http://twitter.com/geosolutions_it >> >> *AVVERTENZE AI SENSI DEL D.Lgs. 196/2003* >> >> Le informazioni contenute in questo messaggio di posta elettronica e/o >> nel/i file/s allegato/i sono da considerarsi strettamente riservate. Il >> loro utilizzo è consentito esclusivamente al destinatario del messaggio, >> per le finalità indicate nel messaggio stesso. Qualora riceviate questo >> messaggio senza esserne il destinatario, Vi preghiamo cortesemente di >> darcene notizia via e-mail e di procedere alla distruzione del messaggio >> stesso, cancellandolo dal Vostro sistema. Conservare il messaggio stesso, >> divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo, od >> utilizzarlo per finalità diverse, costituisce comportamento contrario ai >> principi dettati dal D.Lgs. 196/2003. >> >> >> >> The information in this message and/or attachments, is intended solely >> for the attention and use of the named addressee(s) and may be confidential >> or proprietary in nature or covered by the provisions of privacy act >> (Legislative Decree June, 30 2003, no.196 - Italy's New Data Protection >> Code).Any use not in accord with its purpose, any disclosure, reproduction, >> copying, distribution, or either dissemination, either whole or partial, is >> strictly forbidden except previous formal approval of the named >> addressee(s). If you are not the intended recipient, please contact >> immediately the sender by telephone, fax or e-mail and delete the >> information in this message that has been received in error. The sender >> does not give any warranty or accept liability as the content, accuracy or >> completeness of sent messages and accepts no responsibility for changes >> made after they were sent or for other risks which arise as a result of >> e-mail transmission, viruses, etc. >> >> ------------------------------------------------------- >> > >
------------------------------------------------------------------------------
_______________________________________________ GeoTools-Devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
