Merge pull request #2234 from hongwei1/develop

refactor/added error handling and the debug log for createSystemView

obp-api/src/main/resources/logback-test.xml

@@ -9,4 +9,10 @@
   <root level="DEBUG">
     <appender-ref ref="STDOUT"/>
   </root>
+
+  <!-- LOG "com.gargoylesoftware.htmlunit.javascript*" at OFF (closed) level -->
+  <logger name="com.gargoylesoftware.htmlunit.javascript"  level="OFF" additivity="false">
+    <appender-ref ref="RollingFile" />
+    <appender-ref ref="Console" />
+  </logger>
 </configuration>

obp-api/src/main/scala/code/api/v2_0_0/APIMethods200.scala

@@ -1133,13 +1133,15 @@ trait APIMethods200 {
               "", //added new field in V220
               List.empty
             )
+            //1 Create or Update the `Owner` for the new account
+            //2 Add permission to the user
+            //3 Set the user as the account holder
+            _ = BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, postedOrLoggedInUser, callContext)
+            dataContext = DataContext(cc.user, Some(bankAccount.bankId), Some(bankAccount.accountId), Empty, Empty, Empty)
+            links = code.api.util.APIUtil.getHalLinks(CallerContext(createAccount), codeContext, dataContext)
+            json = JSONFactory200.createCoreAccountJSON(bankAccount, links)
+
           } yield {
-            BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, postedOrLoggedInUser, Some(cc))
-
-            val dataContext = DataContext(cc.user, Some(bankAccount.bankId), Some(bankAccount.accountId), Empty, Empty, Empty)
-            val links = code.api.util.APIUtil.getHalLinks(CallerContext(createAccount), codeContext, dataContext)
-            val json = JSONFactory200.createCoreAccountJSON(bankAccount, links)
-
             successJsonResponse(Extraction.decompose(json))
           }
         }

obp-api/src/main/scala/code/api/v2_2_0/APIMethods220.scala

@@ -809,11 +809,11 @@ trait APIMethods220 {
               List(AccountRouting(createAccountJson.account_routing.scheme, createAccountJson.account_routing.address)),
               callContext
             )
-          } yield {
             //1 Create or Update the `Owner` for the new account
             //2 Add permission to the user
             //3 Set the user as the account holder
-            BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, postedOrLoggedInUser, callContext)
+            _ = BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, postedOrLoggedInUser, callContext)
+          } yield {
             (JSONFactory220.createAccountJSON(userIdAccountOwner, bankAccount), HttpCode.`200`(callContext))
 
           }

obp-api/src/main/scala/code/api/v3_1_0/APIMethods310.scala

@@ -5423,11 +5423,11 @@ trait APIMethods310 {
               None,
               callContext: Option[CallContext]
             )
-          } yield {
             //1 Create or Update the `Owner` for the new account
             //2 Add permission to the user
             //3 Set the user as the account holder
-            BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, postedOrLoggedInUser, callContext)
+            _ = BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, postedOrLoggedInUser, callContext)
+          } yield {
             (JSONFactory310.createAccountJSON(userIdAccountOwner, bankAccount, accountAttributes), HttpCode.`201`(callContext))
           }
         }

obp-api/src/main/scala/code/api/v4_0_0/APIMethods400.scala

@@ -532,11 +532,11 @@ trait APIMethods400 {
               None,
               callContext: Option[CallContext]
             )
-          } yield {
             //1 Create or Update the `Owner` for the new account
             //2 Add permission to the user
             //3 Set the user as the account holder
-            BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, postedOrLoggedInUser, callContext)
+            _ = BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, postedOrLoggedInUser, callContext)
+          } yield {
             (JSONFactory400.createSettlementAccountJson(userIdAccountOwner, bankAccount, accountAttributes), HttpCode.`201`(callContext))
           }
       }
@@ -2627,11 +2627,11 @@ trait APIMethods400 {
               None,
               callContext: Option[CallContext]
             )
-          } yield {
             //1 Create or Update the `Owner` for the new account
             //2 Add permission to the user
             //3 Set the user as the account holder
-            BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, postedOrLoggedInUser, callContext)
+            _ = BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, postedOrLoggedInUser, callContext)
+          } yield {
             (JSONFactory310.createAccountJSON(userIdAccountOwner, bankAccount, accountAttributes), HttpCode.`201`(callContext))
           }
         }

obp-api/src/main/scala/code/api/v5_0_0/APIMethods500.scala

@@ -388,11 +388,11 @@ trait APIMethods500 {
               None,
               callContext: Option[CallContext]
             )
-          } yield {
             //1 Create or Update the `Owner` for the new account
             //2 Add permission to the user
             //3 Set the user as the account holder
-            BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, postedOrLoggedInUser, callContext)
+            _ = BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, postedOrLoggedInUser, callContext)
+          } yield {
             (JSONFactory310.createAccountJSON(userIdAccountOwner, bankAccount, accountAttributes), HttpCode.`201`(callContext))
           }
         }

obp-api/src/main/scala/code/bankconnectors/LocalMappedConnector.scala

@@ -2234,7 +2234,7 @@ object LocalMappedConnector extends Connector with MdcLoggable {
       List.empty
     )
 
-    Full((bank, account))
+    account.map(account => (bank, account))
   }
 
   override def updateBankAccount(
@@ -2409,11 +2409,9 @@ object LocalMappedConnector extends Connector with MdcLoggable {
                                        ): Box[BankAccount] = {
 
     for {
-      (bank, _) <- getBankLegacy(bankId, None) //bank is not really used, but doing this will ensure account creations fails if the bank doesn't
-    } yield {
-
-      val balanceInSmallestCurrencyUnits = Helper.convertToSmallestCurrencyUnits(initialBalance, currency)
-      createAccountIfNotExisting(
+      (_, _) <- getBankLegacy(bankId, None) //bank is not really used, but doing this will ensure account creations fails if the bank doesn't
+      balanceInSmallestCurrencyUnits = Helper.convertToSmallestCurrencyUnits(initialBalance, currency)
+      account <- createAccountIfNotExisting (
         bankId,
         accountId,
         accountNumber,
@@ -2424,7 +2422,9 @@ object LocalMappedConnector extends Connector with MdcLoggable {
         accountHolderName,
         branchId,
         accountRoutings
-      )
+        ) ?~! AccountRoutingAlreadyExist
+    } yield {
+      account
     }
 
   }
@@ -2441,12 +2441,12 @@ object LocalMappedConnector extends Connector with MdcLoggable {
                                           accountHolderName: String,
                                           branchId: String,
                                           accountRoutings: List[AccountRouting],
-                                        ): BankAccount = {
+                                        ): Box[BankAccount] = {
     getBankAccountOld(bankId, accountId) match {
       case Full(a) =>
         logger.debug(s"account with id $accountId at bank with id $bankId already exists. No need to create a new one.")
-        a
-      case _ =>
+        Full(a)
+      case _ => tryo {
         accountRoutings.map(accountRouting =>
           BankAccountRouting.create
             .BankId(bankId.value)
@@ -2466,6 +2466,7 @@ object LocalMappedConnector extends Connector with MdcLoggable {
           .holder(accountHolderName)
           .mBranchId(branchId)
           .saveMe()
+      }
     }
   }
 

obp-api/src/main/scala/code/model/dataAccess/AuthUser.scala

@@ -1420,8 +1420,8 @@ def restoreSomeSessions(): Unit = {
           _ = AccountHolders.accountHolders.vend.deleteAccountHolder(user,cbsRemovedBankAccountId)
           cbsAccount = accountsHeld.find(cbsAccount =>cbsAccount.bankId == bankId.value && cbsAccount.accountId == accountId.value)
           viewId <- cbsAccount.map(_.viewsToGenerate).getOrElse(List.empty[String])
+          _=UserRefreshes.UserRefreshes.vend.createOrUpdateRefreshUser(user.userId)
         } yield {
-          UserRefreshes.UserRefreshes.vend.createOrUpdateRefreshUser(user.userId)
           Views.views.vend.removeCustomView(ViewId(viewId), cbsRemovedBankAccountId)
         }
 
@@ -1433,9 +1433,10 @@ def restoreSomeSessions(): Unit = {
           accountId = newBankAccountId.accountId
           newBankAccount = accountsHeld.find(cbsAccount =>cbsAccount.bankId == bankId.value && cbsAccount.accountId == accountId.value)
           viewId <- newBankAccount.map(_.viewsToGenerate).getOrElse(List.empty[String])
-          view <- Views.views.vend.getOrCreateSystemViewFromCbs(viewId)//this method will return both system views and custom views back.
+          _ = logger.debug("refreshViewsAccountAccessAndHolders.csbNewBankAccountIds-------" + csbNewBankAccountIds)
+          view <- Views.views.vend.getOrCreateSystemViewFromCbs(viewId)//TODO, only support system views so far, may add custom views later.
+          _=UserRefreshes.UserRefreshes.vend.createOrUpdateRefreshUser(user.userId)
         } yield {
-          UserRefreshes.UserRefreshes.vend.createOrUpdateRefreshUser(user.userId)
           if (view.isSystem)//if the view is a system view, we will call `grantAccessToSystemView`
             Views.views.vend.grantAccessToSystemView(bankId, accountId, view, user)
           else //otherwise, we will call `grantAccessToCustomView`
@@ -1451,9 +1452,11 @@ def restoreSomeSessions(): Unit = {
             obpViewsForAccount = Views.views.vend.privateViewsUserCanAccessForAccount(user, bankAccountId).map(_.viewId.value)
 
             cbsViewsForAccount = accountsHeld.find(account => account.bankId.equals(bankAccountId.bankId.value) && account.accountId.equals(bankAccountId.accountId.value)).map(_.viewsToGenerate).getOrElse(Nil)
-
+            _ = logger.debug("refreshViewsAccountAccessAndHolders.obpViewsForAccount-------" + obpViewsForAccount)
+            _ = logger.debug("refreshViewsAccountAccessAndHolders.cbsViewsForAccount-------" + cbsViewsForAccount)
             //cbs removed these views, but OBP still contains the data for them, so we need to clean data in OBP side.
             cbsRemovedViewsForAccount = obpViewsForAccount diff cbsViewsForAccount
+            _ = logger.debug("refreshViewsAccountAccessAndHolders.cbsRemovedViewsForAccount-------" + cbsRemovedViewsForAccount)
             _ = if(cbsRemovedViewsForAccount.nonEmpty){
               val cbsRemovedViewIdBankIdAccountIds = cbsRemovedViewsForAccount.map(view => ViewIdBankIdAccountId(ViewId(view), bankAccountId.bankId, bankAccountId.accountId))
               Views.views.vend.revokeAccessToMultipleViews(cbsRemovedViewIdBankIdAccountIds, user) 
@@ -1462,16 +1465,18 @@ def restoreSomeSessions(): Unit = {
             } 
             //cbs has new views which are not in obp yet, we need to create new data for these accounts.
             csbNewViewsForAccount = cbsViewsForAccount diff obpViewsForAccount
+            _ = logger.debug("refreshViewsAccountAccessAndHolders.csbNewViewsForAccount-------" + csbNewViewsForAccount)
             _ = if(csbNewViewsForAccount.nonEmpty){
               for{
                 newViewForAccount <- csbNewViewsForAccount
-                view <- Views.views.vend.getOrCreateSystemViewFromCbs(newViewForAccount) //this method will return both system views and custom views back.
+                view <- Views.views.vend.getOrCreateSystemViewFromCbs(newViewForAccount)//TODO, only support system views so far, may add custom views later.
+                _ = UserRefreshes.UserRefreshes.vend.createOrUpdateRefreshUser(user.userId)
               }yield{
                 if (view.isSystem)//if the view is a system view, we will call `grantAccessToSystemView`
                   Views.views.vend.grantAccessToSystemView(bankAccountId.bankId, bankAccountId.accountId, view, user)
                 else //otherwise, we will call `grantAccessToCustomView`
                   Views.views.vend.grantAccessToCustomView(view.uid, user)
-                UserRefreshes.UserRefreshes.vend.createOrUpdateRefreshUser(user.userId)
+
               }
             } 
           } yield {

obp-api/src/main/scala/code/snippet/CreateTestAccountForm.scala

@@ -91,9 +91,12 @@ object CreateTestAccountForm{
           s"Account with id $accountId already exists at bank $bankId")
         bankAccount <- Connector.connector.vend.createBankAccountLegacy(bankId, accountId, accountType, accountLabel, currency, initialBalanceAsNumber, user.name,
                                                                          "", List.empty)//added field in V220
-
+
+        //1 Create or Update the `Owner` for the new account
+        //2 Add permission to the user
+        //3 Set the user as the account holder
+        _ = BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, user, callContext)
       } yield {
-        BankAccountCreation.setAccountHolderAndRefreshUserAccountAccess(bankId, accountId, user, None)
         bankAccount
       }
     }

obp-api/src/main/scala/code/views/MapperViews.scala

@@ -96,7 +96,7 @@ object MapperViews extends Views with MdcLoggable {
       By(AccountAccess.bank_id, bankId), 
       By(AccountAccess.account_id, accountId), 
       By(AccountAccess.view_id, viewDefinition.viewId.value)) == 0) {
-      //logger.debug(s"saving AccountAccessList for user ${user.resourceUserId.value} for view ${vImpl.id}")
+      logger.debug(s"getOrGrantAccessToCustomView AccountAccess.create user(UserId(${user.userId}), ViewId(${viewDefinition.viewId.value}), bankId($bankId), accountId($accountId)")
       // SQL Insert AccountAccessList
       val saved = AccountAccess.create.
         user_fk(user.userPrimaryKey.value).
@@ -112,7 +112,10 @@ object MapperViews extends Views with MdcLoggable {
         //logger.debug("failed to save AccountAccessList")
         Empty ~> APIFailure("Server error adding permission", 500) //TODO: move message + code logic to api level
       }
-    } else Full(viewDefinition) //accountAccess already exists, no need to create one
+    } else {
+      logger.debug(s"getOrGrantAccessToCustomView AccountAccess is already existing (UserId(${user.userId}), ViewId(${viewDefinition.viewId.value}), bankId($bankId), accountId($accountId))")
+      Full(viewDefinition)
+    } //accountAccess already exists, no need to create one
   }
   // This is an idempotent function 
   private def getOrGrantAccessToSystemView(bankId: BankId, accountId: AccountId, user: User, view: View): Box[View] = {
@@ -627,7 +630,7 @@ object MapperViews extends Views with MdcLoggable {
         Failure(ViewIdNotSupported+ s"Your input viewId is :$viewId")
       }
 
-    logger.debug(s"-->getOrCreateAccountView.${viewId } : ${theView} ")
+    logger.debug(s"-->getOrCreateSystemViewFromCbs.${viewId } : ${theView} ")
 
     theView
   }
@@ -662,15 +665,15 @@ object MapperViews extends Views with MdcLoggable {
     }
   }
 
-  def createDefaultSystemView(name: String): Box[View] = {
-    createAndSaveSystemView(name)
+  def createDefaultSystemView(viewId: String): Box[View] = {
+    createAndSaveSystemView(viewId)
   }
 
-  def createDefaultCustomPublicView(bankId: BankId, accountId: AccountId, name: String): Box[View] = {
+  def createDefaultCustomPublicView(bankId: BankId, accountId: AccountId, description: String): Box[View] = {
     if(!allowPublicViews) {
       return Failure(PublicViewsNotAllowedOnThisInstance)
     }
-    createAndSaveDefaultPublicCustomView(bankId, accountId, "Public View")
+    createAndSaveDefaultPublicCustomView(bankId, accountId, description)
   }
 
   def getExistingCustomView(bankId: BankId, accountId: AccountId, viewId: String): Box[View] = {
@@ -680,6 +683,7 @@ object MapperViews extends Views with MdcLoggable {
   }
   def getExistingSystemView(viewId: String): Box[View] = {
     val res = ViewDefinition.findSystemView(viewId)
+    logger.debug(s"-->getExistingSystemView(viewId($viewId)) = result ${res} ")
     if(res.isDefined && res.openOrThrowException(attemptedToOpenAnEmptyBox).isPublic && !allowPublicViews) return Failure(PublicViewsNotAllowedOnThisInstance)
     res
   }
@@ -805,7 +809,9 @@ object MapperViews extends Views with MdcLoggable {
   }
 
   def createAndSaveSystemView(viewId: String) : Box[View] = {
+    logger.debug(s"-->createAndSaveSystemView.viewId: ${viewId} ")
     val res = unsavedSystemView(viewId).saveMe
+    logger.debug(s"-->createAndSaveSystemView: ${res} ")
     Full(res)
   }
 

obp-api/src/test/scala/code/setup/TestConnectorSetup.scala

@@ -146,7 +146,7 @@ trait TestConnectorSetup {
   final protected def createPaymentTestBank() : Bank =
     createBank("payment-test-bank")
 
-  protected def getOrCreateSystemView(name: String): View
+  protected def getOrCreateSystemView(viewId: String): View
   protected def createPublicView(bankId: BankId, accountId: AccountId) : View
   protected def createCustomRandomView(bankId: BankId, accountId: AccountId) : View
 

obp-api/src/test/scala/code/setup/TestConnectorSetupWithStandardPermissions.scala

@@ -25,8 +25,8 @@ trait TestConnectorSetupWithStandardPermissions extends TestConnectorSetup {
     AccountHolders.accountHolders.vend.getOrCreateAccountHolder(user, BankIdAccountId(bankId, accountId))
   }
 
-  protected def getOrCreateSystemView(name: String) : View = {
-    Views.views.vend.getOrCreateSystemView(name).openOrThrowException(attemptedToOpenAnEmptyBox)
+  protected def getOrCreateSystemView(viewId: String) : View = {
+    Views.views.vend.getOrCreateSystemView(viewId).openOrThrowException(attemptedToOpenAnEmptyBox)
   }
 
   protected def createPublicView(bankId: BankId, accountId: AccountId) : View = {