From c31f08d5b8f4debf968298e9e6b95d2629f42df3 Mon Sep 17 00:00:00 2001 From: filipriec Date: Mon, 16 Jun 2025 14:42:49 +0200 Subject: [PATCH] fixing post with links --- Cargo.lock | 3 + Cargo.toml | 1 + client/Cargo.toml | 1 + client/src/services/grpc_client.rs | 28 ++-- common/Cargo.toml | 2 + common/proto/tables_data.proto | 5 +- common/src/proto/descriptor.bin | Bin 22335 -> 26905 bytes common/src/proto/multieko2.tables_data.rs | 8 +- server/Cargo.toml | 1 + .../tables_data/handlers/post_table_data.rs | 108 +++++++++----- .../tables_data/handlers/put_table_data.rs | 133 +++++++++--------- 11 files changed, 169 insertions(+), 121 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8e39ec1..b2858e2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -449,6 +449,7 @@ dependencies = [ "dotenvy", "lazy_static", "prost", + "prost-types", "ratatui", "serde", "serde_json", @@ -487,6 +488,7 @@ name = "common" version = "0.3.13" dependencies = [ "prost", + "prost-types", "serde", "tantivy", "tonic", @@ -2843,6 +2845,7 @@ dependencies = [ "jsonwebtoken", "lazy_static", "prost", + "prost-types", "regex", "rstest", "rust-stemmers", diff --git a/Cargo.toml b/Cargo.toml index 1a875ed..fcd15d1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ tokio = { version = "1.44.2", features = ["full"] } tonic = "0.13.0" prost = "0.13.5" async-trait = "0.1.88" +prost-types = "0.13.0" # Data Handling & Serialization serde = { version = "1.0.219", features = ["derive"] } diff --git a/client/Cargo.toml b/client/Cargo.toml index 223f580..3fdf47e 100644 --- a/client/Cargo.toml +++ b/client/Cargo.toml @@ -9,6 +9,7 @@ anyhow = "1.0.98" async-trait = "0.1.88" common = { path = "../common" } +prost-types = { workspace = true } crossterm = "0.28.1" dirs = "6.0.0" dotenvy = "0.15.7" diff --git a/client/src/services/grpc_client.rs b/client/src/services/grpc_client.rs index f2e2eea..1f793d0 100644 --- a/client/src/services/grpc_client.rs +++ b/client/src/services/grpc_client.rs @@ -23,8 +23,9 @@ use common::proto::multieko2::tables_data::{ use common::proto::multieko2::search::{ searcher_client::SearcherClient, SearchRequest, SearchResponse, }; -use anyhow::{Context, Result}; // Added Context -use std::collections::HashMap; // NEW +use anyhow::{Context, Result}; +use std::collections::HashMap; +use prost_types::Value; #[derive(Clone)] pub struct GrpcClient { @@ -48,7 +49,6 @@ impl GrpcClient { TableDefinitionClient::new(channel.clone()); let table_script_client = TableScriptClient::new(channel.clone()); let tables_data_client = TablesDataClient::new(channel.clone()); - // NEW: Instantiate the search client let search_client = SearcherClient::new(channel.clone()); Ok(Self { @@ -56,7 +56,7 @@ impl GrpcClient { table_definition_client, table_script_client, tables_data_client, - search_client, // NEW + search_client, }) } @@ -135,7 +135,7 @@ impl GrpcClient { Ok(response.into_inner().count as u64) } - pub async fn get_table_data_by_position( +pub async fn get_table_data_by_position( &mut self, profile_name: String, table_name: String, @@ -155,16 +155,22 @@ impl GrpcClient { Ok(response.into_inner()) } - pub async fn post_table_data( +pub async fn post_table_data( &mut self, profile_name: String, table_name: String, data: HashMap, ) -> Result { + // 2. CONVERT THE HASHMAP + let data: HashMap = data + .into_iter() + .map(|(k, v)| (k, Value::from(v))) + .collect(); + let grpc_request = PostTableDataRequest { profile_name, table_name, - data, + data, // This is now the correct type }; let request = tonic::Request::new(grpc_request); let response = self @@ -182,11 +188,17 @@ impl GrpcClient { id: i64, data: HashMap, ) -> Result { + // 2. CONVERT THE HASHMAP + let data: HashMap = data + .into_iter() + .map(|(k, v)| (k, Value::from(v))) + .collect(); + let grpc_request = PutTableDataRequest { profile_name, table_name, id, - data, + data, // This is now the correct type }; let request = tonic::Request::new(grpc_request); let response = self diff --git a/common/Cargo.toml b/common/Cargo.toml index e5f8b7b..5d19597 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -5,6 +5,8 @@ edition.workspace = true license.workspace = true [dependencies] +prost-types = { workspace = true } + tonic = "0.13.0" prost = "0.13.5" serde = { version = "1.0.219", features = ["derive"] } diff --git a/common/proto/tables_data.proto b/common/proto/tables_data.proto index c0e613c..ccef666 100644 --- a/common/proto/tables_data.proto +++ b/common/proto/tables_data.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package multieko2.tables_data; import "common.proto"; +import "google/protobuf/struct.proto"; service TablesData { rpc PostTableData (PostTableDataRequest) returns (PostTableDataResponse); @@ -16,7 +17,7 @@ service TablesData { message PostTableDataRequest { string profile_name = 1; string table_name = 2; - map data = 3; + map data = 3; } message PostTableDataResponse { @@ -29,7 +30,7 @@ message PutTableDataRequest { string profile_name = 1; string table_name = 2; int64 id = 3; - map data = 4; + map data = 4; } message PutTableDataResponse { diff --git a/common/src/proto/descriptor.bin b/common/src/proto/descriptor.bin index 642c133a0d3393bcb0da768003e0c1190adcabfc..30d062c9ef05d791060c10baba5c5b8fa3f80523 100644 GIT binary patch delta 6741 zcmb7I&2JmW6<_WyDRT5-Yh_z9t&fRp%dt&;M6q2*ZY)#eO4^tv6@J)J12>|`mCT0X z3S3f-iUJGd+M_N_+EWif&;mu#9C9g86zH+1-i!VdJ@i!c_hx3-vR$XC5B9#9_kQzx zAM@VOfBIeQi@(Kw|K)1%?^!mv?|J)eck!_6^}OBw-lE^@_8YwgAzNqUu9UDl`->P$ zRD_0gk0th6ZoBDE#JKVH;PT~#SCki|;=;PnvOB%*aaoz9KV*aJEt9NI{+^u(GAw>n zZ};5^oyP!m;Wenov|E-+EBf8o>>qR%7mMJ*I{kM06A2Ilz{qm?HGm>ig#M;hPLaMe zzRZR?{exiF?J9dB5QEMjma!UXAuO%VzEU!vlp-a$DwSp!+x5JVhY6J=okeR>;fHJp z`UtKIW)?Z;HE8yK#x7qBvS3j1_Bz!Y7Zjg_?acf_mNBha%YPo?!l2=``C5N(&+Ynro=d1} zK5y20^?4< z-vuA;d;M<16)U@~PQ809i1``*yw!Wgy{`Dz>x1Zl*KF;z8g)X*@OszfIQpQ~>$y#i zdwkSt;-mMh-oszyUc2o*M=|q;*J-xMDDcS&KyGgZr0CBLMJ;6Jb8j!C)bN@vKj`~C z_};67D#6xwy(1TpDrSs#yk4u}X0X}vIaMMdL`UL8xT^RBiV$MaXxCc@ZWqD)?<{cI zsc3{O;AOLqU`0gO3nt%#NDwi3jR>Pu5Ny+H^bg!lPehr>FCqmVcDfwZ;&xm0w*Mm9 zB1hO{obnq6G4iVDB8RXf;71F@In2U@48ei zVDE9a)5I!(y2RqZ>!AunRC=g!&_jjtJ!~-GA|bxF*LzN-LQDgc5%&+>29*)aT2x(K zDxi)mCErJtETA;1n@)vSO6%1p*|N>?y;UwfcJg+fuWe(~=DE_=cG=n3tn$rLA#ax} zJX_3TwOB1XYqe^rTw#1RTY>4U*vS^RxgC78RkkY?UMh3v(N@6$9PrE8V%4!L8SWHw zg<9SzZe%zhc(GJvyx=@?s?e*JGUD9V?RaUOKeEfYO~7Q=oPtx`7ALMd)gm3XUMe%5 zrM6b7I@Ma$<{PC_UWB`1mmfR1z^>fmg%V|8 zo!2ThnB=q7tT-DWh#2kq}A9@F$x#){*O?j8`_Gm{d?9x$0>>oQQ%!vR`=0i}psr*|3W_n+TN%=ZRCX zuOnv;$ZZfPMfyn=r=nPJGLq?9}B0ng?i10^4-qY5gHsu@^SYPn4n zc3grai|LlZwF&x4TE?t4!%|F7&d5)>cv)K)i-VRypK|evmJy3F%b-uWcvZ^;Vii#M zR;zQ`tXO20L7!6fn)a4hykZ&jDHq??Fw}AURm-5yZw#iJnq@9)w`1wA489}h?+$wP zA&sgI`azw`he*GRrhow$)VX*FWT?aVXjn8I(2?Z(j`8QuT8(FDIEU0|n#br^m^SV9 zF{gpul(s&u2@S0Sm(yhL*Y{n_s_=Bo7BNZDu>Ahvq1UC^w0jID9W>MclOv)h8tejJ zM-Os8s~;S;F+*A%#{EXObtsrc_!ujny8MGmO)KD7B$r1J*Gfhp`EaR5W(-~gFJU|l%AxZ+MzcC4n4Y4?x zCdl?&C;|4?`AeZ>K$@Omf6-AYG0S{E&>qFo-|BovJQiOml{XxvilR}+Kz6yDXBhf; zS>p+Uf9ds1Uo`zX`}9RGVYf~%6a=Qx@A{ZYvJit_SbvC6okgz&`8r)dmsSHF6L(Ll zQvBS{FtVtSC?77~7acK2kt833_o$AUjB1DZF3KLS1K8F($KeKws!5L~%+~-;xZSRY zX9rr=cV$lnC`_Ips8Sb0jNeZVvst7rmc$KQ)eA=YoWP^EcSIfflq2ZCVl_F$t_5U5 z+0L3%=`>zmr*0}mRJt76iQHz*VTqy>N_J!-l#tu(rAc;OKw>LrPNpYchHQHM-L`uQ zGyoet7AIxJO($=1gX0P{2Gm^ol0rrIvtLJ*i9Ur&pgKV92vi5C9f9fqwE=pA1>`=Y z4Razr^)gpgG_SxKfZZ^Yik$)2jnr5u5zvKkHZLMzT80Cz{AJWr?T=CzxKV6P+#4q% zjN4&25n&U^4hTzRP)-ajLyae1xT$85ixUrjMqFIhmc#80|r`rVmk$NJ0tK&YWeR z!(UUk0`p`1XiFdQo#@A@^7nbSeuy74_<7>>afPUJsQ8N66x}HwgueUczKkqfsdu~e zWAY?G=cU^GI62BzP&PW<#i#mYdY;Ng<}*6)4gHW9(6xpwYVN5XNyz)t!L#8WPI)>i zuN5?r_NVDfyDTAw%De2Vw@!YwdUr1Uo&PH{`_C9Vx8?aK^SSED(u0Sw(#g*rOwVP$ z*Z-lMeEs03iO+R5w$+DyUTgJiBW+hDD4ADiw!a}Aqpg^KcrBQfURUg+**=Q5y|t{!#xBhdCO5|P)zBYYKt0E zDq)2Z%!1UpYoQ3c)bhHr!yeN=R9lo^mN*|uv^PE;HEs+|EQNcp8@jix%+N=MBQxp?!-+|y7>C*L z)JiD9Y}nqW7}EpqtaeVs*bFF=XOokJCm#)?dYWuV0Q#sd8>I~SXiCI@_JB7!aZ#B8 z!_wsx(ZnpUA~OoPrTQ!OU}mAe%G1Cw7}FQjaXN~9OurIJup7H7a*E@q!H;F;1${g^ zE@){&k7i~4{%fC}bfGNeN6FT!gIzHXV5+FO!6v;t!?c}d`jzA>eO z4*~S26b(qgn^M7-SAR+cU#Fv{6%7b5n~uzg{j>@`_CRA=1z*>}envkN2*QwHH>2o7 zg569q>_S9;W_U!gguqqKkQU^OtDHfC8CN-jM4BpRhGkyXu1T*+>$0jiBcN`1MJ3G; zUSCm3g9ObhDrt~_cSR)4G!0^SRpkr<*k4s%L!v#EGe|JIs&ZzK*K@jTp%CJT|D5s~ zBJAeSZ>ahq!EWyS1(7t&a;m4U^d4P&tDD8aEUTNKm|?as~--H&o8V zytpZHhSw7$*xiimi2hBLH0;6drb-&-gJop&3xXj8m}L}uNHEJ_1`&HmFw2}fFWAQ| zb5VOovZo%gn4A&p<6s{cODgz~fVZTA4+)w}D)^9qw}jwR38Q zJ0m3EtxQb|yd-`DX%8e`5?b#kxrlC(;O?8E9h(q9@4k8)LW1IbdK(7F7DV9P$K#ON zVv?w@n(`pDSvQ=RZW~+!Ixs|faHEmteNI`2Yc~!urh(=B&1p?r5D!7oy z|D4JSMBwFAji!i3UL^$r?DCNv(a5W$U=L<_m6Q}E#a3a105e-91rp5cq->^;U}k^w JkJ8S-e*mY}j6ozN++!@~)yN!JvJ8oh-Q4*IXfu`gGC8bH#eDI+n(WJ15ShW%>5JAxW z>=1jeV8ahULO_228@3e^TM+E|6+Gw888085^PYRodydE7{TcuEQ~dMy56!EU>FDCX zt^OSs^G~k7p8k9J(e#gt$J5_GKAe7b`Gxr}p8j?DtKiFjp8k;LzqE_Y+c-L(esgs? z{c$}nZ^T7WBv};uBy+xK7B-7a1N8`|b>}@Z)RkbhGdu|l!#d0EjQ#}RHM&D$i z07PQHakD1M^*iCRY%DJ=gojgs)R$m2H7LQntvYFcp*ye7(f&ZEj*a2DfoW)>)Ev0F z6@zTR)MCEzb}3q^x|ggaw-m}5>3(QrVCILYH`F#DGKXqDM7^P!AB-Al8vwG=OeWor z)O=#dMryv110S2Y&@X^&tlgV3)Ev8_4x$_1xT$Rd%epjU5|*~C-a$OKtUH4wQ+Jkl zyBgiDrk1Q#J-I}tUelccw6SK&YH1*uYq~UudTYA0M66rahXHi!IyEFQ-5G>zU3Zqq z)DshaSr$Mx(WxP16FxVslIsW2O?v&XH0$l1==G|7`ElNHt2C--x?zIdq>&_uFbdK)G%C!N~j|ncHeUM7?dymlNh|y%W7v)yv4*(G$+3 z-mba^}0iN4eHy~vD zdchE~eJ)tKZ+LqU-LJZDkaeJ^*pTiI<+5ddh|@mQ3xmi!)YFEjcc`cBynPTotm-+k z9=Mg@u9JF4HhhjPKzm2F`bj|~9yO~wgeZ5kSbYbak^LXr>dObAJD%ypjpJtZ5hjN2 z7#C%%+}lUddDTWv)+09wZRFC%i7o}8+=-eCk$9p@fhc!Exl;ZNa&hofw*sI$o#~{F zQ(X!%bf>zMT$XaCwgJe_bSV(BGgo~~A!KLW`RCb}UtPbr-Yvdu6#m7t=bwN2^3}5s G8vg^M2Y+w? diff --git a/common/src/proto/multieko2.tables_data.rs b/common/src/proto/multieko2.tables_data.rs index 1ae4d33..3aecea0 100644 --- a/common/src/proto/multieko2.tables_data.rs +++ b/common/src/proto/multieko2.tables_data.rs @@ -5,10 +5,10 @@ pub struct PostTableDataRequest { pub profile_name: ::prost::alloc::string::String, #[prost(string, tag = "2")] pub table_name: ::prost::alloc::string::String, - #[prost(map = "string, string", tag = "3")] + #[prost(map = "string, message", tag = "3")] pub data: ::std::collections::HashMap< ::prost::alloc::string::String, - ::prost::alloc::string::String, + ::prost_types::Value, >, } #[derive(Clone, PartialEq, ::prost::Message)] @@ -28,10 +28,10 @@ pub struct PutTableDataRequest { pub table_name: ::prost::alloc::string::String, #[prost(int64, tag = "3")] pub id: i64, - #[prost(map = "string, string", tag = "4")] + #[prost(map = "string, message", tag = "4")] pub data: ::std::collections::HashMap< ::prost::alloc::string::String, - ::prost::alloc::string::String, + ::prost_types::Value, >, } #[derive(Clone, PartialEq, ::prost::Message)] diff --git a/server/Cargo.toml b/server/Cargo.toml index 56c8f03..235806f 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -10,6 +10,7 @@ search = { path = "../search" } anyhow = { workspace = true } tantivy = { workspace = true } +prost-types = { workspace = true } chrono = { version = "0.4.40", features = ["serde"] } dotenvy = "0.15.7" prost = "0.13.5" diff --git a/server/src/tables_data/handlers/post_table_data.rs b/server/src/tables_data/handlers/post_table_data.rs index cb310d1..af761b2 100644 --- a/server/src/tables_data/handlers/post_table_data.rs +++ b/server/src/tables_data/handlers/post_table_data.rs @@ -8,16 +8,15 @@ use common::proto::multieko2::tables_data::{PostTableDataRequest, PostTableDataR use std::collections::HashMap; use std::sync::Arc; use crate::shared::schema_qualifier::qualify_table_name_for_data; +use prost_types::value::Kind; // NEW: Import the Kind enum use crate::steel::server::execution::{self, Value}; use crate::steel::server::functions::SteelContext; -// Add these imports use crate::indexer::{IndexCommand, IndexCommandData}; use tokio::sync::mpsc; use tracing::error; -// MODIFIED: Function signature now accepts the indexer sender pub async fn post_table_data( db_pool: &PgPool, request: PostTableDataRequest, @@ -25,11 +24,12 @@ pub async fn post_table_data( ) -> Result { let profile_name = request.profile_name; let table_name = request.table_name; - let mut data = HashMap::new(); - for (key, value) in request.data { - data.insert(key, value.trim().to_string()); - } + // REMOVED: The old data conversion loop. We will process request.data directly. + // let mut data = HashMap::new(); + // for (key, value) in request.data { + // data.insert(key, value.trim().to_string()); + // } // Lookup profile let profile = sqlx::query!( @@ -94,13 +94,28 @@ pub async fn post_table_data( // Validate all data columns let user_columns: Vec<&String> = columns.iter().map(|(name, _)| name).collect(); - for key in data.keys() { + for key in request.data.keys() { // CHANGED: Use request.data if !system_columns_set.contains(key.as_str()) && !user_columns.contains(&&key.to_string()) { return Err(Status::invalid_argument(format!("Invalid column: {}", key))); } } + // NEW: Create a string-based map for backwards compatibility with Steel scripts. + let mut string_data_for_scripts = HashMap::new(); + for (key, proto_value) in &request.data { + let str_val = match &proto_value.kind { + Some(Kind::StringValue(s)) => s.clone(), + Some(Kind::NumberValue(n)) => n.to_string(), + Some(Kind::BoolValue(b)) => b.to_string(), + Some(Kind::NullValue(_)) => String::new(), + Some(Kind::StructValue(_)) | Some(Kind::ListValue(_)) | None => { + return Err(Status::invalid_argument(format!("Unsupported type for script validation in column '{}'", key))); + } + }; + string_data_for_scripts.insert(key.clone(), str_val); + } + // Validate Steel scripts let scripts = sqlx::query!( "SELECT target_column, script FROM table_scripts WHERE table_definitions_id = $1", @@ -114,20 +129,20 @@ pub async fn post_table_data( let target_column = script_record.target_column; // Ensure target column exists in submitted data - let user_value = data.get(&target_column) + let user_value = string_data_for_scripts.get(&target_column) // CHANGED: Use the new string map .ok_or_else(|| Status::invalid_argument( format!("Script target column '{}' is required", target_column) ))?; // Create execution context let context = SteelContext { - current_table: table_name.clone(), // Keep base name for scripts + current_table: table_name.clone(), profile_id, - row_data: data.clone(), + row_data: string_data_for_scripts.clone(), // CHANGED: Use the new string map db_pool: Arc::new(db_pool.clone()), }; - // Execute validation script + // ... (rest of script execution is the same) let script_result = execution::execute_script( script_record.script, "STRINGS", @@ -138,7 +153,6 @@ pub async fn post_table_data( format!("Script execution failed for '{}': {}", target_column, e) ))?; - // Validate script output let Value::Strings(mut script_output) = script_result else { return Err(Status::internal("Script must return string values")); }; @@ -160,11 +174,12 @@ pub async fn post_table_data( let mut placeholders = Vec::new(); let mut param_idx = 1; - for (col, value) in data { + // CHANGED: This entire loop is rewritten to handle prost_types::Value + for (col, proto_value) in request.data { let sql_type = if system_columns_set.contains(col.as_str()) { match col.as_str() { "deleted" => "BOOLEAN", - _ if col.ends_with("_id") => "BIGINT", // Handle foreign keys + _ if col.ends_with("_id") => "BIGINT", _ => return Err(Status::invalid_argument("Invalid system column")), } } else { @@ -174,36 +189,55 @@ pub async fn post_table_data( .ok_or_else(|| Status::invalid_argument(format!("Column not found: {}", col)))? }; + let kind = proto_value.kind.ok_or_else(|| { + Status::invalid_argument(format!("Value for column '{}' cannot be null", col)) + })?; + match sql_type { "TEXT" | "VARCHAR(15)" | "VARCHAR(255)" => { - if let Some(max_len) = sql_type.strip_prefix("VARCHAR(") - .and_then(|s| s.strip_suffix(')')) - .and_then(|s| s.parse::().ok()) - { - if value.len() > max_len { - return Err(Status::internal(format!("Value too long for {}", col))); + if let Kind::StringValue(value) = kind { + if let Some(max_len) = sql_type.strip_prefix("VARCHAR(") + .and_then(|s| s.strip_suffix(')')) + .and_then(|s| s.parse::().ok()) + { + if value.len() > max_len { + return Err(Status::internal(format!("Value too long for {}", col))); + } } + params.add(value) + .map_err(|e| Status::invalid_argument(format!("Failed to add text parameter for {}: {}", col, e)))?; + } else { + return Err(Status::invalid_argument(format!("Expected string for column '{}'", col))); } - params.add(value) - .map_err(|e| Status::invalid_argument(format!("Failed to add text parameter for {}: {}", col, e)))?; }, "BOOLEAN" => { - let val = value.parse::() - .map_err(|_| Status::invalid_argument(format!("Invalid boolean for {}", col)))?; - params.add(val) - .map_err(|e| Status::invalid_argument(format!("Failed to add boolean parameter for {}: {}", col, e)))?; + if let Kind::BoolValue(val) = kind { + params.add(val) + .map_err(|e| Status::invalid_argument(format!("Failed to add boolean parameter for {}: {}", col, e)))?; + } else { + return Err(Status::invalid_argument(format!("Expected boolean for column '{}'", col))); + } }, "TIMESTAMPTZ" => { - let dt = DateTime::parse_from_rfc3339(&value) - .map_err(|_| Status::invalid_argument(format!("Invalid timestamp for {}", col)))?; - params.add(dt.with_timezone(&Utc)) - .map_err(|e| Status::invalid_argument(format!("Failed to add timestamp parameter for {}: {}", col, e)))?; + if let Kind::StringValue(value) = kind { + let dt = DateTime::parse_from_rfc3339(&value) + .map_err(|_| Status::invalid_argument(format!("Invalid timestamp for {}", col)))?; + params.add(dt.with_timezone(&Utc)) + .map_err(|e| Status::invalid_argument(format!("Failed to add timestamp parameter for {}: {}", col, e)))?; + } else { + return Err(Status::invalid_argument(format!("Expected ISO 8601 string for column '{}'", col))); + } }, "BIGINT" => { - let val = value.parse::() - .map_err(|_| Status::invalid_argument(format!("Invalid integer for {}", col)))?; - params.add(val) - .map_err(|e| Status::invalid_argument(format!("Failed to add integer parameter for {}: {}", col, e)))?; + if let Kind::NumberValue(val) = kind { + if val.fract() != 0.0 { + return Err(Status::invalid_argument(format!("Expected integer for column '{}', but got a float", col))); + } + params.add(val as i64) + .map_err(|e| Status::invalid_argument(format!("Failed to add integer parameter for {}: {}", col, e)))?; + } else { + return Err(Status::invalid_argument(format!("Expected number for column '{}'", col))); + } }, _ => return Err(Status::invalid_argument(format!("Unsupported type {}", sql_type))), } @@ -227,7 +261,7 @@ pub async fn post_table_data( placeholders.join(", ") ); - // Execute query with enhanced error handling + // ... (rest of the function is unchanged) let result = sqlx::query_scalar_with::<_, i64, _>(&sql, params) .fetch_one(db_pool) .await; @@ -235,7 +269,6 @@ pub async fn post_table_data( let inserted_id = match result { Ok(id) => id, Err(e) => { - // Handle "relation does not exist" error specifically if let Some(db_err) = e.as_database_error() { if db_err.code() == Some(std::borrow::Cow::Borrowed("42P01")) { return Err(Status::internal(format!( @@ -248,15 +281,12 @@ pub async fn post_table_data( } }; - // After a successful insert, send a command to the indexer. let command = IndexCommand::AddOrUpdate(IndexCommandData { table_name: table_name.clone(), row_id: inserted_id, }); if let Err(e) = indexer_tx.send(command).await { - // If sending fails, the DB is updated but the index will be stale. - // This is a critical situation to log and monitor. error!( "CRITICAL: DB insert for table '{}' (id: {}) succeeded but failed to queue for indexing: {}. Search index is now inconsistent.", table_name, inserted_id, e diff --git a/server/src/tables_data/handlers/put_table_data.rs b/server/src/tables_data/handlers/put_table_data.rs index 6eef2d1..8f61338 100644 --- a/server/src/tables_data/handlers/put_table_data.rs +++ b/server/src/tables_data/handlers/put_table_data.rs @@ -4,8 +4,8 @@ use sqlx::{PgPool, Arguments, Postgres}; use sqlx::postgres::PgArguments; use chrono::{DateTime, Utc}; use common::proto::multieko2::tables_data::{PutTableDataRequest, PutTableDataResponse}; -use std::collections::HashMap; -use crate::shared::schema_qualifier::qualify_table_name_for_data; // Import schema qualifier +use crate::shared::schema_qualifier::qualify_table_name_for_data; +use prost_types::value::Kind; pub async fn put_table_data( db_pool: &PgPool, @@ -15,20 +15,9 @@ pub async fn put_table_data( let table_name = request.table_name; let record_id = request.id; - // Preprocess and validate data - let mut processed_data = HashMap::new(); - let mut null_fields = Vec::new(); - - // CORRECTED: Generic handling for all fields. - // Any field with an empty string will be added to the null_fields list. - // The special, hardcoded logic for "firma" has been removed. - for (key, value) in request.data { - let trimmed = value.trim().to_string(); - if trimmed.is_empty() { - null_fields.push(key); - } else { - processed_data.insert(key, trimmed); - } + // If no data is provided to update, it's an invalid request. + if request.data.is_empty() { + return Err(Status::invalid_argument("No fields provided to update.")); } // Lookup profile @@ -70,14 +59,29 @@ pub async fn put_table_data( columns.push((name, sql_type)); } - // CORRECTED: "firma" is not a system column. - // It should be treated as a user-defined column. - let system_columns = ["deleted"]; + // Get all foreign key columns for this table (needed for validation) + let fk_columns = sqlx::query!( + r#"SELECT ltd.table_name + FROM table_definition_links tdl + JOIN table_definitions ltd ON tdl.linked_table_id = ltd.id + WHERE tdl.source_table_id = $1"#, + table_def.id + ) + .fetch_all(db_pool) + .await + .map_err(|e| Status::internal(format!("Foreign key lookup error: {}", e)))?; + + let mut system_columns = vec!["deleted".to_string()]; + for fk in fk_columns { + let base_name = fk.table_name.split_once('_').map_or(fk.table_name.as_str(), |(_, rest)| rest); + system_columns.push(format!("{}_id", base_name)); + } + let system_columns_set: std::collections::HashSet<_> = system_columns.iter().map(|s| s.as_str()).collect(); let user_columns: Vec<&String> = columns.iter().map(|(name, _)| name).collect(); // Validate input columns - for key in processed_data.keys() { - if !system_columns.contains(&key.as_str()) && !user_columns.contains(&key) { + for key in request.data.keys() { + if !system_columns_set.contains(key.as_str()) && !user_columns.contains(&key) { return Err(Status::invalid_argument(format!("Invalid column: {}", key))); } } @@ -87,54 +91,65 @@ pub async fn put_table_data( let mut set_clauses = Vec::new(); let mut param_idx = 1; - // Add data parameters for non-empty fields - for (col, value) in &processed_data { - // CORRECTED: The logic for "firma" is removed from this match. - // It will now fall through to the `else` block and have its type - // correctly looked up from the `columns` vector. - let sql_type = if system_columns.contains(&col.as_str()) { + for (col, proto_value) in request.data { + let sql_type = if system_columns_set.contains(col.as_str()) { match col.as_str() { "deleted" => "BOOLEAN", + _ if col.ends_with("_id") => "BIGINT", _ => return Err(Status::invalid_argument("Invalid system column")), } } else { columns.iter() - .find(|(name, _)| name == col) + .find(|(name, _)| name == &col) .map(|(_, sql_type)| sql_type.as_str()) .ok_or_else(|| Status::invalid_argument(format!("Column not found: {}", col)))? }; + // A provided value cannot be null or empty in a PUT request. + // To clear a field, it should be set to an empty string "" for text, + // or a specific value for other types if needed (though typically not done). + // For now, we reject nulls. + let kind = proto_value.kind.ok_or_else(|| { + Status::invalid_argument(format!("Value for column '{}' cannot be empty in a PUT request. To clear a text field, send an empty string.", col)) + })?; + match sql_type { "TEXT" | "VARCHAR(15)" | "VARCHAR(255)" => { - if let Some(max_len) = sql_type.strip_prefix("VARCHAR(") - .and_then(|s| s.strip_suffix(')')) - .and_then(|s| s.parse::().ok()) - { - if value.len() > max_len { - return Err(Status::internal(format!("Value too long for {}", col))); - } + if let Kind::StringValue(value) = kind { + params.add(value) + .map_err(|e| Status::internal(format!("Failed to add text parameter for {}: {}", col, e)))?; + } else { + return Err(Status::invalid_argument(format!("Expected string for column '{}'", col))); } - params.add(value) - .map_err(|e| Status::internal(format!("Failed to add text parameter for {}: {}", col, e)))?; }, "BOOLEAN" => { - let val = value.parse::() - .map_err(|_| Status::invalid_argument(format!("Invalid boolean for {}", col)))?; - params.add(val) - .map_err(|e| Status::internal(format!("Failed to add boolean parameter for {}: {}", col, e)))?; + if let Kind::BoolValue(val) = kind { + params.add(val) + .map_err(|e| Status::internal(format!("Failed to add boolean parameter for {}: {}", col, e)))?; + } else { + return Err(Status::invalid_argument(format!("Expected boolean for column '{}'", col))); + } }, "TIMESTAMPTZ" => { - let dt = DateTime::parse_from_rfc3339(value) - .map_err(|_| Status::invalid_argument(format!("Invalid timestamp for {}", col)))?; - params.add(dt.with_timezone(&Utc)) - .map_err(|e| Status::internal(format!("Failed to add timestamp parameter for {}: {}", col, e)))?; + if let Kind::StringValue(value) = kind { + let dt = DateTime::parse_from_rfc3339(&value) + .map_err(|_| Status::invalid_argument(format!("Invalid timestamp for {}", col)))?; + params.add(dt.with_timezone(&Utc)) + .map_err(|e| Status::internal(format!("Failed to add timestamp parameter for {}: {}", col, e)))?; + } else { + return Err(Status::invalid_argument(format!("Expected ISO 8601 string for column '{}'", col))); + } }, - // ADDED: BIGINT handling for completeness, if needed for other columns. "BIGINT" => { - let val = value.parse::() - .map_err(|_| Status::invalid_argument(format!("Invalid integer for {}", col)))?; - params.add(val) - .map_err(|e| Status::internal(format!("Failed to add integer parameter for {}: {}", col, e)))?; + if let Kind::NumberValue(val) = kind { + if val.fract() != 0.0 { + return Err(Status::invalid_argument(format!("Expected integer for column '{}', but got a float", col))); + } + params.add(val as i64) + .map_err(|e| Status::internal(format!("Failed to add integer parameter for {}: {}", col, e)))?; + } else { + return Err(Status::invalid_argument(format!("Expected number for column '{}'", col))); + } }, _ => return Err(Status::invalid_argument(format!("Unsupported type {}", sql_type))), } @@ -143,27 +158,10 @@ pub async fn put_table_data( param_idx += 1; } - // Add NULL clauses for empty fields - for field in null_fields { - // Make sure the field is valid - if !system_columns.contains(&field.as_str()) && !user_columns.contains(&&field) { - return Err(Status::invalid_argument(format!("Invalid column to set NULL: {}", field))); - } - set_clauses.push(format!("\"{}\" = NULL", field)); - } - - // Ensure we have at least one field to update - if set_clauses.is_empty() { - return Err(Status::invalid_argument("No valid fields to update")); - } - - // Add ID parameter at the end params.add(record_id) .map_err(|e| Status::internal(format!("Failed to add record_id parameter: {}", e)))?; - // Qualify table name with schema let qualified_table = qualify_table_name_for_data(&table_name)?; - let set_clause = set_clauses.join(", "); let sql = format!( "UPDATE {} SET {} WHERE id = ${} AND deleted = FALSE RETURNING id", @@ -184,7 +182,6 @@ pub async fn put_table_data( }), Ok(None) => Err(Status::not_found("Record not found or already deleted")), Err(e) => { - // Handle "relation does not exist" error specifically if let Some(db_err) = e.as_database_error() { if db_err.code() == Some(std::borrow::Cow::Borrowed("42P01")) { return Err(Status::internal(format!(