commit d5da1b1deacd75db922582b700cf15ae21975ef2
parent 362aba71864f4860a78b5b9dc707cd4ae535ea55
Author: Antoine A <>
Date: Wed, 6 Dec 2023 17:07:02 +0000
Fix PATCH semantic
Diffstat:
8 files changed, 179 insertions(+), 117 deletions(-)
diff --git a/API_CHANGES.md b/API_CHANGES.md
@@ -14,6 +14,7 @@ This files contains all the API changes for the current release:
- GET /accounts: add payto_uri, is_public and is_taler_exchange
- GET /accounts/USERNAME: add is_public and is_taler_exchange
- GET /public-accounts: add is_taler_exchange and rename account_name to username
+- PATCH /accounts: fix PATCH semantic
## bank cli
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt b/bank/src/main/kotlin/tech/libeufin/bank/CoreBankApi.kt
@@ -159,8 +159,8 @@ suspend fun createAccount(db: Database, ctx: BankConfig, req: RegisterAccountReq
val res = db.account.create(
login = req.username,
name = req.name,
- email = contactData?.email,
- phone = contactData?.phone,
+ email = contactData?.email?.get(),
+ phone = contactData?.phone?.get(),
cashoutPayto = req.cashout_payto_uri,
password = req.password,
internalPaytoUri = internalPayto,
@@ -181,10 +181,10 @@ suspend fun patchAccount(db: Database, ctx: BankConfig, req: AccountReconfigurat
return db.account.reconfig(
login = username,
name = req.name,
- cashoutPayto = req.cashout_payto_uri,
- emailAddress = contactData?.email,
+ cashoutPayto = req.cashout_payto_uri,
+ email = contactData?.email ?: Option.None,
+ phone = contactData?.phone ?: Option.None,
isPublic = req.is_public,
- phoneNumber = contactData?.phone,
debtLimit = req.debit_threshold,
isAdmin = isAdmin,
allowEditName = ctx.allowEditName,
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/Main.kt b/bank/src/main/kotlin/tech/libeufin/bank/Main.kt
@@ -135,7 +135,6 @@ fun Application.corebankWebApp(db: Database, ctx: BankConfig) {
@OptIn(ExperimentalSerializationApi::class)
explicitNulls = false
encodeDefaults = true
- prettyPrint = true
ignoreUnknownKeys = true
})
}
@@ -369,10 +368,11 @@ class EditAccount : CliktCommand(
is_taler_exchange = exchange,
is_public = is_public,
contact_data = ChallengeContactData(
- email = email,
- phone = phone,
+ // PATCH semantic, if not given do not change, if empty remove
+ email = if (email == null) Option.None else Option.Some(if (email != "") email else null),
+ phone = if (phone == null) Option.None else Option.Some(if (phone != "") phone else null),
),
- cashout_payto_uri = cashout_payto_uri,
+ cashout_payto_uri = Option.Some(cashout_payto_uri),
debit_threshold = debit_threshold
)
when (patchAccount(db, ctx, req, username, true)) {
@@ -445,8 +445,8 @@ class CreateAccount : CliktCommand(
is_public = is_public,
is_taler_exchange = exchange,
contact_data = ChallengeContactData(
- email = email,
- phone = phone,
+ email = Option.Some(email),
+ phone = Option.Some(phone),
),
cashout_payto_uri = cashout_payto_uri,
internal_payto_uri = internal_payto_uri,
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/TalerMessage.kt b/bank/src/main/kotlin/tech/libeufin/bank/TalerMessage.kt
@@ -21,7 +21,7 @@ package tech.libeufin.bank
import io.ktor.http.*
import io.ktor.server.application.*
-import kotlinx.serialization.Serializable
+import kotlinx.serialization.*
import net.taler.wallet.crypto.Base32Crockford
import net.taler.wallet.crypto.EncodingException
import java.time.Duration
@@ -72,6 +72,45 @@ enum class Timeframe {
year
}
+@Serializable(with = Option.Serializer::class)
+sealed class Option<out T> {
+ object None : Option<Nothing>()
+ data class Some<T>(val value: T) : Option<T>()
+
+ fun get(): T? {
+ return when (this) {
+ Option.None -> null
+ is Option.Some -> this.value
+ }
+ }
+
+ inline fun some(lambda: (T) -> Unit) {
+ if (this is Option.Some) {
+ lambda(value)
+ }
+ }
+
+ fun isSome(): Boolean = this is Option.Some
+
+ @OptIn(ExperimentalSerializationApi::class)
+ internal class Serializer<T> (
+ private val valueSerializer: KSerializer<T>
+ ) : KSerializer<Option<T>> {
+ override val descriptor: SerialDescriptor = valueSerializer.descriptor
+
+ override fun serialize(encoder: Encoder, value: Option<T>) {
+ when (value) {
+ Option.None -> encoder.encodeNull()
+ is Option.Some -> valueSerializer.serialize(encoder, value.value)
+ }
+ }
+
+ override fun deserialize(decoder: Decoder): Option<T> {
+ return Option.Some(valueSerializer.deserialize(decoder))
+ }
+ }
+}
+
/**
* HTTP response type of successful token refresh.
* access_token is the Crockford encoding of the 32 byte
@@ -89,14 +128,14 @@ data class TokenSuccessResponse(
* users, to let them complete cashout operations. */
@Serializable
data class ChallengeContactData(
- val email: String? = null,
- val phone: String? = null
+ val email: Option<String?> = Option.None,
+ val phone: Option<String?> = Option.None
) {
init {
- if (email != null && !EMAIL_PATTERN.matches(email))
+ if (email.get()?.let { !EMAIL_PATTERN.matches(it) } ?: false)
throw badRequest("email contact data '$email' is malformed")
- if (phone != null && !PHONE_PATTERN.matches(phone))
+ if (phone.get()?.let { !PHONE_PATTERN.matches(it) } ?: false)
throw badRequest("phone contact data '$phone' is malformed")
}
companion object {
@@ -133,7 +172,7 @@ data class RegisterAccountResponse(
@Serializable
data class AccountReconfiguration(
val contact_data: ChallengeContactData? = null,
- val cashout_payto_uri: IbanPayTo? = null,
+ val cashout_payto_uri: Option<IbanPayTo?> = Option.None,
val name: String? = null,
val is_public: Boolean? = null,
val debit_threshold: TalerAmount? = null,
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/db/AccountDAO.kt b/bank/src/main/kotlin/tech/libeufin/bank/db/AccountDAO.kt
@@ -206,53 +206,90 @@ class AccountDAO(private val db: Database) {
suspend fun reconfig(
login: String,
name: String?,
- cashoutPayto: IbanPayTo?,
- phoneNumber: String?,
- emailAddress: String?,
+ cashoutPayto: Option<IbanPayTo?>,
+ phone: Option<String?>,
+ email: Option<String?>,
isPublic: Boolean?,
debtLimit: TalerAmount?,
isAdmin: Boolean,
allowEditName: Boolean,
allowEditCashout: Boolean,
- ): AccountPatchResult = db.serializable { conn ->
- println("$name $cashoutPayto $allowEditName $allowEditCashout")
- val stmt = conn.prepareStatement("""
+ ): AccountPatchResult = db.serializable { it.transaction { conn ->
+ val checkName = !isAdmin && !allowEditName && name != null
+ val checkCashout = !isAdmin && !allowEditCashout && cashoutPayto.isSome()
+ val checkDebtLimit = !isAdmin && debtLimit != null
+
+ // Get user ID and check reconfig rights
+ val customer_id = conn.prepareStatement("""
SELECT
- out_not_found,
- out_name_change,
- out_cashout_change,
- out_debt_limit_change
- FROM account_reconfig(?, ?, ?, ?, ?, ?, (?, ?)::taler_amount, ?, ?, ?)
- """)
- stmt.setString(1, login)
- stmt.setString(2, name)
- stmt.setString(3, phoneNumber)
- stmt.setString(4, emailAddress)
- stmt.setString(5, cashoutPayto?.canonical)
- if (isPublic == null)
- stmt.setNull(6, Types.NULL)
- else stmt.setBoolean(6, isPublic)
- if (debtLimit == null) {
- stmt.setNull(7, Types.NULL)
- stmt.setNull(8, Types.NULL)
- } else {
- stmt.setLong(7, debtLimit.value)
- stmt.setInt(8, debtLimit.frac)
- }
- stmt.setBoolean(9, isAdmin)
- stmt.setBoolean(10, allowEditName)
- stmt.setBoolean(11, allowEditCashout)
- stmt.executeQuery().use {
- when {
- !it.next() -> throw internalServerError("accountReconfig() returned nothing")
- it.getBoolean("out_not_found") -> AccountPatchResult.UnknownAccount
- it.getBoolean("out_name_change") -> AccountPatchResult.NonAdminName
- it.getBoolean("out_cashout_change") -> AccountPatchResult.NonAdminCashout
- it.getBoolean("out_debt_limit_change") -> AccountPatchResult.NonAdminDebtLimit
- else -> AccountPatchResult.Success
+ customer_id
+ ,(${ if (checkName) "name != ? " else "false" }) as name_change
+ ,(${ if (checkCashout) "cashout_payto IS DISTINCT FROM ?" else "false" }) as cashout_change
+ ,(${ if (checkDebtLimit) "max_debt != (?, ?)::taler_amount" else "false" }) as debt_limit_change
+ FROM customers
+ JOIN bank_accounts
+ ON customer_id=owning_customer_id
+ WHERE login=?
+ """).run {
+ var idx = 1;
+ if (checkName) {
+ setString(idx, name); idx++
+ }
+ if (checkCashout) {
+ setString(idx, cashoutPayto.get()?.canonical); idx++
+ }
+ if (checkDebtLimit) {
+ setLong(idx, debtLimit!!.value); idx++
+ setInt(idx, debtLimit.frac); idx++
+ }
+ setString(idx, login)
+ executeQuery().use {
+ when {
+ !it.next() -> return@transaction AccountPatchResult.UnknownAccount
+ it.getBoolean("name_change") -> return@transaction AccountPatchResult.NonAdminName
+ it.getBoolean("cashout_change") -> return@transaction AccountPatchResult.NonAdminCashout
+ it.getBoolean("debt_limit_change") -> return@transaction AccountPatchResult.NonAdminDebtLimit
+ else -> it.getLong("customer_id")
+ }
}
}
- }
+
+ // Update bank info
+ conn.dynamicUpdate(
+ "bank_accounts",
+ sequence {
+ if (isPublic != null) yield("is_public=?")
+ if (debtLimit != null) yield("max_debt=(?, ?)::taler_amount")
+ },
+ "WHERE owning_customer_id = ?",
+ sequence {
+ isPublic?.let { yield(it) }
+ debtLimit?.let { yield(it.value); yield(it.frac) }
+ yield(customer_id)
+ }
+ )
+
+ // Update customer info
+ conn.dynamicUpdate(
+ "customers",
+ sequence {
+ cashoutPayto.some { yield("cashout_payto=?") }
+ phone.some { yield("phone=?") }
+ email.some { yield("email=?") }
+ name?.let { yield("name=?") }
+ },
+ "WHERE customer_id = ?",
+ sequence {
+ cashoutPayto.some { yield(it?.canonical) }
+ phone.some { yield(it) }
+ email.some { yield(it) }
+ name?.let { yield(it) }
+ yield(customer_id)
+ }
+ )
+
+ AccountPatchResult.Success
+ }}
/** Result status of customer account auth patch */
@@ -357,8 +394,8 @@ class AccountDAO(private val db: Database) {
AccountData(
name = it.getString("name"),
contact_data = ChallengeContactData(
- email = it.getString("email"),
- phone = it.getString("phone")
+ email = Option.Some(it.getString("email")),
+ phone = Option.Some(it.getString("phone"))
),
cashout_payto_uri = it.getString("cashout_payto"),
payto_uri = it.getString("internal_payto_uri"),
diff --git a/bank/src/test/kotlin/CoreBankApiTest.kt b/bank/src/test/kotlin/CoreBankApiTest.kt
@@ -419,8 +419,22 @@ class CoreBankAccountsApiTest {
client.getA("/accounts/merchant").assertOkJson<AccountData> { obj ->
assertEquals("Roger", obj.name)
assertEquals(cashout.canonical, obj.cashout_payto_uri)
- assertEquals("+99", obj.contact_data?.phone)
- assertEquals("foo@example.com", obj.contact_data?.email)
+ assertEquals("+99", obj.contact_data?.phone?.get())
+ assertEquals("foo@example.com", obj.contact_data?.email?.get())
+ assertEquals(TalerAmount("KUDOS:100"), obj.debit_threshold)
+ assert(obj.is_public)
+ assert(!obj.is_taler_exchange)
+ }
+
+ // Check keep values when there is no changes
+ client.patchA("/accounts/merchant") {
+ json { }
+ }.assertNoContent()
+ client.getA("/accounts/merchant").assertOkJson<AccountData> { obj ->
+ assertEquals("Roger", obj.name)
+ assertEquals(cashout.canonical, obj.cashout_payto_uri)
+ assertEquals("+99", obj.contact_data?.phone?.get())
+ assertEquals("foo@example.com", obj.contact_data?.email?.get())
assertEquals(TalerAmount("KUDOS:100"), obj.debit_threshold)
assert(obj.is_public)
assert(!obj.is_taler_exchange)
@@ -430,6 +444,7 @@ class CoreBankAccountsApiTest {
// Test admin-only account patch
@Test
fun patchRestricted() = bankSetup(conf = "test_restrict.conf") { _ ->
+ // Check restricted
checkAdminOnly(
obj { "name" to "Another Foo" },
TalerErrorCode.BANK_NON_ADMIN_PATCH_LEGAL_NAME
@@ -438,6 +453,16 @@ class CoreBankAccountsApiTest {
obj { "cashout_payto_uri" to genIbanPaytoUri() },
TalerErrorCode.BANK_NON_ADMIN_PATCH_CASHOUT
)
+ // Check idempotent
+ client.getA("/accounts/merchant").assertOkJson<AccountData> { obj ->
+ client.patchA("/accounts/merchant") {
+ json {
+ "name" to obj.name
+ "cashout_payto_uri" to obj.cashout_payto_uri
+ "debit_threshold" to obj.debit_threshold
+ }
+ }.assertNoContent()
+ }
}
// PATCH /accounts/USERNAME/auth
diff --git a/database-versioning/libeufin-bank-procedures.sql b/database-versioning/libeufin-bank-procedures.sql
@@ -117,63 +117,6 @@ END IF;
END $$;
COMMENT ON FUNCTION account_balance_is_sufficient IS 'Check if an account have enough fund to transfer an amount.';
-CREATE OR REPLACE FUNCTION account_reconfig(
- IN in_login TEXT,
- IN in_name TEXT,
- IN in_phone TEXT,
- IN in_email TEXT,
- IN in_cashout_payto TEXT,
- IN in_is_public BOOLEAN,
- IN in_max_debt taler_amount,
- IN in_is_admin BOOLEAN,
- IN in_allow_name BOOLEAN,
- IN in_allow_cashout BOOLEAN,
- OUT out_not_found BOOLEAN,
- OUT out_name_change BOOLEAN,
- OUT out_cashout_change BOOLEAN,
- OUT out_debt_limit_change BOOLEAN
-)
-LANGUAGE plpgsql AS $$
-DECLARE
-my_customer_id INT8;
-BEGIN
-IF (in_max_debt.val IS NULL) THEN
- in_max_debt = NULL;
-END IF;
--- Get user ID and check reconfig rights
-SELECT
- customer_id,
- in_name IS NOT NULL AND name != in_name AND NOT in_is_admin AND NOT in_allow_name,
- cashout_payto IS DISTINCT FROM in_cashout_payto AND NOT in_is_admin AND NOT in_allow_cashout,
- in_max_debt IS NOT NULL AND max_debt != in_max_debt AND NOT in_is_admin
- INTO my_customer_id, out_name_change, out_cashout_change, out_debt_limit_change
- FROM customers
- JOIN bank_accounts
- ON customer_id=owning_customer_id
- WHERE login=in_login;
-IF NOT FOUND THEN
- out_not_found=TRUE;
- RETURN;
-ELSIF out_name_change OR out_cashout_change OR out_debt_limit_change THEN
- RETURN;
-END IF;
-
--- Update bank info
-UPDATE bank_accounts SET
- is_public = COALESCE(in_is_public, is_public),
- max_debt = COALESCE(in_max_debt, max_debt)
-WHERE owning_customer_id = my_customer_id;
--- Update customer info
-UPDATE customers SET
- cashout_payto=in_cashout_payto,
- phone=in_phone,
- email=in_email,
- name = COALESCE(in_name, name)
-WHERE customer_id = my_customer_id;
-END $$;
-COMMENT ON FUNCTION account_reconfig
- IS 'Updates values on customer and bank account rows based on the input data.';
-
CREATE OR REPLACE FUNCTION customer_delete(
IN in_login TEXT,
OUT out_nx_customer BOOLEAN,
diff --git a/util/src/main/kotlin/DB.kt b/util/src/main/kotlin/DB.kt
@@ -187,6 +187,23 @@ fun PreparedStatement.executeProcedureViolation(): Boolean {
}
}
+// TODO comment
+fun PgConnection.dynamicUpdate(
+ table: String,
+ fields: Sequence<String>,
+ filter: String,
+ bind: Sequence<Any?>,
+) {
+ val sql = fields.joinToString()
+ if (sql.isEmpty()) return
+ prepareStatement("UPDATE $table SET $sql $filter").run {
+ for ((idx, value) in bind.withIndex()) {
+ setObject(idx+1, value)
+ }
+ executeUpdate()
+ }
+}
+
/**
* Only runs versioning.sql if the _v schema is not found.
*
