[libeufin] branch master updated: PATCHing accounts.

[gnunet]

This is an automated email from the git hooks/post-receive script.

ms pushed a commit to branch master
in repository libeufin.

The following commit(s) were added to refs/heads/master by this push:
     new d84b9461 PATCHing accounts.
d84b9461 is described below

commit d84b94617f562056d7a87d38f5302c1701e7d3e2
Author: MS <ms@taler.net>
AuthorDate: Wed Oct 4 15:28:08 2023 +0200

    PATCHing accounts.
    
    HTTP level tests and handling of null inputs.
---
 .../main/kotlin/tech/libeufin/bank/BankMessages.kt |  2 +-
 .../tech/libeufin/bank/CorebankApiHandlers.kt      | 25 +++++++--
 .../src/main/kotlin/tech/libeufin/bank/Database.kt | 27 ++++++----
 bank/src/test/kotlin/DatabaseTest.kt               | 40 +++++++++++---
 bank/src/test/kotlin/LibeuFinApiTest.kt            | 63 ++++++++++++++++++++++
 database-versioning/procedures.sql                 | 19 +++++--
 6 files changed, 149 insertions(+), 27 deletions(-)

diff --git a/bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt 
b/bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt
index 34b03472..ee5cf866 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/BankMessages.kt
@@ -741,7 +741,7 @@ data class AccountReconfiguration(
     val challenge_contact_data: ChallengeContactData?,
     val cashout_address: String?,
     val name: String?,
-    val is_exchange: Boolean
+    val is_exchange: Boolean?
 )
 
 /**
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/CorebankApiHandlers.kt 
b/bank/src/main/kotlin/tech/libeufin/bank/CorebankApiHandlers.kt
index 8b269b15..5cfbfb49 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/CorebankApiHandlers.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/CorebankApiHandlers.kt
@@ -351,7 +351,7 @@ fun Routing.accountsMgmtHandlers(db: Database, ctx: 
BankApplicationContext) {
             talerEc = TalerErrorCode.TALER_EC_END // FIXME, define EC.
         )
         // Check if a non-admin user tried to change their legal name
-        if (c.login != "admin" && req.name != accountCustomer.name)
+        if ((c.login != "admin") && (req.name != null) && (req.name != 
accountCustomer.name))
             throw forbidden("non-admin user cannot change their legal name")
         // Preventing identical data to be overridden.
         val bankAccount = 
db.bankAccountGetFromOwnerId(accountCustomer.expectRowId())
@@ -366,8 +366,27 @@ fun Routing.accountsMgmtHandlers(db: Database, ctx: 
BankApplicationContext) {
             call.respond(HttpStatusCode.NoContent)
             return@patch
         }
-        // Not identical, go on writing the DB.
-        throw NotImplementedError("DB part missing.")
+        val dbRes = db.accountReconfig(
+            login = accountCustomer.login,
+            name = req.name,
+            cashoutPayto = req.cashout_address,
+            emailAddress = req.challenge_contact_data?.email,
+            isTalerExchange = req.is_exchange,
+            phoneNumber = req.challenge_contact_data?.phone
+            )
+        when (dbRes) {
+            AccountReconfigDBResult.SUCCESS -> 
call.respond(HttpStatusCode.NoContent)
+            AccountReconfigDBResult.CUSTOMER_NOT_FOUND -> {
+                // Rare case.  Only possible if a deletion happened before the 
flow reaches here.
+                logger.warn("Authenticated customer wasn't found any more in 
the database")
+                throw notFound("Customer not found", 
TalerErrorCode.TALER_EC_END) // FIXME: needs EC
+            }
+            AccountReconfigDBResult.BANK_ACCOUNT_NOT_FOUND -> {
+                // Bank's fault: no customer should lack a bank account.
+                throw internalServerError("Customer '${accountCustomer.login}' 
lacks bank account")
+            }
+        }
+        return@patch
     }
     // WITHDRAWAL ENDPOINTS
     post("/accounts/{USERNAME}/withdrawals") {
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Database.kt 
b/bank/src/main/kotlin/tech/libeufin/bank/Database.kt
index dc7eb04b..066d0cc2 100644
--- a/bank/src/main/kotlin/tech/libeufin/bank/Database.kt
+++ b/bank/src/main/kotlin/tech/libeufin/bank/Database.kt
@@ -28,10 +28,7 @@ import tech.libeufin.util.microsToJavaInstant
 import tech.libeufin.util.stripIbanPayto
 import tech.libeufin.util.toDbMicros
 import java.io.File
-import java.sql.DriverManager
-import java.sql.PreparedStatement
-import java.sql.ResultSet
-import java.sql.SQLException
+import java.sql.*
 import java.time.Instant
 import java.util.*
 import kotlin.math.abs
@@ -404,16 +401,22 @@ class Database(private val dbConfig: String, private val 
bankCurrency: String) {
      * The 'login' parameter decides which customer and bank account rows
      * will get the update.
      *
+     * Meaning of null in the parameters: when 'name' and 'isTalerExchange'
+     * are null, NOTHING gets changed.  If any of the other values are null,
+     * WARNING: their value will be overridden with null.  No parameter gets
+     * null as the default, as to always keep the caller aware of what gets in
+     * the database.
+     *
      * The return type expresses either success, or that the target rows
      * could not be found.
      */
     fun accountReconfig(
         login: String,
-        name: String,
-        cashoutPayto: String,
-        phoneNumber: String,
-        emailAddress: String,
-        isTalerExchange: Boolean
+        name: String?,
+        cashoutPayto: String?,
+        phoneNumber: String?,
+        emailAddress: String?,
+        isTalerExchange: Boolean?
     ): AccountReconfigDBResult {
         reconnect()
         val stmt = prepare("""
@@ -427,7 +430,11 @@ class Database(private val dbConfig: String, private val 
bankCurrency: String) {
         stmt.setString(3, phoneNumber)
         stmt.setString(4, emailAddress)
         stmt.setString(5, cashoutPayto)
-        stmt.setBoolean(6, isTalerExchange)
+
+        if (isTalerExchange == null)
+            stmt.setNull(6, Types.NULL)
+        else stmt.setBoolean(6, isTalerExchange)
+
         val res = stmt.executeQuery()
         res.use {
             if (!it.next()) throw internalServerError("accountReconfig() 
returned nothing")
diff --git a/bank/src/test/kotlin/DatabaseTest.kt 
b/bank/src/test/kotlin/DatabaseTest.kt
index 23423eb5..93196193 100644
--- a/bank/src/test/kotlin/DatabaseTest.kt
+++ b/bank/src/test/kotlin/DatabaseTest.kt
@@ -28,6 +28,7 @@ import java.util.UUID
 import kotlin.experimental.inv
 import kotlin.test.assertEquals
 import kotlin.test.assertNotEquals
+import kotlin.test.assertNotNull
 import kotlin.test.assertTrue
 
 // Foo pays Bar with custom subject.
@@ -486,9 +487,9 @@ class DatabaseTest {
             "+99",
             "foo@example.com",
             true
-        ).apply { assertEquals(this, 
AccountReconfigDBResult.CUSTOMER_NOT_FOUND) }
+        ).apply { assertEquals(AccountReconfigDBResult.CUSTOMER_NOT_FOUND, 
this) }
         // creating the customer
-        assertNotEquals(db.customerCreate(customerFoo), null)
+        assertNotNull(db.customerCreate(customerFoo))
 
         // asserting for the bank account not being found.
         db.accountReconfig(
@@ -498,7 +499,7 @@ class DatabaseTest {
             "+99",
             "foo@example.com",
             true
-        ).apply { assertEquals(this, 
AccountReconfigDBResult.BANK_ACCOUNT_NOT_FOUND) }
+        ).apply { assertEquals(AccountReconfigDBResult.BANK_ACCOUNT_NOT_FOUND, 
this) }
         // Giving foo a bank account
         assert(db.bankAccountCreate(bankAccountFoo) != null)
         // asserting for success.
@@ -512,16 +513,40 @@ class DatabaseTest {
         ).apply { assertEquals(this, AccountReconfigDBResult.SUCCESS) }
         // Getting the updated account from the database and checking values.
         db.customerGetFromLogin("foo").apply {
-            assertNotEquals(this, null)
-            assert((this!!.login == "foo") &&
+            assertNotNull(this)
+            assert((this.login == "foo") &&
                     (this.name == "Bar") &&
                     (this.cashoutPayto) == "payto://cashout" &&
                     (this.email) == "foo@example.com" &&
                     this.phone == "+99"
             )
             db.bankAccountGetFromOwnerId(this.expectRowId()).apply {
-                assertNotEquals(this, null)
-                assertTrue(this!!.isTalerExchange)
+                assertNotNull(this)
+                assertTrue(this.isTalerExchange)
+            }
+        }
+        // Testing the null cases.
+        // Sets everything to null, leaving name and the Taler exchange flag 
untouched.
+        assertEquals(db.accountReconfig(
+            "foo",
+            null,
+            null,
+            null,
+            null,
+            null),
+            AccountReconfigDBResult.SUCCESS
+        )
+        db.customerGetFromLogin("foo").apply {
+            assertNotNull(this)
+            assert((this.login == "foo") &&
+                    (this.name == "Bar") &&
+                    (this.cashoutPayto) == null &&
+                    (this.email) == null &&
+                    this.phone == null
+            )
+            db.bankAccountGetFromOwnerId(this.expectRowId()).apply {
+                assertNotNull(this)
+                assertTrue(this.isTalerExchange)
             }
         }
     }
@@ -529,4 +554,3 @@ class DatabaseTest {
 
 
 
-
diff --git a/bank/src/test/kotlin/LibeuFinApiTest.kt 
b/bank/src/test/kotlin/LibeuFinApiTest.kt
index 9984a72c..229414d2 100644
--- a/bank/src/test/kotlin/LibeuFinApiTest.kt
+++ b/bank/src/test/kotlin/LibeuFinApiTest.kt
@@ -17,6 +17,9 @@ import java.time.Duration
 import java.time.Instant
 import java.time.temporal.ChronoUnit
 import kotlin.random.Random
+import kotlin.test.assertEquals
+import kotlin.test.assertNotEquals
+import kotlin.test.assertNotNull
 
 class LibeuFinApiTest {
     private val customerFoo = Customer(
@@ -707,6 +710,66 @@ class LibeuFinApiTest {
         }
     }
 
+    /**
+     * Tests reconfiguration of account data.
+     */
+    @Test
+    fun accountReconfig() {
+        val db = initDb()
+        val ctx = getTestContext()
+        testApplication {
+            application {
+                corebankWebApp(db, ctx)
+            }
+            assertNotNull(db.customerCreate(customerFoo))
+            val validReq = json {
+                put("is_exchange", true)
+            }
+            // First call expects 500, because foo lacks a bank account
+            client.patch("/accounts/foo") {
+                basicAuth("foo", "pw")
+                jsonBody(validReq)
+            }.assertStatus(HttpStatusCode.InternalServerError)
+            // Creating foo's bank account.
+            assertNotNull(db.bankAccountCreate(genBankAccount(1L)))
+            // Successful attempt now.
+            client.patch("/accounts/foo") {
+                basicAuth("foo", "pw")
+                jsonBody(AccountReconfiguration(
+                    cashout_address = "payto://new-cashout-address",
+                    challenge_contact_data = ChallengeContactData(
+                        email = "new@example.com",
+                        phone = "+987"
+                    ),
+                    is_exchange = true,
+                    name = null
+                ))
+            }.assertStatus(HttpStatusCode.NoContent)
+            // Checking ordinary user doesn't get to patch their name.
+            client.patch("/accounts/foo") {
+                basicAuth("foo", "pw")
+                jsonBody(json {
+                    put("name", "Another Foo")
+                })
+            }.assertStatus(HttpStatusCode.Forbidden)
+            // Finally checking that admin does get to patch foo's name.
+            assertNotNull(db.customerCreate(Customer(
+                login = "admin",
+                passwordHash = CryptoUtil.hashpw("secret"),
+                name = "CFO"
+            )))
+            client.patch("/accounts/foo") {
+                basicAuth("admin", "secret")
+                jsonBody(json {
+                    put("name", "Another Foo")
+                })
+            }.assertStatus(HttpStatusCode.NoContent)
+            val fooFromDb = db.customerGetFromLogin("foo")
+            assertNotNull(fooFromDb)
+            assertEquals("Another Foo", fooFromDb.name)
+        }
+    }
+
     /**
      * Tests the GET /accounts endpoint.
      */
diff --git a/database-versioning/procedures.sql 
b/database-versioning/procedures.sql
index 802ca7f4..cdf875aa 100644
--- a/database-versioning/procedures.sql
+++ b/database-versioning/procedures.sql
@@ -113,25 +113,34 @@ THEN
 END IF;
 out_nx_customer=FALSE;
 
-UPDATE bank_accounts
-  SET is_taler_exchange = in_is_taler_exchange
-  WHERE owning_customer_id = my_customer_id;
-IF NOT FOUND
+-- optionally updating the Taler exchange flag
+IF in_is_taler_exchange IS NOT NULL
+THEN
+  UPDATE bank_accounts
+    SET is_taler_exchange = in_is_taler_exchange
+    WHERE owning_customer_id = my_customer_id;
+END IF;
+IF in_is_taler_exchange IS NOT NULL AND NOT FOUND
 THEN
   out_nx_bank_account=TRUE;
   RETURN;
 END IF;
 out_nx_bank_account=FALSE;
+
 -- bank account patching worked, custom must as well
 -- since this runs in a DB transaction and the customer
 -- was found earlier in this function.
 UPDATE customers
 SET
-  name=in_name,
   cashout_payto=in_cashout_payto,
   phone=in_phone,
   email=in_email
 WHERE customer_id = my_customer_id;
+-- optionally updating the name
+IF in_name IS NOT NULL
+THEN
+  UPDATE customers SET name=in_name WHERE customer_id = my_customer_id;
+END IF;
 END $$;
 
 COMMENT ON FUNCTION account_reconfig(TEXT, TEXT, TEXT, TEXT, TEXT, BOOLEAN)

-- 
To stop receiving notification emails like this one, please contact
gnunet@gnunet.org.