diff --git a/CHANGES.md b/CHANGES.md index a0a27bc3..e18053bc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,42 +1,67 @@ # Changes ## Unreleased -* **Breaking**: Upgrade to `ndarray 0.15` - * -* Implement wrapper for `OGR_L_TestCapability` - * -* **Breaking**: Use `DatasetOptions` to pass as `Dataset::open_ex` parameters and - add support for extended open flags. - - ```rust - use gdal::{ Dataset, DatasetOptions } - - let dataset = Dataset::open_ex( - "roads.geojson", - DatasetOptions { - open_flags: GdalOpenFlags::GDAL_OF_UPDATE|GdalOpenFlags::GDAL_OF_VECTOR, - ..DatasetOptions::default() - } - ) - .unwrap(); - ``` - - `GDALAccess` values are supported usinf [`From`] implementation - - ```rust - Dataset::open_ex( - "roads.geojson", - DatasetOptions { - open_flags: GDALAccess::GA_Update.into(), - ..DatasetOptions::default() - }, - ) - .unwrap(); - ``` - -* Add more functions to SpatialRef implementation - * -* **Breaking**: Change `Feature::field` return type from + +- **Breaking**: Make `Layer::features` iterator reset to + beginning, and borrow mutably. + + - closes + +- **Breaking**: [Enforce borrow + semantics](https://github.com/georust/gdal/pull/161) on + methods of `Dataset`, `RasterBand`, and `Layer`. + + 1. Methods that do not modify the underlying structure take `&self`. + 1. Methods that modify the underlying structure take `&mut self`. + + ```rust + let ds = Dataset::open(...); + + // ds need not be mutable to open layer + let mut band = ds.rasterband(1)?; + + // band needs to be mutable to set no-data value + band.set_no_data_value(0.0)?; + ``` + +- **Breaking**: Upgrade to `ndarray 0.15` + - +- Implement wrapper for `OGR_L_TestCapability` + + - + +- **Breaking**: Use `DatasetOptions` to pass as `Dataset::open_ex` parameters and + add support for extended open flags. + + ```rust + use gdal::{ Dataset, DatasetOptions } + + let dataset = Dataset::open_ex( + "roads.geojson", + DatasetOptions { + open_flags: GdalOpenFlags::GDAL_OF_UPDATE|GdalOpenFlags::GDAL_OF_VECTOR, + ..DatasetOptions::default() + } + ) + .unwrap(); + ``` + + `GDALAccess` values are supported using [`From`] implementation + + ```rust + Dataset::open_ex( + "roads.geojson", + DatasetOptions { + open_flags: GDALAccess::GA_Update.into(), + ..DatasetOptions::default() + }, + ) + .unwrap(); + ``` + +- Add more functions to SpatialRef implementation + - +- **Breaking**: Change `Feature::field` return type from `Result` to `Result>`. Fields can be null. Before this change, if a field was null, the value returned was the default value for the underlying type. @@ -66,96 +91,101 @@ .into_string() .unwrap(); ``` - * -* Fixed potential race condition wrt. GDAL driver initialization - * -* Add basic support to read overviews -* Added a `Dataset::build_overviews` method - * -* BREAKING: update geo-types to 0.7.0. geo-types Coordinate now implement `Debug` - * -* Deprecated `SpatialRef::get_axis_mapping_strategy` - migrate to + + - + +- Fixed potential race condition wrt. GDAL driver initialization + - +- Add basic support to read overviews +- Added a `Dataset::build_overviews` method + - +- BREAKING: update geo-types to 0.7.0. geo-types Coordinate now implement `Debug` + - +- Deprecated `SpatialRef::get_axis_mapping_strategy` - migrate to `SpatialRef::axis_mapping_strategy` instead. -* Add support for reading and setting rasterband colour interpretations - * -* Fixed memory leak in `Geometry::from_wkt` - * +- Add support for reading and setting rasterband colour interpretations + - +- Fixed memory leak in `Geometry::from_wkt` + - ## 0.7.1 -* fix docs.rs build for gdal-sys - * + +- fix docs.rs build for gdal-sys + - ## 0.6.0 - 0.7.0 -* Dataset layer iteration and FieldValue types - * https://github.com/georust/gdal/pull/126 -* Fix i8 ptr instead of c_char ptr passed to OSRImportFromESRI() - * -* Rename spatial_reference to spatial_ref - * -* Replace get_extent force flag by get_extent and try_get_extent - * -* Add support for transactions on datasets - * -* Add feature_count{,_force} and implement Iterator::size_hint - * -* Replace failure with thiserror - * -* Ability to read into preallocated slice for rasterband - * -* Datasets are Send (requires GDAL >= 2.3) - * -* User GDALOpenEx - * -* GDAL 2.0 conform structure / drop GDAL 1.x - * -* Inplace functions use mutable refs - * -* Detect GDAL version at build time / remove version features - * -* Add support for delaunay_triangulation and simplify functions - * -* Add support for 3d points - * -* Additional metadata retrieval options - * -* Support for GDAL 3 in CI - * -* Support for Integer64 - * -* Geometry Intersection trait - * -* Rust 2018 - * -* support for date and time fields - * -* Prebuild bindings - * -* Support for ndarray - * + +- Dataset layer iteration and FieldValue types + - https://github.com/georust/gdal/pull/126 +- Fix i8 ptr instead of c_char ptr passed to OSRImportFromESRI() + - +- Rename spatial_reference to spatial_ref + - +- Replace get_extent force flag by get_extent and try_get_extent + - +- Add support for transactions on datasets + - +- Add feature_count{,\_force} and implement Iterator::size_hint + - +- Replace failure with thiserror + - +- Ability to read into preallocated slice for rasterband + - +- Datasets are Send (requires GDAL >= 2.3) + - +- User GDALOpenEx + - +- GDAL 2.0 conform structure / drop GDAL 1.x + - +- Inplace functions use mutable refs + - +- Detect GDAL version at build time / remove version features + - +- Add support for delaunay_triangulation and simplify functions + - +- Add support for 3d points + - +- Additional metadata retrieval options + - +- Support for GDAL 3 in CI + - +- Support for Integer64 + - +- Geometry Intersection trait + - +- Rust 2018 + - +- support for date and time fields + - +- Prebuild bindings + - +- Support for ndarray + - ## 0.5.0 -* [Bump geo-types from 0.3 -> 0.4](https://github.com/georust/gdal/pull/71) -* [Allow reading block-size of Rasters](https://github.com/georust/gdal/pull/67) -* [Add prebuilt-bindings GDAL 2.3 and GDAL 2.4](https://github.com/georust/gdal/pull/69) -* [Make GdalType trait public](https://github.com/georust/gdal/pull/66) -* [RasterBand to Ndarray, with failure](https://github.com/georust/gdal/pull/68) +- [Bump geo-types from 0.3 -> 0.4](https://github.com/georust/gdal/pull/71) +- [Allow reading block-size of Rasters](https://github.com/georust/gdal/pull/67) +- [Add prebuilt-bindings GDAL 2.3 and GDAL 2.4](https://github.com/georust/gdal/pull/69) +- [Make GdalType trait public](https://github.com/georust/gdal/pull/66) +- [RasterBand to Ndarray, with failure](https://github.com/georust/gdal/pull/68) ## 0.4.0 -* [Migrate to the `geo-types` crate](https://github.com/georust/gdal/pull/60) -* [Replace `error-chain` with `failure`](https://github.com/georust/gdal/pull/58) -* [Use `bindgen` to generate the low-level bindings](https://github.com/georust/gdal/pull/55) + +- [Migrate to the `geo-types` crate](https://github.com/georust/gdal/pull/60) +- [Replace `error-chain` with `failure`](https://github.com/georust/gdal/pull/58) +- [Use `bindgen` to generate the low-level bindings](https://github.com/georust/gdal/pull/55) ## 0.3.0 -* [Add support for creating a SpatialRef from a esri "wkt" definition](https://github.com/georust/gdal/pull/37) -* [Travis now uses GDAL 2.x](https://github.com/georust/gdal/pull/36) -* [API extensions](https://github.com/georust/gdal/pull/35) -* [Extend the existing possibilities of writing ogr datasets](https://github.com/georust/gdal/pull/31) -* [Allow to transform ogr geometries to other SRS](https://github.com/georust/gdal/pull/29) -* [Move ffi into a seperate crate](https://github.com/georust/gdal/pull/26) -* [Added rasterband.rs and moved all band functions](https://github.com/georust/gdal/pull/24) +- [Add support for creating a SpatialRef from a esri "wkt" definition](https://github.com/georust/gdal/pull/37) +- [Travis now uses GDAL 2.x](https://github.com/georust/gdal/pull/36) +- [API extensions](https://github.com/georust/gdal/pull/35) +- [Extend the existing possibilities of writing ogr datasets](https://github.com/georust/gdal/pull/31) +- [Allow to transform ogr geometries to other SRS](https://github.com/georust/gdal/pull/29) +- [Move ffi into a seperate crate](https://github.com/georust/gdal/pull/26) +- [Added rasterband.rs and moved all band functions](https://github.com/georust/gdal/pull/24) ## 0.2.1 -* [First version of metadata handling](https://github.com/georust/gdal/pull/21) +- [First version of metadata handling](https://github.com/georust/gdal/pull/21) diff --git a/Cargo.toml b/Cargo.toml index 015170e9..8c6878b0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,6 +31,7 @@ semver = "0.11" [dev-dependencies] tempfile = "3.1" +trybuild = "1.0.39" [workspace] members = ["gdal-sys"] diff --git a/examples/read_write_ogr.rs b/examples/read_write_ogr.rs index dd8e64a2..57f3c500 100644 --- a/examples/read_write_ogr.rs +++ b/examples/read_write_ogr.rs @@ -6,8 +6,8 @@ use std::fs; use std::path::Path; fn run() -> Result<()> { - let mut dataset_a = Dataset::open(Path::new("fixtures/roads.geojson"))?; - let layer_a = dataset_a.layer(0)?; + let dataset_a = Dataset::open(Path::new("fixtures/roads.geojson"))?; + let mut layer_a = dataset_a.layer(0)?; let fields_defn = layer_a .defn() .fields() diff --git a/examples/read_write_ogr_datetime.rs b/examples/read_write_ogr_datetime.rs index 9bcb212f..bd0943e8 100644 --- a/examples/read_write_ogr_datetime.rs +++ b/examples/read_write_ogr_datetime.rs @@ -8,8 +8,8 @@ fn run() -> gdal::errors::Result<()> { println!("gdal crate was build with datetime support"); - let mut dataset_a = Dataset::open(Path::new("fixtures/points_with_datetime.json"))?; - let layer_a = dataset_a.layer(0)?; + let dataset_a = Dataset::open(Path::new("fixtures/points_with_datetime.json"))?; + let mut layer_a = dataset_a.layer(0)?; // Create a new dataset: let path = std::env::temp_dir().join("later.geojson"); diff --git a/src/dataset.rs b/src/dataset.rs index 46d2abdd..3ef59d24 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -253,7 +253,7 @@ impl Dataset { } /// Set the projection reference string for this dataset. - pub fn set_projection(&self, projection: &str) -> Result<()> { + pub fn set_projection(&mut self, projection: &str) -> Result<()> { let c_projection = CString::new(projection)?; unsafe { gdal_sys::GDALSetProjection(self.c_dataset, c_projection.as_ptr()) }; Ok(()) @@ -267,7 +267,7 @@ impl Dataset { #[cfg(major_ge_3)] /// Set the spatial reference system for this dataset. - pub fn set_spatial_ref(&self, spatial_ref: &SpatialRef) -> Result<()> { + pub fn set_spatial_ref(&mut self, spatial_ref: &SpatialRef) -> Result<()> { let rv = unsafe { gdal_sys::GDALSetSpatialRef(self.c_dataset, spatial_ref.to_c_hsrs()) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); @@ -362,7 +362,7 @@ impl Dataset { /// /// Applies to vector datasets, and fetches by the given /// _0-based_ index. - pub fn layer(&mut self, idx: isize) -> Result { + pub fn layer(&self, idx: isize) -> Result { let c_layer = unsafe { gdal_sys::OGR_DS_GetLayer(self.c_dataset, idx as c_int) }; if c_layer.is_null() { return Err(_last_null_pointer_err("OGR_DS_GetLayer")); @@ -371,7 +371,7 @@ impl Dataset { } /// Fetch a layer by name. - pub fn layer_by_name(&mut self, name: &str) -> Result { + pub fn layer_by_name(&self, name: &str) -> Result { let c_name = CString::new(name)?; let c_layer = unsafe { gdal_sys::OGR_DS_GetLayerByName(self.c_dataset(), c_name.as_ptr()) }; if c_layer.is_null() { @@ -437,7 +437,7 @@ impl Dataset { /// y-coordinate of the top-left corner pixel /// column rotation (typically zero) /// height of a pixel (y-resolution, typically negative) - pub fn set_geo_transform(&self, transformation: &GeoTransform) -> Result<()> { + pub fn set_geo_transform(&mut self, transformation: &GeoTransform) -> Result<()> { assert_eq!(transformation.len(), 6); let rv = unsafe { gdal_sys::GDALSetGeoTransform(self.c_dataset, transformation.as_ptr() as *mut f64) @@ -567,7 +567,7 @@ impl Dataset { /// /// let ds = Dataset::open(Path::new("fixtures/roads.geojson")).unwrap(); /// let query = "SELECT kind, is_bridge, highway FROM roads WHERE highway = 'pedestrian'"; - /// let result_set = ds.execute_sql(query, None, sql::Dialect::DEFAULT).unwrap().unwrap(); + /// let mut result_set = ds.execute_sql(query, None, sql::Dialect::DEFAULT).unwrap().unwrap(); /// /// assert_eq!(10, result_set.feature_count()); /// diff --git a/src/lib.rs b/src/lib.rs index 11017979..8dbb7875 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,8 +8,8 @@ //! use std::path::Path; //! use gdal::Dataset; //! -//! let mut dataset = Dataset::open(Path::new("fixtures/roads.geojson")).unwrap(); -//! let layer = dataset.layer(0).unwrap(); +//! let dataset = Dataset::open(Path::new("fixtures/roads.geojson")).unwrap(); +//! let mut layer = dataset.layer(0).unwrap(); //! for feature in layer.features() { //! let highway_field = feature.field("highway").unwrap().unwrap(); //! let geometry = feature.geometry(); diff --git a/src/raster/rasterband.rs b/src/raster/rasterband.rs index c227810d..303f67ba 100644 --- a/src/raster/rasterband.rs +++ b/src/raster/rasterband.rs @@ -209,7 +209,7 @@ impl<'a> RasterBand<'a> { /// * window - the window position from top left /// * window_size - the window size (GDAL will interpolate data if window_size != Buffer.size) pub fn write( - &self, + &mut self, window: (isize, isize), window_size: (usize, usize), buffer: &Buffer, @@ -254,7 +254,7 @@ impl<'a> RasterBand<'a> { } /// Set the no data value of this band. - pub fn set_no_data_value(&self, no_data: f64) -> Result<()> { + pub fn set_no_data_value(&mut self, no_data: f64) -> Result<()> { let rv = unsafe { gdal_sys::GDALSetRasterNoDataValue(self.c_rasterband, no_data) }; if rv != CPLErr::CE_None { return Err(_last_cpl_err(rv)); diff --git a/src/raster/tests.rs b/src/raster/tests.rs index 0ade16c9..c77d724d 100644 --- a/src/raster/tests.rs +++ b/src/raster/tests.rs @@ -96,7 +96,7 @@ fn test_write_raster() { }; // epand it to fill the image (20x10) - let rb = dataset.rasterband(1).unwrap(); + let mut rb = dataset.rasterband(1).unwrap(); let res = rb.write((0, 0), (20, 10), &raster); @@ -218,7 +218,7 @@ fn test_create_copy() { #[allow(clippy::float_cmp)] fn test_geo_transform() { let driver = Driver::get("MEM").unwrap(); - let dataset = driver.create("", 20, 10, 1).unwrap(); + let mut dataset = driver.create("", 20, 10, 1).unwrap(); let transform = [0., 1., 0., 0., 0., 1.]; assert!(dataset.set_geo_transform(&transform).is_ok()); assert_eq!(dataset.geo_transform().unwrap(), transform); @@ -360,7 +360,7 @@ fn test_get_no_data_value() { fn test_set_no_data_value() { let driver = Driver::get("MEM").unwrap(); let dataset = driver.create("", 20, 10, 1).unwrap(); - let rasterband = dataset.rasterband(1).unwrap(); + let mut rasterband = dataset.rasterband(1).unwrap(); assert_eq!(rasterband.no_data_value(), None); assert!(rasterband.set_no_data_value(1.23).is_ok()); assert_eq!(rasterband.no_data_value(), Some(1.23)); diff --git a/src/vector/layer.rs b/src/vector/layer.rs index f41abece..70649159 100644 --- a/src/vector/layer.rs +++ b/src/vector/layer.rs @@ -87,8 +87,8 @@ impl LayerCaps { /// use std::path::Path; /// use gdal::Dataset; /// -/// let mut dataset = Dataset::open(Path::new("fixtures/roads.geojson")).unwrap(); -/// let layer = dataset.layer(0).unwrap(); +/// let dataset = Dataset::open(Path::new("fixtures/roads.geojson")).unwrap(); +/// let mut layer = dataset.layer(0).unwrap(); /// for feature in layer.features() { /// // do something with each feature /// } @@ -138,10 +138,6 @@ impl<'a> Layer<'a> { /// Not all drivers support this efficiently; however, the call should always work if the /// feature exists, as a fallback implementation just scans all the features in the layer /// looking for the desired feature. - /// - /// **Important**: do not call `feature` while iterating over features using the - /// [`features`](Layer::features) function. Even if you don't mutate anything, this can cause - /// the the iterator to get interrupted. This is a limitation of the GDAL library. pub fn feature(&self, fid: u64) -> Option { let c_feature = unsafe { gdal_sys::OGR_L_GetFeature(self.c_layer, fid as i64) }; if c_feature.is_null() { @@ -151,16 +147,25 @@ impl<'a> Layer<'a> { } } - /// Iterate over all features in this layer. - pub fn features(&self) -> FeatureIterator { + /// Returns iterator over the features in this layer. + /// + /// **Note.** This method resets the current index to + /// the beginning before iteration. It also borrows the + /// layer mutably, preventing any overlapping borrows. + pub fn features(&mut self) -> FeatureIterator { + self.reset_feature_reading(); FeatureIterator::_with_layer(self) } - pub fn set_spatial_filter(&self, geometry: &Geometry) { + /// Set a spatial filter on this layer. + /// + /// Refer [OGR_L_SetSpatialFilter](https://gdal.org/doxygen/classOGRLayer.html#a75c06b4993f8eb76b569f37365cd19ab) + pub fn set_spatial_filter(&mut self, geometry: &Geometry) { unsafe { gdal_sys::OGR_L_SetSpatialFilter(self.c_layer, geometry.c_geometry()) }; } - pub fn clear_spatial_filter(&self) { + /// Clear spatial filters set on this layer. + pub fn clear_spatial_filter(&mut self) { unsafe { gdal_sys::OGR_L_SetSpatialFilter(self.c_layer, null_mut()) }; } @@ -309,6 +314,9 @@ impl<'a> Layer<'a> { } } + /// Fetch the spatial reference system for this layer. + /// + /// Refer [OGR_L_GetSpatialRef](https://gdal.org/doxygen/classOGRLayer.html#a75c06b4993f8eb76b569f37365cd19ab) pub fn spatial_ref(&self) -> Result { let c_obj = unsafe { gdal_sys::OGR_L_GetSpatialRef(self.c_layer) }; if c_obj.is_null() { @@ -316,10 +324,18 @@ impl<'a> Layer<'a> { } SpatialRef::from_c_obj(c_obj) } + + fn reset_feature_reading(&mut self) { + unsafe { + gdal_sys::OGR_L_ResetReading(self.c_layer); + } + } } pub struct FeatureIterator<'a> { - layer: &'a Layer<'a>, + defn: &'a Defn, + c_layer: OGRLayerH, + size_hint: Option, } impl<'a> Iterator for FeatureIterator<'a> { @@ -327,21 +343,16 @@ impl<'a> Iterator for FeatureIterator<'a> { #[inline] fn next(&mut self) -> Option> { - let c_feature = unsafe { gdal_sys::OGR_L_GetNextFeature(self.layer.c_layer) }; + let c_feature = unsafe { gdal_sys::OGR_L_GetNextFeature(self.c_layer) }; if c_feature.is_null() { None } else { - Some(unsafe { Feature::from_c_feature(self.layer.defn(), c_feature) }) + Some(unsafe { Feature::from_c_feature(self.defn, c_feature) }) } } fn size_hint(&self) -> (usize, Option) { - match self - .layer - .try_feature_count() - .map(|s| s.try_into().ok()) - .flatten() - { + match self.size_hint { Some(size) => (size, Some(size)), None => (0, None), } @@ -350,7 +361,16 @@ impl<'a> Iterator for FeatureIterator<'a> { impl<'a> FeatureIterator<'a> { pub fn _with_layer(layer: &'a Layer) -> FeatureIterator<'a> { - FeatureIterator { layer } + let defn = layer.defn(); + let size_hint = layer + .try_feature_count() + .map(|s| s.try_into().ok()) + .flatten(); + FeatureIterator { + c_layer: layer.c_layer, + size_hint, + defn, + } } } diff --git a/src/vector/mod.rs b/src/vector/mod.rs index 71b25221..3dbe64e8 100644 --- a/src/vector/mod.rs +++ b/src/vector/mod.rs @@ -6,8 +6,8 @@ //! use std::path::Path; //! use gdal::Dataset; //! -//! let mut dataset = Dataset::open(Path::new("fixtures/roads.geojson")).unwrap(); -//! let layer = dataset.layer(0).unwrap(); +//! let dataset = Dataset::open(Path::new("fixtures/roads.geojson")).unwrap(); +//! let mut layer = dataset.layer(0).unwrap(); //! for feature in layer.features() { //! let highway_field = feature.field("highway").unwrap().unwrap(); //! let geometry = feature.geometry(); diff --git a/src/vector/sql.rs b/src/vector/sql.rs index decd23d5..ef7d717d 100644 --- a/src/vector/sql.rs +++ b/src/vector/sql.rs @@ -1,4 +1,4 @@ -use std::ops::Deref; +use std::ops::{Deref, DerefMut}; use gdal_sys::GDALDatasetH; @@ -21,6 +21,12 @@ impl<'a> Deref for ResultSet<'a> { } } +impl<'a> DerefMut for ResultSet<'a> { + fn deref_mut(&mut self) -> &mut ::Target { + &mut self.layer + } +} + impl<'a> Drop for ResultSet<'a> { fn drop(&mut self) { unsafe { gdal_sys::GDALDatasetReleaseResultSet(self.dataset, self.layer.c_layer()) }; diff --git a/src/vector/vector_tests/mod.rs b/src/vector/vector_tests/mod.rs index 04961749..2231a13f 100644 --- a/src/vector/vector_tests/mod.rs +++ b/src/vector/vector_tests/mod.rs @@ -41,7 +41,7 @@ fn test_many_layer_count() { #[test] fn test_layer_get_extent() { - let mut ds = Dataset::open(fixture!("roads.geojson")).unwrap(); + let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); let layer = ds.layer(0).unwrap(); let extent = layer.get_extent().unwrap(); assert_almost_eq(extent.MinX, 26.100768); @@ -52,14 +52,14 @@ fn test_layer_get_extent() { #[test] fn test_layer_try_get_extent() { - let mut ds = Dataset::open(fixture!("roads.geojson")).unwrap(); + let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); let layer = ds.layer(0).unwrap(); assert!(layer.try_get_extent().unwrap().is_none()); } #[test] fn test_layer_spatial_ref() { - let mut ds = Dataset::open(fixture!("roads.geojson")).unwrap(); + let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); let layer = ds.layer(0).unwrap(); let srs = layer.spatial_ref().unwrap(); assert_eq!(srs.auth_code().unwrap(), 4326); @@ -67,7 +67,7 @@ fn test_layer_spatial_ref() { #[test] fn test_layer_capabilities() { - let mut ds = Dataset::open(fixture!("roads.geojson")).unwrap(); + let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); let layer = ds.layer(0).unwrap(); assert!(!layer.has_capability(OLCFastSpatialFilter)); @@ -81,7 +81,7 @@ fn ds_with_layer(ds_name: &str, layer_name: &str, f: F) where F: Fn(Layer), { - let mut ds = Dataset::open(fixture!(ds_name)).unwrap(); + let ds = Dataset::open(fixture!(ds_name)).unwrap(); let layer = ds.layer_by_name(layer_name).unwrap(); f(layer); } @@ -90,7 +90,7 @@ fn with_layer(name: &str, f: F) where F: Fn(Layer), { - let mut ds = Dataset::open(fixture!(name)).unwrap(); + let ds = Dataset::open(fixture!(name)).unwrap(); let layer = ds.layer(0).unwrap(); f(layer); } @@ -99,7 +99,7 @@ fn with_features(name: &str, f: F) where F: Fn(FeatureIterator), { - with_layer(name, |layer| f(layer.features())); + with_layer(name, |mut layer| f(layer.features())); } fn with_feature(name: &str, fid: u64, f: F) @@ -219,7 +219,7 @@ mod tests { #[test] fn test_field_in_layer() { - ds_with_layer("three_layer_ds.s3db", "layer_0", |layer| { + ds_with_layer("three_layer_ds.s3db", "layer_0", |mut layer| { let feature = layer.features().next().unwrap(); assert_eq!(feature.field("id").unwrap(), None); }); @@ -337,7 +337,7 @@ mod tests { #[test] fn test_schema() { - let mut ds = Dataset::open(fixture!("roads.geojson")).unwrap(); + let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); let layer = ds.layer(0).unwrap(); // The layer name is "roads" in GDAL 2.2 assert!(layer.name() == "OGRGeoJSON" || layer.name() == "roads"); @@ -363,7 +363,7 @@ mod tests { #[test] fn test_geom_fields() { - let mut ds = Dataset::open(fixture!("roads.geojson")).unwrap(); + let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); let layer = ds.layer(0).unwrap(); let name_list = layer .defn() @@ -386,7 +386,7 @@ mod tests { #[test] fn test_get_layer_by_name() { - let mut ds = Dataset::open(fixture!("roads.geojson")).unwrap(); + let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); // The layer name is "roads" in GDAL 2.2 if let Ok(layer) = ds.layer_by_name("OGRGeoJSON") { assert_eq!(layer.name(), "OGRGeoJSON"); @@ -404,8 +404,8 @@ mod tests { #[test] fn test_spatial_filter() { - let mut ds = Dataset::open(fixture!("roads.geojson")).unwrap(); - let layer = ds.layer(0).unwrap(); + let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); + let mut layer = ds.layer(0).unwrap(); assert_eq!(layer.features().count(), 21); let bbox = Geometry::bbox(26.1017, 44.4297, 26.1025, 44.4303).unwrap(); @@ -493,9 +493,9 @@ mod tests { // dataset is closed here } - let mut ds = Dataset::open(fixture!("output.geojson")).unwrap(); + let ds = Dataset::open(fixture!("output.geojson")).unwrap(); fs::remove_file(fixture!("output.geojson")).unwrap(); - let layer = ds.layer(0).unwrap(); + let mut layer = ds.layer(0).unwrap(); let ft = layer.features().next().unwrap(); assert_eq!(ft.geometry().wkt().unwrap(), "POINT (1 2)"); assert_eq!( @@ -505,4 +505,30 @@ mod tests { assert_eq!(ft.field("Value").unwrap().unwrap().into_real(), Some(45.78)); assert_eq!(ft.field("Int_value").unwrap().unwrap().into_int(), Some(1)); } + + #[test] + fn test_features_reset() { + with_layer("roads.geojson", |mut layer| { + assert_eq!(layer.features().count(), layer.features().count(),); + }); + } + + #[test] + fn test_features_aliasing_compile_fail() { + let t = trybuild::TestCases::new(); // A compilation test that should fail. + t.compile_fail("tests/compile-tests/01-features-aliasing-errors.rs"); + } + + // It tries to iterate over a layer's features, while + // also trying to read the layer's definition. The + // features iterator borrows layer as a mutable ref as + // it increments the internal state of the layer. + // + // fn test_features_mut_lifetime_enforce() { + // with_layer("roads.geojson", |mut layer| { + // for _ in layer.features() { + // let _ = layer.defn(); + // } + // }); + // } } diff --git a/src/vector/vector_tests/sql.rs b/src/vector/vector_tests/sql.rs index 82fffc51..34e0d198 100644 --- a/src/vector/vector_tests/sql.rs +++ b/src/vector/vector_tests/sql.rs @@ -10,7 +10,7 @@ use crate::{ fn test_sql() { let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); let query = "SELECT kind, is_bridge, highway FROM roads WHERE highway = 'pedestrian'"; - let result_set = ds + let mut result_set = ds .execute_sql(query, None, sql::Dialect::DEFAULT) .unwrap() .unwrap(); @@ -46,7 +46,7 @@ fn test_sql_with_spatial_filter() { let query = "SELECT * FROM roads WHERE highway = 'pedestrian'"; let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); let bbox = Geometry::bbox(26.1017, 44.4297, 26.1025, 44.4303).unwrap(); - let result_set = ds + let mut result_set = ds .execute_sql(query, Some(&bbox), sql::Dialect::DEFAULT) .unwrap() .unwrap(); @@ -77,7 +77,7 @@ fn test_sql_with_dialect() { let query = "SELECT * FROM roads WHERE highway = 'pedestrian' and NumPoints(GEOMETRY) = 3"; let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); let bbox = Geometry::bbox(26.1017, 44.4297, 26.1025, 44.4303).unwrap(); - let result_set = ds + let mut result_set = ds .execute_sql(query, Some(&bbox), sql::Dialect::SQLITE) .unwrap() .unwrap(); @@ -99,7 +99,7 @@ fn test_sql_with_dialect() { fn test_sql_empty_result() { let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); let query = "SELECT kind, is_bridge, highway FROM roads WHERE highway = 'jazz hands 👐'"; - let result_set = ds + let mut result_set = ds .execute_sql(query, None, sql::Dialect::DEFAULT) .unwrap() .unwrap(); diff --git a/tests/compile-tests/01-features-aliasing-errors.rs b/tests/compile-tests/01-features-aliasing-errors.rs new file mode 100644 index 00000000..eb531f12 --- /dev/null +++ b/tests/compile-tests/01-features-aliasing-errors.rs @@ -0,0 +1,11 @@ +mod utils; + +use gdal::Dataset; + +fn main() { + let ds = Dataset::open(fixture!("roads.geojson")).unwrap(); + let mut layer = ds.layer(0).unwrap(); + for _ in layer.features() { + let _ = layer.defn(); + } +} diff --git a/tests/compile-tests/01-features-aliasing-errors.stderr b/tests/compile-tests/01-features-aliasing-errors.stderr new file mode 100644 index 00000000..016a7356 --- /dev/null +++ b/tests/compile-tests/01-features-aliasing-errors.stderr @@ -0,0 +1,10 @@ +error[E0502]: cannot borrow `layer` as immutable because it is also borrowed as mutable + --> $DIR/01-features-aliasing-errors.rs:9:17 + | +8 | for _ in layer.features() { + | ---------------- + | | + | mutable borrow occurs here + | mutable borrow later used here +9 | let _ = layer.defn(); + | ^^^^^ immutable borrow occurs here diff --git a/tests/compile-tests/utils.rs b/tests/compile-tests/utils.rs new file mode 100644 index 00000000..29e6368a --- /dev/null +++ b/tests/compile-tests/utils.rs @@ -0,0 +1,14 @@ +#[macro_export] +macro_rules! fixture { + ($name:expr) => { + std::path::Path::new(file!()) + .parent() + .unwrap() + .parent() + .unwrap() + .join("fixtures") + .as_path() + .join($name) + .as_path() + }; +}