Comments fix, code update to correspond latest SAPI version

Comments fix
Changed sapi::StatusOr to absl::StatusOr
Code changes according to new CStr(absl::string_view) constructor

oss-internship-2020/gdal/raster_to_gtiff/CMakeLists.txt

@@ -69,7 +69,7 @@ add_sapi_library(gdal_sapi
 
     INPUTS "${GDAL_HEADER_PREFIX}/gdal.h"
     LIBRARY libgdal
-    LIBRARY_NAME gdal
+    LIBRARY_NAME Gdal
     
     NAMESPACE "gdal::sandbox"
 )

oss-internship-2020/gdal/raster_to_gtiff/README.md

@@ -79,14 +79,16 @@ PROJ uses `proj.db` database to work correctly with different transformations an
 
 You can use environment variables to set path to proj:
 ```
-export PROJ_PATH=/path/to/proj.db
+export PROJ_DB_PATH=/path/to/proj.db
 ```
 The code will check this variable and if it represents a valid file it will be mounted inside the sandbox.
 Alternatively, if there is no such environment variable program will try to use the default path `/usr/local/share/proj/proj.db`.
 There is a simple command-line utility that takes path to the GeoTIFF file and absolute path to the output file as arguments, parses raster data from the input file and, re-creates the same GeoTIFF file (except some metadata) inside the sandbox.
 
 You can run it in the following way:
-`./raster_to_gtiff path/to/input.tif /absolute/path/to/output.tif`
+```
+./raster_to_gtiff path/to/input.tif /absolute/path/to/output.tif
+```
 After that, you can compare both files using the `gdalinfo` utility.
 Also, there are unit tests that automatically convert a few files and then compare input and output raster data to make sure that they are equal.
 To run tests your CMake build must use `-DENABLE_TESTS=ON`, then you can run tests using `./tests`.

oss-internship-2020/gdal/raster_to_gtiff/gdal_sandbox.h

@@ -19,19 +19,16 @@
 
 #include <string>
 
-#include "sandboxed_api/sandbox2/util/fileops.h"
-
 #include "gdal_sapi.sapi.h"  // NOLINT(build/include)
 
 namespace gdal::sandbox {
 
-class GdalSapiSandbox : public gdalSandbox {
+class GdalSapiSandbox : public GdalSandbox {
  public:
   GdalSapiSandbox(std::string out_directory_path,
                   std::string proj_db_path, 
                   time_t time_limit = 0)
-    : gdalSandbox(),
+    : out_directory_path_(std::move(out_directory_path)),
-      out_directory_path_(std::move(out_directory_path)),
       proj_db_path_(std::move(proj_db_path))
     {
       SetWallTimeLimit(time_limit).IgnoreError();

oss-internship-2020/gdal/raster_to_gtiff/get_raster_data.cc

@@ -65,8 +65,7 @@ RasterDataset GetRasterBandsFromFile(const std::string& filename) {
     int data_type  = static_cast<int>(GDALGetRasterDataType(band));
     int color_interp = static_cast<int>(GDALGetRasterColorInterpretation(band));
 
-    std::vector<int> band_raster_data;
+    std::vector<int32_t> band_raster_data(width * height);
-    band_raster_data.resize(width * height);
 
     // GDALRasterIO with GF_Write should use the same type (GDT_Int32)
     GDALRasterIO(band, GF_Read, 0, 0, width, height, band_raster_data.data(),

oss-internship-2020/gdal/raster_to_gtiff/gtiff_converter.cc

@@ -14,6 +14,8 @@
 
 #include "gtiff_converter.h"  // NOLINT(build/include)
 
+#include "sandboxed_api/sandbox2/util/fileops.h"
+
 namespace gdal::sandbox {
 
 namespace {
@@ -23,12 +25,12 @@ namespace {
 }  // namespace
 
 RasterToGTiffProcessor::RasterToGTiffProcessor(std::string out_file_full_path, 
-                                                std::string out_file_folder,
                                                 std::string proj_db_path,
                                                 parser::RasterDataset data,
                                                 int retry_count) 
   : sapi::Transaction(std::make_unique<GdalSapiSandbox>(
-      std::move(out_file_folder), std::move(proj_db_path))),
+      sandbox2::file_util::fileops::StripBasename(out_file_full_path), 
+      std::move(proj_db_path))),
     out_file_full_path_(std::move(out_file_full_path)),
     data_(std::move(data))
 {
@@ -37,12 +39,12 @@ RasterToGTiffProcessor::RasterToGTiffProcessor(std::string out_file_full_path,
 }
 
 absl::Status RasterToGTiffProcessor::Main() {
-  gdalApi api(sandbox());
+  GdalApi api(sandbox());
   SAPI_RETURN_IF_ERROR(api.GDALAllRegister());
 
-  sapi::v::ConstCStr driver_name_ptr(kDriverName.data());
+  sapi::v::CStr driver_name_ptr(kDriverName);
 
-  SAPI_ASSIGN_OR_RETURN(sapi::StatusOr<GDALDriverH> driver, 
+  SAPI_ASSIGN_OR_RETURN(absl::StatusOr<GDALDriverH> driver, 
       api.GDALGetDriverByName(driver_name_ptr.PtrBefore()));
 
   TRANSACTION_FAIL_IF_NOT(driver.value() != nullptr, 
@@ -56,7 +58,7 @@ absl::Status RasterToGTiffProcessor::Main() {
     static_cast<GDALDataType>(data_.bands[0].data_type)
       : GDALDataType::GDT_Unknown;
 
-  SAPI_ASSIGN_OR_RETURN(sapi::StatusOr<GDALDatasetH> dataset, 
+  SAPI_ASSIGN_OR_RETURN(absl::StatusOr<GDALDatasetH> dataset, 
       api.GDALCreate(&driver_ptr, out_file_full_path_ptr.PtrBefore(), 
         data_.width, data_.height, data_.bands.size(), type, 
         &create_options));
@@ -65,44 +67,47 @@ absl::Status RasterToGTiffProcessor::Main() {
       "Error creating dataset");  
   sapi::v::RemotePtr dataset_ptr(dataset.value());
 
-  for (int i = 0; i < data_.bands.size(); ++i) {
+  int current_band = 1;
-    SAPI_ASSIGN_OR_RETURN(sapi::StatusOr<GDALRasterBandH> band,
+  for (auto& band_data : data_.bands) {
-        api.GDALGetRasterBand(&dataset_ptr, i + 1));
+    SAPI_ASSIGN_OR_RETURN(absl::StatusOr<GDALRasterBandH> band,
+        api.GDALGetRasterBand(&dataset_ptr, current_band));
     TRANSACTION_FAIL_IF_NOT(band.value() != nullptr, 
         "Error getting band from dataset");
     sapi::v::RemotePtr band_ptr(band.value());
 
-    sapi::v::Array<int> data_array(data_.bands[i].data.data(), 
+    sapi::v::Array<int> data_array(band_data.data.data(), 
-        data_.bands[i].data.size());
+        band_data.data.size());
 
-    SAPI_ASSIGN_OR_RETURN(sapi::StatusOr<CPLErr> result, 
+    SAPI_ASSIGN_OR_RETURN(absl::StatusOr<CPLErr> result, 
         api.GDALRasterIO(&band_ptr, GF_Write, 0, 0,
-          data_.bands[i].width, data_.bands[i].height, data_array.PtrBefore(),
+          band_data.width, band_data.height, data_array.PtrBefore(),
-          data_.bands[i].width, data_.bands[i].height, GDT_Int32, 0, 0));
+          band_data.width, band_data.height, GDT_Int32, 0, 0));
 
     TRANSACTION_FAIL_IF_NOT(result.value() == CPLErr::CE_None, 
         "Error writing band to dataset");
 
     SAPI_ASSIGN_OR_RETURN(result, api.GDALSetRasterColorInterpretation(
         &band_ptr, static_cast<GDALColorInterp>(
-              data_.bands[i].color_interp)));
+              band_data.color_interp)));
 
     TRANSACTION_FAIL_IF_NOT(result.value() == CPLErr::CE_None,
         "Error setting color interpretation");
 
-    if (data_.bands[i].no_data_value.has_value()) {
+    if (band_data.no_data_value.has_value()) {
       SAPI_ASSIGN_OR_RETURN(result, 
           api.GDALSetRasterNoDataValue(&band_ptr, 
-            data_.bands[i].no_data_value.value()));
+            band_data.no_data_value.value()));
 
       TRANSACTION_FAIL_IF_NOT(result.value() == CPLErr::CE_None, 
           "Error setting no data value for the band");
     }
+
+    ++current_band;
   }
 
   if (data_.wkt_projection.length() > 0) {
     sapi::v::ConstCStr wkt_projection_ptr(data_.wkt_projection.c_str());
-    SAPI_ASSIGN_OR_RETURN(sapi::StatusOr<CPLErr> result, 
+    SAPI_ASSIGN_OR_RETURN(absl::StatusOr<CPLErr> result, 
         api.GDALSetProjection(&dataset_ptr, wkt_projection_ptr.PtrBefore())); 
     TRANSACTION_FAIL_IF_NOT(result.value() == CPLErr::CE_None,
         "Error setting wkt projection");
@@ -111,7 +116,7 @@ absl::Status RasterToGTiffProcessor::Main() {
   if (data_.geo_transform.size() > 0) {
     sapi::v::Array<double> geo_transform_ptr(data_.geo_transform.data(), 
         data_.geo_transform.size());
-    SAPI_ASSIGN_OR_RETURN(sapi::StatusOr<CPLErr> result, 
+    SAPI_ASSIGN_OR_RETURN(absl::StatusOr<CPLErr> result, 
         api.GDALSetGeoTransform(&dataset_ptr, geo_transform_ptr.PtrBefore()));
 
     TRANSACTION_FAIL_IF_NOT(result.value() == CPLErr::CE_None, 

oss-internship-2020/gdal/raster_to_gtiff/gtiff_converter.h

@@ -27,7 +27,6 @@ namespace gdal::sandbox {
 class RasterToGTiffProcessor : public sapi::Transaction {
  public:
   RasterToGTiffProcessor(std::string out_file_full_path, 
-                          std::string out_file_folder,
                           std::string proj_db_path,
                           parser::RasterDataset data,
                           int retry_count = 0);

oss-internship-2020/gdal/raster_to_gtiff/raster_to_gtiff.cc

@@ -17,6 +17,9 @@
 #include <optional>
 #include <string>
 
+#include "sandboxed_api/sandbox2/util/fileops.h"
+#include "sandboxed_api/sandbox2/util/path.h"
+
 #include "get_raster_data.h"  // NOLINT(build/include)
 #include "gtiff_converter.h"  // NOLINT(build/include)
 #include "utils.h"  // NOLINT(build/include)
@@ -25,23 +28,15 @@ namespace {
 
 absl::Status SaveToGTiff(gdal::sandbox::parser::RasterDataset bands_data, 
     std::string out_file) {
-  using namespace gdal::sandbox;
+  std::optional<std::string> proj_db_path = 
-
+      gdal::sandbox::utils::FindProjDbPath();
-  std::string output_data_folder = "";
-
-  if (!sandbox2::file_util::fileops::RemoveLastPathComponent(out_file,
-      &output_data_folder)) {
-    return absl::FailedPreconditionError("Error getting output file directory");
-  }
-
-  std::optional<std::string> proj_db_path = utils::FindProjDbPath();
 
   if (proj_db_path == std::nullopt) {
     return absl::FailedPreconditionError("Specified proj.db does not exist");
   }
 
-  RasterToGTiffProcessor processor(out_file, output_data_folder, 
+  gdal::sandbox::RasterToGTiffProcessor processor(std::move(out_file), 
-                                    proj_db_path.value(), bands_data);
+      std::move(proj_db_path.value()), std::move(bands_data));
 
   return processor.Run();
 }
@@ -58,7 +53,7 @@ void Usage() {
 int main(int argc, char** argv) {
   using namespace gdal::sandbox;
 
-  if (argc < 3 || !gdal::sandbox::utils::IsAbsolute(argv[2])) {
+  if (argc < 3 || !sandbox2::file::IsAbsolutePath(argv[2])) {
     Usage();
     return EXIT_FAILURE;
   }

oss-internship-2020/gdal/raster_to_gtiff/tests.cc

@@ -16,14 +16,13 @@
 #include <string>
 
 #include "gtest/gtest.h"
+#include "sandboxed_api/sandbox2/util/fileops.h"
 
 #include "gdal_sandbox.h"  // NOLINT(build/include)
 #include "get_raster_data.h"  // NOLINT(build/include)
 #include "gtiff_converter.h" // NOLINT(build/include)
 #include "utils.h"  // NOLINT(build/include)
 
-namespace gdal::sandbox::tests {
-
 namespace {
 
 inline constexpr absl::string_view kTempFilePrefix = "temp_data";
@@ -41,7 +40,7 @@ class TestGTiffProcessor : public testing::TestWithParam<absl::string_view> {
   {}
 
  protected:
-  const utils::TempFile tempfile_;
+  const gdal::sandbox::utils::TempFile tempfile_;
 };
 
 TEST_P(TestGTiffProcessor, TestProcessorOnGTiffData) {
@@ -50,23 +49,23 @@ TEST_P(TestGTiffProcessor, TestProcessorOnGTiffData) {
   ASSERT_TRUE(tempfile_.HasValue()) 
       << "Error creating temporary output file";
 
-  parser::RasterDataset original_bands_data = 
+  gdal::sandbox::parser::RasterDataset original_bands_data = 
-      parser::GetRasterBandsFromFile(filename);
+      gdal::sandbox::parser::GetRasterBandsFromFile(filename);
 
-  std::optional<std::string> proj_db_path = utils::FindProjDbPath();
+  std::optional<std::string> proj_db_path = 
+      gdal::sandbox::utils::FindProjDbPath();
   ASSERT_TRUE(proj_db_path != std::nullopt)
       << "Specified proj.db does not exist";
 
-  RasterToGTiffProcessor processor(absl::StrCat(
+  gdal::sandbox::RasterToGTiffProcessor processor(absl::StrCat(
           sandbox2::file_util::fileops::GetCWD(), "/", tempfile_.GetPath()),
-      sandbox2::file_util::fileops::GetCWD(), std::move(proj_db_path.value()),
+      std::move(proj_db_path.value()), original_bands_data);
-      original_bands_data);
   
   ASSERT_EQ(processor.Run(), absl::OkStatus())
       << "Error creating new GTiff dataset inside sandbox";
 
   ASSERT_EQ(original_bands_data, 
-      parser::GetRasterBandsFromFile(tempfile_.GetPath()))
+      gdal::sandbox::parser::GetRasterBandsFromFile(tempfile_.GetPath()))
       << "New dataset doesn't match the original one";
 }
 
@@ -77,5 +76,3 @@ INSTANTIATE_TEST_CASE_P(
                           kSecondTestDataPath
                           )
 );
-
-}  // namespace gdal::sandbox::tests

oss-internship-2020/gdal/raster_to_gtiff/utils.cc

@@ -23,7 +23,7 @@ namespace gdal::sandbox::utils {
 
 namespace {
 
-  inline constexpr absl::string_view kProjDbEnvVariableName = "PROJ_PATH";
+  constexpr char kProjDbEnvVariableName[] = "PROJ_DB_PATH";
   inline constexpr absl::string_view kDefaultProjDbPath 
       = "/usr/local/share/proj/proj.db";
 
@@ -56,10 +56,12 @@ std::string TempFile::GetPath() const {
 }
 
 std::optional<std::string> FindProjDbPath() {
-  const char* proj_db_path_ptr = std::getenv(kProjDbEnvVariableName.data());
+  std::string proj_db_path(kDefaultProjDbPath);
   
-  std::string proj_db_path = proj_db_path_ptr == nullptr ? 
+  if (const char* proj_db_env_var = std::getenv(kProjDbEnvVariableName); 
-      std::string(kDefaultProjDbPath) : std::string(proj_db_path_ptr);
+      proj_db_env_var != nullptr) {
+    proj_db_path = proj_db_env_var;
+  }
 
   if (!sandbox2::file_util::fileops::Exists(proj_db_path, false)) {
     return std::nullopt;
@@ -68,8 +70,4 @@ std::optional<std::string> FindProjDbPath() {
   return proj_db_path;
 }
 
-bool IsAbsolute(absl::string_view path) {
-  return path.length() > 0 && path.at(0) == '/';
-}
-
 }  // namespace gdal::sandbox::utils

oss-internship-2020/gdal/raster_to_gtiff/utils.h

@@ -42,7 +42,6 @@ class TempFile {
 
 // Helper function to retrieve potential proj.db path from environment variable
 std::optional<std::string> FindProjDbPath();
-bool IsAbsolute(absl::string_view path);
 
 }  // namespace gdal::sandbox::utils