From e82465d6428b8db1da50d4ff5931b98ee1fc910e Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sun, 15 Apr 2018 01:08:24 +0530 Subject: [PATCH 01/51] Initial CosmosDB implementation with CRUD support --- common/scala/build.gradle | 2 + .../main/scala/whisk/core/WhiskConfig.scala | 1 + .../cosmosdb/CosmosDBArtifactStore.scala | 225 ++++++++++++++++++ .../CosmosDBArtifactStoreProvider.scala | 111 +++++++++ .../database/cosmosdb/CosmosDBSupport.scala | 75 ++++++ .../cosmosdb/CosmosDBViewMapper.scala | 24 ++ .../database/cosmosdb/ReferenceCounted.scala | 49 ++++ .../cosmosdb/RxObservableImplicits.scala | 55 +++++ .../cosmosdb/CosmosDBArtifactStoreTests.scala | 50 ++++ .../cosmosdb/ReferenceCountedTests.scala | 87 +++++++ 10 files changed, 679 insertions(+) create mode 100644 common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala create mode 100644 common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala create mode 100644 common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala create mode 100644 common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala create mode 100644 common/scala/src/main/scala/whisk/core/database/cosmosdb/ReferenceCounted.scala create mode 100644 common/scala/src/main/scala/whisk/core/database/cosmosdb/RxObservableImplicits.scala create mode 100644 tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala create mode 100644 tests/src/test/scala/whisk/core/database/cosmosdb/ReferenceCountedTests.scala diff --git a/common/scala/build.gradle b/common/scala/build.gradle index 02e94531b96..60f1223671a 100644 --- a/common/scala/build.gradle +++ b/common/scala/build.gradle @@ -72,6 +72,8 @@ dependencies { compile 'io.zipkin.reporter2:zipkin-sender-okhttp3:2.6.1' compile 'io.zipkin.reporter2:zipkin-reporter:2.6.1' + compile 'io.reactivex:rxscala_2.11:0.26.5' + compile 'com.microsoft.azure:azure-cosmosdb:1.0.0' scoverage gradle.scoverage.deps } diff --git a/common/scala/src/main/scala/whisk/core/WhiskConfig.scala b/common/scala/src/main/scala/whisk/core/WhiskConfig.scala index be9f7b0b0a9..fd6eeec7e5b 100644 --- a/common/scala/src/main/scala/whisk/core/WhiskConfig.scala +++ b/common/scala/src/main/scala/whisk/core/WhiskConfig.scala @@ -200,6 +200,7 @@ object ConfigKeys { val buildInformation = "whisk.info" val couchdb = "whisk.couchdb" + val cosmosdb = "whisk.cosmosdb" val kafka = "whisk.kafka" val kafkaCommon = s"$kafka.common" val kafkaProducer = s"$kafka.producer" diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala new file mode 100644 index 00000000000..a9d7abf9dad --- /dev/null +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -0,0 +1,225 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import akka.actor.ActorSystem +import akka.http.scaladsl.model.{ContentType, StatusCodes} +import akka.stream.ActorMaterializer +import akka.stream.scaladsl.{Sink, Source} +import akka.util.ByteString +import com.microsoft.azure.cosmosdb.internal.Constants.Properties.{E_TAG, ID, SELF_LINK} +import com.microsoft.azure.cosmosdb.rx.AsyncDocumentClient +import com.microsoft.azure.cosmosdb.{AccessCondition, Document, DocumentClientException, RequestOptions} +import spray.json.{DefaultJsonProtocol, JsObject, JsString, JsValue, RootJsonFormat, _} +import whisk.common.{Logging, LoggingMarkers, TransactionId} +import whisk.core.database.StoreUtils.{checkDocHasRevision, deserialize, reportFailure} +import whisk.core.database._ +import whisk.core.database.cosmosdb.CosmosDBArtifactStore._computed +import whisk.core.database.cosmosdb.CosmosDBArtifactStoreProvider.DocumentClientRef +import whisk.core.entity._ +import whisk.http.Messages + +import scala.collection.JavaConverters._ +import scala.concurrent.{ExecutionContext, Future} + +object CosmosDBArtifactStore { + val _computed = "_c" +} + +class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val collName: String, + protected val config: CosmosDBConfig, + clientRef: DocumentClientRef, + documentHandler: DocumentHandler, + viewMapper: CosmosDBViewMapper)( + implicit system: ActorSystem, + val logging: Logging, + jsonFormat: RootJsonFormat[DocumentAbstraction], + materializer: ActorMaterializer, + docReader: DocumentReader) + extends ArtifactStore[DocumentAbstraction] + with DefaultJsonProtocol + with DocumentProvider + with CosmosDBSupport + with RxObservableImplicits { + + protected val client: AsyncDocumentClient = clientRef.get.client + private val (database, collection) = initialize() + + private val _id = "_id" + private val _rev = "_rev" + + override protected[core] implicit val executionContext: ExecutionContext = system.dispatcher + + override protected[database] def put(d: DocumentAbstraction)(implicit transid: TransactionId): Future[DocInfo] = { + val asJson = d.toDocumentRecord + + //TODO Batching support + val doc = toCosmosDoc(asJson) + val docinfoStr = s"id: ${doc.getId}, rev: ${doc.getETag}" + val start = transid.started(this, LoggingMarkers.DATABASE_SAVE, s"[PUT] '$collName' saving document: '$docinfoStr'") + + val o = if (doc.getETag == null) { + client.createDocument(collection.getSelfLink, doc, null, true) + } else { + client.replaceDocument(doc, matchRevOption(doc.getETag)) + } + + val f = o + .head() + .transform( + r => toDocInfo(r.getResource), { + case e: DocumentClientException + if e.getStatusCode == StatusCodes.Conflict.intValue || e.getStatusCode == StatusCodes.PreconditionFailed.intValue => + DocumentConflictException("conflict on 'put'") + case e => e + }) + + f.onFailure({ + case _: DocumentConflictException => + transid.finished(this, start, s"[PUT] '$collName', document: '$docinfoStr'; conflict.") + }) + + f.onSuccess({ + case _ => transid.finished(this, start, s"[PUT] '$collName' completed document: '$docinfoStr'") + }) + + reportFailure(f, start, failure => s"[PUT] '$collName' internal error, failure: '${failure.getMessage}'") + } + + override protected[database] def del(doc: DocInfo)(implicit transid: TransactionId): Future[Boolean] = { + checkDocHasRevision(doc) + val start = transid.started(this, LoggingMarkers.DATABASE_DELETE, s"[DEL] '$collName' deleting document: '$doc'") + val f = client + .deleteDocument(createSelfLink(doc.id.id), matchRevOption(doc.rev.rev)) + .head() + .transform( + _ => true, { + case e: DocumentClientException if e.getStatusCode == StatusCodes.NotFound.intValue => + NoDocumentException("not found on 'delete'") + case e: DocumentClientException if e.getStatusCode == StatusCodes.PreconditionFailed.intValue => + DocumentConflictException("conflict on 'delete'") + case e => e + }) + + reportFailure( + f, + start, + failure => s"[DEL] '$collName' internal error, doc: '$doc', failure: '${failure.getMessage}'") + } + + override protected[database] def get[A <: DocumentAbstraction](doc: DocInfo)(implicit transid: TransactionId, + ma: Manifest[A]): Future[A] = { + val start = transid.started(this, LoggingMarkers.DATABASE_GET, s"[GET] '$collName' finding document: '$doc'") + + require(doc != null, "doc undefined") + val f = client + .queryDocuments(collection.getSelfLink, querySpec(doc.id.id), null) + .head() + .map { fr => + val cdoc = fr.getResults.asScala.headOption.getOrElse({ + transid.finished(this, start, s"[GET] '$collName', document: '$doc'; not found.") + // for compatibility + throw NoDocumentException("not found on 'get'") + }) + + val js = toWhiskJsonDoc(cdoc) + transid.finished(this, start, s"[GET] '$collName' completed: found document '$doc'") + deserialize[A, DocumentAbstraction](doc, js) + } + .recoverWith { + case _: DeserializationException => throw DocumentUnreadable(Messages.corruptedEntity) + } + + reportFailure( + f, + start, + failure => s"[DEL] '$collName' internal error, doc: '$doc', failure: '${failure.getMessage}'") + + } + + override protected[core] def query(table: String, + startKey: List[Any], + endKey: List[Any], + skip: Int, + limit: Int, + includeDocs: Boolean, + descending: Boolean, + reduce: Boolean, + stale: StaleParameter)(implicit transid: TransactionId): Future[List[JsObject]] = + ??? + + override protected[core] def count(table: String, + startKey: List[Any], + endKey: List[Any], + skip: Int, + stale: StaleParameter)(implicit transid: TransactionId): Future[Long] = ??? + + override protected[core] def attach( + doc: DocInfo, + name: String, + contentType: ContentType, + docStream: Source[ByteString, _])(implicit transid: TransactionId): Future[DocInfo] = ??? + + override protected[core] def readAttachment[T](doc: DocInfo, name: String, sink: Sink[ByteString, Future[T]])( + implicit transid: TransactionId): Future[(ContentType, T)] = ??? + + override protected[core] def deleteAttachments[T](doc: DocInfo)(implicit transid: TransactionId): Future[Boolean] = + ??? + + override def shutdown(): Unit = clientRef.close() + + override protected[database] def get(id: DocId)(implicit transid: TransactionId): Future[Option[JsObject]] = ??? + + private def toCosmosDoc(json: JsObject): Document = { + val computed = documentHandler.computedFields(json) + val computedOpt = if (computed.fields.nonEmpty) Some(computed) else None + val fieldsToAdd = + Seq( + (ID, Some(JsString(escapeId(json.fields(_id).convertTo[String])))), + (E_TAG, json.fields.get(_rev)), + (_computed, computedOpt)) + val fieldsToRemove = Seq(_id, _rev) + val mapped = transform(json, fieldsToAdd, fieldsToRemove) + val doc = new Document(mapped.compactPrint) + doc.set(SELF_LINK, createSelfLink(doc.getId)) + doc + } + + private def toWhiskJsonDoc(doc: Document): JsObject = { + val js = doc.toJson.parseJson.asJsObject + val fieldsToAdd = Seq((_id, Some(JsString(unescapeId(doc.getId)))), (_rev, Some(JsString(doc.getETag)))) + transform(js, fieldsToAdd, Seq.empty) + } + + private def transform(json: JsObject, fieldsToAdd: Seq[(String, Option[JsValue])], fieldsToRemove: Seq[String]) = { + val fields = json.fields ++ fieldsToAdd.flatMap(f => f._2.map((f._1, _))) -- fieldsToRemove + JsObject(fields) + } + + private def toDocInfo(doc: Document) = DocInfo(DocId(doc.getId), DocRevision(doc.getETag)) + + private def createSelfLink(id: String) = s"dbs/${database.getId}/colls/${collection.getId}/docs/$id" + + private def matchRevOption(etag: String) = { + val options = new RequestOptions + val condition = new AccessCondition + condition.setCondition(etag) + options.setAccessCondition(condition) + options + } +} diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala new file mode 100644 index 00000000000..9780de77318 --- /dev/null +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import java.io.Closeable + +import akka.actor.ActorSystem +import akka.stream.ActorMaterializer +import com.microsoft.azure.cosmosdb.{ConnectionPolicy, ConsistencyLevel} +import com.microsoft.azure.cosmosdb.rx.AsyncDocumentClient +import spray.json.RootJsonFormat +import whisk.common.Logging +import whisk.core.database._ +import pureconfig._ +import whisk.core.ConfigKeys +import whisk.core.entity.{DocumentReader, WhiskActivation, WhiskAuth, WhiskEntity} + +import scala.reflect.ClassTag + +case class CosmosDBConfig(host: String, key: String, db: String) + +case class ClientHolder(client: AsyncDocumentClient) extends Closeable { + override def close(): Unit = client.close() +} + +object CosmosDBArtifactStoreProvider extends ArtifactStoreProvider { + type DocumentClientRef = ReferenceCounted[ClientHolder]#CountedReference + private lazy val config = loadConfigOrThrow[CosmosDBConfig](ConfigKeys.cosmosdb) + private var clientRef: ReferenceCounted[ClientHolder] = _ + + override def makeStore[D <: DocumentSerializer: ClassTag](useBatching: Boolean)( + implicit jsonFormat: RootJsonFormat[D], + docReader: DocumentReader, + actorSystem: ActorSystem, + logging: Logging, + materializer: ActorMaterializer): ArtifactStore[D] = { + makeStoreForClient(config, useBatching, getOrCreateReference(config)) + } + + def makeStore[D <: DocumentSerializer: ClassTag](config: CosmosDBConfig, useBatching: Boolean)( + implicit jsonFormat: RootJsonFormat[D], + docReader: DocumentReader, + actorSystem: ActorSystem, + logging: Logging, + materializer: ActorMaterializer): ArtifactStore[D] = { + + makeStoreForClient(config, useBatching, createReference(config).reference()) + } + + private def makeStoreForClient[D <: DocumentSerializer: ClassTag](config: CosmosDBConfig, + useBatching: Boolean, + clientRef: DocumentClientRef)( + implicit jsonFormat: RootJsonFormat[D], + docReader: DocumentReader, + actorSystem: ActorSystem, + logging: Logging, + materializer: ActorMaterializer): ArtifactStore[D] = { + + val classTag = implicitly[ClassTag[D]] + val (dbName, handler, viewMapper) = handlerAndMapper(classTag) + + new CosmosDBArtifactStore(dbName, config, clientRef, handler, viewMapper) + } + + private def handlerAndMapper[D](entityType: ClassTag[D])( + implicit actorSystem: ActorSystem, + logging: Logging, + materializer: ActorMaterializer): (String, DocumentHandler, CosmosDBViewMapper) = { + entityType.runtimeClass match { + case x if x == classOf[WhiskEntity] => + ("whisks", WhisksHandler, WhisksViewMapper) + case x if x == classOf[WhiskActivation] => + ("activations", ActivationHandler, ActivationViewMapper) + case x if x == classOf[WhiskAuth] => + ("subjects", SubjectHandler, SubjectViewMapper) + } + } + + private def getOrCreateReference(config: CosmosDBConfig) = synchronized { + if (clientRef == null || clientRef.isClosed) { + clientRef = createReference(config) + } + clientRef.reference() + } + + private def createReference(config: CosmosDBConfig) = + new ReferenceCounted[ClientHolder](ClientHolder(createClient(config))) + + private def createClient(config: CosmosDBConfig): AsyncDocumentClient = + new AsyncDocumentClient.Builder() + .withServiceEndpoint(config.host) + .withMasterKey(config.key) + .withConnectionPolicy(ConnectionPolicy.GetDefault) + .withConsistencyLevel(ConsistencyLevel.Session) + .build +} diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala new file mode 100644 index 00000000000..ff587f03118 --- /dev/null +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import com.microsoft.azure.cosmosdb._ +import com.microsoft.azure.cosmosdb.rx.AsyncDocumentClient +import _root_.rx.lang.scala._ +import _root_.rx.lang.scala.JavaConverters._ + +import scala.collection.JavaConverters._ + +trait CosmosDBSupport { + protected def config: CosmosDBConfig + protected def collName: String + protected def client: AsyncDocumentClient + + def initialize(): (Database, DocumentCollection) = { + val db = getOrCreateDatabase() + (db, getOrCreateCollection(db)) + } + + private def getOrCreateDatabase(): Database = { + blockingResult[Database](client.queryDatabases(querySpec(config.db), null).asScala).getOrElse { + val databaseDefinition = new Database + databaseDefinition.setId(config.db) + blockingResult(client.createDatabase(databaseDefinition, null).asScala) + } + } + + private def getOrCreateCollection(database: Database) = { + blockingResult[DocumentCollection]( + client + .queryCollections(database.getSelfLink, querySpec(collName), null) + .asScala).getOrElse { + val collectionDefinition = new DocumentCollection + collectionDefinition.setId(collName) + blockingResult(client.createCollection(database.getSelfLink, collectionDefinition, null).asScala) + } + } + + private def blockingResult[T <: Resource](response: Observable[FeedResponse[T]]) = { + val value = response.toList.toBlocking.single + value.head.getResults.asScala.headOption + } + + private def blockingResult[T <: Resource](response: Observable[ResourceResponse[T]]) = { + response.toBlocking.single.getResource + } + + protected def querySpec(id: String) = + new SqlQuerySpec("SELECT * FROM root r WHERE r.id=@id", new SqlParameterCollection(new SqlParameter("@id", id))) + + /** + * CosmosDB id considers '/', '\' , '?' and '#' as invalid. EntityNames can include '/' so + * that need to be escaped. For that we use '|' as the replacement char + */ + protected def escapeId(id: String): String = id.replace("/", "|") + + protected def unescapeId(id: String): String = id.replace("|", "/") +} diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala new file mode 100644 index 00000000000..225e68f8652 --- /dev/null +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -0,0 +1,24 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +trait CosmosDBViewMapper {} + +object WhisksViewMapper extends CosmosDBViewMapper +object ActivationViewMapper extends CosmosDBViewMapper +object SubjectViewMapper extends CosmosDBViewMapper diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/ReferenceCounted.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/ReferenceCounted.scala new file mode 100644 index 00000000000..aee6f76c375 --- /dev/null +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/ReferenceCounted.scala @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import java.util.concurrent.atomic.{AtomicBoolean, AtomicInteger} + +case class ReferenceCounted[T <: AutoCloseable](private val inner: T) { + private val count = new AtomicInteger(0) + + private def inc(): Unit = count.incrementAndGet() + + private def dec(): Unit = { + val newCount = count.decrementAndGet() + if (newCount == 0) { + inner.close() + count.decrementAndGet() + } + } + + def isClosed: Boolean = count.get() < 0 + + def reference(): CountedReference = { + require(count.get >= 0, "Reference is already closed") + new CountedReference + } + + class CountedReference extends AutoCloseable { + private val closed = new AtomicBoolean() + inc() + override def close(): Unit = if (closed.compareAndSet(false, true)) dec() + + def get: T = inner + } +} diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/RxObservableImplicits.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/RxObservableImplicits.scala new file mode 100644 index 00000000000..50166082602 --- /dev/null +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/RxObservableImplicits.scala @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import rx.lang.scala.JavaConverters._ +import rx.Observable + +import scala.concurrent.{Future, Promise} + +trait RxObservableImplicits { + + implicit class RxScalaObservable[T](observable: Observable[T]) { + + /** + * Returns the head of the [[Observable]] in a [[scala.concurrent.Future]]. + * + * @return the head result of the [[Observable]]. + */ + def head(): Future[T] = { + val promise = Promise[T]() + observable.asScala.single.subscribe(x => promise.success(x), e => promise.failure(e)) + promise.future + } + + /** + * Collects the [[Observable]] results and converts to a [[scala.concurrent.Future]]. + * + * Automatically subscribes to the `Observable` and uses the [[Observable#toList]] method to aggregate the results. + * + * @note If the Observable is large then this will consume lots of memory! + * If the underlying Observable is infinite this Observable will never complete. + * @return a future representation of the whole Observable + */ + def toFuture(): Future[Seq[T]] = { + val promise = Promise[Seq[T]]() + observable.asScala.toList.subscribe(x => promise.success(x), e => promise.failure(e)) + promise.future + } + } +} diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala new file mode 100644 index 00000000000..f1b69ae11e4 --- /dev/null +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import org.junit.runner.RunWith +import org.scalatest.FlatSpec +import org.scalatest.junit.JUnitRunner +import whisk.core.database.test.behavior.ArtifactStoreCRUDBehaviors +import whisk.core.entity._ + +import scala.reflect.classTag + +@RunWith(classOf[JUnitRunner]) +class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreCRUDBehaviors { + override def storeType = "CosmosDB" + + override val authStore = { + implicit val docReader: DocumentReader = WhiskDocumentReader + CosmosDBArtifactStoreProvider.makeStore[WhiskAuth]() + } + + override val entityStore = + CosmosDBArtifactStoreProvider.makeStore[WhiskEntity]()( + classTag[WhiskEntity], + WhiskEntityJsonFormat, + WhiskDocumentReader, + actorSystem, + logging, + materializer) + + override val activationStore = { + implicit val docReader: DocumentReader = WhiskDocumentReader + CosmosDBArtifactStoreProvider.makeStore[WhiskActivation]() + } +} diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/ReferenceCountedTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/ReferenceCountedTests.scala new file mode 100644 index 00000000000..a357a13ccce --- /dev/null +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/ReferenceCountedTests.scala @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import org.scalatest.{FlatSpec, Matchers} + +class ReferenceCountedTests extends FlatSpec with Matchers { + + class CloseProbe extends AutoCloseable { + var closed: Boolean = _ + var closedCount: Int = _ + override def close(): Unit = { + closed = true + closedCount += 1 + } + } + + behavior of "ReferenceCounted" + + it should "close only once" in { + val probe = new CloseProbe + val refCounted = ReferenceCounted(probe) + + val ref1 = refCounted.reference() + + ref1.get should be theSameInstanceAs probe + ref1.close() + ref1.close() + + probe.closed shouldBe true + probe.closedCount shouldBe 1 + } + + it should "not close with one reference active" in { + val probe = new CloseProbe + val refCounted = ReferenceCounted(probe) + + val ref1 = refCounted.reference() + val ref2 = refCounted.reference() + + ref1.close() + ref1.close() + + probe.closed shouldBe false + } + + it should "be closed when all reference close" in { + val probe = new CloseProbe + val refCounted = ReferenceCounted(probe) + + val ref1 = refCounted.reference() + val ref2 = refCounted.reference() + + ref1.close() + ref2.close() + + probe.closed shouldBe true + probe.closedCount shouldBe 1 + } + + it should "throw exception if closed" in { + val probe = new CloseProbe + val refCounted = ReferenceCounted(probe) + + val ref1 = refCounted.reference() + ref1.close() + + intercept[IllegalArgumentException] { + refCounted.reference() + } + } +} From 1c45317751e521fb0416f6cddd83ba5e57faa4a0 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Mon, 16 Apr 2018 10:16:45 +0530 Subject: [PATCH 02/51] Add missed annotation --- .../whisk/core/database/cosmosdb/ReferenceCountedTests.scala | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/ReferenceCountedTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/ReferenceCountedTests.scala index a357a13ccce..0da706ba388 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/ReferenceCountedTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/ReferenceCountedTests.scala @@ -17,8 +17,11 @@ package whisk.core.database.cosmosdb +import org.junit.runner.RunWith +import org.scalatest.junit.JUnitRunner import org.scalatest.{FlatSpec, Matchers} +@RunWith(classOf[JUnitRunner]) class ReferenceCountedTests extends FlatSpec with Matchers { class CloseProbe extends AutoCloseable { From ef03a558c4183a43fd3f649a92e8d2113680ba1f Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Mon, 16 Apr 2018 10:28:34 +0530 Subject: [PATCH 03/51] Add CosmosDB config --- tests/src/test/resources/application.conf.j2 | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/src/test/resources/application.conf.j2 b/tests/src/test/resources/application.conf.j2 index 62fa019d1ef..7a40c243505 100644 --- a/tests/src/test/resources/application.conf.j2 +++ b/tests/src/test/resources/application.conf.j2 @@ -46,6 +46,12 @@ whisk { } } + cosmosdb { + host = ${?COSMOSDB_HOST} + key = ${?COSMOSDB_KEY} + db = "owtest" + } + controller { protocol = {{ controller.protocol }} https { From dd9b6b4bf3ff909fc5f380dc8597b430690253d8 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Mon, 16 Apr 2018 15:04:15 +0530 Subject: [PATCH 04/51] Use readDocument instead of query documents for single document read --- .../cosmosdb/CosmosDBArtifactStore.scala | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index a9d7abf9dad..e5d589d0bf3 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -34,7 +34,6 @@ import whisk.core.database.cosmosdb.CosmosDBArtifactStoreProvider.DocumentClient import whisk.core.entity._ import whisk.http.Messages -import scala.collection.JavaConverters._ import scala.concurrent.{ExecutionContext, Future} object CosmosDBArtifactStore { @@ -128,19 +127,20 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected require(doc != null, "doc undefined") val f = client - .queryDocuments(collection.getSelfLink, querySpec(doc.id.id), null) + .readDocument(createSelfLink(doc.id.id), null) .head() - .map { fr => - val cdoc = fr.getResults.asScala.headOption.getOrElse({ - transid.finished(this, start, s"[GET] '$collName', document: '$doc'; not found.") - // for compatibility - throw NoDocumentException("not found on 'get'") + .transform( + { rr => + val js = toWhiskJsonDoc(rr.getResource) + transid.finished(this, start, s"[GET] '$collName' completed: found document '$doc'") + deserialize[A, DocumentAbstraction](doc, js) + }, { + case e: DocumentClientException if e.getStatusCode == StatusCodes.NotFound.intValue => + transid.finished(this, start, s"[GET] '$collName', document: '${doc}'; not found.") + // for compatibility + throw NoDocumentException("not found on 'get'") + case e => e }) - - val js = toWhiskJsonDoc(cdoc) - transid.finished(this, start, s"[GET] '$collName' completed: found document '$doc'") - deserialize[A, DocumentAbstraction](doc, js) - } .recoverWith { case _: DeserializationException => throw DocumentUnreadable(Messages.corruptedEntity) } From bde7ca562d73a023d507d7cc8f3824f0e75b8534 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 18 Apr 2018 20:27:05 +0530 Subject: [PATCH 05/51] Initial support for CosmosDB queries --- common/scala/build.gradle | 1 + .../cosmosdb/CosmosDBArtifactStore.scala | 81 ++++++++++--- .../database/cosmosdb/CosmosDBSupport.scala | 3 + .../core/database/cosmosdb/CosmosDBUtil.scala | 79 ++++++++++++ .../cosmosdb/CosmosDBViewMapper.scala | 112 +++++++++++++++++- .../cosmosdb/CosmosDBArtifactStoreTests.scala | 4 +- 6 files changed, 257 insertions(+), 23 deletions(-) create mode 100644 common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala diff --git a/common/scala/build.gradle b/common/scala/build.gradle index 60f1223671a..98ac1163169 100644 --- a/common/scala/build.gradle +++ b/common/scala/build.gradle @@ -73,6 +73,7 @@ dependencies { compile 'io.zipkin.reporter2:zipkin-reporter:2.6.1' compile 'io.reactivex:rxscala_2.11:0.26.5' + compile 'io.reactivex:rxjava-reactive-streams:1.2.1' compile 'com.microsoft.azure:azure-cosmosdb:1.0.0' scoverage gradle.scoverage.deps } diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index e5d589d0bf3..cf57bd54f56 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -17,29 +17,26 @@ package whisk.core.database.cosmosdb +import _root_.rx.RxReactiveStreams import akka.actor.ActorSystem import akka.http.scaladsl.model.{ContentType, StatusCodes} import akka.stream.ActorMaterializer import akka.stream.scaladsl.{Sink, Source} import akka.util.ByteString -import com.microsoft.azure.cosmosdb.internal.Constants.Properties.{E_TAG, ID, SELF_LINK} +import com.microsoft.azure.cosmosdb._ + import com.microsoft.azure.cosmosdb.rx.AsyncDocumentClient -import com.microsoft.azure.cosmosdb.{AccessCondition, Document, DocumentClientException, RequestOptions} import spray.json.{DefaultJsonProtocol, JsObject, JsString, JsValue, RootJsonFormat, _} import whisk.common.{Logging, LoggingMarkers, TransactionId} import whisk.core.database.StoreUtils.{checkDocHasRevision, deserialize, reportFailure} import whisk.core.database._ -import whisk.core.database.cosmosdb.CosmosDBArtifactStore._computed +import whisk.core.database.cosmosdb.CosmosDBConstants._ import whisk.core.database.cosmosdb.CosmosDBArtifactStoreProvider.DocumentClientRef import whisk.core.entity._ import whisk.http.Messages import scala.concurrent.{ExecutionContext, Future} -object CosmosDBArtifactStore { - val _computed = "_c" -} - class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val collName: String, protected val config: CosmosDBConfig, clientRef: DocumentClientRef, @@ -131,12 +128,12 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected .head() .transform( { rr => - val js = toWhiskJsonDoc(rr.getResource) + val js = getResultToWhiskJsonDoc(rr.getResource) transid.finished(this, start, s"[GET] '$collName' completed: found document '$doc'") deserialize[A, DocumentAbstraction](doc, js) }, { case e: DocumentClientException if e.getStatusCode == StatusCodes.NotFound.intValue => - transid.finished(this, start, s"[GET] '$collName', document: '${doc}'; not found.") + transid.finished(this, start, s"[GET] '$collName', document: '$doc'; not found.") // for compatibility throw NoDocumentException("not found on 'get'") case e => e @@ -160,8 +157,41 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected includeDocs: Boolean, descending: Boolean, reduce: Boolean, - stale: StaleParameter)(implicit transid: TransactionId): Future[List[JsObject]] = - ??? + stale: StaleParameter)(implicit transid: TransactionId): Future[List[JsObject]] = { + require(!(reduce && includeDocs), "reduce and includeDocs cannot both be true") + require(!reduce, "Reduce scenario not supported") //TODO Investigate reduce + documentHandler.checkIfTableSupported(table) + + val Array(ddoc, viewName) = table.split("/") + + val start = transid.started(this, LoggingMarkers.DATABASE_QUERY, s"[QUERY] '$collName' searching '$table'") + val realIncludeDocs = includeDocs | documentHandler.shouldAlwaysIncludeDocs(ddoc, viewName) + val realLimit = if (limit > 0) skip + limit else limit + + val querySpec = viewMapper.prepareQuery(ddoc, viewName, startKey, endKey, realLimit, realIncludeDocs, descending) + + val options = new FeedOptions() + options.setEnableCrossPartitionQuery(true) + + val publisher = RxReactiveStreams.toPublisher(client.queryDocuments(collection.getSelfLink, querySpec, options)) + val f = Source + .fromPublisher(publisher) + .mapConcat(asSeq) + .drop(skip) + .map(queryResultToWhiskJsonDoc) + .map(js => + documentHandler + .transformViewResult(ddoc, viewName, startKey, endKey, realIncludeDocs, js, CosmosDBArtifactStore.this)) + .mapAsync(1)(identity) + .mapConcat(identity) + .runWith(Sink.seq) + .map(_.toList) + + f.onSuccess({ + case out => transid.finished(this, start, s"[QUERY] '$collName' completed: matched ${out.size}") + }) + reportFailure(f, start, failure => s"[QUERY] '$collName' internal error, failure: '${failure.getMessage}'") + } override protected[core] def count(table: String, startKey: List[Any], @@ -190,20 +220,32 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val computedOpt = if (computed.fields.nonEmpty) Some(computed) else None val fieldsToAdd = Seq( - (ID, Some(JsString(escapeId(json.fields(_id).convertTo[String])))), - (E_TAG, json.fields.get(_rev)), + (cid, Some(JsString(escapeId(json.fields(_id).convertTo[String])))), + (etag, json.fields.get(_rev)), (_computed, computedOpt)) val fieldsToRemove = Seq(_id, _rev) val mapped = transform(json, fieldsToAdd, fieldsToRemove) val doc = new Document(mapped.compactPrint) - doc.set(SELF_LINK, createSelfLink(doc.getId)) + doc.set(selfLink, createSelfLink(doc.getId)) doc } - private def toWhiskJsonDoc(doc: Document): JsObject = { + private def queryResultToWhiskJsonDoc(doc: Document): JsObject = { + val docJson = doc.toJson.parseJson.asJsObject + //If includeDocs is true then document json is to be used + val js = if (doc.has(queryResultAlias)) docJson.fields(queryResultAlias).asJsObject else docJson + val id = js.fields(cid).convertTo[String] + toWhiskJsonDoc(js, id, None) + } + + private def getResultToWhiskJsonDoc(doc: Document): JsObject = { val js = doc.toJson.parseJson.asJsObject - val fieldsToAdd = Seq((_id, Some(JsString(unescapeId(doc.getId)))), (_rev, Some(JsString(doc.getETag)))) - transform(js, fieldsToAdd, Seq.empty) + toWhiskJsonDoc(js, doc.getId, Some(JsString(doc.getETag))) + } + + private def toWhiskJsonDoc(js: JsObject, id: String, etag: Option[JsString]): JsObject = { + val fieldsToAdd = Seq((_id, Some(JsString(unescapeId(id)))), (_rev, etag)) + transform(stripInternalFields(js), fieldsToAdd, Seq.empty) } private def transform(json: JsObject, fieldsToAdd: Seq[(String, Option[JsValue])], fieldsToRemove: Seq[String]) = { @@ -211,6 +253,11 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected JsObject(fields) } + private def stripInternalFields(js: JsObject) = { + //Strip out all field name starting with '_' which are considered as db specific internal fields + JsObject(js.fields.filter { case (k, _) => !k.startsWith("_") && k != cid }) + } + private def toDocInfo(doc: Document) = DocInfo(DocId(doc.getId), DocRevision(doc.getETag)) private def createSelfLink(id: String) = s"dbs/${database.getId}/colls/${collection.getId}/docs/$id" diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala index ff587f03118..342890ff826 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala @@ -23,6 +23,7 @@ import _root_.rx.lang.scala._ import _root_.rx.lang.scala.JavaConverters._ import scala.collection.JavaConverters._ +import scala.collection.immutable trait CosmosDBSupport { protected def config: CosmosDBConfig @@ -72,4 +73,6 @@ trait CosmosDBSupport { protected def escapeId(id: String): String = id.replace("/", "|") protected def unescapeId(id: String): String = id.replace("|", "/") + + protected def asSeq[T <: Resource](r: FeedResponse[T]): immutable.Seq[T] = r.getResults.asScala.to[immutable.Seq] } diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala new file mode 100644 index 00000000000..b1e6ffe5d3c --- /dev/null +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import scala.collection.immutable.Iterable +import whisk.core.database.cosmosdb.CosmosDBConstants._ +import com.microsoft.azure.cosmosdb.internal.Constants.Properties.{E_TAG, ID, SELF_LINK} + +object CosmosDBConstants { + val _computed: String = "_c" + + val queryResultAlias: String = "view" + + val cid: String = ID + + val etag: String = E_TAG + + val selfLink: String = SELF_LINK +} + +object CosmosDBUtil { + + /** + * Prepares the json like select clause + * {{{ + * Seq("a", "b", "c.d.e") => + * { "a" : r['a'], "b" : r['b'], "c" : { "d" : { "e" : r['c']['d']['e']}}, "id" : r['id']} AS view + * }}} + * Here it uses {{{r['keyName']}}} notation to avoid issues around using reserved words as field name + */ + def prepareFieldClause(fields: Iterable[String]): String = { + val m = fields.foldLeft(Map.empty[String, Any]) { (map, name) => + addToMap(name, map) + } + val withId = addToMap(cid, m) + val json = asJsonLikeString(withId) + s"$json AS $queryResultAlias" + } + + private def addToMap(name: String, map: Map[String, _]): Map[String, Any] = name.split('.').toList match { + case Nil => sys.error(s"Should not reach here $name") + case x :: xs => addToMap(x, xs, Nil, map) + } + + private def addToMap(key: String, + children: List[String], + keyPath: List[String], + map: Map[String, Any]): Map[String, Any] = children match { + case Nil => map + (key -> s"r${makeKeyPath(key :: keyPath)}") + case x :: xs => + map + (key -> addToMap(x, xs, key :: keyPath, map.getOrElse(key, Map.empty).asInstanceOf[Map[String, Any]])) + } + + private def makeKeyPath(keyPath: List[String]) = keyPath.reverse.map(f => s"['$f']").mkString + + private def asJsonLikeString(m: Map[_, _]) = + m.map { case (k, v) => s""" "$k" : ${asString(v)}""" }.mkString("{", ",", "}") + + private def asString(v: Any): String = v match { + case m: Map[_, _] => asJsonLikeString(m) + case x => x.toString + } + +} diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala index 225e68f8652..a6df0a09a39 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -17,8 +17,112 @@ package whisk.core.database.cosmosdb -trait CosmosDBViewMapper {} +import com.microsoft.azure.cosmosdb.{SqlParameter, SqlParameterCollection, SqlQuerySpec} +import whisk.core.database._ +import whisk.core.database.cosmosdb.CosmosDBConstants._computed +import whisk.core.entity.WhiskEntityQueries.TOP -object WhisksViewMapper extends CosmosDBViewMapper -object ActivationViewMapper extends CosmosDBViewMapper -object SubjectViewMapper extends CosmosDBViewMapper +trait CosmosDBViewMapper { + protected val NOTHING = "" + protected val ALL_FIELDS = "*" + protected def handler: DocumentHandler + + def prepareQuery(ddoc: String, + viewName: String, + startKey: List[Any], + endKey: List[Any], + limit: Int, + includeDocs: Boolean, + descending: Boolean): SqlQuerySpec = { + checkKeys(startKey, endKey) + + val selectClause = select(ddoc, viewName, limit, includeDocs) + val whereClause = where(ddoc, viewName, startKey, endKey) + val orderField = orderByField(ddoc, viewName) + val order = if (descending) "DESC" else NOTHING + + val query = s"SELECT $selectClause FROM root r WHERE ${whereClause._1} ORDER BY $orderField $order" + + val params = new SqlParameterCollection + whereClause._2.foreach { case (k, v) => params.add(new SqlParameter(k, v)) } + + new SqlQuerySpec(query, params) + } + + protected def checkKeys(startKey: List[Any], endKey: List[Any]): Unit = { + require(startKey.nonEmpty) + require(endKey.nonEmpty) + require(startKey.head == endKey.head, s"First key should be same => ($startKey) - ($endKey)") + } + + protected def select(ddoc: String, viewName: String, limit: Int, includeDocs: Boolean): String = { + val fieldClause = if (includeDocs) ALL_FIELDS else prepareFieldClause(ddoc, viewName) + s"${top(limit)} $fieldClause" + } + + private def top(limit: Int): String = { + if (limit > 0) s"TOP $limit" else NOTHING + } + + private def prepareFieldClause(ddoc: String, viewName: String) = + CosmosDBUtil.prepareFieldClause(handler.fieldsRequiredForView(ddoc, viewName)) + + protected def where(ddoc: String, + viewName: String, + startKey: List[Any], + endKey: List[Any]): (String, List[(String, Any)]) = ??? + + protected def orderByField(ddoc: String, viewName: String): String = ??? + +} + +object WhisksViewMapper extends CosmosDBViewMapper { + val handler = WhisksHandler +} +object ActivationViewMapper extends CosmosDBViewMapper { + private val NS = "r.namespace" + private val NS_WITH_PATH = s"r.${_computed}.${ActivationHandler.NS_PATH}" + private val START = "r.start" + + val handler = ActivationHandler + + override protected def where(ddoc: String, + view: String, + startKey: List[Any], + endKey: List[Any]): (String, List[(String, Any)]) = { + val nsValue = startKey.head.asInstanceOf[String] + view match { + //whisks-filters ddoc uses namespace + invoking action path as first key + case "activations" if ddoc.startsWith("whisks-filters") => + filterActivation(NS_WITH_PATH, nsValue, startKey, endKey) + //whisks ddoc uses namespace as first key + case "activations" if ddoc.startsWith("whisks") => filterActivation(NS, nsValue, startKey, endKey) + case _ => throw UnsupportedView(s"$ddoc/$view") + } + } + + private def filterActivation(nsKey: String, + nsValue: String, + startKey: List[Any], + endKey: List[Any]): (String, List[(String, Any)]) = { + val params = ("@nsvalue", nsValue) :: Nil + val filter = (startKey, endKey) match { + case (_ :: Nil, _ :: `TOP` :: Nil) => + (s"$nsKey = @nsvalue", params) + case (_ :: (since: Number) :: Nil, _ :: `TOP` :: `TOP` :: Nil) => + (s"$nsKey = @nsvalue AND $START >= @start", ("@start", since) :: params) + case (_ :: (since: Number) :: Nil, _ :: (upto: Number) :: `TOP` :: Nil) => + (s"$nsKey = @nsvalue AND $START >= @start AND $START <= @upto", ("@upto", upto) :: ("@start", since) :: params) + case _ => throw UnsupportedQueryKeys(s"$startKey, $endKey") + } + filter + } + + override protected def orderByField(ddoc: String, view: String): String = view match { + case "activations" if ddoc.startsWith("whisks") => START + case _ => throw UnsupportedView(s"$ddoc/$view") + } +} +object SubjectViewMapper extends CosmosDBViewMapper { + val handler = SubjectHandler +} diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala index f1b69ae11e4..5281722536f 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala @@ -20,13 +20,13 @@ package whisk.core.database.cosmosdb import org.junit.runner.RunWith import org.scalatest.FlatSpec import org.scalatest.junit.JUnitRunner -import whisk.core.database.test.behavior.ArtifactStoreCRUDBehaviors +import whisk.core.database.test.behavior.ArtifactStoreQueryBehaviors import whisk.core.entity._ import scala.reflect.classTag @RunWith(classOf[JUnitRunner]) -class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreCRUDBehaviors { +class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreQueryBehaviors { override def storeType = "CosmosDB" override val authStore = { From d49ec107ea3183e06b487d29a702ad9c48587484 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 19 Apr 2018 11:07:38 +0530 Subject: [PATCH 06/51] Switch to CRUD test for now as not all query tests are passing --- .../core/database/cosmosdb/CosmosDBArtifactStoreTests.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala index 5281722536f..f1b69ae11e4 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala @@ -20,13 +20,13 @@ package whisk.core.database.cosmosdb import org.junit.runner.RunWith import org.scalatest.FlatSpec import org.scalatest.junit.JUnitRunner -import whisk.core.database.test.behavior.ArtifactStoreQueryBehaviors +import whisk.core.database.test.behavior.ArtifactStoreCRUDBehaviors import whisk.core.entity._ import scala.reflect.classTag @RunWith(classOf[JUnitRunner]) -class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreQueryBehaviors { +class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreCRUDBehaviors { override def storeType = "CosmosDB" override val authStore = { From 340e1912792c06dbe3c9553fabe53b44a8aff859 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 19 Apr 2018 14:11:19 +0530 Subject: [PATCH 07/51] Check for document having id and etag set Documents returned from query result may not have id set. However ones from explicit lookup must have them set. This check ensures that correct method is used to convert the docs --- .../database/cosmosdb/CosmosDBArtifactStore.scala | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index cf57bd54f56..05835db1dab 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -239,6 +239,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected } private def getResultToWhiskJsonDoc(doc: Document): JsObject = { + checkDoc(doc) val js = doc.toJson.parseJson.asJsObject toWhiskJsonDoc(js, doc.getId, Some(JsString(doc.getETag))) } @@ -258,7 +259,10 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected JsObject(js.fields.filter { case (k, _) => !k.startsWith("_") && k != cid }) } - private def toDocInfo(doc: Document) = DocInfo(DocId(doc.getId), DocRevision(doc.getETag)) + private def toDocInfo(doc: Document) = { + checkDoc(doc) + DocInfo(DocId(unescapeId(doc.getId)), DocRevision(doc.getETag)) + } private def createSelfLink(id: String) = s"dbs/${database.getId}/colls/${collection.getId}/docs/$id" @@ -269,4 +273,9 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected options.setAccessCondition(condition) options } + + private def checkDoc(doc: Document) = { + require(doc.getId != null, s"$doc does not have id field set") + require(doc.getETag != null, s"$doc does not have etag field set") + } } From f936bf7ffb507fce9fcc7d86e1cd249a3b478475 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 19 Apr 2018 14:12:00 +0530 Subject: [PATCH 08/51] Implement whisks view query support --- .../cosmosdb/CosmosDBViewMapper.scala | 51 ++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala index a6df0a09a39..b715a372ee9 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -55,7 +55,7 @@ trait CosmosDBViewMapper { require(startKey.head == endKey.head, s"First key should be same => ($startKey) - ($endKey)") } - protected def select(ddoc: String, viewName: String, limit: Int, includeDocs: Boolean): String = { + private def select(ddoc: String, viewName: String, limit: Int, includeDocs: Boolean): String = { val fieldClause = if (includeDocs) ALL_FIELDS else prepareFieldClause(ddoc, viewName) s"${top(limit)} $fieldClause" } @@ -77,7 +77,56 @@ trait CosmosDBViewMapper { } object WhisksViewMapper extends CosmosDBViewMapper { + private val NS = "r.namespace" + private val ROOT_NS = s"r.${_computed}.${WhisksHandler.ROOT_NS}" + private val TYPE = "r.entityType" + private val UPDATED = "r.updated" + private val PUBLISH = "r.publish" + private val BINDING = "r.binding" + val handler = WhisksHandler + + override protected def where(ddoc: String, + view: String, + startKey: List[Any], + endKey: List[Any]): (String, List[(String, Any)]) = { + val entityType = WhisksHandler.getEntityTypeForDesignDoc(ddoc, view) + val namespace = startKey.head + + val (vc, vcParams) = + viewConditions(ddoc, view).map(q => (s"${q._1} AND", q._2)).getOrElse((NOTHING, Nil)) + + val params = ("@entityType", entityType) :: ("@namespace", namespace) :: vcParams + val baseCondition = s"$vc $TYPE = @entityType AND ($NS = @namespace OR $ROOT_NS = @namespace)" + + (startKey, endKey) match { + case (_ :: Nil, _ :: `TOP` :: Nil) => + (baseCondition, params) + + case (_ :: (since: Number) :: Nil, _ :: `TOP` :: `TOP` :: Nil) => + (s"$baseCondition AND $UPDATED >= @since", ("@since", since) :: params) + + case (_ :: (since: Number) :: Nil, _ :: (upto: Number) :: `TOP` :: Nil) => + (s"$baseCondition AND ($UPDATED BETWEEN @since AND @upto)", ("@upto", upto) :: ("@since", since) :: params) + + case _ => throw UnsupportedQueryKeys(s"$ddoc/$view -> ($startKey, $endKey)") + } + } + + private def viewConditions(ddoc: String, view: String): Option[(String, List[(String, Any)])] = { + view match { + case "packages-public" if ddoc.startsWith("whisks") => + Some(s"$PUBLISH = true AND (!IS_OBJECT($BINDING) OR $BINDING = {})", Nil) + case _ => None + } + } + + override protected def orderByField(ddoc: String, view: String): String = view match { + case "actions" | "rules" | "triggers" | "packages" | "packages-public" if ddoc.startsWith("whisks") => + UPDATED + case _ => throw UnsupportedView(s"$ddoc/$view") + } + } object ActivationViewMapper extends CosmosDBViewMapper { private val NS = "r.namespace" From 9d34f89fe41bcfec118d0d1d933d33d81cbfe134 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 19 Apr 2018 16:08:26 +0530 Subject: [PATCH 09/51] Enable count support --- .../cosmosdb/CosmosDBArtifactStore.scala | 31 ++++++++++++++++--- .../core/database/cosmosdb/CosmosDBUtil.scala | 4 ++- .../cosmosdb/CosmosDBViewMapper.scala | 19 ++++++++++-- .../cosmosdb/CosmosDBArtifactStoreTests.scala | 4 +-- 4 files changed, 48 insertions(+), 10 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index 05835db1dab..353a5dc8c6f 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -35,6 +35,7 @@ import whisk.core.database.cosmosdb.CosmosDBArtifactStoreProvider.DocumentClient import whisk.core.entity._ import whisk.http.Messages +import scala.collection.JavaConverters._ import scala.concurrent.{ExecutionContext, Future} class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val collName: String, @@ -101,7 +102,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected checkDocHasRevision(doc) val start = transid.started(this, LoggingMarkers.DATABASE_DELETE, s"[DEL] '$collName' deleting document: '$doc'") val f = client - .deleteDocument(createSelfLink(doc.id.id), matchRevOption(doc.rev.rev)) + .deleteDocument(selfLinkOf(doc.id), matchRevOption(doc.rev.rev)) .head() .transform( _ => true, { @@ -124,7 +125,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected require(doc != null, "doc undefined") val f = client - .readDocument(createSelfLink(doc.id.id), null) + .readDocument(selfLinkOf(doc.id), null) .head() .transform( { rr => @@ -197,7 +198,27 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected startKey: List[Any], endKey: List[Any], skip: Int, - stale: StaleParameter)(implicit transid: TransactionId): Future[Long] = ??? + stale: StaleParameter)(implicit transid: TransactionId): Future[Long] = { + val Array(ddoc, viewName) = table.split("/") + + val start = transid.started(this, LoggingMarkers.DATABASE_QUERY, s"[COUNT] '$collName' searching '$table") + val querySpec = viewMapper.prepareCountQuery(ddoc, viewName, startKey, endKey) + + val options = new FeedOptions() + options.setEnableCrossPartitionQuery(true) + + //For aggregates the value is in _aggregates fields + val f = client + .queryDocuments(collection.getSelfLink, querySpec, options) + .head() + .map(_.getResults.asScala.head.getLong(aggregate).longValue()) + + f.onSuccess({ + case out => transid.finished(this, start, s"[COUNT] '$collName' completed: count $out") + }) + + reportFailure(f, start, failure => s"[COUNT] '$collName' internal error, failure: '${failure.getMessage}'") + } override protected[core] def attach( doc: DocInfo, @@ -264,6 +285,8 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected DocInfo(DocId(unescapeId(doc.getId)), DocRevision(doc.getETag)) } + private def selfLinkOf(id: DocId) = createSelfLink(escapeId(id.id)) + private def createSelfLink(id: String) = s"dbs/${database.getId}/colls/${collection.getId}/docs/$id" private def matchRevOption(etag: String) = { @@ -274,7 +297,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected options } - private def checkDoc(doc: Document) = { + private def checkDoc(doc: Document): Unit = { require(doc.getId != null, s"$doc does not have id field set") require(doc.getETag != null, s"$doc does not have etag field set") } diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala index b1e6ffe5d3c..1664b843168 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala @@ -19,7 +19,7 @@ package whisk.core.database.cosmosdb import scala.collection.immutable.Iterable import whisk.core.database.cosmosdb.CosmosDBConstants._ -import com.microsoft.azure.cosmosdb.internal.Constants.Properties.{E_TAG, ID, SELF_LINK} +import com.microsoft.azure.cosmosdb.internal.Constants.Properties.{AGGREGATE, E_TAG, ID, SELF_LINK} object CosmosDBConstants { val _computed: String = "_c" @@ -30,6 +30,8 @@ object CosmosDBConstants { val etag: String = E_TAG + val aggregate: String = AGGREGATE + val selfLink: String = SELF_LINK } diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala index b715a372ee9..c4eea8f75a0 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -43,10 +43,16 @@ trait CosmosDBViewMapper { val query = s"SELECT $selectClause FROM root r WHERE ${whereClause._1} ORDER BY $orderField $order" - val params = new SqlParameterCollection - whereClause._2.foreach { case (k, v) => params.add(new SqlParameter(k, v)) } + prepareSpec(whereClause._2, query) + } + + def prepareCountQuery(ddoc: String, viewName: String, startKey: List[Any], endKey: List[Any]): SqlQuerySpec = { + checkKeys(startKey, endKey) + + val whereClause = where(ddoc, viewName, startKey, endKey) + val query = s"SELECT TOP 1 VALUE COUNT(r) FROM root r WHERE ${whereClause._1}" - new SqlQuerySpec(query, params) + prepareSpec(whereClause._2, query) } protected def checkKeys(startKey: List[Any], endKey: List[Any]): Unit = { @@ -55,6 +61,13 @@ trait CosmosDBViewMapper { require(startKey.head == endKey.head, s"First key should be same => ($startKey) - ($endKey)") } + private def prepareSpec(params: List[(String, Any)], query: String) = { + val paramColl = new SqlParameterCollection + params.foreach { case (k, v) => paramColl.add(new SqlParameter(k, v)) } + + new SqlQuerySpec(query, paramColl) + } + private def select(ddoc: String, viewName: String, limit: Int, includeDocs: Boolean): String = { val fieldClause = if (includeDocs) ALL_FIELDS else prepareFieldClause(ddoc, viewName) s"${top(limit)} $fieldClause" diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala index f1b69ae11e4..120b0530d39 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala @@ -20,13 +20,13 @@ package whisk.core.database.cosmosdb import org.junit.runner.RunWith import org.scalatest.FlatSpec import org.scalatest.junit.JUnitRunner -import whisk.core.database.test.behavior.ArtifactStoreCRUDBehaviors +import whisk.core.database.test.behavior.{ArtifactStoreCRUDBehaviors, ArtifactStoreQueryBehaviors} import whisk.core.entity._ import scala.reflect.classTag @RunWith(classOf[JUnitRunner]) -class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreCRUDBehaviors { +class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreQueryBehaviors with ArtifactStoreCRUDBehaviors { override def storeType = "CosmosDB" override val authStore = { From f60968a9b30db7877e235980b29b546a3e020a98 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 19 Apr 2018 16:23:50 +0530 Subject: [PATCH 10/51] Fix the not condition handling --- .../core/database/cosmosdb/CosmosDBViewMapper.scala | 2 +- .../cosmosdb/CosmosDBArtifactStoreTests.scala | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala index c4eea8f75a0..6b729274f98 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -129,7 +129,7 @@ object WhisksViewMapper extends CosmosDBViewMapper { private def viewConditions(ddoc: String, view: String): Option[(String, List[(String, Any)])] = { view match { case "packages-public" if ddoc.startsWith("whisks") => - Some(s"$PUBLISH = true AND (!IS_OBJECT($BINDING) OR $BINDING = {})", Nil) + Some(s"$PUBLISH = true AND (NOT IS_OBJECT($BINDING) OR $BINDING = {})", Nil) case _ => None } } diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala index 120b0530d39..127505a0c20 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala @@ -20,13 +20,21 @@ package whisk.core.database.cosmosdb import org.junit.runner.RunWith import org.scalatest.FlatSpec import org.scalatest.junit.JUnitRunner -import whisk.core.database.test.behavior.{ArtifactStoreCRUDBehaviors, ArtifactStoreQueryBehaviors} +import whisk.core.database.test.behavior.{ + ArtifactStoreCRUDBehaviors, + ArtifactStoreQueryBehaviors, + ArtifactStoreWhisksQueryBehaviors +} import whisk.core.entity._ import scala.reflect.classTag @RunWith(classOf[JUnitRunner]) -class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreQueryBehaviors with ArtifactStoreCRUDBehaviors { +class CosmosDBArtifactStoreTests + extends FlatSpec + with ArtifactStoreQueryBehaviors + with ArtifactStoreCRUDBehaviors + with ArtifactStoreWhisksQueryBehaviors { override def storeType = "CosmosDB" override val authStore = { From 6997b9a8260872ac8e648c5facd0bbebdcf5fe59 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Fri, 20 Apr 2018 16:18:33 +0530 Subject: [PATCH 11/51] Log the logs in case of test failure --- .../test/behavior/ArtifactStoreBehaviorBase.scala | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala index 38ef12e4b5d..12ff623d159 100644 --- a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala +++ b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala @@ -71,6 +71,14 @@ trait ArtifactStoreBehaviorBase assertAttachmentStoresAreClosed() } + override protected def withFixture(test: NoArgTest) = { + val outcome = super.withFixture(test) + if (outcome.isFailed) { + println(logLines.mkString("\n")) + } + outcome + } + //~----------------------------------------< utility methods > protected def query[A <: WhiskEntity]( From 849b5ad3c2416176bbbd06792f322254ce4df8db Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Fri, 20 Apr 2018 16:22:48 +0530 Subject: [PATCH 12/51] Implement subject query support --- .../cosmosdb/CosmosDBArtifactStore.scala | 25 +++++++-- .../cosmosdb/CosmosDBViewMapper.scala | 52 +++++++++++++++++-- .../cosmosdb/CosmosDBArtifactStoreTests.scala | 12 +---- 3 files changed, 73 insertions(+), 16 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index 353a5dc8c6f..c33ca65bd51 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -146,8 +146,29 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected reportFailure( f, start, - failure => s"[DEL] '$collName' internal error, doc: '$doc', failure: '${failure.getMessage}'") + failure => s"[GET] '$collName' internal error, doc: '$doc', failure: '${failure.getMessage}'") + + } + + override protected[database] def get(id: DocId)(implicit transid: TransactionId): Future[Option[JsObject]] = { + val start = transid.started(this, LoggingMarkers.DATABASE_GET, s"[GET_BY_ID] '$collName' finding document: '$id'") + + val f = client + .readDocument(selfLinkOf(id), null) + .head() + .map { rr => + val js = getResultToWhiskJsonDoc(rr.getResource) + transid.finished(this, start, s"[GET_BY_ID] '$collName' completed: found document '$id'") + Some(js) + } + .recoverWith { + case e: DocumentClientException if e.getStatusCode == StatusCodes.NotFound.intValue => Future.successful(None) + } + reportFailure( + f, + start, + failure => s"[GET_BY_ID] '$collName' internal error, doc: '$id', failure: '${failure.getMessage}'") } override protected[core] def query(table: String, @@ -234,8 +255,6 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected override def shutdown(): Unit = clientRef.close() - override protected[database] def get(id: DocId)(implicit transid: TransactionId): Future[Option[JsObject]] = ??? - private def toCosmosDoc(json: JsObject): Document = { val computed = documentHandler.computedFields(json) val computedOpt = if (computed.fields.nonEmpty) Some(computed) else None diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala index 6b729274f98..1cca8c19edd 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -20,6 +20,7 @@ package whisk.core.database.cosmosdb import com.microsoft.azure.cosmosdb.{SqlParameter, SqlParameterCollection, SqlQuerySpec} import whisk.core.database._ import whisk.core.database.cosmosdb.CosmosDBConstants._computed +import whisk.core.database.cosmosdb.CosmosDBConstants.queryResultAlias import whisk.core.entity.WhiskEntityQueries.TOP trait CosmosDBViewMapper { @@ -43,7 +44,7 @@ trait CosmosDBViewMapper { val query = s"SELECT $selectClause FROM root r WHERE ${whereClause._1} ORDER BY $orderField $order" - prepareSpec(whereClause._2, query) + prepareSpec(query, whereClause._2) } def prepareCountQuery(ddoc: String, viewName: String, startKey: List[Any], endKey: List[Any]): SqlQuerySpec = { @@ -52,7 +53,7 @@ trait CosmosDBViewMapper { val whereClause = where(ddoc, viewName, startKey, endKey) val query = s"SELECT TOP 1 VALUE COUNT(r) FROM root r WHERE ${whereClause._1}" - prepareSpec(whereClause._2, query) + prepareSpec(query, whereClause._2) } protected def checkKeys(startKey: List[Any], endKey: List[Any]): Unit = { @@ -61,7 +62,7 @@ trait CosmosDBViewMapper { require(startKey.head == endKey.head, s"First key should be same => ($startKey) - ($endKey)") } - private def prepareSpec(params: List[(String, Any)], query: String) = { + protected def prepareSpec(query: String, params: List[(String, Any)]) = { val paramColl = new SqlParameterCollection params.foreach { case (k, v) => paramColl.add(new SqlParameter(k, v)) } @@ -187,4 +188,49 @@ object ActivationViewMapper extends CosmosDBViewMapper { } object SubjectViewMapper extends CosmosDBViewMapper { val handler = SubjectHandler + + override def prepareQuery(ddoc: String, + view: String, + startKey: List[Any], + endKey: List[Any], + limit: Int, + includeDocs: Boolean, + descending: Boolean): SqlQuerySpec = { + require(startKey == endKey, s"startKey: $startKey and endKey: $endKey must be same for $ddoc/$view") + (ddoc, view) match { + case ("subjects", "identities") => + queryForMatchingSubjectOrNamespace(ddoc, view, startKey, endKey) + case ("namespaceThrottlings", "blockedNamespaces") => + queryForBlacklistedNamespace() + case _ => + throw UnsupportedView(s"$ddoc/$view") + } + } + + private def queryForMatchingSubjectOrNamespace(ddoc: String, + view: String, + startKey: List[Any], + endKey: List[Any]): SqlQuerySpec = { + val notBlocked = "(NOT(IS_DEFINED(r.blocked)) OR r.blocked = false)" + + val (where, params) = startKey match { + case (ns: String) :: Nil => + (s"$notBlocked AND (r.subject = @name OR n.name = @name)", ("@name", ns) :: Nil) + case (uuid: String) :: (key: String) :: Nil => + ( + s"$notBlocked AND ((r.uuid = @uuid AND r.key = @key) OR (n.uuid = @uuid AND n.key = @key))", + ("@uuid", uuid) :: ("@key", key) :: Nil) + case _ => throw UnsupportedQueryKeys(s"$ddoc/$view -> ($startKey, $endKey)") + } + prepareSpec(s"SELECT r AS $queryResultAlias FROM root r JOIN n in r.namespaces WHERE $where", params) + } + + private def queryForBlacklistedNamespace(): SqlQuerySpec = + prepareSpec( + s"""SELECT r AS $queryResultAlias + FROM root r + WHERE r.blocked = true + OR r.concurrentInvocations = 0 + OR r.invocationsPerMinute = 0 """, + Nil) } diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala index 127505a0c20..1cc1c9dc6e2 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala @@ -20,21 +20,13 @@ package whisk.core.database.cosmosdb import org.junit.runner.RunWith import org.scalatest.FlatSpec import org.scalatest.junit.JUnitRunner -import whisk.core.database.test.behavior.{ - ArtifactStoreCRUDBehaviors, - ArtifactStoreQueryBehaviors, - ArtifactStoreWhisksQueryBehaviors -} +import whisk.core.database.test.behavior.ArtifactStoreBehavior import whisk.core.entity._ import scala.reflect.classTag @RunWith(classOf[JUnitRunner]) -class CosmosDBArtifactStoreTests - extends FlatSpec - with ArtifactStoreQueryBehaviors - with ArtifactStoreCRUDBehaviors - with ArtifactStoreWhisksQueryBehaviors { +class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreBehavior { override def storeType = "CosmosDB" override val authStore = { From ee5cc8ab09c202ae8914d784df11813dc4609e62 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Fri, 20 Apr 2018 16:58:13 +0530 Subject: [PATCH 13/51] Reset log stream post each run This ensures that logs are collected for only that test --- .../core/database/test/behavior/ArtifactStoreBehaviorBase.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala index 12ff623d159..c86e4802a46 100644 --- a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala +++ b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala @@ -59,6 +59,7 @@ trait ArtifactStoreBehaviorBase override def afterEach(): Unit = { cleanup() + stream.reset() } override def afterAll(): Unit = { From 7d0c961a007fbe43e7791d08c90eba7084d6e37b Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Fri, 20 Apr 2018 17:00:21 +0530 Subject: [PATCH 14/51] Refactor the base trait Refactor such that common logic specific to Whisks and Activations are part of SimpleMapper. While Subjects which has very specific queries implements queries directly --- .../cosmosdb/CosmosDBViewMapper.scala | 78 ++++++++++++------- 1 file changed, 52 insertions(+), 26 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala index 1cca8c19edd..3eb0faa139d 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -28,6 +28,32 @@ trait CosmosDBViewMapper { protected val ALL_FIELDS = "*" protected def handler: DocumentHandler + def prepareQuery(ddoc: String, + viewName: String, + startKey: List[Any], + endKey: List[Any], + limit: Int, + includeDocs: Boolean, + descending: Boolean): SqlQuerySpec + + def prepareCountQuery(ddoc: String, viewName: String, startKey: List[Any], endKey: List[Any]): SqlQuerySpec + + protected def checkKeys(startKey: List[Any], endKey: List[Any]): Unit = { + require(startKey.nonEmpty) + require(endKey.nonEmpty) + require(startKey.head == endKey.head, s"First key should be same => ($startKey) - ($endKey)") + } + + protected def prepareSpec(query: String, params: List[(String, Any)]): SqlQuerySpec = { + val paramColl = new SqlParameterCollection + params.foreach { case (k, v) => paramColl.add(new SqlParameter(k, v)) } + + new SqlQuerySpec(query, paramColl) + } +} + +abstract class SimpleMapper extends CosmosDBViewMapper { + def prepareQuery(ddoc: String, viewName: String, startKey: List[Any], @@ -56,19 +82,6 @@ trait CosmosDBViewMapper { prepareSpec(query, whereClause._2) } - protected def checkKeys(startKey: List[Any], endKey: List[Any]): Unit = { - require(startKey.nonEmpty) - require(endKey.nonEmpty) - require(startKey.head == endKey.head, s"First key should be same => ($startKey) - ($endKey)") - } - - protected def prepareSpec(query: String, params: List[(String, Any)]) = { - val paramColl = new SqlParameterCollection - params.foreach { case (k, v) => paramColl.add(new SqlParameter(k, v)) } - - new SqlQuerySpec(query, paramColl) - } - private def select(ddoc: String, viewName: String, limit: Int, includeDocs: Boolean): String = { val fieldClause = if (includeDocs) ALL_FIELDS else prepareFieldClause(ddoc, viewName) s"${top(limit)} $fieldClause" @@ -84,13 +97,12 @@ trait CosmosDBViewMapper { protected def where(ddoc: String, viewName: String, startKey: List[Any], - endKey: List[Any]): (String, List[(String, Any)]) = ??? - - protected def orderByField(ddoc: String, viewName: String): String = ??? + endKey: List[Any]): (String, List[(String, Any)]) + protected def orderByField(ddoc: String, viewName: String): String } -object WhisksViewMapper extends CosmosDBViewMapper { +object WhisksViewMapper extends SimpleMapper { private val NS = "r.namespace" private val ROOT_NS = s"r.${_computed}.${WhisksHandler.ROOT_NS}" private val TYPE = "r.entityType" @@ -142,7 +154,7 @@ object WhisksViewMapper extends CosmosDBViewMapper { } } -object ActivationViewMapper extends CosmosDBViewMapper { +object ActivationViewMapper extends SimpleMapper { private val NS = "r.namespace" private val NS_WITH_PATH = s"r.${_computed}.${ActivationHandler.NS_PATH}" private val START = "r.start" @@ -195,13 +207,23 @@ object SubjectViewMapper extends CosmosDBViewMapper { endKey: List[Any], limit: Int, includeDocs: Boolean, - descending: Boolean): SqlQuerySpec = { + descending: Boolean): SqlQuerySpec = + prepareQuery(ddoc, view, startKey, endKey, count = false) + + override def prepareCountQuery(ddoc: String, view: String, startKey: List[Any], endKey: List[Any]): SqlQuerySpec = + prepareQuery(ddoc, view, startKey, endKey, count = true) + + private def prepareQuery(ddoc: String, + view: String, + startKey: List[Any], + endKey: List[Any], + count: Boolean): SqlQuerySpec = { require(startKey == endKey, s"startKey: $startKey and endKey: $endKey must be same for $ddoc/$view") (ddoc, view) match { case ("subjects", "identities") => - queryForMatchingSubjectOrNamespace(ddoc, view, startKey, endKey) + queryForMatchingSubjectOrNamespace(ddoc, view, startKey, endKey, count) case ("namespaceThrottlings", "blockedNamespaces") => - queryForBlacklistedNamespace() + queryForBlacklistedNamespace(count) case _ => throw UnsupportedView(s"$ddoc/$view") } @@ -210,9 +232,9 @@ object SubjectViewMapper extends CosmosDBViewMapper { private def queryForMatchingSubjectOrNamespace(ddoc: String, view: String, startKey: List[Any], - endKey: List[Any]): SqlQuerySpec = { + endKey: List[Any], + count: Boolean): SqlQuerySpec = { val notBlocked = "(NOT(IS_DEFINED(r.blocked)) OR r.blocked = false)" - val (where, params) = startKey match { case (ns: String) :: Nil => (s"$notBlocked AND (r.subject = @name OR n.name = @name)", ("@name", ns) :: Nil) @@ -222,15 +244,19 @@ object SubjectViewMapper extends CosmosDBViewMapper { ("@uuid", uuid) :: ("@key", key) :: Nil) case _ => throw UnsupportedQueryKeys(s"$ddoc/$view -> ($startKey, $endKey)") } - prepareSpec(s"SELECT r AS $queryResultAlias FROM root r JOIN n in r.namespaces WHERE $where", params) + prepareSpec( + s"SELECT ${selectClause(count)} AS $queryResultAlias FROM root r JOIN n in r.namespaces WHERE $where", + params) } - private def queryForBlacklistedNamespace(): SqlQuerySpec = + private def queryForBlacklistedNamespace(count: Boolean): SqlQuerySpec = prepareSpec( - s"""SELECT r AS $queryResultAlias + s"""SELECT ${selectClause(count)} AS $queryResultAlias FROM root r WHERE r.blocked = true OR r.concurrentInvocations = 0 OR r.invocationsPerMinute = 0 """, Nil) + + private def selectClause(count: Boolean) = if (count) "TOP 1 VALUE COUNT(r)" else "r" } From afaa5c519d788f95b195fc123d38f2956148ca00 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Fri, 20 Apr 2018 20:06:58 +0530 Subject: [PATCH 15/51] Change constant name to alias --- .../core/database/cosmosdb/CosmosDBArtifactStore.scala | 2 +- .../scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala | 4 ++-- .../whisk/core/database/cosmosdb/CosmosDBViewMapper.scala | 8 +++----- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index c33ca65bd51..594b93cb92b 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -273,7 +273,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected private def queryResultToWhiskJsonDoc(doc: Document): JsObject = { val docJson = doc.toJson.parseJson.asJsObject //If includeDocs is true then document json is to be used - val js = if (doc.has(queryResultAlias)) docJson.fields(queryResultAlias).asJsObject else docJson + val js = if (doc.has(alias)) docJson.fields(alias).asJsObject else docJson val id = js.fields(cid).convertTo[String] toWhiskJsonDoc(js, id, None) } diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala index 1664b843168..30a5581c37a 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala @@ -24,7 +24,7 @@ import com.microsoft.azure.cosmosdb.internal.Constants.Properties.{AGGREGATE, E_ object CosmosDBConstants { val _computed: String = "_c" - val queryResultAlias: String = "view" + val alias: String = "view" val cid: String = ID @@ -51,7 +51,7 @@ object CosmosDBUtil { } val withId = addToMap(cid, m) val json = asJsonLikeString(withId) - s"$json AS $queryResultAlias" + s"$json AS $alias" } private def addToMap(name: String, map: Map[String, _]): Map[String, Any] = name.split('.').toList match { diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala index 3eb0faa139d..c988c36e17d 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -20,7 +20,7 @@ package whisk.core.database.cosmosdb import com.microsoft.azure.cosmosdb.{SqlParameter, SqlParameterCollection, SqlQuerySpec} import whisk.core.database._ import whisk.core.database.cosmosdb.CosmosDBConstants._computed -import whisk.core.database.cosmosdb.CosmosDBConstants.queryResultAlias +import whisk.core.database.cosmosdb.CosmosDBConstants.alias import whisk.core.entity.WhiskEntityQueries.TOP trait CosmosDBViewMapper { @@ -244,14 +244,12 @@ object SubjectViewMapper extends CosmosDBViewMapper { ("@uuid", uuid) :: ("@key", key) :: Nil) case _ => throw UnsupportedQueryKeys(s"$ddoc/$view -> ($startKey, $endKey)") } - prepareSpec( - s"SELECT ${selectClause(count)} AS $queryResultAlias FROM root r JOIN n in r.namespaces WHERE $where", - params) + prepareSpec(s"SELECT ${selectClause(count)} AS $alias FROM root r JOIN n in r.namespaces WHERE $where", params) } private def queryForBlacklistedNamespace(count: Boolean): SqlQuerySpec = prepareSpec( - s"""SELECT ${selectClause(count)} AS $queryResultAlias + s"""SELECT ${selectClause(count)} AS $alias FROM root r WHERE r.blocked = true OR r.concurrentInvocations = 0 From 488e8b1fd869ceb6d16c3876257fdeb971e8f343 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sat, 21 Apr 2018 10:29:21 +0530 Subject: [PATCH 16/51] Enable test runs only when CosmosDB config is configured The tests would now run in following cases -- There is a travis build for main repo -- If CosmosDB config is found For test runs on forks or PR the run would be considered cancelled if config is not found --- tests/src/test/scala/common/TestUtils.java | 18 ++++++++++++++---- .../cosmosdb/CosmosDBArtifactStoreTests.scala | 11 ++++++++--- .../behavior/ArtifactStoreBehaviorBase.scala | 17 +++++++++++++++-- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/tests/src/test/scala/common/TestUtils.java b/tests/src/test/scala/common/TestUtils.java index a4e7a4bcaa4..9fa1082c850 100644 --- a/tests/src/test/scala/common/TestUtils.java +++ b/tests/src/test/scala/common/TestUtils.java @@ -27,10 +27,7 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.text.SimpleDateFormat; -import java.util.Collections; -import java.util.Date; -import java.util.HashMap; -import java.util.Map; +import java.util.*; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -200,6 +197,19 @@ public static String getTime() { return sdf.format(date); } + /** + * Determines if the test build is running for main repo and not on any fork or PR + */ + public static boolean isBuildingOnMainRepo(){ + //Based on https://docs.travis-ci.com/user/environment-variables/#Default-Environment-Variables + String repoName = System.getenv("TRAVIS_REPO_SLUG"); + if (repoName == null) { + return false; //Not a travis build + } else { + return repoName.startsWith("apache/") && "false".equals(System.getenv("TRAVIS_PULL_REQUEST")); + } + } + /** * Encapsulates the result of running a native command, providing: * exitCode the exit code of the process diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala index 1cc1c9dc6e2..81a8cfcbe46 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala @@ -20,21 +20,26 @@ package whisk.core.database.cosmosdb import org.junit.runner.RunWith import org.scalatest.FlatSpec import org.scalatest.junit.JUnitRunner +import pureconfig.loadConfigOrThrow +import whisk.core.ConfigKeys import whisk.core.database.test.behavior.ArtifactStoreBehavior import whisk.core.entity._ import scala.reflect.classTag +import scala.util.Try @RunWith(classOf[JUnitRunner]) class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreBehavior { override def storeType = "CosmosDB" - override val authStore = { + override lazy val storeAvailableCheck = Try { loadConfigOrThrow[CosmosDBConfig](ConfigKeys.cosmosdb) } + + override lazy val authStore = { implicit val docReader: DocumentReader = WhiskDocumentReader CosmosDBArtifactStoreProvider.makeStore[WhiskAuth]() } - override val entityStore = + override lazy val entityStore = CosmosDBArtifactStoreProvider.makeStore[WhiskEntity]()( classTag[WhiskEntity], WhiskEntityJsonFormat, @@ -43,7 +48,7 @@ class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreBehavior { logging, materializer) - override val activationStore = { + override lazy val activationStore = { implicit val docReader: DocumentReader = WhiskDocumentReader CosmosDBArtifactStoreProvider.makeStore[WhiskActivation]() } diff --git a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala index c86e4802a46..c2aeb73454d 100644 --- a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala +++ b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala @@ -20,7 +20,7 @@ package whisk.core.database.test.behavior import java.time.Instant import akka.stream.ActorMaterializer -import common.{StreamLogging, WskActorSystem} +import common.{StreamLogging, TestUtils, WskActorSystem} import org.scalatest.concurrent.{IntegrationPatience, ScalaFutures} import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach, FlatSpec, Matchers} import spray.json.{JsObject, JsValue} @@ -31,7 +31,7 @@ import whisk.core.database.{ArtifactStore, AttachmentStore, StaleParameter} import whisk.core.entity._ import whisk.utils.JsHelpers -import scala.util.Random +import scala.util.{Failure, Random, Success, Try} trait ArtifactStoreBehaviorBase extends FlatSpec @@ -73,6 +73,7 @@ trait ArtifactStoreBehaviorBase } override protected def withFixture(test: NoArgTest) = { + assume(storeAvailable(), s"$storeType not configured or available") val outcome = super.withFixture(test) if (outcome.isFailed) { println(logLines.mkString("\n")) @@ -80,6 +81,18 @@ trait ArtifactStoreBehaviorBase outcome } + private def storeAvailable(): Boolean = { + storeAvailableCheck match { + case Success(_) => true + case Failure(x) => + //If running on master on main repo build tests MUST be run + //For non main repo runs like in fork or for PR its fine for test + //to be cancelled + if (TestUtils.isBuildingOnMainRepo) throw x else false + } + } + + protected def storeAvailableCheck: Try[Any] = Try(true) //~----------------------------------------< utility methods > protected def query[A <: WhiskEntity]( From 9c3329e73bdf335c2bff8626dd997f2f9eb5e297 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sat, 21 Apr 2018 10:37:47 +0530 Subject: [PATCH 17/51] Fix count with skip --- .../whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index 594b93cb92b..48eb32bad39 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -232,7 +232,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val f = client .queryDocuments(collection.getSelfLink, querySpec, options) .head() - .map(_.getResults.asScala.head.getLong(aggregate).longValue()) + .map(_.getResults.asScala.head.getLong(aggregate).longValue() - skip) f.onSuccess({ case out => transid.finished(this, start, s"[COUNT] '$collName' completed: count $out") From 70401955b1ff301b03acddf44e9b14fce88d69cd Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sat, 21 Apr 2018 13:14:26 +0530 Subject: [PATCH 18/51] Implement Attachment support For now use default CosmosDB attachment storage. Later this would be replaced with AttachmentStore SPI usage Also added few logs around completion of put and del --- .../cosmosdb/CosmosDBArtifactStore.scala | 138 ++++++++++++++---- 1 file changed, 112 insertions(+), 26 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index 48eb32bad39..66fc0a323e4 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -17,21 +17,22 @@ package whisk.core.database.cosmosdb +import java.io.ByteArrayInputStream + import _root_.rx.RxReactiveStreams import akka.actor.ActorSystem import akka.http.scaladsl.model.{ContentType, StatusCodes} import akka.stream.ActorMaterializer -import akka.stream.scaladsl.{Sink, Source} -import akka.util.ByteString +import akka.stream.scaladsl.{Sink, Source, StreamConverters} +import akka.util.{ByteString, ByteStringBuilder} import com.microsoft.azure.cosmosdb._ - import com.microsoft.azure.cosmosdb.rx.AsyncDocumentClient import spray.json.{DefaultJsonProtocol, JsObject, JsString, JsValue, RootJsonFormat, _} import whisk.common.{Logging, LoggingMarkers, TransactionId} import whisk.core.database.StoreUtils.{checkDocHasRevision, deserialize, reportFailure} import whisk.core.database._ -import whisk.core.database.cosmosdb.CosmosDBConstants._ import whisk.core.database.cosmosdb.CosmosDBArtifactStoreProvider.DocumentClientRef +import whisk.core.database.cosmosdb.CosmosDBConstants._ import whisk.core.entity._ import whisk.http.Messages @@ -79,22 +80,16 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val f = o .head() .transform( - r => toDocInfo(r.getResource), { - case e: DocumentClientException - if e.getStatusCode == StatusCodes.Conflict.intValue || e.getStatusCode == StatusCodes.PreconditionFailed.intValue => + { r => + transid.finished(this, start, s"[PUT] '$collName' completed document: '$docinfoStr'") + toDocInfo(r.getResource) + }, { + case e: DocumentClientException if isConflict(e) => + transid.finished(this, start, s"[PUT] '$collName', document: '$docinfoStr'; conflict.") DocumentConflictException("conflict on 'put'") case e => e }) - f.onFailure({ - case _: DocumentConflictException => - transid.finished(this, start, s"[PUT] '$collName', document: '$docinfoStr'; conflict.") - }) - - f.onSuccess({ - case _ => transid.finished(this, start, s"[PUT] '$collName' completed document: '$docinfoStr'") - }) - reportFailure(f, start, failure => s"[PUT] '$collName' internal error, failure: '${failure.getMessage}'") } @@ -105,10 +100,15 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected .deleteDocument(selfLinkOf(doc.id), matchRevOption(doc.rev.rev)) .head() .transform( - _ => true, { - case e: DocumentClientException if e.getStatusCode == StatusCodes.NotFound.intValue => + { _ => + transid.finished(this, start, s"[DEL] '$collName' completed document: '$doc'") + true + }, { + case e: DocumentClientException if isNotFound(e) => + transid.finished(this, start, s"[DEL] '$collName', document: '${doc}'; not found.") NoDocumentException("not found on 'delete'") - case e: DocumentClientException if e.getStatusCode == StatusCodes.PreconditionFailed.intValue => + case e: DocumentClientException if isConflict(e) => + transid.finished(this, start, s"[DEL] '$collName', document: '${doc}'; conflict.") DocumentConflictException("conflict on 'delete'") case e => e }) @@ -133,7 +133,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected transid.finished(this, start, s"[GET] '$collName' completed: found document '$doc'") deserialize[A, DocumentAbstraction](doc, js) }, { - case e: DocumentClientException if e.getStatusCode == StatusCodes.NotFound.intValue => + case e: DocumentClientException if isNotFound(e) => transid.finished(this, start, s"[GET] '$collName', document: '$doc'; not found.") // for compatibility throw NoDocumentException("not found on 'get'") @@ -162,7 +162,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected Some(js) } .recoverWith { - case e: DocumentClientException if e.getStatusCode == StatusCodes.NotFound.intValue => Future.successful(None) + case e: DocumentClientException if isNotFound(e) => Future.successful(None) } reportFailure( @@ -245,16 +245,102 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected doc: DocInfo, name: String, contentType: ContentType, - docStream: Source[ByteString, _])(implicit transid: TransactionId): Future[DocInfo] = ??? + docStream: Source[ByteString, _])(implicit transid: TransactionId): Future[DocInfo] = { + val start = transid.started( + this, + LoggingMarkers.DATABASE_ATT_SAVE, + s"[ATT_PUT] '$collName' uploading attachment '$name' of document '$doc'") + + checkDocHasRevision(doc) + val options = new MediaOptions + options.setContentType(contentType.toString()) + options.setSlug(name) + + //TODO Temporary implementation till AttachmentStore PR is merged + val f = docStream + .runFold(new ByteStringBuilder)((builder, b) => builder ++= b) + .map(_.result().toArray) + .map(new ByteArrayInputStream(_)) + .flatMap(s => client.upsertAttachment(selfLinkOf(doc.id), s, options, matchRevOption(doc.rev.rev)).head()) + .transform( + { _ => + transid + .finished(this, start, s"[ATT_PUT] '$collName' completed uploading attachment '$name' of document '$doc'") + doc //Adding attachment does not change the revision of document. So retain the doc info + }, { + case e: DocumentClientException if isConflict(e) => + transid + .finished(this, start, s"[ATT_PUT] '$collName' uploading attachment '$name' of document '$doc'; conflict") + DocumentConflictException("conflict on 'attachment put'") + case e => e + }) + + reportFailure( + f, + start, + failure => s"[ATT_PUT] '$collName' internal error, name: '$name', doc: '$doc', failure: '${failure.getMessage}'") + } override protected[core] def readAttachment[T](doc: DocInfo, name: String, sink: Sink[ByteString, Future[T]])( - implicit transid: TransactionId): Future[(ContentType, T)] = ??? + implicit transid: TransactionId): Future[(ContentType, T)] = { + val start = transid.started( + this, + LoggingMarkers.DATABASE_ATT_GET, + s"[ATT_GET] '$collName' finding attachment '$name' of document '$doc'") + checkDocHasRevision(doc) + + val f = client + .readAttachment(s"${selfLinkOf(doc.id)}/attachments/$name", matchRevOption(doc.rev.rev)) + .head() + .flatMap(a => client.readMedia(a.getResource.getMediaLink).head()) + .transform( + { r => + //Here stream can only be fetched once + StreamConverters + .fromInputStream(() => r.getMedia) + .runWith(sink) + .map((parseContentType(r), _)) + }, { + case e: DocumentClientException if isNotFound(e) => + transid.finished( + this, + start, + s"[ATT_GET] '$collName', retrieving attachment '$name' of document '$doc'; not found.") + NoDocumentException("not found on 'delete'") + case e => e + }) + .flatMap(identity) + + reportFailure( + f, + start, + failure => s"[ATT_GET] '$collName' internal error, name: '$name', doc: '$doc', failure: '${failure.getMessage}'") + } override protected[core] def deleteAttachments[T](doc: DocInfo)(implicit transid: TransactionId): Future[Boolean] = - ??? + // NOTE: this method is not intended for standalone use for CosmosDB. + // To delete attachments, it is expected that the entire document is deleted. + Future.successful(true) override def shutdown(): Unit = clientRef.close() + private def parseContentType(r: MediaResponse): ContentType = { + val typeString = r.getResponseHeaders.asScala.getOrElse( + "Content-Type", + throw new RuntimeException(s"Content-Type header not found in response ${r.getResponseHeaders}")) + ContentType.parse(typeString) match { + case Right(ct) => ct + case Left(_) => throw new RuntimeException(s"Invalid Content-Type header $typeString") //Should not happen + } + } + + private def isNotFound[A <: DocumentAbstraction](e: DocumentClientException) = + e.getStatusCode == StatusCodes.NotFound.intValue + + private def isConflict(e: DocumentClientException) = { + e.getStatusCode == StatusCodes.Conflict.intValue || e.getStatusCode == StatusCodes.PreconditionFailed.intValue + } + private def toCosmosDoc(json: JsObject): Document = { val computed = documentHandler.computedFields(json) val computedOpt = if (computed.fields.nonEmpty) Some(computed) else None @@ -299,7 +385,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected JsObject(js.fields.filter { case (k, _) => !k.startsWith("_") && k != cid }) } - private def toDocInfo(doc: Document) = { + private def toDocInfo[T <: Resource](doc: T) = { checkDoc(doc) DocInfo(DocId(unescapeId(doc.getId)), DocRevision(doc.getETag)) } @@ -316,7 +402,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected options } - private def checkDoc(doc: Document): Unit = { + private def checkDoc[T <: Resource](doc: T): Unit = { require(doc.getId != null, s"$doc does not have id field set") require(doc.getETag != null, s"$doc does not have etag field set") } From d87df1d61fbf18172fa98ad94d66f395eefd5043 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sat, 21 Apr 2018 13:15:07 +0530 Subject: [PATCH 19/51] Use a separate db for travis tests --- tests/src/test/resources/application.conf.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/test/resources/application.conf.j2 b/tests/src/test/resources/application.conf.j2 index 7a40c243505..f431bf09732 100644 --- a/tests/src/test/resources/application.conf.j2 +++ b/tests/src/test/resources/application.conf.j2 @@ -49,7 +49,7 @@ whisk { cosmosdb { host = ${?COSMOSDB_HOST} key = ${?COSMOSDB_KEY} - db = "owtest" + db = "owtest-travis" } controller { From 435c8040bfc858956152c538ea65c0702f30a562 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sat, 21 Apr 2018 14:03:44 +0530 Subject: [PATCH 20/51] Pickup db name from environment variable --- tests/src/test/resources/application.conf.j2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/test/resources/application.conf.j2 b/tests/src/test/resources/application.conf.j2 index f431bf09732..6f374790f46 100644 --- a/tests/src/test/resources/application.conf.j2 +++ b/tests/src/test/resources/application.conf.j2 @@ -49,7 +49,7 @@ whisk { cosmosdb { host = ${?COSMOSDB_HOST} key = ${?COSMOSDB_KEY} - db = "owtest-travis" + db = ${?COSMOSDB_NAME} } controller { From 1641b7bd8ea2257fbbcd5148dac831c246df1e19 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sun, 22 Apr 2018 11:43:07 +0530 Subject: [PATCH 21/51] Change the config name --- .../database/cosmosdb/CosmosDBArtifactStoreProvider.scala | 4 ++-- tests/src/test/resources/application.conf.j2 | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala index 9780de77318..545b561ea61 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala @@ -32,7 +32,7 @@ import whisk.core.entity.{DocumentReader, WhiskActivation, WhiskAuth, WhiskEntit import scala.reflect.ClassTag -case class CosmosDBConfig(host: String, key: String, db: String) +case class CosmosDBConfig(endpoint: String, key: String, db: String) case class ClientHolder(client: AsyncDocumentClient) extends Closeable { override def close(): Unit = client.close() @@ -103,7 +103,7 @@ object CosmosDBArtifactStoreProvider extends ArtifactStoreProvider { private def createClient(config: CosmosDBConfig): AsyncDocumentClient = new AsyncDocumentClient.Builder() - .withServiceEndpoint(config.host) + .withServiceEndpoint(config.endpoint) .withMasterKey(config.key) .withConnectionPolicy(ConnectionPolicy.GetDefault) .withConsistencyLevel(ConsistencyLevel.Session) diff --git a/tests/src/test/resources/application.conf.j2 b/tests/src/test/resources/application.conf.j2 index 6f374790f46..59e33f04ee9 100644 --- a/tests/src/test/resources/application.conf.j2 +++ b/tests/src/test/resources/application.conf.j2 @@ -47,9 +47,9 @@ whisk { } cosmosdb { - host = ${?COSMOSDB_HOST} - key = ${?COSMOSDB_KEY} - db = ${?COSMOSDB_NAME} + endpoint = ${?COSMOSDB_ENDPOINT} + key = ${?COSMOSDB_KEY} + db = ${?COSMOSDB_NAME} } controller { From fb009c0c6539465a5b9c94c027565d929dd002f2 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 25 Apr 2018 15:13:23 +0530 Subject: [PATCH 22/51] Implement indexing support --- .../cosmosdb/CosmosDBArtifactStore.scala | 47 ++++--- .../CosmosDBArtifactStoreProvider.scala | 9 +- .../database/cosmosdb/CosmosDBSupport.scala | 34 ++++- .../core/database/cosmosdb/CosmosDBUtil.scala | 15 ++- .../cosmosdb/CosmosDBViewMapper.scala | 59 ++++++++- .../database/cosmosdb/IndexingPolicy.scala | 123 ++++++++++++++++++ .../cosmosdb/CosmosDBArtifactStoreTests.scala | 4 +- .../cosmosdb/CosmosDBSupportTests.scala | 69 ++++++++++ .../database/cosmosdb/CosmosDBTestUtil.scala | 70 ++++++++++ .../behavior/ArtifactStoreBehaviorBase.scala | 18 +-- .../test/behavior/ArtifactStoreTestUtil.scala | 36 +++++ 11 files changed, 426 insertions(+), 58 deletions(-) create mode 100644 common/scala/src/main/scala/whisk/core/database/cosmosdb/IndexingPolicy.scala create mode 100644 tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBSupportTests.scala create mode 100644 tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestUtil.scala create mode 100644 tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreTestUtil.scala diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index 66fc0a323e4..4fd99761b66 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -43,7 +43,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected protected val config: CosmosDBConfig, clientRef: DocumentClientRef, documentHandler: DocumentHandler, - viewMapper: CosmosDBViewMapper)( + protected val viewMapper: CosmosDBViewMapper)( implicit system: ActorSystem, val logging: Logging, jsonFormat: RootJsonFormat[DocumentAbstraction], @@ -68,13 +68,14 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected //TODO Batching support val doc = toCosmosDoc(asJson) - val docinfoStr = s"id: ${doc.getId}, rev: ${doc.getETag}" + val id = doc.getId + val docinfoStr = s"id: $id, rev: ${doc.getETag}" val start = transid.started(this, LoggingMarkers.DATABASE_SAVE, s"[PUT] '$collName' saving document: '$docinfoStr'") val o = if (doc.getETag == null) { - client.createDocument(collection.getSelfLink, doc, null, true) + client.createDocument(collection.getSelfLink, doc, newRequestOption(id), true) } else { - client.replaceDocument(doc, matchRevOption(doc.getETag)) + client.replaceDocument(doc, matchRevOption(id, doc.getETag)) } val f = o @@ -97,7 +98,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected checkDocHasRevision(doc) val start = transid.started(this, LoggingMarkers.DATABASE_DELETE, s"[DEL] '$collName' deleting document: '$doc'") val f = client - .deleteDocument(selfLinkOf(doc.id), matchRevOption(doc.rev.rev)) + .deleteDocument(selfLinkOf(doc.id), matchRevOption(doc.id.id, doc.rev.rev)) .head() .transform( { _ => @@ -125,7 +126,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected require(doc != null, "doc undefined") val f = client - .readDocument(selfLinkOf(doc.id), null) + .readDocument(selfLinkOf(doc.id), newRequestOption(doc.id.id)) .head() .transform( { rr => @@ -154,7 +155,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val start = transid.started(this, LoggingMarkers.DATABASE_GET, s"[GET_BY_ID] '$collName' finding document: '$id'") val f = client - .readDocument(selfLinkOf(id), null) + .readDocument(selfLinkOf(id), newRequestOption(id.id)) .head() .map { rr => val js = getResultToWhiskJsonDoc(rr.getResource) @@ -192,10 +193,8 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val querySpec = viewMapper.prepareQuery(ddoc, viewName, startKey, endKey, realLimit, realIncludeDocs, descending) - val options = new FeedOptions() - options.setEnableCrossPartitionQuery(true) - - val publisher = RxReactiveStreams.toPublisher(client.queryDocuments(collection.getSelfLink, querySpec, options)) + val publisher = + RxReactiveStreams.toPublisher(client.queryDocuments(collection.getSelfLink, querySpec, newFeedOptions())) val f = Source .fromPublisher(publisher) .mapConcat(asSeq) @@ -225,12 +224,9 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val start = transid.started(this, LoggingMarkers.DATABASE_QUERY, s"[COUNT] '$collName' searching '$table") val querySpec = viewMapper.prepareCountQuery(ddoc, viewName, startKey, endKey) - val options = new FeedOptions() - options.setEnableCrossPartitionQuery(true) - //For aggregates the value is in _aggregates fields val f = client - .queryDocuments(collection.getSelfLink, querySpec, options) + .queryDocuments(collection.getSelfLink, querySpec, newFeedOptions()) .head() .map(_.getResults.asScala.head.getLong(aggregate).longValue() - skip) @@ -261,7 +257,8 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected .runFold(new ByteStringBuilder)((builder, b) => builder ++= b) .map(_.result().toArray) .map(new ByteArrayInputStream(_)) - .flatMap(s => client.upsertAttachment(selfLinkOf(doc.id), s, options, matchRevOption(doc.rev.rev)).head()) + .flatMap(s => + client.upsertAttachment(selfLinkOf(doc.id), s, options, matchRevOption(doc.id.id, doc.rev.rev)).head()) .transform( { _ => transid @@ -290,7 +287,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected checkDocHasRevision(doc) val f = client - .readAttachment(s"${selfLinkOf(doc.id)}/attachments/$name", matchRevOption(doc.rev.rev)) + .readAttachment(s"${selfLinkOf(doc.id)}/attachments/$name", matchRevOption(doc.id.id, doc.rev.rev)) .head() .flatMap(a => client.readMedia(a.getResource.getMediaLink).head()) .transform( @@ -394,14 +391,26 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected private def createSelfLink(id: String) = s"dbs/${database.getId}/colls/${collection.getId}/docs/$id" - private def matchRevOption(etag: String) = { - val options = new RequestOptions + private def matchRevOption(id: String, etag: String) = { + val options = newRequestOption(id) val condition = new AccessCondition condition.setCondition(etag) options.setAccessCondition(condition) options } + private def newRequestOption(id: String) = { + val options = new RequestOptions + options.setPartitionKey(new PartitionKey(escapeId(id))) + options + } + + private def newFeedOptions() = { + val options = new FeedOptions() + options.setEnableCrossPartitionQuery(true) + options + } + private def checkDoc[T <: Resource](doc: T): Unit = { require(doc.getId != null, s"$doc does not have id field set") require(doc.getETag != null, s"$doc does not have etag field set") diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala index 545b561ea61..e547b03a3c8 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala @@ -21,13 +21,13 @@ import java.io.Closeable import akka.actor.ActorSystem import akka.stream.ActorMaterializer -import com.microsoft.azure.cosmosdb.{ConnectionPolicy, ConsistencyLevel} import com.microsoft.azure.cosmosdb.rx.AsyncDocumentClient import spray.json.RootJsonFormat import whisk.common.Logging import whisk.core.database._ import pureconfig._ import whisk.core.ConfigKeys +import whisk.core.database.cosmosdb.CosmosDBUtil.createClient import whisk.core.entity.{DocumentReader, WhiskActivation, WhiskAuth, WhiskEntity} import scala.reflect.ClassTag @@ -101,11 +101,4 @@ object CosmosDBArtifactStoreProvider extends ArtifactStoreProvider { private def createReference(config: CosmosDBConfig) = new ReferenceCounted[ClientHolder](ClientHolder(createClient(config))) - private def createClient(config: CosmosDBConfig): AsyncDocumentClient = - new AsyncDocumentClient.Builder() - .withServiceEndpoint(config.endpoint) - .withMasterKey(config.key) - .withConnectionPolicy(ConnectionPolicy.GetDefault) - .withConsistencyLevel(ConsistencyLevel.Session) - .build } diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala index 342890ff826..e3dcd0042ba 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala @@ -29,6 +29,7 @@ trait CosmosDBSupport { protected def config: CosmosDBConfig protected def collName: String protected def client: AsyncDocumentClient + protected def viewMapper: CosmosDBViewMapper def initialize(): (Database, DocumentCollection) = { val db = getOrCreateDatabase() @@ -44,16 +45,35 @@ trait CosmosDBSupport { } private def getOrCreateCollection(database: Database) = { - blockingResult[DocumentCollection]( - client - .queryCollections(database.getSelfLink, querySpec(collName), null) - .asScala).getOrElse { - val collectionDefinition = new DocumentCollection - collectionDefinition.setId(collName) - blockingResult(client.createCollection(database.getSelfLink, collectionDefinition, null).asScala) + val collOpt = blockingResult[DocumentCollection]( + client.queryCollections(database.getSelfLink, querySpec(collName), null).asScala) + .map { coll => + if (matchingIndexingPolicy(coll)) { + coll + } else { + //Modify the found collection as its selfLink is set + coll.setIndexingPolicy(viewMapper.indexingPolicy.asJava()) + blockingResult(client.replaceCollection(coll, null).asScala) + } + } + + collOpt.getOrElse { + val defn: DocumentCollection = newDatabaseCollection + blockingResult(client.createCollection(database.getSelfLink, defn, null).asScala) } } + private def matchingIndexingPolicy(coll: DocumentCollection): Boolean = + IndexingPolicy.isSame(viewMapper.indexingPolicy, IndexingPolicy(coll.getIndexingPolicy)) + + private def newDatabaseCollection = { + val defn = new DocumentCollection + defn.setId(collName) + defn.setIndexingPolicy(viewMapper.indexingPolicy.asJava()) + defn.setPartitionKey(viewMapper.partitionKeyDefn) + defn + } + private def blockingResult[T <: Resource](response: Observable[FeedResponse[T]]) = { val value = response.toList.toBlocking.single value.head.getResults.asScala.headOption diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala index 30a5581c37a..6d861988d19 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala @@ -17,9 +17,12 @@ package whisk.core.database.cosmosdb -import scala.collection.immutable.Iterable -import whisk.core.database.cosmosdb.CosmosDBConstants._ +import com.microsoft.azure.cosmosdb._ import com.microsoft.azure.cosmosdb.internal.Constants.Properties.{AGGREGATE, E_TAG, ID, SELF_LINK} +import com.microsoft.azure.cosmosdb.rx.AsyncDocumentClient +import whisk.core.database.cosmosdb.CosmosDBConstants._ + +import scala.collection.immutable.Iterable object CosmosDBConstants { val _computed: String = "_c" @@ -78,4 +81,12 @@ object CosmosDBUtil { case x => x.toString } + def createClient(config: CosmosDBConfig): AsyncDocumentClient = + new AsyncDocumentClient.Builder() + .withServiceEndpoint(config.endpoint) + .withMasterKey(config.key) + .withConnectionPolicy(ConnectionPolicy.GetDefault) + .withConsistencyLevel(ConsistencyLevel.Session) + .build + } diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala index c988c36e17d..179a47eb326 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -17,8 +17,21 @@ package whisk.core.database.cosmosdb -import com.microsoft.azure.cosmosdb.{SqlParameter, SqlParameterCollection, SqlQuerySpec} -import whisk.core.database._ +import java.util.Collections + +import com.microsoft.azure.cosmosdb.IndexKind.{Hash, Range} +import com.microsoft.azure.cosmosdb.DataType.{Number, String} +import com.microsoft.azure.cosmosdb.{PartitionKeyDefinition, SqlParameter, SqlParameterCollection, SqlQuerySpec} +import whisk.core.database.ActivationHandler.NS_PATH +import whisk.core.database.WhisksHandler.ROOT_NS +import whisk.core.database.{ + ActivationHandler, + DocumentHandler, + SubjectHandler, + UnsupportedQueryKeys, + UnsupportedView, + WhisksHandler +} import whisk.core.database.cosmosdb.CosmosDBConstants._computed import whisk.core.database.cosmosdb.CosmosDBConstants.alias import whisk.core.entity.WhiskEntityQueries.TOP @@ -38,6 +51,14 @@ trait CosmosDBViewMapper { def prepareCountQuery(ddoc: String, viewName: String, startKey: List[Any], endKey: List[Any]): SqlQuerySpec + def indexingPolicy: IndexingPolicy + + val partitionKeyDefn: PartitionKeyDefinition = { + val defn = new PartitionKeyDefinition + defn.setPaths(Collections.singletonList("/id")) + defn + } + protected def checkKeys(startKey: List[Any], endKey: List[Any]): Unit = { require(startKey.nonEmpty) require(endKey.nonEmpty) @@ -104,7 +125,7 @@ abstract class SimpleMapper extends CosmosDBViewMapper { object WhisksViewMapper extends SimpleMapper { private val NS = "r.namespace" - private val ROOT_NS = s"r.${_computed}.${WhisksHandler.ROOT_NS}" + private val ROOT_NS_C = s"r.${_computed}.$ROOT_NS" private val TYPE = "r.entityType" private val UPDATED = "r.updated" private val PUBLISH = "r.publish" @@ -112,6 +133,14 @@ object WhisksViewMapper extends SimpleMapper { val handler = WhisksHandler + override def indexingPolicy: IndexingPolicy = + IndexingPolicy( + includedPaths = Set( + IncludedPath("/entityType/?", Index(Hash, String, -1)), + IncludedPath("/namespace/?", Index(Hash, String, -1)), + IncludedPath(s"/${_computed}/$ROOT_NS/?", Index(Hash, String, -1)), + IncludedPath("/updated/?", Index(Range, Number, -1)))) + override protected def where(ddoc: String, view: String, startKey: List[Any], @@ -123,7 +152,7 @@ object WhisksViewMapper extends SimpleMapper { viewConditions(ddoc, view).map(q => (s"${q._1} AND", q._2)).getOrElse((NOTHING, Nil)) val params = ("@entityType", entityType) :: ("@namespace", namespace) :: vcParams - val baseCondition = s"$vc $TYPE = @entityType AND ($NS = @namespace OR $ROOT_NS = @namespace)" + val baseCondition = s"$vc $TYPE = @entityType AND ($NS = @namespace OR $ROOT_NS_C = @namespace)" (startKey, endKey) match { case (_ :: Nil, _ :: `TOP` :: Nil) => @@ -156,11 +185,18 @@ object WhisksViewMapper extends SimpleMapper { } object ActivationViewMapper extends SimpleMapper { private val NS = "r.namespace" - private val NS_WITH_PATH = s"r.${_computed}.${ActivationHandler.NS_PATH}" + private val NS_WITH_PATH = s"r.${_computed}.$NS_PATH" private val START = "r.start" val handler = ActivationHandler + override def indexingPolicy: IndexingPolicy = + IndexingPolicy( + includedPaths = Set( + IncludedPath("/namespace/?", Index(Hash, String, -1)), + IncludedPath(s"/${_computed}/$NS_PATH/?", Index(Hash, String, -1)), + IncludedPath("/start/?", Index(Range, Number, -1)))) + override protected def where(ddoc: String, view: String, startKey: List[Any], @@ -201,6 +237,19 @@ object ActivationViewMapper extends SimpleMapper { object SubjectViewMapper extends CosmosDBViewMapper { val handler = SubjectHandler + override def indexingPolicy: IndexingPolicy = + //Booleans are indexed by default + //Specifying less precision for key as match on uuid should be sufficient + //and keys are bigger + IndexingPolicy( + includedPaths = Set( + IncludedPath("/uuid/?", Index(Hash, String, -1)), + IncludedPath("/key/?", Index(Hash, String, 3)), + IncludedPath("/namespaces/[]/uuid/?", Index(Hash, String, -1)), + IncludedPath("/namespaces/[]/key/?", Index(Hash, String, 3)), + IncludedPath("/concurrentInvocations/?", Index(Hash, Number, -1)), + IncludedPath("/invocationsPerMinute/?", Index(Hash, Number, -1)))) + override def prepareQuery(ddoc: String, view: String, startKey: List[Any], diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/IndexingPolicy.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/IndexingPolicy.scala new file mode 100644 index 00000000000..83c182ed7bd --- /dev/null +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/IndexingPolicy.scala @@ -0,0 +1,123 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import com.microsoft.azure.cosmosdb.{ + DataType, + HashIndex, + IndexKind, + IndexingMode, + RangeIndex, + ExcludedPath => JExcludedPath, + IncludedPath => JIncludedPath, + Index => JIndex, + IndexingPolicy => JIndexingPolicy +} + +import scala.collection.JavaConverters._ + +/** + * Scala based IndexingPolicy type which maps to java based IndexingPolicy. This is done for 2 reasons + * + * - Simplify constructing policy instance + * - Enable custom comparison between existing policy and desired policy as policy instances + * obtained from CosmosDB have extra index type configured per included path. Hence the comparison + * needs to be customized + * + */ +case class IndexingPolicy(mode: IndexingMode = IndexingMode.Consistent, + includedPaths: Set[IncludedPath], + excludedPaths: Set[ExcludedPath] = Set(ExcludedPath("/"))) { + + def asJava(): JIndexingPolicy = { + val policy = new JIndexingPolicy() + policy.setIndexingMode(mode) + policy.setIncludedPaths(includedPaths.map(_.asJava()).asJava) + policy.setExcludedPaths(excludedPaths.map(_.asJava()).asJava) + policy + } +} + +object IndexingPolicy { + def apply(policy: JIndexingPolicy): IndexingPolicy = + IndexingPolicy( + policy.getIndexingMode, + policy.getIncludedPaths.asScala.map(IncludedPath(_)).toSet, + policy.getExcludedPaths.asScala.map(ExcludedPath(_)).toSet) + + /** + * IndexingPolicy fetched from CosmosDB contains extra entries. So need to check + * that at least what we expect is present + */ + def isSame(expected: IndexingPolicy, current: IndexingPolicy): Boolean = { + expected.mode == current.mode && expected.excludedPaths == current.excludedPaths && + matchIncludes(expected.includedPaths, current.includedPaths) + } + + private def matchIncludes(expected: Set[IncludedPath], current: Set[IncludedPath]): Boolean = { + expected.size == current.size && expected.forall { i => + current.find(_.path == i.path) match { + case Some(x) => i.indexes.subsetOf(x.indexes) + case None => false + } + } + } +} + +case class IncludedPath(path: String, indexes: Set[Index]) { + def asJava(): JIncludedPath = { + val includedPath = new JIncludedPath() + includedPath.setIndexes(indexes.map(_.asJava()).asJava) + includedPath.setPath(path) + includedPath + } +} + +object IncludedPath { + def apply(ip: JIncludedPath): IncludedPath = IncludedPath(ip.getPath, ip.getIndexes.asScala.map(Index(_)).toSet) + + def apply(path: String, index: Index): IncludedPath = IncludedPath(path, Set(index)) +} + +case class ExcludedPath(path: String) { + def asJava(): JExcludedPath = { + val excludedPath = new JExcludedPath() + excludedPath.setPath(path) + excludedPath + } +} + +object ExcludedPath { + def apply(ep: JExcludedPath): ExcludedPath = ExcludedPath(ep.getPath) +} + +case class Index(kind: IndexKind, dataType: DataType, precision: Int) { + def asJava(): JIndex = kind match { + case IndexKind.Hash => JIndex.Hash(dataType, precision) + case IndexKind.Range => JIndex.Range(dataType, precision) + case _ => throw new RuntimeException(s"Unsupported kind $kind") + } +} + +object Index { + def apply(index: JIndex): Index = index match { + case i: HashIndex => Index(i.getKind, i.getDataType, i.getPrecision) + case i: RangeIndex => Index(i.getKind, i.getDataType, i.getPrecision) + case _ => throw new RuntimeException(s"Unsupported kind $index") + } +} diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala index 81a8cfcbe46..7456ba11583 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala @@ -20,8 +20,6 @@ package whisk.core.database.cosmosdb import org.junit.runner.RunWith import org.scalatest.FlatSpec import org.scalatest.junit.JUnitRunner -import pureconfig.loadConfigOrThrow -import whisk.core.ConfigKeys import whisk.core.database.test.behavior.ArtifactStoreBehavior import whisk.core.entity._ @@ -32,7 +30,7 @@ import scala.util.Try class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreBehavior { override def storeType = "CosmosDB" - override lazy val storeAvailableCheck = Try { loadConfigOrThrow[CosmosDBConfig](ConfigKeys.cosmosdb) } + override lazy val storeAvailableCheck: Try[Any] = CosmosDBTestUtil.storeConfig override lazy val authStore = { implicit val docReader: DocumentReader = WhiskDocumentReader diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBSupportTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBSupportTests.scala new file mode 100644 index 00000000000..8ffc807e7b7 --- /dev/null +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBSupportTests.scala @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import com.microsoft.azure.cosmosdb.IndexKind.Hash +import com.microsoft.azure.cosmosdb.DataType.String +import com.microsoft.azure.cosmosdb.DocumentCollection +import com.microsoft.azure.cosmosdb.rx.AsyncDocumentClient +import org.junit.runner.RunWith +import org.scalamock.scalatest.MockFactory +import org.scalatest.junit.JUnitRunner +import org.scalatest.{FlatSpec, Matchers} + +import scala.collection.JavaConverters._ + +@RunWith(classOf[JUnitRunner]) +class CosmosDBSupportTests extends FlatSpec with CosmosDBTestSupport with MockFactory with Matchers { + + behavior of "index" + + it should "be created and updated on init" in { + val testDb = createTestDB() + val config: CosmosDBConfig = storeConfig.copy(db = testDb.getId) + + val indexedPaths1 = Set("/foo/?", "/bar/?") + val (_, coll) = new CosmosTest(config, client, newMapper(indexedPaths1)).initialize() + indexedPaths(coll) should contain theSameElementsAs indexedPaths1 + + //Test if index definition is updated in code it gets updated in db also + val indexedPaths2 = Set("/foo/?", "/bar2/?") + val (_, coll2) = new CosmosTest(config, client, newMapper(indexedPaths2)).initialize() + indexedPaths(coll2) should contain theSameElementsAs indexedPaths2 + } + + private def newMapper(paths: Set[String]) = { + val mapper = stub[CosmosDBViewMapper] + mapper.indexingPolicy _ when () returns newTestIndexingPolicy(paths) + mapper + } + + private def indexedPaths(coll: DocumentCollection) = + coll.getIndexingPolicy.getIncludedPaths.asScala.map(_.getPath).toList + + protected def newTestIndexingPolicy(paths: Set[String]): IndexingPolicy = + IndexingPolicy(includedPaths = paths.map(p => IncludedPath(p, Index(Hash, String, -1)))) + + private class CosmosTest(override val config: CosmosDBConfig, + override val client: AsyncDocumentClient, + mapper: CosmosDBViewMapper) + extends CosmosDBSupport { + override protected def collName = "test" + override protected def viewMapper = mapper + } +} diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestUtil.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestUtil.scala new file mode 100644 index 00000000000..8a6385ec5ae --- /dev/null +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestUtil.scala @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import java.text.SimpleDateFormat +import java.util.Date + +import _root_.rx.lang.scala.JavaConverters._ +import com.microsoft.azure.cosmosdb.Database +import org.scalatest.{BeforeAndAfterAll, FlatSpec} +import pureconfig.loadConfigOrThrow +import whisk.core.ConfigKeys +import whisk.core.database.test.behavior.ArtifactStoreTestUtil.storeAvailable + +import scala.collection.mutable.ListBuffer +import scala.util.{Random, Try} + +object CosmosDBTestUtil { + + lazy val storeConfig = Try { loadConfigOrThrow[CosmosDBConfig](ConfigKeys.cosmosdb) } + +} + +trait CosmosDBTestSupport extends FlatSpec with BeforeAndAfterAll { + private val dbsToDelete = ListBuffer[Database]() + + lazy val storeConfigTry = Try { loadConfigOrThrow[CosmosDBConfig](ConfigKeys.cosmosdb) } + lazy val client = CosmosDBUtil.createClient(storeConfig) + + def storeConfig = storeConfigTry.get + + override protected def withFixture(test: NoArgTest) = { + assume(storeAvailable(storeConfigTry), "CosmosDB not configured or available") + super.withFixture(test) + } + + protected def generateDBName() = { + val format = new SimpleDateFormat("yyyy-MM-dd") + s"${format.format(new Date())}-${Random.alphanumeric.take(5).mkString}" + } + + protected def createTestDB() = { + val databaseDefinition = new Database + databaseDefinition.setId(generateDBName()) + val db = client.createDatabase(databaseDefinition, null).asScala.toBlocking.single.getResource + dbsToDelete += db + db + } + + override def afterAll(): Unit = { + super.afterAll() + dbsToDelete.foreach(db => client.deleteDatabase(db.getSelfLink, null).asScala.toBlocking.single.getResource) + client.close() + } +} diff --git a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala index c2aeb73454d..21fda4812da 100644 --- a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala +++ b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala @@ -20,18 +20,19 @@ package whisk.core.database.test.behavior import java.time.Instant import akka.stream.ActorMaterializer -import common.{StreamLogging, TestUtils, WskActorSystem} +import common.{StreamLogging, WskActorSystem} import org.scalatest.concurrent.{IntegrationPatience, ScalaFutures} import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach, FlatSpec, Matchers} import spray.json.{JsObject, JsValue} import whisk.common.TransactionId import whisk.core.database.memory.MemoryAttachmentStore import whisk.core.database.test.DbUtils +import whisk.core.database.test.behavior.ArtifactStoreTestUtil.storeAvailable import whisk.core.database.{ArtifactStore, AttachmentStore, StaleParameter} import whisk.core.entity._ import whisk.utils.JsHelpers -import scala.util.{Failure, Random, Success, Try} +import scala.util.{Random, Try} trait ArtifactStoreBehaviorBase extends FlatSpec @@ -73,7 +74,7 @@ trait ArtifactStoreBehaviorBase } override protected def withFixture(test: NoArgTest) = { - assume(storeAvailable(), s"$storeType not configured or available") + assume(storeAvailable(storeAvailableCheck), s"$storeType not configured or available") val outcome = super.withFixture(test) if (outcome.isFailed) { println(logLines.mkString("\n")) @@ -81,17 +82,6 @@ trait ArtifactStoreBehaviorBase outcome } - private def storeAvailable(): Boolean = { - storeAvailableCheck match { - case Success(_) => true - case Failure(x) => - //If running on master on main repo build tests MUST be run - //For non main repo runs like in fork or for PR its fine for test - //to be cancelled - if (TestUtils.isBuildingOnMainRepo) throw x else false - } - } - protected def storeAvailableCheck: Try[Any] = Try(true) //~----------------------------------------< utility methods > diff --git a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreTestUtil.scala b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreTestUtil.scala new file mode 100644 index 00000000000..417a0d0f351 --- /dev/null +++ b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreTestUtil.scala @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.test.behavior + +import common.TestUtils + +import scala.util.{Failure, Success, Try} + +object ArtifactStoreTestUtil { + + def storeAvailable(storeAvailableCheck: Try[Any]): Boolean = { + storeAvailableCheck match { + case Success(_) => true + case Failure(x) => + //If running on master on main repo build tests MUST be run + //For non main repo runs like in fork or for PR its fine for test + //to be cancelled + if (TestUtils.isBuildingOnMainRepo) throw x else false + } + } +} From 368d99fc738895ad5075da681a2b5051af4f873e Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 25 Apr 2018 15:16:31 +0530 Subject: [PATCH 23/51] Refactor matchRevOption --- .../database/cosmosdb/CosmosDBArtifactStore.scala | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index 4fd99761b66..df331704120 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -98,7 +98,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected checkDocHasRevision(doc) val start = transid.started(this, LoggingMarkers.DATABASE_DELETE, s"[DEL] '$collName' deleting document: '$doc'") val f = client - .deleteDocument(selfLinkOf(doc.id), matchRevOption(doc.id.id, doc.rev.rev)) + .deleteDocument(selfLinkOf(doc.id), matchRevOption(doc)) .head() .transform( { _ => @@ -257,8 +257,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected .runFold(new ByteStringBuilder)((builder, b) => builder ++= b) .map(_.result().toArray) .map(new ByteArrayInputStream(_)) - .flatMap(s => - client.upsertAttachment(selfLinkOf(doc.id), s, options, matchRevOption(doc.id.id, doc.rev.rev)).head()) + .flatMap(s => client.upsertAttachment(selfLinkOf(doc.id), s, options, matchRevOption(doc)).head()) .transform( { _ => transid @@ -287,7 +286,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected checkDocHasRevision(doc) val f = client - .readAttachment(s"${selfLinkOf(doc.id)}/attachments/$name", matchRevOption(doc.id.id, doc.rev.rev)) + .readAttachment(s"${selfLinkOf(doc.id)}/attachments/$name", matchRevOption(doc)) .head() .flatMap(a => client.readMedia(a.getResource.getMediaLink).head()) .transform( @@ -391,7 +390,9 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected private def createSelfLink(id: String) = s"dbs/${database.getId}/colls/${collection.getId}/docs/$id" - private def matchRevOption(id: String, etag: String) = { + private def matchRevOption(info: DocInfo): RequestOptions = matchRevOption(info.id.id, info.rev.rev) + + private def matchRevOption(id: String, etag: String): RequestOptions = { val options = newRequestOption(id) val condition = new AccessCondition condition.setCondition(etag) From 6527b17228bc91e78c9d60bec89f057146e72577 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 25 Apr 2018 15:20:47 +0530 Subject: [PATCH 24/51] Make classes package private --- .../whisk/core/database/cosmosdb/CosmosDBSupport.scala | 2 +- .../whisk/core/database/cosmosdb/CosmosDBUtil.scala | 4 ++-- .../core/database/cosmosdb/CosmosDBViewMapper.scala | 10 +++++----- .../core/database/cosmosdb/ReferenceCounted.scala | 2 +- .../core/database/cosmosdb/RxObservableImplicits.scala | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala index e3dcd0042ba..8fe88122ff0 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala @@ -25,7 +25,7 @@ import _root_.rx.lang.scala.JavaConverters._ import scala.collection.JavaConverters._ import scala.collection.immutable -trait CosmosDBSupport { +private[cosmosdb] trait CosmosDBSupport { protected def config: CosmosDBConfig protected def collName: String protected def client: AsyncDocumentClient diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala index 6d861988d19..9bd8c9f7f32 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala @@ -24,7 +24,7 @@ import whisk.core.database.cosmosdb.CosmosDBConstants._ import scala.collection.immutable.Iterable -object CosmosDBConstants { +private[cosmosdb] object CosmosDBConstants { val _computed: String = "_c" val alias: String = "view" @@ -38,7 +38,7 @@ object CosmosDBConstants { val selfLink: String = SELF_LINK } -object CosmosDBUtil { +private[cosmosdb] object CosmosDBUtil { /** * Prepares the json like select clause diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala index 179a47eb326..6538d6ecaba 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -36,7 +36,7 @@ import whisk.core.database.cosmosdb.CosmosDBConstants._computed import whisk.core.database.cosmosdb.CosmosDBConstants.alias import whisk.core.entity.WhiskEntityQueries.TOP -trait CosmosDBViewMapper { +private[cosmosdb] trait CosmosDBViewMapper { protected val NOTHING = "" protected val ALL_FIELDS = "*" protected def handler: DocumentHandler @@ -73,7 +73,7 @@ trait CosmosDBViewMapper { } } -abstract class SimpleMapper extends CosmosDBViewMapper { +private[cosmosdb] abstract class SimpleMapper extends CosmosDBViewMapper { def prepareQuery(ddoc: String, viewName: String, @@ -123,7 +123,7 @@ abstract class SimpleMapper extends CosmosDBViewMapper { protected def orderByField(ddoc: String, viewName: String): String } -object WhisksViewMapper extends SimpleMapper { +private[cosmosdb] object WhisksViewMapper extends SimpleMapper { private val NS = "r.namespace" private val ROOT_NS_C = s"r.${_computed}.$ROOT_NS" private val TYPE = "r.entityType" @@ -183,7 +183,7 @@ object WhisksViewMapper extends SimpleMapper { } } -object ActivationViewMapper extends SimpleMapper { +private[cosmosdb] object ActivationViewMapper extends SimpleMapper { private val NS = "r.namespace" private val NS_WITH_PATH = s"r.${_computed}.$NS_PATH" private val START = "r.start" @@ -234,7 +234,7 @@ object ActivationViewMapper extends SimpleMapper { case _ => throw UnsupportedView(s"$ddoc/$view") } } -object SubjectViewMapper extends CosmosDBViewMapper { +private[cosmosdb] object SubjectViewMapper extends CosmosDBViewMapper { val handler = SubjectHandler override def indexingPolicy: IndexingPolicy = diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/ReferenceCounted.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/ReferenceCounted.scala index aee6f76c375..a05a9140c40 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/ReferenceCounted.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/ReferenceCounted.scala @@ -19,7 +19,7 @@ package whisk.core.database.cosmosdb import java.util.concurrent.atomic.{AtomicBoolean, AtomicInteger} -case class ReferenceCounted[T <: AutoCloseable](private val inner: T) { +private[cosmosdb] case class ReferenceCounted[T <: AutoCloseable](private val inner: T) { private val count = new AtomicInteger(0) private def inc(): Unit = count.incrementAndGet() diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/RxObservableImplicits.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/RxObservableImplicits.scala index 50166082602..eab2871b9d3 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/RxObservableImplicits.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/RxObservableImplicits.scala @@ -22,7 +22,7 @@ import rx.Observable import scala.concurrent.{Future, Promise} -trait RxObservableImplicits { +private[cosmosdb] trait RxObservableImplicits { implicit class RxScalaObservable[T](observable: Observable[T]) { From c32ea05ee93f710434987d4efca02831696c6972 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 25 Apr 2018 15:23:51 +0530 Subject: [PATCH 25/51] Remove '_' from computed such that path expression can be simplified --- .../core/database/cosmosdb/CosmosDBArtifactStore.scala | 6 +++--- .../whisk/core/database/cosmosdb/CosmosDBUtil.scala | 2 +- .../core/database/cosmosdb/CosmosDBViewMapper.scala | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index df331704120..54b1ce04286 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -338,13 +338,13 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected } private def toCosmosDoc(json: JsObject): Document = { - val computed = documentHandler.computedFields(json) - val computedOpt = if (computed.fields.nonEmpty) Some(computed) else None + val computedJs = documentHandler.computedFields(json) + val computedOpt = if (computedJs.fields.nonEmpty) Some(computedJs) else None val fieldsToAdd = Seq( (cid, Some(JsString(escapeId(json.fields(_id).convertTo[String])))), (etag, json.fields.get(_rev)), - (_computed, computedOpt)) + (computed, computedOpt)) val fieldsToRemove = Seq(_id, _rev) val mapped = transform(json, fieldsToAdd, fieldsToRemove) val doc = new Document(mapped.compactPrint) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala index 9bd8c9f7f32..999f2c4db4a 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala @@ -25,7 +25,7 @@ import whisk.core.database.cosmosdb.CosmosDBConstants._ import scala.collection.immutable.Iterable private[cosmosdb] object CosmosDBConstants { - val _computed: String = "_c" + val computed: String = "_c" val alias: String = "view" diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala index 6538d6ecaba..896f578b02f 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -32,7 +32,7 @@ import whisk.core.database.{ UnsupportedView, WhisksHandler } -import whisk.core.database.cosmosdb.CosmosDBConstants._computed +import whisk.core.database.cosmosdb.CosmosDBConstants.computed import whisk.core.database.cosmosdb.CosmosDBConstants.alias import whisk.core.entity.WhiskEntityQueries.TOP @@ -125,7 +125,7 @@ private[cosmosdb] abstract class SimpleMapper extends CosmosDBViewMapper { private[cosmosdb] object WhisksViewMapper extends SimpleMapper { private val NS = "r.namespace" - private val ROOT_NS_C = s"r.${_computed}.$ROOT_NS" + private val ROOT_NS_C = s"r.$computed.$ROOT_NS" private val TYPE = "r.entityType" private val UPDATED = "r.updated" private val PUBLISH = "r.publish" @@ -138,7 +138,7 @@ private[cosmosdb] object WhisksViewMapper extends SimpleMapper { includedPaths = Set( IncludedPath("/entityType/?", Index(Hash, String, -1)), IncludedPath("/namespace/?", Index(Hash, String, -1)), - IncludedPath(s"/${_computed}/$ROOT_NS/?", Index(Hash, String, -1)), + IncludedPath(s"/$computed/$ROOT_NS/?", Index(Hash, String, -1)), IncludedPath("/updated/?", Index(Range, Number, -1)))) override protected def where(ddoc: String, @@ -185,7 +185,7 @@ private[cosmosdb] object WhisksViewMapper extends SimpleMapper { } private[cosmosdb] object ActivationViewMapper extends SimpleMapper { private val NS = "r.namespace" - private val NS_WITH_PATH = s"r.${_computed}.$NS_PATH" + private val NS_WITH_PATH = s"r.$computed.$NS_PATH" private val START = "r.start" val handler = ActivationHandler @@ -194,7 +194,7 @@ private[cosmosdb] object ActivationViewMapper extends SimpleMapper { IndexingPolicy( includedPaths = Set( IncludedPath("/namespace/?", Index(Hash, String, -1)), - IncludedPath(s"/${_computed}/$NS_PATH/?", Index(Hash, String, -1)), + IncludedPath(s"/$computed/$NS_PATH/?", Index(Hash, String, -1)), IncludedPath("/start/?", Index(Range, Number, -1)))) override protected def where(ddoc: String, From 176a7ee95ceb963ba7843114296eda2f14279c11 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 25 Apr 2018 15:45:03 +0530 Subject: [PATCH 26/51] Use field names in index paths Also dropped index for key and instead added index for subject and name --- .../cosmosdb/CosmosDBViewMapper.scala | 79 +++++++++++-------- 1 file changed, 44 insertions(+), 35 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala index 896f578b02f..34a48b5dcf2 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -124,22 +124,22 @@ private[cosmosdb] abstract class SimpleMapper extends CosmosDBViewMapper { } private[cosmosdb] object WhisksViewMapper extends SimpleMapper { - private val NS = "r.namespace" - private val ROOT_NS_C = s"r.$computed.$ROOT_NS" - private val TYPE = "r.entityType" - private val UPDATED = "r.updated" - private val PUBLISH = "r.publish" - private val BINDING = "r.binding" + private val NS = "namespace" + private val ROOT_NS_C = s"$computed.$ROOT_NS" + private val TYPE = "entityType" + private val UPDATED = "updated" + private val PUBLISH = "publish" + private val BINDING = "binding" val handler = WhisksHandler override def indexingPolicy: IndexingPolicy = IndexingPolicy( includedPaths = Set( - IncludedPath("/entityType/?", Index(Hash, String, -1)), - IncludedPath("/namespace/?", Index(Hash, String, -1)), + IncludedPath(s"/$TYPE/?", Index(Hash, String, -1)), + IncludedPath(s"/$NS/?", Index(Hash, String, -1)), IncludedPath(s"/$computed/$ROOT_NS/?", Index(Hash, String, -1)), - IncludedPath("/updated/?", Index(Range, Number, -1)))) + IncludedPath(s"/$UPDATED/?", Index(Range, Number, -1)))) override protected def where(ddoc: String, view: String, @@ -152,17 +152,17 @@ private[cosmosdb] object WhisksViewMapper extends SimpleMapper { viewConditions(ddoc, view).map(q => (s"${q._1} AND", q._2)).getOrElse((NOTHING, Nil)) val params = ("@entityType", entityType) :: ("@namespace", namespace) :: vcParams - val baseCondition = s"$vc $TYPE = @entityType AND ($NS = @namespace OR $ROOT_NS_C = @namespace)" + val baseCondition = s"$vc r.$TYPE = @entityType AND (r.$NS = @namespace OR r.$ROOT_NS_C = @namespace)" (startKey, endKey) match { case (_ :: Nil, _ :: `TOP` :: Nil) => (baseCondition, params) case (_ :: (since: Number) :: Nil, _ :: `TOP` :: `TOP` :: Nil) => - (s"$baseCondition AND $UPDATED >= @since", ("@since", since) :: params) + (s"$baseCondition AND r.$UPDATED >= @since", ("@since", since) :: params) case (_ :: (since: Number) :: Nil, _ :: (upto: Number) :: `TOP` :: Nil) => - (s"$baseCondition AND ($UPDATED BETWEEN @since AND @upto)", ("@upto", upto) :: ("@since", since) :: params) + (s"$baseCondition AND (r.$UPDATED BETWEEN @since AND @upto)", ("@upto", upto) :: ("@since", since) :: params) case _ => throw UnsupportedQueryKeys(s"$ddoc/$view -> ($startKey, $endKey)") } @@ -171,31 +171,31 @@ private[cosmosdb] object WhisksViewMapper extends SimpleMapper { private def viewConditions(ddoc: String, view: String): Option[(String, List[(String, Any)])] = { view match { case "packages-public" if ddoc.startsWith("whisks") => - Some(s"$PUBLISH = true AND (NOT IS_OBJECT($BINDING) OR $BINDING = {})", Nil) + Some(s"r.$PUBLISH = true AND (NOT IS_OBJECT(r.$BINDING) OR r.$BINDING = {})", Nil) case _ => None } } override protected def orderByField(ddoc: String, view: String): String = view match { case "actions" | "rules" | "triggers" | "packages" | "packages-public" if ddoc.startsWith("whisks") => - UPDATED + s"r.$UPDATED" case _ => throw UnsupportedView(s"$ddoc/$view") } } private[cosmosdb] object ActivationViewMapper extends SimpleMapper { - private val NS = "r.namespace" - private val NS_WITH_PATH = s"r.$computed.$NS_PATH" - private val START = "r.start" + private val NS = "namespace" + private val NS_WITH_PATH = s"$computed.$NS_PATH" + private val START = "start" val handler = ActivationHandler override def indexingPolicy: IndexingPolicy = IndexingPolicy( includedPaths = Set( - IncludedPath("/namespace/?", Index(Hash, String, -1)), + IncludedPath(s"/$NS/?", Index(Hash, String, -1)), IncludedPath(s"/$computed/$NS_PATH/?", Index(Hash, String, -1)), - IncludedPath("/start/?", Index(Range, Number, -1)))) + IncludedPath(s"/$START/?", Index(Range, Number, -1)))) override protected def where(ddoc: String, view: String, @@ -219,22 +219,31 @@ private[cosmosdb] object ActivationViewMapper extends SimpleMapper { val params = ("@nsvalue", nsValue) :: Nil val filter = (startKey, endKey) match { case (_ :: Nil, _ :: `TOP` :: Nil) => - (s"$nsKey = @nsvalue", params) + (s"r.$nsKey = @nsvalue", params) case (_ :: (since: Number) :: Nil, _ :: `TOP` :: `TOP` :: Nil) => - (s"$nsKey = @nsvalue AND $START >= @start", ("@start", since) :: params) + (s"r.$nsKey = @nsvalue AND r.$START >= @start", ("@start", since) :: params) case (_ :: (since: Number) :: Nil, _ :: (upto: Number) :: `TOP` :: Nil) => - (s"$nsKey = @nsvalue AND $START >= @start AND $START <= @upto", ("@upto", upto) :: ("@start", since) :: params) + (s"r.$nsKey = @nsvalue AND (r.$START BETWEEN @start AND @upto)", ("@upto", upto) :: ("@start", since) :: params) case _ => throw UnsupportedQueryKeys(s"$startKey, $endKey") } filter } override protected def orderByField(ddoc: String, view: String): String = view match { - case "activations" if ddoc.startsWith("whisks") => START + case "activations" if ddoc.startsWith("whisks") => s"r.$START" case _ => throw UnsupportedView(s"$ddoc/$view") } } private[cosmosdb] object SubjectViewMapper extends CosmosDBViewMapper { + private val UUID = "uuid" + private val KEY = "key" + private val NSS = "namespaces" + private val CONCURRENT_INVOCATIONS = "concurrentInvocations" + private val INVOCATIONS_PER_MIN = "invocationsPerMinute" + private val BLOCKED = "blocked" + private val SUBJECT = "subject" + private val NAME = "name" + val handler = SubjectHandler override def indexingPolicy: IndexingPolicy = @@ -243,12 +252,12 @@ private[cosmosdb] object SubjectViewMapper extends CosmosDBViewMapper { //and keys are bigger IndexingPolicy( includedPaths = Set( - IncludedPath("/uuid/?", Index(Hash, String, -1)), - IncludedPath("/key/?", Index(Hash, String, 3)), - IncludedPath("/namespaces/[]/uuid/?", Index(Hash, String, -1)), - IncludedPath("/namespaces/[]/key/?", Index(Hash, String, 3)), - IncludedPath("/concurrentInvocations/?", Index(Hash, Number, -1)), - IncludedPath("/invocationsPerMinute/?", Index(Hash, Number, -1)))) + IncludedPath(s"/$UUID/?", Index(Hash, String, -1)), + IncludedPath(s"/$NSS/[]/$NAME/?", Index(Hash, String, -1)), + IncludedPath(s"/$SUBJECT/?", Index(Hash, String, -1)), + IncludedPath(s"/$NSS/[]/$UUID/?", Index(Hash, String, -1)), + IncludedPath(s"/$CONCURRENT_INVOCATIONS/?", Index(Hash, Number, -1)), + IncludedPath(s"/$INVOCATIONS_PER_MIN/?", Index(Hash, Number, -1)))) override def prepareQuery(ddoc: String, view: String, @@ -283,13 +292,13 @@ private[cosmosdb] object SubjectViewMapper extends CosmosDBViewMapper { startKey: List[Any], endKey: List[Any], count: Boolean): SqlQuerySpec = { - val notBlocked = "(NOT(IS_DEFINED(r.blocked)) OR r.blocked = false)" + val notBlocked = s"(NOT(IS_DEFINED(r.$BLOCKED)) OR r.$BLOCKED = false)" val (where, params) = startKey match { case (ns: String) :: Nil => - (s"$notBlocked AND (r.subject = @name OR n.name = @name)", ("@name", ns) :: Nil) + (s"$notBlocked AND (r.$SUBJECT = @name OR n.$NAME = @name)", ("@name", ns) :: Nil) case (uuid: String) :: (key: String) :: Nil => ( - s"$notBlocked AND ((r.uuid = @uuid AND r.key = @key) OR (n.uuid = @uuid AND n.key = @key))", + s"$notBlocked AND ((r.$UUID = @uuid AND r.$KEY = @key) OR (n.$UUID = @uuid AND n.$KEY = @key))", ("@uuid", uuid) :: ("@key", key) :: Nil) case _ => throw UnsupportedQueryKeys(s"$ddoc/$view -> ($startKey, $endKey)") } @@ -300,9 +309,9 @@ private[cosmosdb] object SubjectViewMapper extends CosmosDBViewMapper { prepareSpec( s"""SELECT ${selectClause(count)} AS $alias FROM root r - WHERE r.blocked = true - OR r.concurrentInvocations = 0 - OR r.invocationsPerMinute = 0 """, + WHERE r.$BLOCKED = true + OR r.$CONCURRENT_INVOCATIONS = 0 + OR r.$INVOCATIONS_PER_MIN = 0 """, Nil) private def selectClause(count: Boolean) = if (count) "TOP 1 VALUE COUNT(r)" else "r" From bcf00fdf3419e1eed1b16843886b6c6c4f6ffde0 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 25 Apr 2018 16:13:01 +0530 Subject: [PATCH 27/51] Use implicits to simplify blocking calls --- .../cosmosdb/CosmosDBArtifactStore.scala | 3 +- .../database/cosmosdb/CosmosDBSupport.scala | 43 ++++++++----------- .../cosmosdb/RxObservableImplicits.scala | 26 +++++------ .../database/cosmosdb/CosmosDBTestUtil.scala | 7 ++- 4 files changed, 36 insertions(+), 43 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index 54b1ce04286..5433eba53c2 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -52,8 +52,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected extends ArtifactStore[DocumentAbstraction] with DefaultJsonProtocol with DocumentProvider - with CosmosDBSupport - with RxObservableImplicits { + with CosmosDBSupport { protected val client: AsyncDocumentClient = clientRef.get.client private val (database, collection) = initialize() diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala index 8fe88122ff0..64368ede6b7 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala @@ -19,13 +19,11 @@ package whisk.core.database.cosmosdb import com.microsoft.azure.cosmosdb._ import com.microsoft.azure.cosmosdb.rx.AsyncDocumentClient -import _root_.rx.lang.scala._ -import _root_.rx.lang.scala.JavaConverters._ import scala.collection.JavaConverters._ import scala.collection.immutable -private[cosmosdb] trait CosmosDBSupport { +private[cosmosdb] trait CosmosDBSupport extends RxObservableImplicits { protected def config: CosmosDBConfig protected def collName: String protected def client: AsyncDocumentClient @@ -37,30 +35,30 @@ private[cosmosdb] trait CosmosDBSupport { } private def getOrCreateDatabase(): Database = { - blockingResult[Database](client.queryDatabases(querySpec(config.db), null).asScala).getOrElse { - val databaseDefinition = new Database - databaseDefinition.setId(config.db) - blockingResult(client.createDatabase(databaseDefinition, null).asScala) - } + client + .queryDatabases(querySpec(config.db), null) + .blockingOnlyResult() + .getOrElse { + client.createDatabase(newDatabase, null).blockingResult() + } } private def getOrCreateCollection(database: Database) = { - val collOpt = blockingResult[DocumentCollection]( - client.queryCollections(database.getSelfLink, querySpec(collName), null).asScala) + client + .queryCollections(database.getSelfLink, querySpec(collName), null) + .blockingOnlyResult() .map { coll => if (matchingIndexingPolicy(coll)) { coll } else { - //Modify the found collection as its selfLink is set + //Modify the found collection with latest policy as its selfLink is set coll.setIndexingPolicy(viewMapper.indexingPolicy.asJava()) - blockingResult(client.replaceCollection(coll, null).asScala) + client.replaceCollection(coll, null).blockingResult() } } - - collOpt.getOrElse { - val defn: DocumentCollection = newDatabaseCollection - blockingResult(client.createCollection(database.getSelfLink, defn, null).asScala) - } + .getOrElse { + client.createCollection(database.getSelfLink, newDatabaseCollection, null).blockingResult() + } } private def matchingIndexingPolicy(coll: DocumentCollection): Boolean = @@ -74,13 +72,10 @@ private[cosmosdb] trait CosmosDBSupport { defn } - private def blockingResult[T <: Resource](response: Observable[FeedResponse[T]]) = { - val value = response.toList.toBlocking.single - value.head.getResults.asScala.headOption - } - - private def blockingResult[T <: Resource](response: Observable[ResourceResponse[T]]) = { - response.toBlocking.single.getResource + private def newDatabase = { + val databaseDefinition = new Database + databaseDefinition.setId(config.db) + databaseDefinition } protected def querySpec(id: String) = diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/RxObservableImplicits.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/RxObservableImplicits.scala index eab2871b9d3..f9812760ef6 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/RxObservableImplicits.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/RxObservableImplicits.scala @@ -17,9 +17,11 @@ package whisk.core.database.cosmosdb +import com.microsoft.azure.cosmosdb.{FeedResponse, Resource, ResourceResponse} import rx.lang.scala.JavaConverters._ import rx.Observable +import scala.collection.JavaConverters._ import scala.concurrent.{Future, Promise} private[cosmosdb] trait RxObservableImplicits { @@ -36,20 +38,18 @@ private[cosmosdb] trait RxObservableImplicits { observable.asScala.single.subscribe(x => promise.success(x), e => promise.failure(e)) promise.future } + } - /** - * Collects the [[Observable]] results and converts to a [[scala.concurrent.Future]]. - * - * Automatically subscribes to the `Observable` and uses the [[Observable#toList]] method to aggregate the results. - * - * @note If the Observable is large then this will consume lots of memory! - * If the underlying Observable is infinite this Observable will never complete. - * @return a future representation of the whole Observable - */ - def toFuture(): Future[Seq[T]] = { - val promise = Promise[Seq[T]]() - observable.asScala.toList.subscribe(x => promise.success(x), e => promise.failure(e)) - promise.future + implicit class RxScalaResourceObservable[T <: Resource](observable: Observable[ResourceResponse[T]]) { + def blockingResult(): T = observable.toBlocking.single.getResource + } + + implicit class RxScalaFeedObservable[T <: Resource](observable: Observable[FeedResponse[T]]) { + def blockingOnlyResult(): Option[T] = { + val value = observable.asScala.toList.toBlocking.single + val results = value.head.getResults.asScala + require(results.isEmpty || results.size == 1, s"More than one result found $results") + results.headOption } } } diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestUtil.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestUtil.scala index 8a6385ec5ae..4374dd59ad4 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestUtil.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestUtil.scala @@ -20,7 +20,6 @@ package whisk.core.database.cosmosdb import java.text.SimpleDateFormat import java.util.Date -import _root_.rx.lang.scala.JavaConverters._ import com.microsoft.azure.cosmosdb.Database import org.scalatest.{BeforeAndAfterAll, FlatSpec} import pureconfig.loadConfigOrThrow @@ -36,7 +35,7 @@ object CosmosDBTestUtil { } -trait CosmosDBTestSupport extends FlatSpec with BeforeAndAfterAll { +trait CosmosDBTestSupport extends FlatSpec with BeforeAndAfterAll with RxObservableImplicits { private val dbsToDelete = ListBuffer[Database]() lazy val storeConfigTry = Try { loadConfigOrThrow[CosmosDBConfig](ConfigKeys.cosmosdb) } @@ -57,14 +56,14 @@ trait CosmosDBTestSupport extends FlatSpec with BeforeAndAfterAll { protected def createTestDB() = { val databaseDefinition = new Database databaseDefinition.setId(generateDBName()) - val db = client.createDatabase(databaseDefinition, null).asScala.toBlocking.single.getResource + val db = client.createDatabase(databaseDefinition, null).blockingResult() dbsToDelete += db db } override def afterAll(): Unit = { super.afterAll() - dbsToDelete.foreach(db => client.deleteDatabase(db.getSelfLink, null).asScala.toBlocking.single.getResource) + dbsToDelete.foreach(db => client.deleteDatabase(db.getSelfLink, null).blockingResult()) client.close() } } From 1b0e61a7c37fc051637c61b28f04b93e5a44b0be Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 25 Apr 2018 16:35:22 +0530 Subject: [PATCH 28/51] Create new and delete database for tests --- .../cosmosdb/CosmosDBArtifactStoreTests.scala | 12 +++++++----- ...mosDBTestUtil.scala => CosmosDBTestSupport.scala} | 11 +++-------- 2 files changed, 10 insertions(+), 13 deletions(-) rename tests/src/test/scala/whisk/core/database/cosmosdb/{CosmosDBTestUtil.scala => CosmosDBTestSupport.scala} (89%) diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala index 7456ba11583..a1399892ac2 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala @@ -27,18 +27,20 @@ import scala.reflect.classTag import scala.util.Try @RunWith(classOf[JUnitRunner]) -class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreBehavior { +class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreBehavior with CosmosDBTestSupport { override def storeType = "CosmosDB" - override lazy val storeAvailableCheck: Try[Any] = CosmosDBTestUtil.storeConfig + override lazy val storeAvailableCheck: Try[Any] = storeConfigTry + + private lazy val config: CosmosDBConfig = storeConfig.copy(db = createTestDB().getId) override lazy val authStore = { implicit val docReader: DocumentReader = WhiskDocumentReader - CosmosDBArtifactStoreProvider.makeStore[WhiskAuth]() + CosmosDBArtifactStoreProvider.makeStore[WhiskAuth](config, false) } override lazy val entityStore = - CosmosDBArtifactStoreProvider.makeStore[WhiskEntity]()( + CosmosDBArtifactStoreProvider.makeStore[WhiskEntity](config, false)( classTag[WhiskEntity], WhiskEntityJsonFormat, WhiskDocumentReader, @@ -48,6 +50,6 @@ class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreBehavior { override lazy val activationStore = { implicit val docReader: DocumentReader = WhiskDocumentReader - CosmosDBArtifactStoreProvider.makeStore[WhiskActivation]() + CosmosDBArtifactStoreProvider.makeStore[WhiskActivation](config, false) } } diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestUtil.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestSupport.scala similarity index 89% rename from tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestUtil.scala rename to tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestSupport.scala index 4374dd59ad4..06a143f8ddb 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestUtil.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestSupport.scala @@ -29,12 +29,6 @@ import whisk.core.database.test.behavior.ArtifactStoreTestUtil.storeAvailable import scala.collection.mutable.ListBuffer import scala.util.{Random, Try} -object CosmosDBTestUtil { - - lazy val storeConfig = Try { loadConfigOrThrow[CosmosDBConfig](ConfigKeys.cosmosdb) } - -} - trait CosmosDBTestSupport extends FlatSpec with BeforeAndAfterAll with RxObservableImplicits { private val dbsToDelete = ListBuffer[Database]() @@ -49,8 +43,8 @@ trait CosmosDBTestSupport extends FlatSpec with BeforeAndAfterAll with RxObserva } protected def generateDBName() = { - val format = new SimpleDateFormat("yyyy-MM-dd") - s"${format.format(new Date())}-${Random.alphanumeric.take(5).mkString}" + val format = new SimpleDateFormat(s"yyyy-MM-dd") + s"${format.format(new Date())}-${getClass.getSimpleName}-${Random.alphanumeric.take(5).mkString}" } protected def createTestDB() = { @@ -58,6 +52,7 @@ trait CosmosDBTestSupport extends FlatSpec with BeforeAndAfterAll with RxObserva databaseDefinition.setId(generateDBName()) val db = client.createDatabase(databaseDefinition, null).blockingResult() dbsToDelete += db + println(s"Credted database ${db.getId}") db } From d5edbecc1e84ee88535ebb1f6aa3b791ffea2f83 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 25 Apr 2018 21:23:59 +0530 Subject: [PATCH 29/51] Unit test for select fields as json --- .../database/cosmosdb/CosmosDBUtilTest.scala | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBUtilTest.scala diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBUtilTest.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBUtilTest.scala new file mode 100644 index 00000000000..ad0c81a7779 --- /dev/null +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBUtilTest.scala @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import org.junit.runner.RunWith +import org.scalatest.junit.JUnitRunner +import org.scalatest.{FlatSpec, Matchers, OptionValues} +import spray.json._ +import whisk.utils.JsHelpers + +@RunWith(classOf[JUnitRunner]) +class CosmosDBUtilTest extends FlatSpec with Matchers with OptionValues { + + behavior of "prepare field" + + it should "always have id" in { + val result = fieldsAsJson() + val expected = """{"id" : "r['id']"}""".parseJson + result shouldBe expected + result.fields("id") shouldBe JsString("r['id']") + } + + it should "build a json like string" in { + val result = fieldsAsJson("a") + val expected = """{ "a" : "r['a']", "id" : "r['id']"}""".parseJson + result shouldBe expected + result.fields("a") shouldBe JsString("r['a']") + } + + it should "support nested fields" in { + val result = fieldsAsJson("a", "b.c") + val expected = """{ + | "a": "r['a']", + | "b": { + | "c": "r['b']['c']" + | }, + | "id": "r['id']" + |}""".stripMargin.parseJson + result shouldBe expected + JsHelpers.getFieldPath(result, "b", "c").value shouldBe JsString("r['b']['c']") + } + + private def fieldsAsJson(fields: String*) = toJson(CosmosDBUtil.prepareFieldClause(fields.toList)) + + private def toJson(s: String): JsObject = { + //Strip of last `As VIEW` + s.replace(" AS view", "") + .replaceAll(raw"(r[\w'\[\]]+)", "\"$1\"") + .parseJson + .asJsObject + } + +} From f4749345ef27a25e2005b421bdd50b4e0153f24d Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 26 Apr 2018 00:04:37 +0530 Subject: [PATCH 30/51] Unit test for IndexingPolicy --- .../cosmosdb/IndexingPolicyTests.scala | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 tests/src/test/scala/whisk/core/database/cosmosdb/IndexingPolicyTests.scala diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/IndexingPolicyTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/IndexingPolicyTests.scala new file mode 100644 index 00000000000..9f11ce93c9d --- /dev/null +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/IndexingPolicyTests.scala @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import com.microsoft.azure.cosmosdb.DataType.String +import com.microsoft.azure.cosmosdb.IndexKind.{Hash, Range} +import com.microsoft.azure.cosmosdb.IndexingMode +import org.junit.runner.RunWith +import org.scalatest.junit.JUnitRunner +import org.scalatest.{FlatSpec, Matchers} + +@RunWith(classOf[JUnitRunner]) +class IndexingPolicyTests extends FlatSpec with Matchers { + behavior of "IndexingPolicy" + + it should "match same instance" in { + val policy = + IndexingPolicy(mode = IndexingMode.Lazy, includedPaths = Set(IncludedPath("foo", Index(Hash, String, -1)))) + IndexingPolicy.isSame(policy, policy) shouldBe true + } + + it should "match when same path and subset of indexes" in { + val policy = + IndexingPolicy( + mode = IndexingMode.Lazy, + includedPaths = Set(IncludedPath("foo", Index(Hash, String, -1)), IncludedPath("bar", Index(Hash, String, -1)))) + + val policy2 = + IndexingPolicy( + mode = IndexingMode.Lazy, + includedPaths = Set( + IncludedPath("foo", Index(Hash, String, -1)), + IncludedPath("bar", Set(Index(Hash, String, -1), Index(Range, String, -1))))) + + IndexingPolicy.isSame(policy, policy2) shouldBe true + IndexingPolicy.isSame(policy2, policy) shouldBe false + } + + it should "not match when same path are different" in { + val policy = + IndexingPolicy( + mode = IndexingMode.Lazy, + includedPaths = Set(IncludedPath("foo", Index(Hash, String, -1)), IncludedPath("bar", Index(Hash, String, -1)))) + + val policy2 = + IndexingPolicy( + mode = IndexingMode.Lazy, + includedPaths = Set( + IncludedPath("foo2", Index(Hash, String, -1)), + IncludedPath("bar", Set(Index(Hash, String, -1), Index(Range, String, -1))))) + + IndexingPolicy.isSame(policy, policy2) shouldBe false + } + + it should "convert and match java IndexingPolicy" in { + val policy = + IndexingPolicy( + mode = IndexingMode.Lazy, + includedPaths = Set( + IncludedPath("foo", Index(Hash, String, -1)), + IncludedPath("bar", Set(Index(Hash, String, -1), Index(Range, String, -1))))) + + val jpolicy = policy.asJava() + val policy2 = IndexingPolicy(jpolicy) + + policy shouldBe policy2 + } +} From 56bbedeef077e76c472a9fa28b4530ffd19eb02b Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 26 Apr 2018 14:28:46 +0530 Subject: [PATCH 31/51] Use lazy indexing mode for activations --- .../whisk/core/database/cosmosdb/CosmosDBViewMapper.scala | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala index 34a48b5dcf2..f3d7353b9da 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBViewMapper.scala @@ -19,11 +19,13 @@ package whisk.core.database.cosmosdb import java.util.Collections -import com.microsoft.azure.cosmosdb.IndexKind.{Hash, Range} import com.microsoft.azure.cosmosdb.DataType.{Number, String} +import com.microsoft.azure.cosmosdb.IndexKind.{Hash, Range} +import com.microsoft.azure.cosmosdb.IndexingMode.Lazy import com.microsoft.azure.cosmosdb.{PartitionKeyDefinition, SqlParameter, SqlParameterCollection, SqlQuerySpec} import whisk.core.database.ActivationHandler.NS_PATH import whisk.core.database.WhisksHandler.ROOT_NS +import whisk.core.database.cosmosdb.CosmosDBConstants.{alias, computed} import whisk.core.database.{ ActivationHandler, DocumentHandler, @@ -32,8 +34,6 @@ import whisk.core.database.{ UnsupportedView, WhisksHandler } -import whisk.core.database.cosmosdb.CosmosDBConstants.computed -import whisk.core.database.cosmosdb.CosmosDBConstants.alias import whisk.core.entity.WhiskEntityQueries.TOP private[cosmosdb] trait CosmosDBViewMapper { @@ -192,6 +192,7 @@ private[cosmosdb] object ActivationViewMapper extends SimpleMapper { override def indexingPolicy: IndexingPolicy = IndexingPolicy( + mode = Lazy, includedPaths = Set( IncludedPath(s"/$NS/?", Index(Hash, String, -1)), IncludedPath(s"/$computed/$NS_PATH/?", Index(Hash, String, -1)), From 1fe1dfc111492b1331187fe60fcd6c29e2cbfcd0 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Fri, 27 Apr 2018 16:07:09 +0530 Subject: [PATCH 32/51] Add checks to escape logic and add tests Refactored the CosmosDBUtil to a trait and object to simplify imports of utility methods --- .../database/cosmosdb/CosmosDBSupport.scala | 10 +--------- .../core/database/cosmosdb/CosmosDBUtil.scala | 18 +++++++++++++++++- .../database/cosmosdb/CosmosDBUtilTest.scala | 15 +++++++++++++++ 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala index 64368ede6b7..09d512afaca 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala @@ -23,7 +23,7 @@ import com.microsoft.azure.cosmosdb.rx.AsyncDocumentClient import scala.collection.JavaConverters._ import scala.collection.immutable -private[cosmosdb] trait CosmosDBSupport extends RxObservableImplicits { +private[cosmosdb] trait CosmosDBSupport extends RxObservableImplicits with CosmosDBUtil { protected def config: CosmosDBConfig protected def collName: String protected def client: AsyncDocumentClient @@ -81,13 +81,5 @@ private[cosmosdb] trait CosmosDBSupport extends RxObservableImplicits { protected def querySpec(id: String) = new SqlQuerySpec("SELECT * FROM root r WHERE r.id=@id", new SqlParameterCollection(new SqlParameter("@id", id))) - /** - * CosmosDB id considers '/', '\' , '?' and '#' as invalid. EntityNames can include '/' so - * that need to be escaped. For that we use '|' as the replacement char - */ - protected def escapeId(id: String): String = id.replace("/", "|") - - protected def unescapeId(id: String): String = id.replace("|", "/") - protected def asSeq[T <: Resource](r: FeedResponse[T]): immutable.Seq[T] = r.getResults.asScala.to[immutable.Seq] } diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala index 999f2c4db4a..e05ff892f10 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala @@ -38,7 +38,7 @@ private[cosmosdb] object CosmosDBConstants { val selfLink: String = SELF_LINK } -private[cosmosdb] object CosmosDBUtil { +private[cosmosdb] trait CosmosDBUtil { /** * Prepares the json like select clause @@ -89,4 +89,20 @@ private[cosmosdb] object CosmosDBUtil { .withConsistencyLevel(ConsistencyLevel.Session) .build + /** + * CosmosDB id considers '/', '\' , '?' and '#' as invalid. EntityNames can include '/' so + * that need to be escaped. For that we use '|' as the replacement char + */ + def escapeId(id: String): String = { + require(!id.contains("|"), s"Id [$id] should not contain '|'") + id.replace("/", "|") + } + + def unescapeId(id: String): String = { + require(!id.contains("/"), s"Escaped Id [$id] should not contain '/'") + id.replace("|", "/") + } + } + +private[cosmosdb] object CosmosDBUtil extends CosmosDBUtil diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBUtilTest.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBUtilTest.scala index ad0c81a7779..50ec7f2c1a5 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBUtilTest.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBUtilTest.scala @@ -65,4 +65,19 @@ class CosmosDBUtilTest extends FlatSpec with Matchers with OptionValues { .asJsObject } + behavior of "escaping" + + it should "escape /" in { + CosmosDBUtil.escapeId("foo/bar") shouldBe "foo|bar" + } + + it should "throw exception when input contains replacement char |" in { + an[IllegalArgumentException] should be thrownBy CosmosDBUtil.escapeId("foo/bar|baz") + } + + it should "support unescaping" in { + val s = "foo/bar" + CosmosDBUtil.unescapeId(CosmosDBUtil.escapeId(s)) shouldBe s + } + } From f20e1f3afd387d59def2d93ab6e03fceaca4cf6f Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Fri, 27 Apr 2018 17:41:46 +0530 Subject: [PATCH 33/51] Assert ReferenceCount.isClosed --- .../whisk/core/database/cosmosdb/ReferenceCountedTests.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/ReferenceCountedTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/ReferenceCountedTests.scala index 0da706ba388..e9b0d85981b 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/ReferenceCountedTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/ReferenceCountedTests.scala @@ -47,6 +47,7 @@ class ReferenceCountedTests extends FlatSpec with Matchers { probe.closed shouldBe true probe.closedCount shouldBe 1 + refCounted.isClosed shouldBe true } it should "not close with one reference active" in { From 371600fdcff2c1e79768f6338a3399cf69263d64 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 2 May 2018 15:30:09 +0530 Subject: [PATCH 34/51] Document the CosmosDB config --- common/scala/src/main/resources/application.conf | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/common/scala/src/main/resources/application.conf b/common/scala/src/main/resources/application.conf index bbac5f4b646..86e5213b96c 100644 --- a/common/scala/src/main/resources/application.conf +++ b/common/scala/src/main/resources/application.conf @@ -136,6 +136,14 @@ whisk { # } #} + # CosmosDB related configuration + # For example: + # cosmosdb { + # endpoint = # Endpoint URL like https://.documents.azure.com:443/ + # key = # Access key + # db = # Database name + #} + # transaction ID related configuration transactions { header = "X-Request-ID" From 464a0431b144c91848d28906c0d14373c3850743 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 2 May 2018 16:29:11 +0530 Subject: [PATCH 35/51] Only escape when id is from DocId Use DummyImplicit to enable method overloading as DocId is of type AnyVal and post erasure there is ambiguity in which `newRequestOption` to use. DummyImplicit acts as a workaround for such a case --- .../database/cosmosdb/CosmosDBArtifactStore.scala | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index 5433eba53c2..18b510c8b63 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -125,7 +125,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected require(doc != null, "doc undefined") val f = client - .readDocument(selfLinkOf(doc.id), newRequestOption(doc.id.id)) + .readDocument(selfLinkOf(doc.id), newRequestOption(doc.id)) .head() .transform( { rr => @@ -154,7 +154,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val start = transid.started(this, LoggingMarkers.DATABASE_GET, s"[GET_BY_ID] '$collName' finding document: '$id'") val f = client - .readDocument(selfLinkOf(id), newRequestOption(id.id)) + .readDocument(selfLinkOf(id), newRequestOption(id)) .head() .map { rr => val js = getResultToWhiskJsonDoc(rr.getResource) @@ -389,7 +389,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected private def createSelfLink(id: String) = s"dbs/${database.getId}/colls/${collection.getId}/docs/$id" - private def matchRevOption(info: DocInfo): RequestOptions = matchRevOption(info.id.id, info.rev.rev) + private def matchRevOption(info: DocInfo): RequestOptions = matchRevOption(escapeId(info.id.id), info.rev.rev) private def matchRevOption(id: String, etag: String): RequestOptions = { val options = newRequestOption(id) @@ -399,9 +399,12 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected options } + //Using DummyImplicit to allow overloading work with type erasure of DocId AnyVal + private def newRequestOption(id: DocId)(implicit i: DummyImplicit): RequestOptions = newRequestOption(escapeId(id.id)) + private def newRequestOption(id: String) = { val options = new RequestOptions - options.setPartitionKey(new PartitionKey(escapeId(id))) + options.setPartitionKey(new PartitionKey(id)) options } From 41f8bc5f9e98f7af6f2d397900bf56e5009cb553 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 2 May 2018 16:29:33 +0530 Subject: [PATCH 36/51] Update CosmosDB dependency to 1.0.1 --- common/scala/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/scala/build.gradle b/common/scala/build.gradle index 98ac1163169..7786cd567c2 100644 --- a/common/scala/build.gradle +++ b/common/scala/build.gradle @@ -74,7 +74,7 @@ dependencies { compile 'io.reactivex:rxscala_2.11:0.26.5' compile 'io.reactivex:rxjava-reactive-streams:1.2.1' - compile 'com.microsoft.azure:azure-cosmosdb:1.0.0' + compile 'com.microsoft.azure:azure-cosmosdb:1.0.1' scoverage gradle.scoverage.deps } From 3e609045ca6b109bc497ee762b8970e3dd31dbf2 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 3 May 2018 16:50:03 +0530 Subject: [PATCH 37/51] Python script to create and prune database for tests --- tools/db/cosmosDbUtil.py | 194 +++++++++++++++++++++++++++++++++++++++ tools/travis/setup.sh | 3 + 2 files changed, 197 insertions(+) create mode 100755 tools/db/cosmosDbUtil.py diff --git a/tools/db/cosmosDbUtil.py b/tools/db/cosmosDbUtil.py new file mode 100755 index 00000000000..87c3b23e287 --- /dev/null +++ b/tools/db/cosmosDbUtil.py @@ -0,0 +1,194 @@ +#!/usr/bin/env python + +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +from collections import namedtuple +import glob +import sys +import os +import argparse +import traceback +import pydocumentdb.documents as documents +import pydocumentdb.errors as document_errors +import pydocumentdb.document_client as document_client + +try: + import argcomplete +except ImportError: + argcomplete = False + +CLI_DIR = os.path.dirname(os.path.realpath(sys.argv[0])) +# ROOT_DIR is the OpenWhisk repository root +ROOT_DIR = os.path.join(os.path.join(CLI_DIR, os.pardir), os.pardir) + +DbContext = namedtuple('DbContext', ['client', 'db', 'whisks', 'subjects', 'activations']) +verbose = False + + +def main(): + global verbose + exit_code = 0 + try: + args = parse_args() + verbose = args.verbose + client = init_client(args) + exit_code = { + 'init': init_cmd, + 'prune': prune_cmd, + 'drop': drop_cmd + }[args.cmd](args, client) + except Exception as e: + print('Exception: ', e) + traceback.print_exc() + exit_code = 1 + + sys.exit(exit_code) + + +def parse_args(): + parser = argparse.ArgumentParser(description='OpenWhisk CosmosDB bootstrap tool') + parser.add_argument('--endpoint', help='DB Endpoint url like https://example.documents.azure.com:443/', + required=True) + parser.add_argument('--key', help='DB access key', required=True) + parser.add_argument('-v', '--verbose', help='Verbose mode', action="store_true") + + subparsers = parser.add_subparsers(title='available commands', dest='cmd') + + propmenu = subparsers.add_parser('init', help='initialize database') + propmenu.add_argument('db', help='Database name under which the collections would be created') + propmenu.add_argument('--dir', help='Directory under which auth files are stored') + + propmenu = subparsers.add_parser('prune', help='remove stale databases created by test') + propmenu.add_argument('--prefix', help='Database name prefix which are matched for removal', default="travis-") + + propmenu = subparsers.add_parser('drop', help='drop database') + propmenu.add_argument('db', help='Database name to be removed') + + if argcomplete: + argcomplete.autocomplete(parser) + return parser.parse_args() + + +def init_cmd(args, client): + db = get_or_create_db(client, args.db) + + whisks = init_coll(client, db, "whisks") + subjects = init_coll(client, db, "subjects") + activations = init_coll(client, db, "activations") + + db_ctx = DbContext(client, db, whisks, subjects, activations) + init_auth(db_ctx) + return 0 + + +def prune_cmd(args, client): + # Remove database which are one day old + pass + + +def drop_cmd(args, client): + db = get_db(client, args.db) + if db is not None: + client.DeleteDatabase(db['_self']) + log("Removed database : %s" % args.db) + else: + log("Database %s not found" % args.db) + + +def init_auth(ctx): + for subject in find_default_subjects(): + link = create_link(ctx.db, ctx.subjects, subject['id']) + options = {'partitionKey': subject.get('id')} + try: + ctx.client.ReadDocument(link, options) + log('Subject already exists : ' + subject['id']) + except document_errors.HTTPFailure as e: + if e.status_code == 404: + ctx.client.CreateDocument(ctx.subjects['_self'], subject, options) + log('Created subject : ' + subject['id']) + else: + raise e + + +def create_link(db, coll, doc_id): + return 'dbs/' + db['id'] + '/colls/' + coll['id'] + '/docs/' + doc_id + + +def find_default_subjects(): + files_dir = os.path.join(ROOT_DIR, "ansible/files") + for name in glob.glob1(files_dir, "auth.*"): + auth_file = open(os.path.join(files_dir, name), 'r') + uuid, key = auth_file.read().strip().split(":") + subject = name[name.index('.') + 1:] + doc = { + 'id': subject, + 'subject': subject, + 'namespaces': [ + { + 'name': subject, + 'uuid': uuid, + 'key': key + } + ] + } + auth_file.close() + yield doc + + +def init_client(args): + return document_client.DocumentClient(args.endpoint, {'masterKey': args.key}) + + +def get_db(client, db_name): + query = client.QueryDatabases('SELECT * FROM root r WHERE r.id=\'' + db_name + '\'') + return next(iter(query), None) + + +def get_or_create_db(client, db_name): + db = get_db(client, db_name) + if db is None: + db = client.CreateDatabase({'id': db_name}) + log('Created database "%s"' % db_name) + return db + + +def init_coll(client, db, coll_name): + query = client.QueryCollections(db['_self'], 'SELECT * FROM root r WHERE r.id=\'' + coll_name + '\'') + it = iter(query) + coll = next(it, None) + if coll is None: + collection_definition = {'id': coll_name, + 'partitionKey': + { + 'paths': ['/id'], + 'kind': documents.PartitionKind.Hash + } + } + collection_options = {} # {'offerThroughput': 10100} + coll = client.CreateCollection(db['_self'], collection_definition, collection_options) + log('Created collection "%s"' % coll_name) + return coll + + +def log(msg): + if verbose: + print(msg) + + +if __name__ == '__main__': + main() diff --git a/tools/travis/setup.sh b/tools/travis/setup.sh index 89cc76bdc04..bc167740066 100755 --- a/tools/travis/setup.sh +++ b/tools/travis/setup.sh @@ -33,3 +33,6 @@ pip install --user couchdb # Ansible pip install --user ansible==2.5.2 + +# Azure CosmosDB +pip install --user pydocumentdb From ef98cf24116200a325c312b191b630f46eb3ef65 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Fri, 4 May 2018 15:38:10 +0530 Subject: [PATCH 38/51] Ensure that skip and limits are non negative i.e. >= 0 Also remove TODO for batching as that cannot be supported --- .../database/cosmosdb/CosmosDBArtifactStore.scala | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index 18b510c8b63..887b33e23c2 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -65,7 +65,6 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected override protected[database] def put(d: DocumentAbstraction)(implicit transid: TransactionId): Future[DocInfo] = { val asJson = d.toDocumentRecord - //TODO Batching support val doc = toCosmosDoc(asJson) val id = doc.getId val docinfoStr = s"id: $id, rev: ${doc.getETag}" @@ -182,6 +181,8 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected stale: StaleParameter)(implicit transid: TransactionId): Future[List[JsObject]] = { require(!(reduce && includeDocs), "reduce and includeDocs cannot both be true") require(!reduce, "Reduce scenario not supported") //TODO Investigate reduce + require(skip >= 0, "skip should be non negative") + require(limit >= 0, "limit should be non negative") documentHandler.checkIfTableSupported(table) val Array(ddoc, viewName) = table.split("/") @@ -218,6 +219,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected endKey: List[Any], skip: Int, stale: StaleParameter)(implicit transid: TransactionId): Future[Long] = { + require(skip >= 0, "skip should be non negative") val Array(ddoc, viewName) = table.split("/") val start = transid.started(this, LoggingMarkers.DATABASE_QUERY, s"[COUNT] '$collName' searching '$table") @@ -227,11 +229,11 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val f = client .queryDocuments(collection.getSelfLink, querySpec, newFeedOptions()) .head() - .map(_.getResults.asScala.head.getLong(aggregate).longValue() - skip) - - f.onSuccess({ - case out => transid.finished(this, start, s"[COUNT] '$collName' completed: count $out") - }) + .map { r => + val count = r.getResults.asScala.head.getLong(aggregate).longValue() + transid.finished(this, start, s"[COUNT] '$collName' completed: count $count") + if (count > skip) count - skip else 0L + } reportFailure(f, start, failure => s"[COUNT] '$collName' internal error, failure: '${failure.getMessage}'") } From 02ccf122b84f9d9e98be54c8ba95d6eb12e5bd77 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sat, 5 May 2018 12:10:50 +0530 Subject: [PATCH 39/51] Prefix test db name with "travis" This would simplify pruning of orphan dbs via cosmosDbUtil.py script --- .../whisk/core/database/cosmosdb/CosmosDBTestSupport.scala | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestSupport.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestSupport.scala index 06a143f8ddb..274353832f1 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestSupport.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBTestSupport.scala @@ -17,9 +17,6 @@ package whisk.core.database.cosmosdb -import java.text.SimpleDateFormat -import java.util.Date - import com.microsoft.azure.cosmosdb.Database import org.scalatest.{BeforeAndAfterAll, FlatSpec} import pureconfig.loadConfigOrThrow @@ -43,8 +40,7 @@ trait CosmosDBTestSupport extends FlatSpec with BeforeAndAfterAll with RxObserva } protected def generateDBName() = { - val format = new SimpleDateFormat(s"yyyy-MM-dd") - s"${format.format(new Date())}-${getClass.getSimpleName}-${Random.alphanumeric.take(5).mkString}" + s"travis-${getClass.getSimpleName}-${Random.alphanumeric.take(5).mkString}" } protected def createTestDB() = { From b15bae3aba46ec668e6f4ad5717c9b14cdbcbc3b Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Wed, 23 May 2018 10:18:45 +0530 Subject: [PATCH 40/51] Switch to azure-cosmosdb:1.0.2 --- common/scala/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/scala/build.gradle b/common/scala/build.gradle index 7786cd567c2..fd81b930879 100644 --- a/common/scala/build.gradle +++ b/common/scala/build.gradle @@ -74,7 +74,7 @@ dependencies { compile 'io.reactivex:rxscala_2.11:0.26.5' compile 'io.reactivex:rxjava-reactive-streams:1.2.1' - compile 'com.microsoft.azure:azure-cosmosdb:1.0.1' + compile 'com.microsoft.azure:azure-cosmosdb:1.0.2' scoverage gradle.scoverage.deps } From 98abace56d90a0b12664e5260959fb5676eddbcb Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 14 Jun 2018 20:40:38 +0530 Subject: [PATCH 41/51] Use AttachmentStore --- .../cosmosdb/CosmosDBArtifactStore.scala | 151 ++++++++++++++---- .../CosmosDBArtifactStoreProvider.scala | 21 ++- .../cosmosdb/CosmosDBArtifactStoreTests.scala | 33 +--- .../CosmosDBAttachmentStoreTests.scala | 36 +++++ .../cosmosdb/CosmosDBStoreBehaviorBase.scala | 65 ++++++++ 5 files changed, 242 insertions(+), 64 deletions(-) create mode 100644 tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBAttachmentStoreTests.scala create mode 100644 tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBStoreBehaviorBase.scala diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index 887b33e23c2..e38dc410ab5 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -21,7 +21,7 @@ import java.io.ByteArrayInputStream import _root_.rx.RxReactiveStreams import akka.actor.ActorSystem -import akka.http.scaladsl.model.{ContentType, StatusCodes} +import akka.http.scaladsl.model.{ContentType, StatusCodes, Uri} import akka.stream.ActorMaterializer import akka.stream.scaladsl.{Sink, Source, StreamConverters} import akka.util.{ByteString, ByteStringBuilder} @@ -33,6 +33,7 @@ import whisk.core.database.StoreUtils.{checkDocHasRevision, deserialize, reportF import whisk.core.database._ import whisk.core.database.cosmosdb.CosmosDBArtifactStoreProvider.DocumentClientRef import whisk.core.database.cosmosdb.CosmosDBConstants._ +import whisk.core.entity.Attachments.Attached import whisk.core.entity._ import whisk.http.Messages @@ -43,16 +44,22 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected protected val config: CosmosDBConfig, clientRef: DocumentClientRef, documentHandler: DocumentHandler, - protected val viewMapper: CosmosDBViewMapper)( + protected val viewMapper: CosmosDBViewMapper, + val inliningConfig: InliningConfig, + val attachmentStore: Option[AttachmentStore])( implicit system: ActorSystem, val logging: Logging, jsonFormat: RootJsonFormat[DocumentAbstraction], - materializer: ActorMaterializer, + val materializer: ActorMaterializer, docReader: DocumentReader) extends ArtifactStore[DocumentAbstraction] with DefaultJsonProtocol with DocumentProvider - with CosmosDBSupport { + with CosmosDBSupport + with AttachmentInliner { + + private val cosmosScheme = "cosmos" + val attachmentScheme: String = attachmentStore.map(_.scheme).getOrElse(cosmosScheme) protected val client: AsyncDocumentClient = clientRef.get.client private val (database, collection) = initialize() @@ -118,8 +125,10 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected failure => s"[DEL] '$collName' internal error, doc: '$doc', failure: '${failure.getMessage}'") } - override protected[database] def get[A <: DocumentAbstraction](doc: DocInfo)(implicit transid: TransactionId, - ma: Manifest[A]): Future[A] = { + override protected[database] def get[A <: DocumentAbstraction](doc: DocInfo, + attachmentHandler: Option[(A, Attached) => A] = None)( + implicit transid: TransactionId, + ma: Manifest[A]): Future[A] = { val start = transid.started(this, LoggingMarkers.DATABASE_GET, s"[GET] '$collName' finding document: '$doc'") require(doc != null, "doc undefined") @@ -130,6 +139,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected { rr => val js = getResultToWhiskJsonDoc(rr.getResource) transid.finished(this, start, s"[GET] '$collName' completed: found document '$doc'") + //TODO Invoke DocumentHandler deserialize[A, DocumentAbstraction](doc, js) }, { case e: DocumentClientException if isNotFound(e) => @@ -238,11 +248,91 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected reportFailure(f, start, failure => s"[COUNT] '$collName' internal error, failure: '${failure.getMessage}'") } - override protected[core] def attach( - doc: DocInfo, - name: String, + override protected[database] def putAndAttach[A <: DocumentAbstraction]( + doc: A, + update: (A, Attached) => A, + contentType: ContentType, + docStream: Source[ByteString, _], + oldAttachment: Option[Attached])(implicit transid: TransactionId): Future[(DocInfo, Attached)] = { + + attachmentStore match { + case Some(_) => + attachToExternalStore(doc, update, contentType, docStream, oldAttachment) + case None => + attachToCosmos(doc, update, contentType, docStream) + } + } + + private def attachToCosmos[A <: DocumentAbstraction]( + doc: A, + update: (A, Attached) => A, + contentType: ContentType, + docStream: Source[ByteString, _])(implicit transid: TransactionId) = { + //TODO Read whole stream and then compute size and digest in advance. + //Then we do not need documentHandler in get part + for { + (bytes, tailSource) <- inlineAndTail(docStream) + uri <- Future.successful(uriOf(bytes, UUID().asString)) + attached <- { + val a = if (isInlined(uri)) { + Attached(uri.toString, contentType, Some(bytes.size), Some(digest(bytes))) + } else { + Attached(uri.toString, contentType) + } + Future.successful(a) + } + i1 <- put(update(doc, attached)) + i2 <- if (isInlined(uri)) { + Future.successful(i1) + } else { + attach(i1, uri.path.toString, attached.attachmentType, combinedSource(bytes, tailSource)) + } + } yield (i2, attached) + } + + private def attachToExternalStore[A <: DocumentAbstraction]( + doc: A, + update: (A, Attached) => A, contentType: ContentType, - docStream: Source[ByteString, _])(implicit transid: TransactionId): Future[DocInfo] = { + docStream: Source[ByteString, _], + oldAttachment: Option[Attached])(implicit transid: TransactionId) = { + val as = attachmentStore.get + val asJson = doc.toDocumentRecord + val id = asJson.fields("_id").convertTo[String].trim + + for { + (bytes, tailSource) <- inlineAndTail(docStream) + uri <- Future.successful(uriOf(bytes, UUID().asString)) + attached <- { + // Upload if cannot be inlined + if (isInlined(uri)) { + val a = Attached(uri.toString, contentType, Some(bytes.size), Some(digest(bytes))) + Future.successful(a) + } else { + as.attach(DocId(id), uri.path.toString, contentType, combinedSource(bytes, tailSource)) + .map { r => + Attached(uri.toString, contentType, Some(r.length), Some(r.digest)) + } + } + } + i1 <- put(update(doc, attached)) + + //Remove old attachment if it was part of attachmentStore + _ <- oldAttachment + .map { old => + val oldUri = Uri(old.attachmentName) + if (oldUri.scheme == as.scheme) { + as.deleteAttachment(DocId(id), oldUri.path.toString) + } else { + Future.successful(true) + } + } + .getOrElse(Future.successful(true)) + } yield (i1, attached) + } + + private def attach(doc: DocInfo, name: String, contentType: ContentType, docStream: Source[ByteString, _])( + implicit transid: TransactionId): Future[DocInfo] = { val start = transid.started( this, LoggingMarkers.DATABASE_ATT_SAVE, @@ -253,7 +343,6 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected options.setContentType(contentType.toString()) options.setSlug(name) - //TODO Temporary implementation till AttachmentStore PR is merged val f = docStream .runFold(new ByteStringBuilder)((builder, b) => builder ++= b) .map(_.result().toArray) @@ -278,8 +367,27 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected failure => s"[ATT_PUT] '$collName' internal error, name: '$name', doc: '$doc', failure: '${failure.getMessage}'") } - override protected[core] def readAttachment[T](doc: DocInfo, name: String, sink: Sink[ByteString, Future[T]])( - implicit transid: TransactionId): Future[(ContentType, T)] = { + override protected[core] def readAttachment[T](doc: DocInfo, attached: Attached, sink: Sink[ByteString, Future[T]])( + implicit transid: TransactionId): Future[T] = { + val name = attached.attachmentName + val attachmentUri = Uri(name) + attachmentUri.scheme match { + case AttachmentInliner.MemScheme => + memorySource(attachmentUri).runWith(sink) + case s if s == cosmosScheme || attachmentUri.isRelative => + //relative case is for compatibility with earlier naming approach where attachment name would be like 'jarfile' + //Compared to current approach of ':' + readAttachmentFromCosmos(doc, attachmentUri, sink) + case s if attachmentStore.isDefined && attachmentStore.get.scheme == s => + attachmentStore.get.readAttachment(doc.id, attachmentUri.path.toString, sink) + case _ => + throw new IllegalArgumentException(s"Unknown attachment scheme in attachment uri $attachmentUri") + } + } + + private def readAttachmentFromCosmos[T](doc: DocInfo, attachmentUri: Uri, sink: Sink[ByteString, Future[T]])( + implicit transid: TransactionId): Future[T] = { + val name = attachmentUri.path val start = transid.started( this, LoggingMarkers.DATABASE_ATT_GET, @@ -296,7 +404,6 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected StreamConverters .fromInputStream(() => r.getMedia) .runWith(sink) - .map((parseContentType(r), _)) }, { case e: DocumentClientException if isNotFound(e) => transid.finished( @@ -315,22 +422,12 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected } override protected[core] def deleteAttachments[T](doc: DocInfo)(implicit transid: TransactionId): Future[Boolean] = - // NOTE: this method is not intended for standalone use for CosmosDB. - // To delete attachments, it is expected that the entire document is deleted. - Future.successful(true) + attachmentStore + .map(as => as.deleteAttachments(doc.id)) + .getOrElse(Future.successful(true)) // For CosmosDB it is expected that the entire document is deleted. override def shutdown(): Unit = clientRef.close() - private def parseContentType(r: MediaResponse): ContentType = { - val typeString = r.getResponseHeaders.asScala.getOrElse( - "Content-Type", - throw new RuntimeException(s"Content-Type header not found in response ${r.getResponseHeaders}")) - ContentType.parse(typeString) match { - case Right(ct) => ct - case Left(_) => throw new RuntimeException(s"Invalid Content-Type header $typeString") //Should not happen - } - } - private def isNotFound[A <: DocumentAbstraction](e: DocumentClientException) = e.getStatusCode == StatusCodes.NotFound.intValue diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala index e547b03a3c8..6e0a5421ffe 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala @@ -26,6 +26,7 @@ import spray.json.RootJsonFormat import whisk.common.Logging import whisk.core.database._ import pureconfig._ +import whisk.core.entity.size._ import whisk.core.ConfigKeys import whisk.core.database.cosmosdb.CosmosDBUtil.createClient import whisk.core.entity.{DocumentReader, WhiskActivation, WhiskAuth, WhiskEntity} @@ -49,22 +50,23 @@ object CosmosDBArtifactStoreProvider extends ArtifactStoreProvider { actorSystem: ActorSystem, logging: Logging, materializer: ActorMaterializer): ArtifactStore[D] = { - makeStoreForClient(config, useBatching, getOrCreateReference(config)) + makeStoreForClient(config, getOrCreateReference(config), getAttachmentStore()) } - def makeStore[D <: DocumentSerializer: ClassTag](config: CosmosDBConfig, useBatching: Boolean)( + def makeArtifactStore[D <: DocumentSerializer: ClassTag](config: CosmosDBConfig, + attachmentStore: Option[AttachmentStore])( implicit jsonFormat: RootJsonFormat[D], docReader: DocumentReader, actorSystem: ActorSystem, logging: Logging, materializer: ActorMaterializer): ArtifactStore[D] = { - makeStoreForClient(config, useBatching, createReference(config).reference()) + makeStoreForClient(config, createReference(config).reference(), attachmentStore) } private def makeStoreForClient[D <: DocumentSerializer: ClassTag](config: CosmosDBConfig, - useBatching: Boolean, - clientRef: DocumentClientRef)( + clientRef: DocumentClientRef, + attachmentStore: Option[AttachmentStore])( implicit jsonFormat: RootJsonFormat[D], docReader: DocumentReader, actorSystem: ActorSystem, @@ -74,7 +76,14 @@ object CosmosDBArtifactStoreProvider extends ArtifactStoreProvider { val classTag = implicitly[ClassTag[D]] val (dbName, handler, viewMapper) = handlerAndMapper(classTag) - new CosmosDBArtifactStore(dbName, config, clientRef, handler, viewMapper) + new CosmosDBArtifactStore( + dbName, + config, + clientRef, + handler, + viewMapper, + loadConfigOrThrow[InliningConfig](ConfigKeys.db), + attachmentStore) } private def handlerAndMapper[D](entityType: ClassTag[D])( diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala index a1399892ac2..1ac93eff955 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala @@ -20,36 +20,7 @@ package whisk.core.database.cosmosdb import org.junit.runner.RunWith import org.scalatest.FlatSpec import org.scalatest.junit.JUnitRunner -import whisk.core.database.test.behavior.ArtifactStoreBehavior -import whisk.core.entity._ - -import scala.reflect.classTag -import scala.util.Try +import whisk.core.database.test.behavior.ArtifactStoreAttachmentBehaviors @RunWith(classOf[JUnitRunner]) -class CosmosDBArtifactStoreTests extends FlatSpec with ArtifactStoreBehavior with CosmosDBTestSupport { - override def storeType = "CosmosDB" - - override lazy val storeAvailableCheck: Try[Any] = storeConfigTry - - private lazy val config: CosmosDBConfig = storeConfig.copy(db = createTestDB().getId) - - override lazy val authStore = { - implicit val docReader: DocumentReader = WhiskDocumentReader - CosmosDBArtifactStoreProvider.makeStore[WhiskAuth](config, false) - } - - override lazy val entityStore = - CosmosDBArtifactStoreProvider.makeStore[WhiskEntity](config, false)( - classTag[WhiskEntity], - WhiskEntityJsonFormat, - WhiskDocumentReader, - actorSystem, - logging, - materializer) - - override lazy val activationStore = { - implicit val docReader: DocumentReader = WhiskDocumentReader - CosmosDBArtifactStoreProvider.makeStore[WhiskActivation](config, false) - } -} +class CosmosDBArtifactStoreTests extends FlatSpec with CosmosDBStoreBehaviorBase with ArtifactStoreAttachmentBehaviors {} diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBAttachmentStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBAttachmentStoreTests.scala new file mode 100644 index 00000000000..5e5da0f9775 --- /dev/null +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBAttachmentStoreTests.scala @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import org.junit.runner.RunWith +import org.scalatest.FlatSpec +import org.scalatest.junit.JUnitRunner +import whisk.core.database.DocumentSerializer +import whisk.core.database.memory.MemoryAttachmentStoreProvider +import whisk.core.database.test.behavior.ArtifactStoreAttachmentBehaviors + +import scala.reflect.ClassTag + +@RunWith(classOf[JUnitRunner]) +class CosmosDBAttachmentStoreTests + extends FlatSpec + with CosmosDBStoreBehaviorBase + with ArtifactStoreAttachmentBehaviors { + override protected def getAttachmentStore[D <: DocumentSerializer: ClassTag]() = + Some(MemoryAttachmentStoreProvider.makeStore[D]()) +} diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBStoreBehaviorBase.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBStoreBehaviorBase.scala new file mode 100644 index 00000000000..2c12a9bd537 --- /dev/null +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBStoreBehaviorBase.scala @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package whisk.core.database.cosmosdb + +import org.scalatest.FlatSpec +import whisk.core.database.test.behavior.ArtifactStoreBehaviorBase +import whisk.core.database.{ArtifactStore, AttachmentStore, DocumentSerializer} +import whisk.core.entity.{ + DocumentReader, + WhiskActivation, + WhiskAuth, + WhiskDocumentReader, + WhiskEntity, + WhiskEntityJsonFormat +} + +import scala.reflect.{classTag, ClassTag} +import scala.util.Try + +trait CosmosDBStoreBehaviorBase extends FlatSpec with ArtifactStoreBehaviorBase with CosmosDBTestSupport { + override def storeType = "CosmosDB" + + override lazy val storeAvailableCheck: Try[Any] = storeConfigTry + + private lazy val config: CosmosDBConfig = storeConfig.copy(db = createTestDB().getId) + + override lazy val authStore = { + implicit val docReader: DocumentReader = WhiskDocumentReader + CosmosDBArtifactStoreProvider.makeArtifactStore[WhiskAuth](config, getAttachmentStore[WhiskAuth]()) + } + + override lazy val entityStore = + CosmosDBArtifactStoreProvider.makeArtifactStore[WhiskEntity](config, getAttachmentStore[WhiskEntity]())( + classTag[WhiskEntity], + WhiskEntityJsonFormat, + WhiskDocumentReader, + actorSystem, + logging, + materializer) + + override lazy val activationStore = { + implicit val docReader: DocumentReader = WhiskDocumentReader + CosmosDBArtifactStoreProvider.makeArtifactStore[WhiskActivation](config, getAttachmentStore[WhiskActivation]()) + } + + override protected def getAttachmentStore(store: ArtifactStore[_]) = + store.asInstanceOf[CosmosDBArtifactStore[_]].attachmentStore + + protected def getAttachmentStore[D <: DocumentSerializer: ClassTag](): Option[AttachmentStore] = None +} From 708972f1a9c14e3b583201fe367a6a97aa5af815 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 14 Jun 2018 20:53:31 +0530 Subject: [PATCH 42/51] Compute digest and length for attachments stored in Cosmos itself --- .../cosmosdb/CosmosDBArtifactStore.scala | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index e38dc410ab5..855e3886a4c 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -139,7 +139,6 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected { rr => val js = getResultToWhiskJsonDoc(rr.getResource) transid.finished(this, start, s"[GET] '$collName' completed: found document '$doc'") - //TODO Invoke DocumentHandler deserialize[A, DocumentAbstraction](doc, js) }, { case e: DocumentClientException if isNotFound(e) => @@ -268,16 +267,16 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected update: (A, Attached) => A, contentType: ContentType, docStream: Source[ByteString, _])(implicit transid: TransactionId) = { - //TODO Read whole stream and then compute size and digest in advance. - //Then we do not need documentHandler in get part + //Convert Source to ByteString as Cosmos API works with InputStream only for { - (bytes, tailSource) <- inlineAndTail(docStream) + allBytes <- toByteString(docStream) + (bytes, tailSource) <- inlineAndTail(Source.single(allBytes)) uri <- Future.successful(uriOf(bytes, UUID().asString)) attached <- { val a = if (isInlined(uri)) { Attached(uri.toString, contentType, Some(bytes.size), Some(digest(bytes))) } else { - Attached(uri.toString, contentType) + Attached(uri.toString, contentType, Some(allBytes.size), Some(digest(allBytes))) } Future.successful(a) } @@ -285,7 +284,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected i2 <- if (isInlined(uri)) { Future.successful(i1) } else { - attach(i1, uri.path.toString, attached.attachmentType, combinedSource(bytes, tailSource)) + attach(i1, uri.path.toString, attached.attachmentType, allBytes) } } yield (i2, attached) } @@ -331,7 +330,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected } yield (i1, attached) } - private def attach(doc: DocInfo, name: String, contentType: ContentType, docStream: Source[ByteString, _])( + private def attach(doc: DocInfo, name: String, contentType: ContentType, allBytes: ByteString)( implicit transid: TransactionId): Future[DocInfo] = { val start = transid.started( this, @@ -342,12 +341,10 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val options = new MediaOptions options.setContentType(contentType.toString()) options.setSlug(name) - - val f = docStream - .runFold(new ByteStringBuilder)((builder, b) => builder ++= b) - .map(_.result().toArray) - .map(new ByteArrayInputStream(_)) - .flatMap(s => client.upsertAttachment(selfLinkOf(doc.id), s, options, matchRevOption(doc)).head()) + val s = new ByteArrayInputStream(allBytes.toArray) + val f = client + .upsertAttachment(selfLinkOf(doc.id), s, options, matchRevOption(doc)) + .head() .transform( { _ => transid @@ -367,6 +364,9 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected failure => s"[ATT_PUT] '$collName' internal error, name: '$name', doc: '$doc', failure: '${failure.getMessage}'") } + private def toByteString(docStream: Source[ByteString, _]) = + docStream.runFold(new ByteStringBuilder)((builder, b) => builder ++= b).map(_.result().compact) + override protected[core] def readAttachment[T](doc: DocInfo, attached: Attached, sink: Sink[ByteString, Future[T]])( implicit transid: TransactionId): Future[T] = { val name = attached.attachmentName From 0a71174229b014f89a5b09ae4c074865961b839a Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 14 Jun 2018 21:24:30 +0530 Subject: [PATCH 43/51] Delete old attachment --- .../cosmosdb/CosmosDBArtifactStore.scala | 27 +++++++++++++++---- .../cosmosdb/CosmosDBArtifactStoreTests.scala | 4 +-- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index 855e3886a4c..08007680997 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -254,19 +254,24 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected docStream: Source[ByteString, _], oldAttachment: Option[Attached])(implicit transid: TransactionId): Future[(DocInfo, Attached)] = { + val asJson = doc.toDocumentRecord + val id = asJson.fields("_id").convertTo[String].trim + attachmentStore match { case Some(_) => - attachToExternalStore(doc, update, contentType, docStream, oldAttachment) + attachToExternalStore(id, doc, update, contentType, docStream, oldAttachment) case None => - attachToCosmos(doc, update, contentType, docStream) + attachToCosmos(id, doc, update, contentType, docStream, oldAttachment) } } private def attachToCosmos[A <: DocumentAbstraction]( + id: String, doc: A, update: (A, Attached) => A, contentType: ContentType, - docStream: Source[ByteString, _])(implicit transid: TransactionId) = { + docStream: Source[ByteString, _], + oldAttachment: Option[Attached])(implicit transid: TransactionId) = { //Convert Source to ByteString as Cosmos API works with InputStream only for { allBytes <- toByteString(docStream) @@ -286,18 +291,30 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected } else { attach(i1, uri.path.toString, attached.attachmentType, allBytes) } + //Remove old attachment if it was part of attachmentStore + _ <- oldAttachment + .map { old => + val oldUri = Uri(old.attachmentName) + if (oldUri.scheme == cosmosScheme) { + val name = oldUri.path.toString + val docId = DocId(id) + client.deleteAttachment(s"${selfLinkOf(docId)}/attachments/$name", newRequestOption(docId)).head() + } else { + Future.successful(true) + } + } + .getOrElse(Future.successful(true)) } yield (i2, attached) } private def attachToExternalStore[A <: DocumentAbstraction]( + id: String, doc: A, update: (A, Attached) => A, contentType: ContentType, docStream: Source[ByteString, _], oldAttachment: Option[Attached])(implicit transid: TransactionId) = { val as = attachmentStore.get - val asJson = doc.toDocumentRecord - val id = asJson.fields("_id").convertTo[String].trim for { (bytes, tailSource) <- inlineAndTail(docStream) diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala index 1ac93eff955..d1e727b215d 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala @@ -20,7 +20,7 @@ package whisk.core.database.cosmosdb import org.junit.runner.RunWith import org.scalatest.FlatSpec import org.scalatest.junit.JUnitRunner -import whisk.core.database.test.behavior.ArtifactStoreAttachmentBehaviors +import whisk.core.database.test.behavior.ArtifactStoreBehavior @RunWith(classOf[JUnitRunner]) -class CosmosDBArtifactStoreTests extends FlatSpec with CosmosDBStoreBehaviorBase with ArtifactStoreAttachmentBehaviors {} +class CosmosDBArtifactStoreTests extends FlatSpec with CosmosDBStoreBehaviorBase with ArtifactStoreBehavior {} From cb0495bbdd2fcb44d5ba6eed263c93fccb078ebe Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 21 Jun 2018 10:49:14 +0530 Subject: [PATCH 44/51] Adapt for changes done for fixing blob inlining --- .../cosmosdb/CosmosDBArtifactStore.scala | 72 ++++--------------- 1 file changed, 15 insertions(+), 57 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala index 08007680997..fac08f33f0e 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStore.scala @@ -56,7 +56,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected with DefaultJsonProtocol with DocumentProvider with CosmosDBSupport - with AttachmentInliner { + with AttachmentSupport[DocumentAbstraction] { private val cosmosScheme = "cosmos" val attachmentScheme: String = attachmentStore.map(_.scheme).getOrElse(cosmosScheme) @@ -111,10 +111,10 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected true }, { case e: DocumentClientException if isNotFound(e) => - transid.finished(this, start, s"[DEL] '$collName', document: '${doc}'; not found.") + transid.finished(this, start, s"[DEL] '$collName', document: '$doc'; not found.") NoDocumentException("not found on 'delete'") case e: DocumentClientException if isConflict(e) => - transid.finished(this, start, s"[DEL] '$collName', document: '${doc}'; conflict.") + transid.finished(this, start, s"[DEL] '$collName', document: '$doc'; conflict.") DocumentConflictException("conflict on 'delete'") case e => e }) @@ -258,8 +258,8 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val id = asJson.fields("_id").convertTo[String].trim attachmentStore match { - case Some(_) => - attachToExternalStore(id, doc, update, contentType, docStream, oldAttachment) + case Some(as) => + attachToExternalStore(doc, update, contentType, docStream, oldAttachment, as) case None => attachToCosmos(id, doc, update, contentType, docStream, oldAttachment) } @@ -271,25 +271,23 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected update: (A, Attached) => A, contentType: ContentType, docStream: Source[ByteString, _], - oldAttachment: Option[Attached])(implicit transid: TransactionId) = { + oldAttachment: Option[Attached])(implicit transid: TransactionId): Future[(DocInfo, Attached)] = { //Convert Source to ByteString as Cosmos API works with InputStream only for { allBytes <- toByteString(docStream) - (bytes, tailSource) <- inlineAndTail(Source.single(allBytes)) - uri <- Future.successful(uriOf(bytes, UUID().asString)) + bytesOrSource <- inlineOrAttach(Source.single(allBytes)) + uri = uriOf(bytesOrSource, UUID().asString) attached <- { - val a = if (isInlined(uri)) { - Attached(uri.toString, contentType, Some(bytes.size), Some(digest(bytes))) - } else { - Attached(uri.toString, contentType, Some(allBytes.size), Some(digest(allBytes))) + val a = bytesOrSource match { + case Left(bytes) => Attached(uri.toString, contentType, Some(bytes.size), Some(digest(bytes))) + case Right(_) => Attached(uri.toString, contentType, Some(allBytes.size), Some(digest(allBytes))) } Future.successful(a) } i1 <- put(update(doc, attached)) - i2 <- if (isInlined(uri)) { - Future.successful(i1) - } else { - attach(i1, uri.path.toString, attached.attachmentType, allBytes) + i2 <- bytesOrSource match { + case Left(_) => Future.successful(i1) + case Right(s) => attach(i1, uri.path.toString, attached.attachmentType, allBytes) } //Remove old attachment if it was part of attachmentStore _ <- oldAttachment @@ -307,46 +305,6 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected } yield (i2, attached) } - private def attachToExternalStore[A <: DocumentAbstraction]( - id: String, - doc: A, - update: (A, Attached) => A, - contentType: ContentType, - docStream: Source[ByteString, _], - oldAttachment: Option[Attached])(implicit transid: TransactionId) = { - val as = attachmentStore.get - - for { - (bytes, tailSource) <- inlineAndTail(docStream) - uri <- Future.successful(uriOf(bytes, UUID().asString)) - attached <- { - // Upload if cannot be inlined - if (isInlined(uri)) { - val a = Attached(uri.toString, contentType, Some(bytes.size), Some(digest(bytes))) - Future.successful(a) - } else { - as.attach(DocId(id), uri.path.toString, contentType, combinedSource(bytes, tailSource)) - .map { r => - Attached(uri.toString, contentType, Some(r.length), Some(r.digest)) - } - } - } - i1 <- put(update(doc, attached)) - - //Remove old attachment if it was part of attachmentStore - _ <- oldAttachment - .map { old => - val oldUri = Uri(old.attachmentName) - if (oldUri.scheme == as.scheme) { - as.deleteAttachment(DocId(id), oldUri.path.toString) - } else { - Future.successful(true) - } - } - .getOrElse(Future.successful(true)) - } yield (i1, attached) - } - private def attach(doc: DocInfo, name: String, contentType: ContentType, allBytes: ByteString)( implicit transid: TransactionId): Future[DocInfo] = { val start = transid.started( @@ -389,7 +347,7 @@ class CosmosDBArtifactStore[DocumentAbstraction <: DocumentSerializer](protected val name = attached.attachmentName val attachmentUri = Uri(name) attachmentUri.scheme match { - case AttachmentInliner.MemScheme => + case AttachmentSupport.MemScheme => memorySource(attachmentUri).runWith(sink) case s if s == cosmosScheme || attachmentUri.isRelative => //relative case is for compatibility with earlier naming approach where attachment name would be like 'jarfile' From 75f6904a058e938eadd15447eb31e9ba355f7efa Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 21 Jun 2018 12:04:40 +0530 Subject: [PATCH 45/51] Adapt large attachment test Use lower size for Cosmos as max limit is 2MB. For ArtifactStore configured with AttachmentStore use large size like 5MB --- .../cosmosdb/CosmosDBArtifactStoreTests.scala | 5 ++++- .../behavior/ArtifactStoreAttachmentBehaviors.scala | 11 ++++++++--- .../test/behavior/ArtifactStoreBehaviorBase.scala | 8 ++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala index d1e727b215d..bb45c723a2d 100644 --- a/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala +++ b/tests/src/test/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreTests.scala @@ -20,7 +20,10 @@ package whisk.core.database.cosmosdb import org.junit.runner.RunWith import org.scalatest.FlatSpec import org.scalatest.junit.JUnitRunner +import whisk.core.entity.size._ import whisk.core.database.test.behavior.ArtifactStoreBehavior @RunWith(classOf[JUnitRunner]) -class CosmosDBArtifactStoreTests extends FlatSpec with CosmosDBStoreBehaviorBase with ArtifactStoreBehavior {} +class CosmosDBArtifactStoreTests extends FlatSpec with CosmosDBStoreBehaviorBase with ArtifactStoreBehavior { + override protected def maxAttachmentSizeWithoutAttachmentStore = 1.MB +} diff --git a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreAttachmentBehaviors.scala b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreAttachmentBehaviors.scala index 82b6b80d330..1438e79d858 100644 --- a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreAttachmentBehaviors.scala +++ b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreAttachmentBehaviors.scala @@ -22,10 +22,10 @@ import java.util.Base64 import akka.http.scaladsl.model.{ContentTypes, Uri} import akka.stream.IOResult +import scala.concurrent.duration.DurationInt import akka.stream.scaladsl.{Sink, StreamConverters} import akka.util.{ByteString, ByteStringBuilder} import whisk.common.TransactionId -import whisk.core.entity.size._ import whisk.core.database.{AttachmentSupport, CacheChangeNotification, NoDocumentException} import whisk.core.entity.Attachments.{Attached, Attachment, Inline} import whisk.core.entity.test.ExecHelpers @@ -113,17 +113,22 @@ trait ArtifactStoreAttachmentBehaviors extends ArtifactStoreBehaviorBase with Ex getAttachmentBytes(i2, attached(action2)).futureValue.result() shouldBe decode(code1) } - it should "put and read 5 MB attachment" in { + it should "put and read large attachment" in { implicit val tid: TransactionId = transid() - val size = Math.max(nonInlinedAttachmentSize(entityStore), 5.MB.toBytes.toInt) + val size = Math.max(nonInlinedAttachmentSize(entityStore), getAttachmentSizeForTest(entityStore)) val base64 = encodedRandomBytes(size) val exec = javaDefault(base64, Some("hello")) val javaAction = WhiskAction(namespace, EntityName("attachment_large"), exec) + //Have more patience as reading large attachments take time specially for remote + //storage like Cosmos + implicit val patienceConfig: PatienceConfig = PatienceConfig(timeout = 1.minute) + val i1 = WhiskAction.put(entityStore, javaAction, old = None).futureValue val action2 = entityStore.get[WhiskAction](i1, attachmentHandler).futureValue + val action3 = WhiskAction.get(entityStore, i1.id, i1.rev).futureValue docsToDelete += ((entityStore, i1)) diff --git a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala index 21fda4812da..d4bcd5c3d0f 100644 --- a/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala +++ b/tests/src/test/scala/whisk/core/database/test/behavior/ArtifactStoreBehaviorBase.scala @@ -30,6 +30,7 @@ import whisk.core.database.test.DbUtils import whisk.core.database.test.behavior.ArtifactStoreTestUtil.storeAvailable import whisk.core.database.{ArtifactStore, AttachmentStore, StaleParameter} import whisk.core.entity._ +import whisk.core.entity.size._ import whisk.utils.JsHelpers import scala.util.{Random, Try} @@ -161,6 +162,13 @@ trait ArtifactStoreBehaviorBase case _ => None } + protected def getAttachmentSizeForTest(store: ArtifactStore[_]): Int = { + val mb = getAttachmentStore(store).map(_ => 5.MB).getOrElse(maxAttachmentSizeWithoutAttachmentStore) + mb.toBytes.toInt + } + + protected def maxAttachmentSizeWithoutAttachmentStore: ByteSize = 5.MB + private def assertAttachmentStoreIsEmpty(): Unit = { Seq(authStore, entityStore, activationStore).foreach { s => for { From 77991a4123d619af781366cddf2dfa407af82723 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Thu, 21 Jun 2018 14:57:40 +0530 Subject: [PATCH 46/51] Update to azure-cosmosdb-2.0.0 to get rid of org.json dependency --- common/scala/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/scala/build.gradle b/common/scala/build.gradle index fd81b930879..493b4320a91 100644 --- a/common/scala/build.gradle +++ b/common/scala/build.gradle @@ -74,7 +74,7 @@ dependencies { compile 'io.reactivex:rxscala_2.11:0.26.5' compile 'io.reactivex:rxjava-reactive-streams:1.2.1' - compile 'com.microsoft.azure:azure-cosmosdb:1.0.2' + compile 'com.microsoft.azure:azure-cosmosdb:2.0.0' scoverage gradle.scoverage.deps } From 5b7909c0f1b577cbd1a802b14d775003b008dc75 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sat, 7 Jul 2018 10:25:11 +0530 Subject: [PATCH 47/51] Simplify by switching to if statement --- .../cosmosdb/CosmosDBArtifactStoreProvider.scala | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala index 6e0a5421ffe..9708fa1f788 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala @@ -90,14 +90,11 @@ object CosmosDBArtifactStoreProvider extends ArtifactStoreProvider { implicit actorSystem: ActorSystem, logging: Logging, materializer: ActorMaterializer): (String, DocumentHandler, CosmosDBViewMapper) = { - entityType.runtimeClass match { - case x if x == classOf[WhiskEntity] => - ("whisks", WhisksHandler, WhisksViewMapper) - case x if x == classOf[WhiskActivation] => - ("activations", ActivationHandler, ActivationViewMapper) - case x if x == classOf[WhiskAuth] => - ("subjects", SubjectHandler, SubjectViewMapper) - } + val entityClass = entityType.runtimeClass + if (entityClass == classOf[WhiskEntity]) ("whisks", WhisksHandler, WhisksViewMapper) + else if (entityClass == classOf[WhiskActivation]) ("activations", ActivationHandler, ActivationViewMapper) + else if (entityClass == classOf[WhiskAuth]) ("subjects", SubjectHandler, SubjectViewMapper) + else throw new IllegalArgumentException(s"Unsupported entity type $entityType") } private def getOrCreateReference(config: CosmosDBConfig) = synchronized { From 4694a25083a4eb73cb64bcaf5fdefd2ffc655a66 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sat, 7 Jul 2018 10:35:41 +0530 Subject: [PATCH 48/51] Throw exception instance of log error --- .../main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala index e05ff892f10..26fc967c755 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBUtil.scala @@ -58,7 +58,7 @@ private[cosmosdb] trait CosmosDBUtil { } private def addToMap(name: String, map: Map[String, _]): Map[String, Any] = name.split('.').toList match { - case Nil => sys.error(s"Should not reach here $name") + case Nil => throw new IllegalStateException(s"'$name' split on '.' should not result in empty list") case x :: xs => addToMap(x, xs, Nil, map) } From 3f6936bbb504d84504b631c2e01759551c47035c Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sat, 7 Jul 2018 10:36:00 +0530 Subject: [PATCH 49/51] Document querySpec intention --- .../scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala index 09d512afaca..d9630943ddb 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBSupport.scala @@ -78,6 +78,9 @@ private[cosmosdb] trait CosmosDBSupport extends RxObservableImplicits with Cosmo databaseDefinition } + /** + * Prepares a query for fetching any resource by id + */ protected def querySpec(id: String) = new SqlQuerySpec("SELECT * FROM root r WHERE r.id=@id", new SqlParameterCollection(new SqlParameter("@id", id))) From a9462841352297c2886d2761b09d1832b884fc40 Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sat, 7 Jul 2018 10:36:16 +0530 Subject: [PATCH 50/51] Document synchronization requirement --- .../database/cosmosdb/CosmosDBArtifactStoreProvider.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala index 9708fa1f788..d3943bfacda 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/CosmosDBArtifactStoreProvider.scala @@ -97,6 +97,10 @@ object CosmosDBArtifactStoreProvider extends ArtifactStoreProvider { else throw new IllegalArgumentException(s"Unsupported entity type $entityType") } + /* + * This method ensures that all store instances share same client instance and thus the underlying connection pool. + * Synchronization is required to ensure concurrent init of various store instances share same ref instance + */ private def getOrCreateReference(config: CosmosDBConfig) = synchronized { if (clientRef == null || clientRef.isClosed) { clientRef = createReference(config) From a27fc645119ff21bdf69ea1c74f7504cf5e55b5b Mon Sep 17 00:00:00 2001 From: Chetan Mehrotra Date: Sat, 7 Jul 2018 10:38:46 +0530 Subject: [PATCH 51/51] Clarify second decrement --- .../scala/whisk/core/database/cosmosdb/ReferenceCounted.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/scala/src/main/scala/whisk/core/database/cosmosdb/ReferenceCounted.scala b/common/scala/src/main/scala/whisk/core/database/cosmosdb/ReferenceCounted.scala index a05a9140c40..693cad373fd 100644 --- a/common/scala/src/main/scala/whisk/core/database/cosmosdb/ReferenceCounted.scala +++ b/common/scala/src/main/scala/whisk/core/database/cosmosdb/ReferenceCounted.scala @@ -26,8 +26,9 @@ private[cosmosdb] case class ReferenceCounted[T <: AutoCloseable](private val in private def dec(): Unit = { val newCount = count.decrementAndGet() - if (newCount == 0) { + if (newCount <= 0) { inner.close() + //Turn count to negative to ensure future reference call fail count.decrementAndGet() } }