So that is similar to the original TreeMap based implementation (which stored features based on featureId order).
-- Jody Garnett On 2 November 2015 at 23:18, Frank Gasdorf <[email protected]> wrote: > 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. >>> >>> ------------------------------------------------------- >>> >> >> > > _______________________________________________ > 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 >
------------------------------------------------------------------------------
_______________________________________________ GeoTools-Devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
