You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by ty...@apache.org on 2019/04/15 18:00:21 UTC
[incubator-openwhisk] branch master updated: prevent deletion of
other s3 bucket keys on doc delete (#4437)
This is an automated email from the ASF dual-hosted git repository.
tysonnorris pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git
The following commit(s) were added to refs/heads/master by this push:
new 865fcf7 prevent deletion of other s3 bucket keys on doc delete (#4437)
865fcf7 is described below
commit 865fcf7f29fbd51d65f8273953cd6f9d56722d03
Author: tysonnorris <ty...@gmail.com>
AuthorDate: Mon Apr 15 11:00:13 2019 -0700
prevent deletion of other s3 bucket keys on doc delete (#4437)
---
.../core/database/cosmosdb/CosmosDBArtifactStore.scala | 2 +-
.../openwhisk/core/database/s3/S3AttachmentStore.scala | 17 +++++++++++++----
.../core/database/test/AttachmentStoreBehaviors.scala | 13 +++++++++++++
3 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/database/cosmosdb/CosmosDBArtifactStore.scala
index c833ae6..09f3bd6 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/database/cosmosdb/CosmosDBArtifactStore.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/database/cosmosdb/CosmosDBArtifactStore.scala
@@ -78,7 +78,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected
this,
s"Initializing CosmosDBArtifactStore for collection [$collName]. Service endpoint [${client.getServiceEndpoint}], " +
s"Read endpoint [${client.getReadEndpoint}], Write endpoint [${client.getWriteEndpoint}], Connection Policy [${client.getConnectionPolicy}], " +
- s"Time to live [${collection.getDefaultTimeToLive} secs, clusterId [${config.clusterId}], soft delete TTL [${config.softDeleteTTL}]")
+ s"Time to live [${collection.getDefaultTimeToLive} secs, clusterId [${config.clusterId}], soft delete TTL [${config.softDeleteTTL}], Consistency Level [${config.consistencyLevel}]")
//Clone the returned instance as these are mutable
def documentCollection(): DocumentCollection = new DocumentCollection(collection.toJson)
diff --git a/common/scala/src/main/scala/org/apache/openwhisk/core/database/s3/S3AttachmentStore.scala b/common/scala/src/main/scala/org/apache/openwhisk/core/database/s3/S3AttachmentStore.scala
index bc786b4..d1f675f 100644
--- a/common/scala/src/main/scala/org/apache/openwhisk/core/database/s3/S3AttachmentStore.scala
+++ b/common/scala/src/main/scala/org/apache/openwhisk/core/database/s3/S3AttachmentStore.scala
@@ -18,6 +18,7 @@
package org.apache.openwhisk.core.database.s3
import akka.actor.ActorSystem
+import akka.event.Logging
import akka.http.scaladsl.model.ContentType
import akka.stream.ActorMaterializer
import akka.stream.alpakka.s3.scaladsl.S3Client
@@ -125,7 +126,8 @@ class S3AttachmentStore(client: S3Client, bucket: String, prefix: String)(implic
.finished(
this,
start,
- s"[ATT_GET] '$prefix', retrieving attachment '$name' of document 'id: $docId'; not found.")
+ s"[ATT_GET] '$prefix', retrieving attachment '$name' of document 'id: $docId'; not found.",
+ logLevel = Logging.ErrorLevel)
NoDocumentException("Not found on 'readAttachment'.")
case e => e
})
@@ -139,14 +141,20 @@ class S3AttachmentStore(client: S3Client, bucket: String, prefix: String)(implic
override protected[core] def deleteAttachments(docId: DocId)(implicit transid: TransactionId): Future[Boolean] = {
val start =
- transid.started(this, DATABASE_ATTS_DELETE, s"[ATT_DELETE] deleting attachments of document 'id: $docId'")
+ transid.started(
+ this,
+ DATABASE_ATTS_DELETE,
+ s"[ATT_DELETE] deleting attachments of document 'id: $docId' with prefix ${objectKeyPrefix(docId)}")
//S3 provides API to delete multiple objects in single call however alpakka client
//currently does not support that and also in current usage 1 docs has at most 1 attachment
//so current approach would also involve 2 remote calls
val f = client
.listBucket(bucket, Some(objectKeyPrefix(docId)))
- .mapAsync(1)(bc => client.deleteObject(bc.bucketName, bc.key))
+ .mapAsync(1) { bc =>
+ logging.info(this, s"[ATT_DELETE] deleting attachment '${bc.key}' of document 'id: $docId'")
+ client.deleteObject(bc.bucketName, bc.key)
+ }
.runWith(Sink.seq)
.map(_ => true)
@@ -181,7 +189,8 @@ class S3AttachmentStore(client: S3Client, bucket: String, prefix: String)(implic
private def objectKey(id: DocId, name: String): String = s"$prefix/${id.id}/$name"
- private def objectKeyPrefix(id: DocId): String = s"$prefix/${id.id}"
+ private def objectKeyPrefix(id: DocId): String =
+ s"$prefix/${id.id}/" //must end with a slash so that ".../<package>/<action>other" does not match for "<package>/<action>"
private def isMissingKeyException(e: Throwable): Boolean = {
//In some case S3Exception is a sub cause. So need to recurse
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/database/test/AttachmentStoreBehaviors.scala b/tests/src/test/scala/org/apache/openwhisk/core/database/test/AttachmentStoreBehaviors.scala
index 0ef1ded..9caabe6 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/database/test/AttachmentStoreBehaviors.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/database/test/AttachmentStoreBehaviors.scala
@@ -83,17 +83,26 @@ trait AttachmentStoreBehaviors
val b3 = randomBytes(3000)
val docId = newDocId()
+ //create another doc with similar name to verify it is unaffected by deletes of the first docs attachments
+ val docId2 = DocId(docId.id + "2")
val r1 = store.attach(docId, "c1", ContentTypes.`application/octet-stream`, chunkedSource(b1)).futureValue
val r2 = store.attach(docId, "c2", ContentTypes.`application/json`, chunkedSource(b2)).futureValue
val r3 = store.attach(docId, "c3", ContentTypes.`application/json`, chunkedSource(b3)).futureValue
+ //create attachments for the other doc
+ val r21 = store.attach(docId2, "c21", ContentTypes.`application/octet-stream`, chunkedSource(b1)).futureValue
+ val r22 = store.attach(docId2, "c22", ContentTypes.`application/json`, chunkedSource(b2)).futureValue
r1.length shouldBe 1000
r2.length shouldBe 2000
r3.length shouldBe 3000
+ r21.length shouldBe 1000
+ r22.length shouldBe 2000
attachmentBytes(docId, "c1").futureValue.result() shouldBe ByteString(b1)
attachmentBytes(docId, "c2").futureValue.result() shouldBe ByteString(b2)
attachmentBytes(docId, "c3").futureValue.result() shouldBe ByteString(b3)
+ attachmentBytes(docId2, "c21").futureValue.result() shouldBe ByteString(b1)
+ attachmentBytes(docId2, "c22").futureValue.result() shouldBe ByteString(b2)
//Delete single attachment
store.deleteAttachment(docId, "c1").futureValue shouldBe true
@@ -108,6 +117,10 @@ trait AttachmentStoreBehaviors
attachmentBytes(docId, "c2").failed.futureValue shouldBe a[NoDocumentException]
attachmentBytes(docId, "c3").failed.futureValue shouldBe a[NoDocumentException]
+
+ //Make sure doc2 attachments are left untouched
+ attachmentBytes(docId2, "c21").futureValue.result() shouldBe ByteString(b1)
+ attachmentBytes(docId2, "c22").futureValue.result() shouldBe ByteString(b2)
}
it should "throw NoDocumentException on reading non existing attachment" in {