googleapis/java-storage-nio

Google Storage NIO: FileAlreadyExistsException exception is not honoured when overwriting files

pditommaso opened this issue · 7 comments

Thanks for stopping by to let us know something could be better!

PLEASE READ: If you have a support contract with Google, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response.

Environment details

  1. Specify the API at the beginning of the title. For example, "BigQuery: ...").
    General, Core, and Other are also allowed as types
  2. OS type and version: any
  3. Java version: Java 8 and later
  4. storage_nio version(s): com.google.cloud:google-cloud-nio:0.122.0 and later

Steps to reproduce

  1. Create a file foo.txt into my-bucket
  2. Create a file bar.txt into my-bucket
  3. Copy foo.txt to bar.txt

Code example

        given:
        def foo = CloudStorageFileSystem.forBucket('nf-bucket').getPath('foo.txt')
        def bar = CloudStorageFileSystem.forBucket('nf-bucket').getPath('bar.txt')
        and:
        Files.createFile(foo)
        Files.createFile(bar)
        when:
        Files.copy(foo, bar)
        then:
        thrown(FileAlreadyExistsException)

Stack trace

com.google.cloud.storage.StorageException: 412 Precondition Failed
PUT https://storage.googleapis.com/upload/storage/v1/b/nf-bucket/o?uploadType=resumable&name=foo.txt&ifGenerationMatch=0&upload_id=ADPycdtER9Udv5f3mdmhElr4rbAf-0U0L59G6pHmMw92d6SUtppicKL0slMnCdx8JhX1_2MPZo9MBUph1Q9NqQXuUjIUVoCeHQ
{
  "error": {
    "code": 412,
    "message": "At least one of the pre-conditions you specified did not hold.",
    "errors": [
      {
        "message": "At least one of the pre-conditions you specified did not hold.",
        "domain": "global",
        "reason": "conditionNotMet",
        "locationType": "header",
        "location": "If-Match"
      }
    ]
  }
}


	at com.google.cloud.storage.spi.v1.HttpStorageRpc.translate(HttpStorageRpc.java:231)
	at com.google.cloud.storage.spi.v1.HttpStorageRpc.writeWithResponse(HttpStorageRpc.java:822)
	at com.google.cloud.storage.BlobWriteChannel$1.run(BlobWriteChannel.java:69)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at com.google.api.gax.retrying.DirectRetryingExecutor.submit(DirectRetryingExecutor.java:105)
	at com.google.cloud.RetryHelper.run(RetryHelper.java:76)
	at com.google.cloud.RetryHelper.runWithRetries(RetryHelper.java:50)
	at com.google.cloud.storage.BlobWriteChannel.flushBuffer(BlobWriteChannel.java:61)
	at com.google.cloud.BaseWriteChannel.close(BaseWriteChannel.java:151)
	at com.google.cloud.storage.contrib.nio.CloudStorageWriteChannel.close(CloudStorageWriteChannel.java:55)
	at java.nio.file.Files.createFile(Files.java:632)

External references such as API reference guides

Any additional information below

This work as expected up to version 0.121.2

Hi @pditommaso,

The error you're receiving is an optimistic copy protection to try and help prevent accidentally overwriting any data.

In this case, since you do in fact want to overwrite you'll need to add an option to your Files.copy call

    Files.copy(foo, bar, java.nio.file.StandardCopyOption.REPLACE_EXISTING);

Adding that option will ensure the request that is sent is tolerant of an overwrite happening.

Hi Ben, thanks for the swift reply. Yes, I've realised that however, it would not make sense to honour the Files.copy contract and throws a FileAlreadyExistsException instead of a custom exception when the file already exists and replace flag is not specified?

Thank you for clarifying, I missed that the first time around.

According to the definition of Files.copy you're correct and I think it makes sense. I'm not super familiar with the internal design of the errors and exception for this library. I'll have to check with some folks and see if there is anything that would prevent us from making the change.

I was finally able to get consensus from several folks and determined we're okay moving forward adding the special handling. Unfortunately, I can't yet give a specific timeframe for when the fix will be implemented and released.

We'll provide an update when we know more time wise.

Sorry for the long delay on this @pditommaso, the fix has just been merged and will go out in the next release (I would guess next week some time).

Nice, thanks a lot. For the records, is the target release number already know (so I can keep an eye on it)?

I expect that it'll release in v 0.123.21. You can follow #816 for release status updates. Our robots will comment and update that issues after it's merged and the artifacts have been pushed to maven central.