API-2532: Extracted common repo code into MongoCrudHelper. (#19)

* API-2532: Extracted common repo code into MongoCrudHelper.

* API-2532: PR feedback fixes.

app/uk/gov/hmrc/apisubscriptionfields/repository/FieldsDefinitionRepository.scala

@@ -19,17 +19,14 @@ package uk.gov.hmrc.apisubscriptionfields.repository
 import javax.inject.{Inject, Singleton}
 
 import com.google.inject.ImplementedBy
-import play.api.Logger
 import play.api.libs.json._
 import reactivemongo.api.indexes.IndexType
-import reactivemongo.api.{Cursor, ReadPreference}
 import reactivemongo.bson.BSONObjectID
-import reactivemongo.play.json.ImplicitBSONHandlers._
+import reactivemongo.play.json.collection.JSONCollection
 import uk.gov.hmrc.apisubscriptionfields.model.{ApiContext, ApiVersion}
 import uk.gov.hmrc.mongo.ReactiveRepository
 import uk.gov.hmrc.mongo.json.ReactiveMongoFormats
 
-import scala.concurrent.ExecutionContext.Implicits.global
 import scala.concurrent.Future
 
 @ImplementedBy(classOf[FieldsDefinitionMongoRepository])
@@ -48,9 +45,9 @@ class FieldsDefinitionMongoRepository @Inject()(mongoDbProvider: MongoDbProvider
   extends ReactiveRepository[FieldsDefinition, BSONObjectID]("fieldsDefinitions", mongoDbProvider.mongo,
     MongoFormatters.FieldsDefinitionJF, ReactiveMongoFormats.objectIdFormats)
   with FieldsDefinitionRepository
-  with MongoIndexCreator
-  with MongoErrorHandler {
+  with MongoCrudHelper[FieldsDefinition] {
 
+  override val col: JSONCollection = collection
   private implicit val format = MongoFormatters.FieldsDefinitionJF
 
   override def indexes = Seq(
@@ -65,25 +62,16 @@ class FieldsDefinitionMongoRepository @Inject()(mongoDbProvider: MongoDbProvider
   )
 
   override def fetchAll(): Future[List[FieldsDefinition]] = {
-    Logger.debug(s"[fetchAll]")
-    collection.find(Json.obj()).cursor[FieldsDefinition](ReadPreference.primary).collect[List](
-      Int.MaxValue, Cursor.FailOnError[List[FieldsDefinition]]()
-    )
+    getMany(Json.obj())
   }
 
   override def fetch(apiContext: ApiContext, apiVersion: ApiVersion): Future[Option[FieldsDefinition]] = {
     val selector = selectorForFieldsDefinition(apiContext, apiVersion)
-    Logger.debug(s"[fetch] selector: $selector")
-    collection.find(selector).one[FieldsDefinition]
+    getOne(selector)
   }
 
   override def save(fieldsDefinition: FieldsDefinition): Future[Boolean] = {
-    collection.update(selector = selectorForFieldsDefinition(fieldsDefinition), update = fieldsDefinition, upsert = true).map {
-      updateWriteResult =>
-        handleSaveError(updateWriteResult, s"Could not save fields definition fields: $fieldsDefinition",
-          updateWriteResult.upserted.nonEmpty
-      )
-    }
+    save(fieldsDefinition, selectorForFieldsDefinition(fieldsDefinition))
   }
 
   private def selectorForFieldsDefinition(apiContext: ApiContext, apiVersion: ApiVersion): JsObject = {

app/uk/gov/hmrc/apisubscriptionfields/repository/MongoCrudHelper.scala

@@ -0,0 +1,60 @@
+/*
+ * Copyright 2017 HM Revenue & Customs
+ *
+ * Licensed 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 uk.gov.hmrc.apisubscriptionfields.repository
+
+import play.api.Logger
+import play.api.libs.json.{JsObject, OWrites, Reads}
+import reactivemongo.api.{Cursor, ReadPreference}
+import reactivemongo.play.json.ImplicitBSONHandlers._
+import reactivemongo.play.json.collection.JSONCollection
+
+import scala.concurrent.ExecutionContext.Implicits.global
+import scala.concurrent.Future
+
+
+
+trait MongoCrudHelper[T] extends MongoIndexCreator with MongoErrorHandler {
+
+  val col: JSONCollection
+
+  def save(entity: T, selector: JsObject)(implicit w: OWrites[T]): Future[Boolean] = {
+    Logger.debug(s"[save] entity: $entity selector: $selector")
+    col.update(selector = selector, update = entity, upsert = true).map {
+      updateWriteResult => handleSaveError(updateWriteResult, s"Could not save entity: $entity")
+    }
+  }
+
+  def getMany(selector: JsObject)(implicit r: Reads[T]):Future[List[T]] = {
+    Logger.debug(s"[getCursor] selector: $selector")
+    col.find(selector).cursor[T](ReadPreference.primary).collect[List](
+      Int.MaxValue, Cursor.FailOnError[List[T]]()
+    )
+  }
+
+  def getOne(selector: JsObject)(implicit r: Reads[T]):Future[Option[T]] = {
+    Logger.debug(s"[getOne] selector: $selector")
+    col.find(selector).one[T]
+  }
+
+  def deleteOne(selector: JsObject): Future[Boolean] = {
+    Logger.debug(s"[deleteOne] selector: $selector")
+    col.remove(selector).map {
+      writeResult => handleDeleteError(writeResult, s"Could not delete entity for selector: $selector")
+    }
+  }
+
+}

app/uk/gov/hmrc/apisubscriptionfields/repository/MongoErrorHandler.scala

@@ -17,23 +17,23 @@
 package uk.gov.hmrc.apisubscriptionfields.repository
 
 import play.api.Logger
-import reactivemongo.api.commands.WriteResult
+import reactivemongo.api.commands.{UpdateWriteResult, WriteResult}
 
 trait MongoErrorHandler {
 
   def handleDeleteError(result: WriteResult, exceptionMsg: => String): Boolean = {
     handleError(result, databaseAltered, exceptionMsg)
   }
 
-  def handleSaveError(result: WriteResult, exceptionMsg: => String, isInserted: Boolean): Boolean = {
+  def handleSaveError(updateWriteResult: UpdateWriteResult, exceptionMsg: => String): Boolean = {
 
     def handleUpsertError(result: WriteResult) =
       if (databaseAltered(result))
-        isInserted
+        updateWriteResult.upserted.nonEmpty
       else
         throw new RuntimeException(exceptionMsg)
 
-    handleError(result, handleUpsertError, exceptionMsg)
+    handleError(updateWriteResult, handleUpsertError, exceptionMsg)
   }
 
   private def handleError(result: WriteResult, f: WriteResult => Boolean, exceptionMsg: String): Boolean = {

app/uk/gov/hmrc/apisubscriptionfields/repository/SubscriptionFieldsRepository.scala

@@ -20,25 +20,22 @@ import java.util.UUID
 import javax.inject.{Inject, Singleton}
 
 import com.google.inject.ImplementedBy
-import play.api.Logger
 import play.api.libs.json._
 import reactivemongo.api.indexes.IndexType
-import reactivemongo.api.{Cursor, ReadPreference}
 import reactivemongo.bson.BSONObjectID
-import reactivemongo.play.json.ImplicitBSONHandlers._
+import reactivemongo.play.json.collection.JSONCollection
 import uk.gov.hmrc.apisubscriptionfields.model.{ApiContext, ApiVersion, ClientId}
 import uk.gov.hmrc.mongo.ReactiveRepository
 import uk.gov.hmrc.mongo.json.ReactiveMongoFormats
 
-import scala.concurrent.ExecutionContext.Implicits.global
 import scala.concurrent.Future
 
 @ImplementedBy(classOf[SubscriptionFieldsMongoRepository])
 trait SubscriptionFieldsRepository {
 
   def save(subscription: SubscriptionFields): Future[Boolean]
 
-  def fetchByClientId(clientId: String): Future[List[SubscriptionFields]]
+  def fetchByClientId(clientId: ClientId): Future[List[SubscriptionFields]]
   def fetch(clientId: ClientId, apiContext: ApiContext, apiVersion: ApiVersion): Future[Option[SubscriptionFields]]
   def fetchByFieldsId(fieldsId: UUID): Future[Option[SubscriptionFields]]
 
@@ -50,8 +47,9 @@ class SubscriptionFieldsMongoRepository @Inject()(mongoDbProvider: MongoDbProvid
   extends ReactiveRepository[SubscriptionFields, BSONObjectID]("subscriptionFields", mongoDbProvider.mongo,
     MongoFormatters.SubscriptionFieldsJF, ReactiveMongoFormats.objectIdFormats)
   with SubscriptionFieldsRepository
-  with MongoIndexCreator
-  with MongoErrorHandler {
+  with MongoCrudHelper[SubscriptionFields] {
+
+  override val col: JSONCollection = collection
 
   private implicit val format = MongoFormatters.SubscriptionFieldsJF
 
@@ -78,25 +76,27 @@ class SubscriptionFieldsMongoRepository @Inject()(mongoDbProvider: MongoDbProvid
   )
 
   override def save(subscription: SubscriptionFields): Future[Boolean] = {
-    collection.update(selector = selectorForSubscriptionFields(subscription), update = subscription, upsert = true).map {
-      updateWriteResult => handleSaveError(updateWriteResult, s"Could not save subscription fields: $subscription",
-        updateWriteResult.upserted.nonEmpty
-      )
-    }
+    save(subscription, selectorForSubscriptionFields(subscription))
   }
 
-  override def fetchByClientId(clientId: String): Future[List[SubscriptionFields]] = {
-    val selector = Json.obj("clientId" -> clientId)
-    Logger.debug(s"[fetchByClientId] selector: $selector")
-    collection.find(selector).cursor[SubscriptionFields](ReadPreference.primary).collect[List](
-      Int.MaxValue, Cursor.FailOnError[List[SubscriptionFields]]()
-    )
+  override def fetchByClientId(clientId: ClientId): Future[List[SubscriptionFields]] = {
+    val selector = Json.obj("clientId" -> clientId.value)
+    getMany(selector)
   }
 
   override def fetch(clientId: ClientId, apiContext: ApiContext, apiVersion: ApiVersion): Future[Option[SubscriptionFields]] = {
     val selector = selectorForSubscriptionFields(clientId.value, apiContext.value, apiVersion.value)
-    Logger.debug(s"[fetch] selector: $selector")
-    collection.find(selector).one[SubscriptionFields]
+    getOne(selector)
+  }
+
+  override def fetchByFieldsId(fieldsId: UUID): Future[Option[SubscriptionFields]] = {
+    val selector = Json.obj("fieldsId" -> fieldsId)
+    getOne(selector)
+  }
+
+  override def delete(clientId: ClientId, apiContext: ApiContext, apiVersion: ApiVersion): Future[Boolean] = {
+    val selector = selectorForSubscriptionFields(clientId.value, apiContext.value, apiVersion.value)
+    deleteOne(selector)
   }
 
   private def selectorForSubscriptionFields(clientId: String, apiContext: String, apiVersion: String): JsObject = {
@@ -111,17 +111,4 @@ class SubscriptionFieldsMongoRepository @Inject()(mongoDbProvider: MongoDbProvid
     selectorForSubscriptionFields(subscription.clientId, subscription.apiContext, subscription.apiVersion)
   }
 
-  override def fetchByFieldsId(fieldsId: UUID): Future[Option[SubscriptionFields]] = {
-    val selector = Json.obj("fieldsId" -> fieldsId)
-    Logger.debug(s"[fetchByFieldsId] selector: $selector")
-    collection.find(selector).one[SubscriptionFields]
-  }
-
-  override def delete(clientId: ClientId, apiContext: ApiContext, apiVersion: ApiVersion): Future[Boolean] = {
-    val selector = selectorForSubscriptionFields(clientId.value, apiContext.value, apiVersion.value)
-    Logger.debug(s"[delete] selector: $selector")
-    collection.remove(selector).map {
-      writeResult => handleDeleteError(writeResult, s"Could not delete subscription fields for clientId: $clientId, apiContext: $apiContext, apiVersion: $apiVersion")
-    }
-  }
 }

app/uk/gov/hmrc/apisubscriptionfields/service/SubscriptionFieldsService.scala

@@ -62,7 +62,7 @@ class SubscriptionFieldsService @Inject()(repository: SubscriptionFieldsReposito
   def get(clientId: ClientId): Future[Option[BulkSubscriptionFieldsResponse]] = {
     Logger.debug(s"[get] ClientId: $clientId")
     (for {
-      list <- repository.fetchByClientId(clientId.value)
+      list <- repository.fetchByClientId(clientId)
     } yield list.map(asResponse)) map {
       case Nil => None
       case list => Some(BulkSubscriptionFieldsResponse(fields = list))

test/unit/uk/gov/hmrc/apisubscriptionfields/repository/SubscriptionFieldsRepositorySpec.scala

@@ -47,14 +47,9 @@ class SubscriptionFieldsRepositorySpec extends UnitSpec
   private val repository = new SubscriptionFieldsMongoRepository(mongoDbProvider) {
 
     import play.api.libs.json._
-    import reactivemongo.play.json.ImplicitBSONHandlers._
 
     def saveByFieldsId(subscription: SubscriptionFields): Future[Boolean] = {
-      collection.update(selector = Json.obj("fieldsId" -> subscription.fieldsId), update = subscription, upsert = true).map {
-        updateWriteResult => handleSaveError(updateWriteResult, s"Could not save subscription fields: $subscription",
-          updateWriteResult.upserted.nonEmpty
-        )
-      }
+      save(subscription, Json.obj("fieldsId" -> subscription.fieldsId))
     }
 
   }
@@ -119,12 +114,12 @@ class SubscriptionFieldsRepositorySpec extends UnitSpec
       await(repository.save(apiSubForApp2Context1))
       collectionSize shouldBe 3
 
-      await(repository.fetchByClientId(fakeRawClientId)) shouldBe List(apiSubForApp1Context1, apiSubForApp1Context2)
-      await(repository.fetchByClientId(fakeRawClientId2)) shouldBe List(apiSubForApp2Context1)
+      await(repository.fetchByClientId(FakeClientId)) shouldBe List(apiSubForApp1Context1, apiSubForApp1Context2)
+      await(repository.fetchByClientId(FakeClientId2)) shouldBe List(apiSubForApp2Context1)
     }
 
     "return an empty list when clientId is not found" in {
-      await(repository.fetchByClientId("CLIENT_ID_DOES_NOT_EXIST_IN_DB")) shouldBe List()
+      await(repository.fetchByClientId(ClientId("CLIENT_ID_DOES_NOT_EXIST_IN_DB"))) shouldBe List()
     }
 
   }

test/unit/uk/gov/hmrc/apisubscriptionfields/service/SubscriptionFieldsServiceSpec.scala

@@ -31,7 +31,7 @@ class SubscriptionFieldsServiceSpec extends UnitSpec with SubscriptionFieldsTest
 
   "A RepositoryFedSubscriptionFieldsService" should {
     "return None when no entry exist in the repo when get by application client id is called" in {
-      (mockSubscriptionFieldsIdRepository fetchByClientId  _) expects fakeRawClientId returns List()
+      (mockSubscriptionFieldsIdRepository fetchByClientId  _) expects FakeClientId returns List()
 
       val result = await(service.get(FakeClientId))
 
@@ -41,7 +41,7 @@ class SubscriptionFieldsServiceSpec extends UnitSpec with SubscriptionFieldsTest
     "return Some response when entry exists in the repo when get by application client id is called" in {
       val subscriptionFields1 = createSubscriptionFieldsWithApiContext()
       val subscriptionFields2 = createSubscriptionFieldsWithApiContext(rawContext = fakeRawContext2)
-      (mockSubscriptionFieldsIdRepository fetchByClientId  _) expects fakeRawClientId returns List(subscriptionFields1, subscriptionFields2)
+      (mockSubscriptionFieldsIdRepository fetchByClientId  _) expects FakeClientId returns List(subscriptionFields1, subscriptionFields2)
 
       val result = await(service.get(FakeClientId))