Skip to content

Commit

Permalink
Merge pull request #11081 from OSGeo/backport-11026-to-release/3.10
Browse files Browse the repository at this point in the history
[Backport release/3.10] Zarr: Create(): remove created files / directories if an error occurs
  • Loading branch information
rouault authored Oct 22, 2024
2 parents 652389d + cd1797e commit 9112dcd
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 1 deletion.
45 changes: 44 additions & 1 deletion autotest/gdrivers/zarr_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import base64
import json
import math
import os
import struct
import sys

Expand Down Expand Up @@ -5532,7 +5533,7 @@ def test_zarr_read_cf1_zarrv3():
@gdaltest.enable_exceptions()
def test_zarr_write_partial_blocks_compressed(tmp_vsimem):

out_filename = "/vsimem/test.zarr"
out_filename = str(tmp_vsimem / "test.zarr")
src_ds = gdal.Open("data/small_world.tif")
gdal.Translate(
out_filename,
Expand All @@ -5541,3 +5542,45 @@ def test_zarr_write_partial_blocks_compressed(tmp_vsimem):
)
out_ds = gdal.Open(out_filename)
assert out_ds.ReadRaster() == src_ds.ReadRaster()


###############################################################################
# Test bug fix for https://github.com/OSGeo/gdal/issues/11023


@gdaltest.enable_exceptions()
@pytest.mark.parametrize("format", ["ZARR_V2", "ZARR_V3"])
def test_zarr_write_cleanup_create_dir_if_bad_blocksize(tmp_path, format):

out_dirname = str(tmp_path / "test.zarr")
with pytest.raises(Exception):
gdal.Translate(
out_dirname,
"data/byte.tif",
options=f"-of ZARR -co FORMAT={format} -co BLOCKSIZE=1,20,20",
)
assert not os.path.exists(out_dirname)


###############################################################################
# Test bug fix for https://github.com/OSGeo/gdal/issues/11023


@gdaltest.enable_exceptions()
@pytest.mark.parametrize("format", ["ZARR_V2", "ZARR_V3"])
def test_zarr_write_cleanup_create_dir_if_bad_blocksize_append_subdataset(
tmp_path, format
):

out_dirname = str(tmp_path / "test.zarr")
gdal.Translate(out_dirname, "data/byte.tif", format="ZARR")
assert os.path.exists(out_dirname)
with pytest.raises(Exception):
gdal.Translate(
out_dirname,
"data/utm.tif",
options=f"-of ZARR -co APPEND_SUBDATASET=YES -co FORMAT={format} -co ARRAY_NAME=other -co BLOCKSIZE=1,20,20",
)
assert os.path.exists(out_dirname)
ds = gdal.Open(out_dirname)
assert ds.GetRasterBand(1).Checksum() == 4672
73 changes: 73 additions & 0 deletions frmts/zarr/zarrdriver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,10 @@ GDALDataset *ZarrDataset::Create(const char *pszName, int nXSize, int nYSize,
int nBandsIn, GDALDataType eType,
char **papszOptions)
{
// To avoid any issue with short-lived string that would be passed to us
const std::string osName = pszName;
pszName = osName.c_str();

if (nBandsIn <= 0 || nXSize <= 0 || nYSize <= 0)
{
CPLError(CE_Failure, CPLE_NotSupported,
Expand Down Expand Up @@ -1028,6 +1032,22 @@ GDALDataset *ZarrDataset::Create(const char *pszName, int nXSize, int nYSize,
}
else
{
VSIStatBufL sStat;
const bool bExists = VSIStatL(pszName, &sStat) == 0;
const bool bIsFile = bExists && !VSI_ISDIR(sStat.st_mode);
const bool bIsDirectory =
!bIsFile && ((bExists && VSI_ISDIR(sStat.st_mode)) ||
!CPLStringList(VSIReadDirEx(pszName, 1)).empty());
if (bIsFile || bIsDirectory || bExists)
{
CPLError(CE_Failure, CPLE_FileIO, "%s %s already exists.",
bIsFile ? "File"
: bIsDirectory ? "Directory"
: "Object",
pszName);
return nullptr;
}

const char *pszFormat =
CSLFetchNameValueDef(papszOptions, "FORMAT", "ZARR_V2");
auto poSharedResource =
Expand Down Expand Up @@ -1059,6 +1079,50 @@ GDALDataset *ZarrDataset::Create(const char *pszName, int nXSize, int nYSize,
poDS->nRasterXSize = nXSize;
poDS->eAccess = GA_Update;

const auto CleanupCreatedFiles =
[bAppendSubDS, pszName, pszArrayName, &poRG, &poDS]()
{
// Make sure all objects are released so that ZarrSharedResource
// is finalized and all files are serialized.
poRG.reset();
poDS.reset();

if (bAppendSubDS)
{
VSIRmdir(CPLFormFilename(pszName, pszArrayName, nullptr));
}
else
{
// Be a bit careful before wiping too much stuff...
// At most 5 files expected for ZARR_V2: .zgroup, .zmetadata,
// one (empty) subdir, . and ..
// and for ZARR_V3: zarr.json, one (empty) subdir, . and ..
const CPLStringList aosFiles(VSIReadDirEx(pszName, 6));
if (aosFiles.size() < 6)
{
for (const char *pszFile : aosFiles)
{
if (pszArrayName && strcmp(pszFile, pszArrayName) == 0)
{
VSIRmdir(CPLFormFilename(pszName, pszFile, nullptr));
}
else if (!pszArrayName &&
strcmp(pszFile, CPLGetBasename(pszName)) == 0)
{
VSIRmdir(CPLFormFilename(pszName, pszFile, nullptr));
}
else if (strcmp(pszFile, ".zgroup") == 0 ||
strcmp(pszFile, ".zmetadata") == 0 ||
strcmp(pszFile, "zarr.json") == 0)
{
VSIUnlink(CPLFormFilename(pszName, pszFile, nullptr));
}
}
VSIRmdir(pszName);
}
}
};

if (bAppendSubDS)
{
auto aoDims = poRG->GetDimensions();
Expand Down Expand Up @@ -1096,7 +1160,10 @@ GDALDataset *ZarrDataset::Create(const char *pszName, int nXSize, int nYSize,
poRG->CreateDimension("X", std::string(), std::string(), nXSize);
}
if (poDS->m_poDimY == nullptr || poDS->m_poDimX == nullptr)
{
CleanupCreatedFiles();
return nullptr;
}

const bool bSingleArray =
CPLTestBool(CSLFetchNameValueDef(papszOptions, "SINGLE_ARRAY", "YES"));
Expand All @@ -1123,7 +1190,10 @@ GDALDataset *ZarrDataset::Create(const char *pszName, int nXSize, int nYSize,
pszNonNullArrayName, apoDims, GDALExtendedDataType::Create(eType),
papszOptions);
if (!poDS->m_poSingleArray)
{
CleanupCreatedFiles();
return nullptr;
}
poDS->SetMetadataItem("INTERLEAVE", bBandInterleave ? "BAND" : "PIXEL",
"IMAGE_STRUCTURE");
for (int i = 0; i < nBandsIn; i++)
Expand All @@ -1145,7 +1215,10 @@ GDALDataset *ZarrDataset::Create(const char *pszName, int nXSize, int nYSize,
: CPLSPrintf("Band%d", i + 1),
apoDims, GDALExtendedDataType::Create(eType), papszOptions);
if (poArray == nullptr)
{
CleanupCreatedFiles();
return nullptr;
}
poDS->SetBand(i + 1, new ZarrRasterBand(poArray));
}
}
Expand Down

0 comments on commit 9112dcd

Please sign in to comment.