diff --git a/CMakeLists.txt b/CMakeLists.txt index e7b7fe9..aceb432 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -320,6 +320,7 @@ if(TRUE) # tests always build; DuckDB is in-tree test/test_telemetry.cpp test/test_abap_codegen.cpp test/test_cli_common.cpp + test/test_cmd_queue.cpp test/test_table_render.cpp test/test_sap_setup.cpp test/bench_ingest.cpp diff --git a/README.md b/README.md index ea3a3d2..75dc0ce 100644 --- a/README.md +++ b/README.md @@ -161,9 +161,13 @@ Add `--print-abap` to any of them to see the ABAP instead of running it, and `--dry-run` to see the plan. Nothing writes to SAP or DuckDB without a terminal confirmation or an explicit `--yes`. -> These commands deploy a short-lived class to `$TMP` to carry their parameters, -> because an ADT classrun takes none. That needs **`S_DEVELOP`** — a stronger -> authorisation than running the equivalent report in SE38. +> Parameters reach SAP as *data*: the CLI writes the command into a DuckDB table +> and the pre-deployed `ZCL_ERPL_REV_CLIDRV` executes it, so these commands need +> **no SAP authorisation** and create nothing. `--queue-only` does not contact +> SAP at all — the periodic `ERPL_REV_DELTA` job picks the command up. +> +> Where the driver is not deployed they fall back to generating a temporary +> class, which does need `S_DEVELOP`. `erpl-rev doctor` reports which applies. ## Then set up the SAP side diff --git a/abap/z_erpl_rev_delta.prog.abap b/abap/z_erpl_rev_delta.prog.abap index 330651b..8b29fed 100644 --- a/abap/z_erpl_rev_delta.prog.abap +++ b/abap/z_erpl_rev_delta.prog.abap @@ -103,6 +103,12 @@ START-OF-SELECTION. DATA lt_run TYPE zcl_erpl_rev_delta=>tt_run. + " Drain anything the CLI queued before running the due targets. This is what + " lets `erpl-rev sync`/`replicate` work for a caller with no SAP + " authorisation at all: the CLI writes a row into a local DuckDB table and + " this job -- already running on a schedule -- picks it up. See issue #85. + PERFORM drain_cli. + IF p_tgt IS NOT INITIAL. APPEND zcl_erpl_rev_delta=>run( p_tgt ) TO lt_run. PERFORM show USING lt_run. @@ -123,6 +129,17 @@ START-OF-SELECTION. PERFORM show USING lt_run. ENDIF. +*&---------------------------------------------------------------------* +*& Run whatever the CLI queued, and report it in the job log. +*&---------------------------------------------------------------------* +FORM drain_cli. + DATA(lt_cmd) = zcl_erpl_rev_clidrv=>drain( ). + LOOP AT lt_cmd INTO DATA(ls_cmd). + WRITE: / |cli { ls_cmd-cmd_id } { ls_cmd-verb } { ls_cmd-status } | && + |{ ls_cmd-result }{ ls_cmd-error }|. + ENDLOOP. +ENDFORM. + *&---------------------------------------------------------------------* *& Show one tick's results as a compact, coloured table. *&---------------------------------------------------------------------* diff --git a/abap/zcl_erpl_rev_clidrv.abap b/abap/zcl_erpl_rev_clidrv.abap new file mode 100644 index 0000000..b16a6b7 --- /dev/null +++ b/abap/zcl_erpl_rev_clidrv.abap @@ -0,0 +1,281 @@ +"!

erpl-rev CLI command driver

+"! +"! Executes commands the CLI queued in the DuckDB table `_erpl_rev_cli_cmd`. +"! +"! Why this exists: `erpl-adt object run` executes a class that takes no +"! parameters, so the first version of the CLI generated a class per command +"! with the parameters written into its source. That works, but it needs +"! S_DEVELOP -- a developer authorisation the erpl-rev service user does not +"! have on a production system -- and it makes every value the user types into +"! ABAP source, which is an injection surface that has to be defended. +"! +"! Here the parameters arrive as *data*, in a JSON column, and are only ever +"! read as values. Nothing is generated, nothing is created, nothing is deleted. +"! +"! Two things can drive it, and both end up here: +"! - `object run ZCL_ERPL_REV_CLIDRV`, when the caller may run a classrun; +"! - the periodic Z_ERPL_REV_DELTA heartbeat, which drains the queue on each +"! tick -- needing no ADT call, and so no SAP authorisation, from the CLI. +CLASS zcl_erpl_rev_clidrv DEFINITION PUBLIC FINAL CREATE PUBLIC. + PUBLIC SECTION. + INTERFACES if_oo_adt_classrun. + + TYPES: BEGIN OF ty_done, + cmd_id TYPE string, + verb TYPE string, + status TYPE string, + result TYPE string, + error TYPE string, + END OF ty_done. + TYPES tt_done TYPE STANDARD TABLE OF ty_done WITH EMPTY KEY. + + "! Run every command currently pending, oldest first. Returns one row per + "! command executed; an empty table means the queue was empty. + "! `iv_max` bounds one drain so a backlog cannot monopolise a job step. + CLASS-METHODS drain + IMPORTING iv_max TYPE i DEFAULT 20 + RETURNING VALUE(rt) TYPE tt_done. + + PRIVATE SECTION. + "! Claim the oldest pending command, returning its fields as one JSON row. + CLASS-METHODS claim + RETURNING VALUE(rs) TYPE zcl_erpl_rev_util=>ty_query. + CLASS-METHODS execute + IMPORTING iv_verb TYPE string + iv_params TYPE string + EXPORTING ev_result TYPE string + ev_error TYPE string. + CLASS-METHODS finish + IMPORTING iv_id TYPE string + iv_result TYPE string + iv_error TYPE string. + "! Read one string out of a flat JSON object. Values are data, never source. + CLASS-METHODS jstr + IMPORTING iv_json TYPE string + iv_key TYPE string + RETURNING VALUE(rv) TYPE string. + CLASS-METHODS jint + IMPORTING iv_json TYPE string + iv_key TYPE string + iv_def TYPE i DEFAULT 0 + RETURNING VALUE(rv) TYPE i. + "! Escape a value for a SQL string literal (doubling the apostrophe). Only + "! ever applied to values we are writing *back*, never to user parameters + "! on the way in -- those are read, not concatenated. + CLASS-METHODS q + IMPORTING iv_in TYPE string + RETURNING VALUE(rv) TYPE string. +ENDCLASS. + +CLASS zcl_erpl_rev_clidrv IMPLEMENTATION. + + METHOD if_oo_adt_classrun~main. + DATA(lt) = drain( ). + out->write( |ERPL-DRV count={ lines( lt ) }| ). + LOOP AT lt INTO DATA(ls). + out->write( |ERPL-DRV id={ ls-cmd_id };verb={ ls-verb };status={ ls-status }| && + |;result={ ls-result };error={ ls-error }| ). + ENDLOOP. + ENDMETHOD. + + METHOD drain. + " One command per iteration. zcl_erpl_rev_util=>query returns its rows as a + " JSON array *string*, so claiming one at a time avoids parsing an array -- + " and it means a command that dumps cannot take the rest of the batch with + " it, since each claim is its own statement. + DO iv_max TIMES. + DATA(ls_q) = claim( ). + IF ls_q-error IS NOT INITIAL. + APPEND VALUE #( status = 'ERROR' error = ls_q-error ) TO rt. + RETURN. + ENDIF. + + DATA(lv_id) = jstr( iv_json = ls_q-rows iv_key = 'cmd_id' ). + IF lv_id IS INITIAL. + RETURN. " queue empty + ENDIF. + + DATA(lv_verb) = jstr( iv_json = ls_q-rows iv_key = 'verb' ). + DATA(lv_par) = jstr( iv_json = ls_q-rows iv_key = 'params' ). + + execute( EXPORTING iv_verb = lv_verb iv_params = lv_par + IMPORTING ev_result = DATA(lv_res) ev_error = DATA(lv_err) ). + finish( iv_id = lv_id iv_result = lv_res iv_error = lv_err ). + + APPEND VALUE #( cmd_id = lv_id verb = lv_verb + status = COND string( WHEN lv_err IS INITIAL THEN 'DONE' ELSE 'ERROR' ) + result = lv_res error = lv_err ) TO rt. + ENDDO. + ENDMETHOD. + + METHOD claim. + " Claim and read in one statement: RETURNING hands back the row we just + " marked, so two drivers racing cannot both take the same command. + rs = zcl_erpl_rev_util=>query( + |UPDATE _erpl_rev_cli_cmd SET status = 'RUNNING', claimed_ts = now() | && + |WHERE cmd_id = ( SELECT cmd_id FROM _erpl_rev_cli_cmd | && + | WHERE status = 'PENDING' ORDER BY cmd_id LIMIT 1 ) | && + |RETURNING cmd_id, verb, params| ). + ENDMETHOD. + + METHOD execute. + CLEAR: ev_result, ev_error. + + CASE iv_verb. + WHEN 'replicate'. + DATA(ls_r) = zcl_erpl_rev_util=>replicate( + iv_tab = jstr( iv_json = iv_params iv_key = 'table' ) + iv_target = jstr( iv_json = iv_params iv_key = 'target' ) + iv_columns = jstr( iv_json = iv_params iv_key = 'columns' ) + iv_where = jstr( iv_json = iv_params iv_key = 'where' ) + iv_params = jstr( iv_json = iv_params iv_key = 'cds_params' ) + iv_init = jstr( iv_json = iv_params iv_key = 'init' ) + iv_mode = COND string( WHEN jstr( iv_json = iv_params iv_key = 'mode' ) IS INITIAL + THEN 'UPSERT' ELSE jstr( iv_json = iv_params iv_key = 'mode' ) ) + iv_batch = COND i( WHEN jint( iv_json = iv_params iv_key = 'batch' ) > 0 + THEN jint( iv_json = iv_params iv_key = 'batch' ) ELSE 50000 ) + iv_maxrows = jint( iv_json = iv_params iv_key = 'maxrows' ) + iv_truncate = COND abap_bool( WHEN jstr( iv_json = iv_params iv_key = 'truncate' ) = 'false' + THEN abap_false ELSE abap_true ) ). + ev_error = ls_r-error. + ev_result = |rows={ ls_r-rows_affected };seconds={ ls_r-seconds }|. + + WHEN 'sync_register'. + ev_error = zcl_erpl_rev_delta=>register( VALUE #( + target = jstr( iv_json = iv_params iv_key = 'target' ) + method = jstr( iv_json = iv_params iv_key = 'method' ) + source_from = jstr( iv_json = iv_params iv_key = 'source_from' ) + keys = jstr( iv_json = iv_params iv_key = 'keys' ) + chg_col = jstr( iv_json = iv_params iv_key = 'chg_col' ) + wm_kind = jstr( iv_json = iv_params iv_key = 'wm_kind' ) + wm_value = jstr( iv_json = iv_params iv_key = 'wm_value' ) + safety_secs = COND i( WHEN jint( iv_json = iv_params iv_key = 'safety_secs' ) > 0 + THEN jint( iv_json = iv_params iv_key = 'safety_secs' ) ELSE 120 ) + cadence = jstr( iv_json = iv_params iv_key = 'cadence' ) + extra = jstr( iv_json = iv_params iv_key = 'extra' ) ) ). + IF ev_error IS INITIAL. + ev_result = |registered { jstr( iv_json = iv_params iv_key = 'target' ) }|. + ENDIF. + + WHEN 'sync_run'. + DATA(lv_tgt) = jstr( iv_json = iv_params iv_key = 'target' ). + DATA lt_run TYPE zcl_erpl_rev_delta=>tt_run. + IF lv_tgt IS INITIAL. + lt_run = zcl_erpl_rev_delta=>run_due( ). + ELSE. + APPEND zcl_erpl_rev_delta=>run( lv_tgt ) TO lt_run. + ENDIF. + LOOP AT lt_run INTO DATA(ls_run). + ev_result = |{ ev_result }{ ls_run-target }:rows={ ls_run-rows },| && + |ins={ ls_run-ins },upd={ ls_run-upd },del={ ls_run-del };|. + IF ls_run-error IS NOT INITIAL. + ev_error = |{ ev_error }{ ls_run-target }: { ls_run-error }; |. + ENDIF. + ENDLOOP. + IF lt_run IS INITIAL. + ev_result = 'nothing due'. + ENDIF. + + WHEN 'schedule'. + ev_result = zcl_erpl_rev_delta=>schedule( + iv_minutes = COND i( WHEN jint( iv_json = iv_params iv_key = 'minutes' ) > 0 + THEN jint( iv_json = iv_params iv_key = 'minutes' ) ELSE 1 ) + iv_remove = COND abap_bool( WHEN jstr( iv_json = iv_params iv_key = 'remove' ) = 'true' + THEN abap_true ELSE abap_false ) ). + IF ev_result CS 'ERROR:'. + ev_error = ev_result. + ENDIF. + + WHEN OTHERS. + ev_error = |unknown verb '{ iv_verb }'|. + ENDCASE. + ENDMETHOD. + + METHOD finish. + " Newlines would break the one-line result contract the CLI parses. + DATA(lv_res) = replace( val = iv_result sub = cl_abap_char_utilities=>newline + with = ` ` occ = 0 ). + DATA(lv_err) = replace( val = iv_error sub = cl_abap_char_utilities=>newline + with = ` ` occ = 0 ). + zcl_erpl_rev_util=>query( + |UPDATE _erpl_rev_cli_cmd SET | && + |status = '{ COND string( WHEN lv_err IS INITIAL THEN 'DONE' ELSE 'ERROR' ) }', | && + |finished_ts = now(), result = '{ q( lv_res ) }', error = '{ q( lv_err ) }' | && + |WHERE cmd_id = { iv_id }| ). + ENDMETHOD. + + METHOD jstr. + " A deliberately small reader for the flat JSON the CLI writes: no nesting, + " no arrays. /ui2/cl_json would drag a structure definition per verb into + " this class for no benefit. + DATA(lv_needle) = |"{ iv_key }":|. + DATA(lv_off) = find( val = iv_json sub = lv_needle ). + IF lv_off < 0. + RETURN. + ENDIF. + DATA(lv_p) = lv_off + strlen( lv_needle ). + WHILE lv_p < strlen( iv_json ) AND iv_json+lv_p(1) = ` `. + lv_p = lv_p + 1. + ENDWHILE. + IF lv_p >= strlen( iv_json ). + RETURN. + ENDIF. + IF iv_json+lv_p(1) <> '"'. + " An unquoted scalar: a number, true/false or null. cmd_id arrives this + " way, and reading only quoted values made the driver silently claim a + " command and then decide there was nothing to run. + WHILE lv_p < strlen( iv_json ). + DATA(lv_u) = iv_json+lv_p(1). + IF lv_u = ',' OR lv_u = '}' OR lv_u = ' '. + EXIT. + ENDIF. + rv = rv && lv_u. + lv_p = lv_p + 1. + ENDWHILE. + IF rv = 'null'. + CLEAR rv. + ENDIF. + RETURN. + ENDIF. + lv_p = lv_p + 1. + WHILE lv_p < strlen( iv_json ). + DATA(lv_c) = iv_json+lv_p(1). + IF lv_c = '\'. + lv_p = lv_p + 1. + IF lv_p < strlen( iv_json ). + DATA(lv_e) = iv_json+lv_p(1). + CASE lv_e. + WHEN 'n'. rv = rv && cl_abap_char_utilities=>newline. + WHEN 't'. rv = rv && cl_abap_char_utilities=>horizontal_tab. + WHEN OTHERS. rv = rv && lv_e. + ENDCASE. + lv_p = lv_p + 1. + ENDIF. + CONTINUE. + ENDIF. + IF lv_c = '"'. + EXIT. + ENDIF. + rv = rv && lv_c. + lv_p = lv_p + 1. + ENDWHILE. + ENDMETHOD. + + METHOD jint. + DATA(lv_s) = jstr( iv_json = iv_json iv_key = iv_key ). + IF lv_s IS INITIAL. + rv = iv_def. + RETURN. + ENDIF. + TRY. + rv = CONV i( lv_s ). + CATCH cx_sy_conversion_error. + rv = iv_def. + ENDTRY. + ENDMETHOD. + + METHOD q. + rv = replace( val = iv_in sub = `'` with = `''` occ = 0 ). + ENDMETHOD. + +ENDCLASS. diff --git a/cmake/embed_abap.cmake b/cmake/embed_abap.cmake index d2062c9..f101e3d 100644 --- a/cmake/embed_abap.cmake +++ b/cmake/embed_abap.cmake @@ -23,6 +23,7 @@ set(ERPL_ABAP_ASSETS "zcl_erpl_rev_mkfm.abap|ZCL_ERPL_REV_MKFM|CLAS/OC||create the Z_DUCKDB_* RFC FMs" "zcl_erpl_rev_setup.abap|ZCL_ERPL_REV_SETUP|CLAS/OC||create the registered destination" "zcl_erpl_rev_diag.abap|ZCL_ERPL_REV_DIAG|CLAS/OC||round-trip probe (STFC_CONNECTION)" + "zcl_erpl_rev_clidrv.abap|ZCL_ERPL_REV_CLIDRV|CLAS/OC||CLI command driver (queue in DuckDB)" "z_erpl_rev_repl_worker.prog.abap|Z_ERPL_REV_REPL_WORKER|PROG/P||parallel-replication worker" "z_erpl_rev_replicate.prog.abap|Z_ERPL_REV_REPLICATE|PROG/P||replicate SAP table -> DuckDB" "z_erpl_rev_sql.prog.abap|Z_ERPL_REV_SQL|PROG/P||DuckDB SQL console" diff --git a/docs/INSTALL.md b/docs/INSTALL.md index 0138985..c12f2b5 100644 --- a/docs/INSTALL.md +++ b/docs/INSTALL.md @@ -13,11 +13,14 @@ Two parts: (1) get the **ABAP objects** into the SAP system, (2) install the > ``` > > **`setup` needs `S_DEVELOP`** (OBJTYPE=CLAS, ACTVT 01 and 02): it creates and -> activates ABAP objects, and `sync`/`replicate` generate a temporary class to -> carry their parameters. That is a developer authorisation and is normally -> absent on production. `doctor` checks it and says so; if it is missing, import -> the transport below instead and use `--print-abap` to get ABAP you can run from -> SE38 as someone who has the rights. +> activates ABAP objects. That is a developer authorisation and is normally +> absent on production, so on production import the transport below instead. +> `doctor` checks it and says so. +> +> The `sync` and `replicate` subcommands do **not** need it once setup (or the +> transport) has deployed `ZCL_ERPL_REV_CLIDRV`: they pass their parameters as +> data through a queue the driver reads. `--queue-only` goes further and does not +> contact SAP at all. > > `setup` deploys the production ABAP objects over ADT, creates the function group, > the type-T destination and the eight `Z_DUCKDB_*` modules, and writes diff --git a/docs/security.md b/docs/security.md index 90f527a..e5eba04 100644 --- a/docs/security.md +++ b/docs/security.md @@ -96,12 +96,18 @@ S_RFC: ACTVT=16, RFC_TYPE=FUGR, RFC_NAME=ZERPL_REV - The **RFC service user** the running server connects as needs only `S_RFC` (`ACTVT=16`, `RFC_TYPE=FUGR`, `RFC_NAME=ZERPL_REV`) — the eight `Z_DUCKDB_*` modules and nothing else. It needs **no** developer rights. -- The user who runs **`erpl-rev setup`**, or the `sync`/`replicate` subcommands, - needs **`S_DEVELOP`** (`OBJTYPE=CLAS`, `ACTVT` 01 and 02, plus PROG/INTF/TABL - for the initial deploy), because those create and activate ABAP. This is a - developer authorisation; do not grant it to the service user to make the CLI - work. On a production system, import the transport and drive the reports from - SE38 instead — `--print-abap` prints exactly what to run. +- The user who runs **`erpl-rev setup`** needs **`S_DEVELOP`** (`OBJTYPE=CLAS`, + `ACTVT` 01 and 02, plus PROG/INTF/TABL), because setup creates and activates + ABAP. This is a developer authorisation; do not grant it to the service user. + On a production system, import the transport instead (docs/INSTALL.md) and + never run setup there at all. +- The `sync` and `replicate` subcommands need **no SAP authorisation** once + `ZCL_ERPL_REV_CLIDRV` is deployed. They write the command into a DuckDB table + and the driver executes it, so the parameters travel as data and nothing is + created in SAP. With `--queue-only` the CLI does not contact SAP at all: the + periodic `ERPL_REV_DELTA` job drains the queue. Without the driver they fall + back to generating a temporary class, which does need `S_DEVELOP` — `doctor` + reports which of the two applies. - `erpl-rev doctor` needs neither: it only reads, and it reports whether `S_DEVELOP` is present so the gap is visible before anyone tries to deploy. diff --git a/scripts/e2e.sh b/scripts/e2e.sh index 5837552..c694986 100755 --- a/scripts/e2e.sh +++ b/scripts/e2e.sh @@ -243,8 +243,57 @@ fi $CLI sync run t000_cli --yes >/dev/null 2>&1 || fail "sync run failed" echo " sync create/ls/run, and the granularity gate still applies" -# 7. Nothing may be left behind in the customer's system. -LEFT="$(adt search 'ZCL_ERPL_REV_CLI*' 2>/dev/null | grep -c ZCL_ERPL_REV_CLI || true)" +# 7. The driver: parameters as data, no generated ABAP, no authorisation. +# Deploy it first -- setup ships it, but this suite deploys via deploy-abap.sh. +adt object create --type CLAS/OC --name ZCL_ERPL_REV_CLIDRV --package '$TMP' \ + --description 'erpl-rev CLI command driver' >/dev/null 2>&1 || true +adt source write ZCL_ERPL_REV_CLIDRV --file "$HERE/abap/zcl_erpl_rev_clidrv.abap" --activate \ + 2>&1 | grep -qE "Activated|Nothing to activate" || fail "clidrv did not activate" + +# --queue-only must not touch SAP: run it with the credentials stripped. +QOUT="$(env -u SAP_USER -u SAP_PASSWORD -u ERPL_REV_SAP_PASSWORD \ + $CLI replicate --table T000 --target t000_q --queue-only --yes 2>&1)" +grep -q "Queued as command" <<<"$QOUT" || fail "--queue-only did not queue: $QOUT" +CID="$(sed -n 's/.*Queued as command \([0-9]*\).*/\1/p' <<<"$QOUT" | head -1)" +[ -n "$CID" ] || fail "no command id in: $QOUT" + +# The heartbeat drains it -- the path that needs no SAP rights from the CLI. +cat > /tmp/erpl_e2e_tick_$$.abap <<'ABAPTICK' +CLASS zcl_erpl_e2e_tick DEFINITION PUBLIC FINAL CREATE PUBLIC. + PUBLIC SECTION. + INTERFACES if_oo_adt_classrun. +ENDCLASS. +CLASS zcl_erpl_e2e_tick IMPLEMENTATION. + METHOD if_oo_adt_classrun~main. + " The report writes a list. A classrun has nowhere to put one, so it has to + " be captured to memory or the SUBMIT dumps. + SUBMIT z_erpl_rev_delta WITH p_once = abap_true + EXPORTING LIST TO MEMORY AND RETURN. + DATA lt TYPE STANDARD TABLE OF abaplist. + CALL FUNCTION 'LIST_FROM_MEMORY' TABLES listobject = lt EXCEPTIONS OTHERS = 1. + CALL FUNCTION 'LIST_FREE_MEMORY' EXCEPTIONS OTHERS = 0. + out->write( 'tick' ). + ENDMETHOD. +ENDCLASS. +ABAPTICK +adt object create --type CLAS/OC --name ZCL_ERPL_E2E_TICK --package '$TMP' \ + --description 'e2e heartbeat tick' >/dev/null 2>&1 || true +adt source write ZCL_ERPL_E2E_TICK --file "/tmp/erpl_e2e_tick_$$.abap" --activate >/dev/null 2>&1 +adt object run ZCL_ERPL_E2E_TICK >/dev/null 2>&1 || fail "heartbeat tick failed" +rm -f "/tmp/erpl_e2e_tick_$$.abap" + +ST="$($CLI sql "SELECT status FROM _erpl_rev_cli_cmd WHERE cmd_id = $CID" \ + --format csv --quiet 2>/dev/null | tail -1)" +[ "$ST" = "DONE" ] || fail "queued command $CID is '$ST', not DONE" +QROWS="$($CLI sql "SELECT count(*) AS n FROM t000_q" --format csv --quiet 2>/dev/null | tail -1)" +[ "${QROWS:-0}" -gt 0 ] || fail "the queued replicate loaded no rows" +adt object delete /sap/bc/adt/oo/classes/zcl_erpl_e2e_tick >/dev/null 2>&1 || true +echo " driver: queued with no SAP credentials, heartbeat ran it, $QROWS rows" + +# 8. Nothing may be left behind in the customer's system. +# The trailing underscore matters: ZCL_ERPL_REV_CLIDRV is the permanent driver, +# ZCL_ERPL_REV_CLI_ are the throwaways that must never survive. +LEFT="$(adt search 'ZCL_ERPL_REV_CLI_*' 2>/dev/null | grep -c 'ZCL_ERPL_REV_CLI_' || true)" [ "$LEFT" -eq 0 ] || fail "$LEFT temporary CLI class(es) leaked into SAP" echo " no temporary classes left behind" diff --git a/src/cmd_sql.cpp b/src/cmd_sql.cpp index bc24095..b40cb79 100644 --- a/src/cmd_sql.cpp +++ b/src/cmd_sql.cpp @@ -24,6 +24,7 @@ bool ParseOption(const std::string &key, const std::function &tak else if (key == "--print-abap") { o.print_abap = true; } else if (key == "--keep-generated") { o.keep_generated = true; } else if (key == "--quiet") { o.quiet = true; } + else if (key == "--queue-only") { o.queue_only = true; } else if (key == "--count") { o.count = true; } else if (key == "--limit") { const std::string v = take(); diff --git a/src/cmd_sync.cpp b/src/cmd_sync.cpp index 8c2842d..81c0ac0 100644 --- a/src/cmd_sync.cpp +++ b/src/cmd_sync.cpp @@ -14,9 +14,19 @@ #include "abap_skeletons.hpp" #include "commands.hpp" #include "db_client.hpp" +#include "json_util.hpp" namespace erpl_rev::cmd { +std::string BuildParams(const std::vector> &kv) { + std::string j = "{"; + for (const auto &[k, v] : kv) { + if (j.size() > 1) j += ","; + j += json::QuoteString(k) + ":" + json::QuoteString(v); + } + return j + "}"; +} + namespace { // Ask before changing anything, exactly as setup does. --non-interactive means @@ -34,6 +44,61 @@ int ConsentGate(const Options &o, const std::string &what) { return 2; } +// Queue a command as data and let ZCL_ERPL_REV_CLIDRV run it. +// +// This is the path that needs no developer authorisation: the parameters go +// into a DuckDB table as JSON and are read by a class that is already deployed, +// so nothing is generated, created or deleted in SAP. Falls back to the +// codegen path when the driver is not there (issue #85). +// +// Returns the command id, or 0 if the queue could not be written. +long long QueueCommand(Options &o, const std::string &verb, const std::string ¶ms_json, + std::string &why) { + try { + const auto ep = dbc::Detect(o.db_path, o.quack_url, o.quack_token); + auto db = dbc::Db::Open(ep); + const QueryResult r = db.Query( + "INSERT INTO _erpl_rev_cli_cmd (cmd_id, verb, params) VALUES " + "(nextval('_erpl_rev_cli_seq'), " + dbc::SqlLiteral(verb) + ", " + + dbc::SqlLiteral(params_json) + ") RETURNING cmd_id"); + if (r.rows.empty()) { why = "the command queue accepted no row"; return 0; } + const auto colon = r.rows[0].find(':'); + return colon == std::string::npos ? 0 : std::atoll(r.rows[0].substr(colon + 1).c_str()); + } catch (const std::exception &e) { + why = e.what(); + return 0; + } +} + +// Read a finished command back out of the queue. +bool CommandResult(Options &o, long long id, std::string &status, + std::string &result, std::string &error) { + try { + const auto ep = dbc::Detect(o.db_path, o.quack_url, o.quack_token); + auto db = dbc::Db::Open(ep); + const QueryResult r = db.Query( + "SELECT status, coalesce(result,'') AS result, coalesce(error,'') AS error " + "FROM _erpl_rev_cli_cmd WHERE cmd_id = " + std::to_string(id)); + if (r.rows.empty()) return false; + const std::string &row = r.rows[0]; + auto pick = [&](const char *k) { + const std::string n = std::string("\"") + k + "\":"; + const auto p = row.find(n); + if (p == std::string::npos) return std::string(); + auto q = row.find('"', p + n.size()); + if (q == std::string::npos) return std::string(); + const auto e = row.find('"', q + 1); + return e == std::string::npos ? std::string() : row.substr(q + 1, e - q - 1); + }; + status = pick("status"); + result = pick("result"); + error = pick("error"); + return !status.empty(); + } catch (...) { + return false; + } +} + // Deploy, run, and read back a nonce-tagged result. `out` gets the raw console // output so a caller can show it when the parse finds nothing. int RunGenerated(const Options &o, const std::string &kind, const std::string &source, @@ -75,6 +140,59 @@ std::string Field(const Options &o, const std::string &name, const std::string & return def; } +// Is the pre-deployed driver available? Cheap and cached: one ADT search. +bool DriverAvailable(const Options &o) { + static int cached = -1; + if (cached >= 0) return cached == 1; + auto r = adt::Run(cli::ToAdtConn(o), {"search", "ZCL_ERPL_REV_CLIDRV"}); + cached = (r.ok() && r.output.find("ZCL_ERPL_REV_CLIDRV") != std::string::npos) ? 1 : 0; + return cached == 1; +} + +// Queue a command, then get it executed: ask the driver to run now if we may, +// otherwise leave it for the periodic heartbeat. Reports which happened, +// because "queued, the job will pick it up within a minute" is a different +// answer from "done" and should not be dressed up as one. +int RunViaDriver(Options &o, const std::string &verb, const std::string ¶ms) { + std::string why; + const long long id = QueueCommand(o, verb, params, why); + if (id == 0) { + std::fprintf(stderr, "erpl-rev: could not queue the command: %s\n", why.c_str()); + return 1; + } + + if (o.queue_only) { + // Deliberately no SAP call: this is the path for a caller with no SAP + // authorisation at all. + std::printf("Queued as command %lld. The periodic ERPL_REV_DELTA job will run it.\n" + " erpl-rev sql \"SELECT status, result, error FROM _erpl_rev_cli_cmd " + "WHERE cmd_id = %lld\"\n", id, id); + return 3; // queued; the outcome is not known yet + } + + auto r = adt::RunClass(cli::ToAdtConn(o), "ZCL_ERPL_REV_CLIDRV"); + if (!r.ok()) { + std::printf("Queued as command %lld. Could not run the driver directly\n" + " (%s),\n" + " so the periodic ERPL_REV_DELTA job will pick it up. Watch it with:\n" + " erpl-rev sql \"SELECT * FROM _erpl_rev_cli_cmd WHERE cmd_id = %lld\"\n", + id, r.output.substr(0, 120).c_str(), id); + return 3; // queued, outcome not yet known + } + + std::string status, result, error; + if (!CommandResult(o, id, status, result, error)) { + std::fprintf(stderr, "erpl-rev: command %lld left no result row.\n", id); + return 1; + } + if (status != "DONE") { + std::fprintf(stderr, "erpl-rev: %s\n", error.empty() ? status.c_str() : error.c_str()); + return 1; + } + std::printf("%s\n", result.c_str()); + return 0; +} + } // namespace // --------------------------------------------------------------------------- @@ -152,6 +270,16 @@ static int SyncCreate(Options &o, const std::string &target) { return 2; } + if (!o.print_abap && !o.dry_run && (o.queue_only || DriverAvailable(o))) { + if (const int rc = ConsentGate(o, "Register sync job '" + st.target + "' on " + o.host)) + return rc; + const std::string j = BuildParams({ + {"target", st.target}, {"method", st.method}, {"source_from", st.source_from}, + {"keys", st.keys}, {"chg_col", st.chg_col}, {"wm_kind", st.wm_kind}, + {"wm_value", st.wm_value}, {"cadence", st.cadence}, {"extra", st.extra}, + {"safety_secs", std::to_string(st.safety_secs)}}); + return RunViaDriver(o, "sync_register", j); + } const std::string nonce = abapgen::MakeNonce(); const std::string src = abapgen::RenderSyncRegister(st, nonce); if (o.print_abap) { std::fputs(src.c_str(), stdout); return 0; } @@ -177,6 +305,14 @@ static int SyncCreate(Options &o, const std::string &target) { } static int SyncRun(Options &o, const std::string &target) { + if (!o.print_abap && !o.dry_run && (o.queue_only || DriverAvailable(o))) { + if (const int rc = ConsentGate(o, target.empty() + ? "Run every due sync job on " + o.host + : "Run sync job '" + target + "' on " + o.host)) + return rc; + return RunViaDriver(o, "sync_run", + BuildParams({{"target", target}})); + } const std::string nonce = abapgen::MakeNonce(); const std::string src = abapgen::RenderSyncRun(target, nonce); if (o.print_abap) { std::fputs(src.c_str(), stdout); return 0; } @@ -215,6 +351,14 @@ static int SyncSchedule(Options &o) { } const long long minutes = every.empty() ? 1 : std::atoll(every.c_str()); + if (!o.print_abap && !o.dry_run && (o.queue_only || DriverAvailable(o))) { + if (const int rc = ConsentGate(o, std::string(remove ? "Remove" : "Install") + + " the periodic job on " + o.host)) + return rc; + const std::string j = BuildParams({{"minutes", std::to_string(minutes)}, + {"remove", remove ? "true" : "false"}}); + return RunViaDriver(o, "schedule", j); + } const std::string nonce = abapgen::MakeNonce(); const std::string src = abapgen::RenderSchedule(minutes, remove, nonce); if (o.print_abap) { std::fputs(src.c_str(), stdout); return 0; } @@ -265,7 +409,9 @@ static int SyncSchedule(Options &o) { int RunSync(Options o) { const auto cfg = cli::ReadConfig(); - cli::ResolveConn(o, cfg, !o.non_interactive && cli::IsTty()); + // --queue-only never contacts SAP, so it must not prompt for a password to + // write a row into a local database. + cli::ResolveConn(o, cfg, !o.queue_only && !o.non_interactive && cli::IsTty()); const std::string sub = o.args.empty() ? "" : o.args.front(); const std::string arg = o.args.size() > 1 && o.args[1].rfind("--", 0) != 0 @@ -295,7 +441,7 @@ int RunSync(Options o) { int RunReplicate(Options o) { const auto cfg = cli::ReadConfig(); - cli::ResolveConn(o, cfg, !o.non_interactive && cli::IsTty()); + cli::ResolveConn(o, cfg, !o.queue_only && !o.non_interactive && cli::IsTty()); abapgen::ReplicateParams p; p.table = Field(o, "--table"); @@ -329,6 +475,21 @@ int RunReplicate(Options o) { } try { + if (!o.print_abap && !o.dry_run && (o.queue_only || DriverAvailable(o))) { + if (const int rc = ConsentGate(o, "Load " + p.table + " into " + p.target + + " from " + o.host)) + return rc; + const std::string j = BuildParams({ + {"table", p.table}, {"target", p.target}, {"columns", p.columns}, + {"where", p.where}, {"cds_params", p.cds_params}, {"init", p.init}, + {"mode", p.mode}, {"batch", std::to_string(p.batch)}, + {"maxrows", std::to_string(p.maxrows)}, + {"truncate", p.truncate ? "true" : "false"}}); + // The driver runs replicate synchronously inside the classrun, which + // is fine for the common case; --detach and the background-job path + // remain available through the codegen route for very long loads. + return RunViaDriver(o, "replicate", j); + } const std::string nonce = abapgen::MakeNonce(); const std::string src = abapgen::RenderReplicate(p, nonce); if (o.print_abap) { std::fputs(src.c_str(), stdout); return 0; } diff --git a/src/commands.hpp b/src/commands.hpp index 9f16416..53b7939 100644 --- a/src/commands.hpp +++ b/src/commands.hpp @@ -7,6 +7,7 @@ #include #include +#include #include #include "cli_common.hpp" @@ -29,6 +30,10 @@ struct Options : cli::ConnOptions { bool print_abap = false; bool keep_generated = false; bool quiet = false; + // Queue the command and return without contacting SAP at all. The periodic + // ERPL_REV_DELTA job drains the queue, so this path needs no SAP + // authorisation whatsoever -- not even the right to run a classrun. + bool queue_only = false; // Positional words after the verb, in order. std::vector args; @@ -40,6 +45,11 @@ bool ParseOption(const std::string &key, const std::function &tak void PrintHelp(); // Exit codes: 0 verified success, 1 verified failure, 2 misuse, 3 unknown. +// Build the JSON a queued command carries. Exposed for tests: these values are +// user input, and they travel through a SQL literal and then an ABAP JSON +// reader, so the escaping has to survive both. +std::string BuildParams(const std::vector> &kv); + int RunSql(Options o); int RunSync(Options o); int RunReplicate(Options o); diff --git a/src/duckdb_bridge.cpp b/src/duckdb_bridge.cpp index c0a56c1..94c92b1 100644 --- a/src/duckdb_bridge.cpp +++ b/src/duckdb_bridge.cpp @@ -260,6 +260,21 @@ DuckDbBridge::DuckDbBridge(const std::string &path, const std::string &init_sql) "provisioned_ts TIMESTAMPTZ, seeded_ts TIMESTAMPTZ, last_run_ts TIMESTAMPTZ, error VARCHAR)"); if (cdc->HasError()) throw std::runtime_error("DuckDB cdc-state init failed: " + cdc->GetError()); + // The CLI command queue. The CLI writes a row here and ABAP picks it up, so + // parameters reach SAP as *data* rather than as generated source -- which is + // what lets sync/replicate work for a user with no developer authorisation, + // and removes the ABAP-escaping surface entirely. See issue #85. + auto cmdq = con.Query( + "CREATE SEQUENCE IF NOT EXISTS _erpl_rev_cli_seq START 1;" + "CREATE TABLE IF NOT EXISTS _erpl_rev_cli_cmd (" + "cmd_id BIGINT PRIMARY KEY, created_ts TIMESTAMPTZ DEFAULT now(), " + "verb VARCHAR NOT NULL, params VARCHAR NOT NULL, " + "status VARCHAR DEFAULT 'PENDING', " + "claimed_ts TIMESTAMPTZ, finished_ts TIMESTAMPTZ, " + "result VARCHAR, error VARCHAR)"); + if (cmdq->HasError()) + throw std::runtime_error("DuckDB cli-queue init failed: " + cmdq->GetError()); + // Boot init: explicit init_sql (CLI/--init-file) wins; else env fallback. Runs // INSTALL/LOAD/CREATE SECRET/ATTACH so replication can publish to external // targets (parquet object stores, postgres, ducklake, bigquery, iceberg). diff --git a/test/test_cmd_queue.cpp b/test/test_cmd_queue.cpp new file mode 100644 index 0000000..50e1fb9 --- /dev/null +++ b/test/test_cmd_queue.cpp @@ -0,0 +1,119 @@ +// Tests for the command queue's payload. +// +// A queued command's parameters are user input that travels a long way: into a +// SQL string literal, into a DuckDB VARCHAR, and finally through a hand-written +// JSON reader in ABAP. Anything that survives all three unchanged is safe; +// anything that does not is a value silently becoming a different value, or an +// escape breaking a statement. These pin the first hop and model the last. +#include +#include + +#include "commands.hpp" +#include "db_client.hpp" + +using namespace erpl_rev; +using Catch::Matchers::ContainsSubstring; + +namespace { + +// A faithful model of ZCL_ERPL_REV_CLIDRV=>jstr: find "key":, then read either +// a quoted string with backslash escapes, or a bare scalar up to , } or space. +// Written independently of the C++ builder so a bug in the builder cannot +// cancel itself out here. +std::string AbapJstr(const std::string &json, const std::string &key) { + const std::string needle = "\"" + key + "\":"; + const auto at = json.find(needle); + if (at == std::string::npos) return {}; + size_t p = at + needle.size(); + while (p < json.size() && json[p] == ' ') p++; + if (p >= json.size()) return {}; + + if (json[p] != '"') { + std::string out; + while (p < json.size() && json[p] != ',' && json[p] != '}' && json[p] != ' ') + out += json[p++]; + return out == "null" ? std::string() : out; + } + p++; + std::string out; + while (p < json.size()) { + if (json[p] == '\\' && p + 1 < json.size()) { + const char e = json[p + 1]; + if (e == 'n') out += '\n'; + else if (e == 't') out += '\t'; + else out += e; + p += 2; + continue; + } + if (json[p] == '"') break; + out += json[p++]; + } + return out; +} + +} // namespace + +TEST_CASE("params round-trip through the ABAP reader", "[queue]") { + const std::string j = cmd::BuildParams({{"table", "MARA"}, {"target", "mara"}}); + CHECK(AbapJstr(j, "table") == "MARA"); + CHECK(AbapJstr(j, "target") == "mara"); + CHECK(AbapJstr(j, "absent").empty()); +} + +TEST_CASE("a value with quotes survives the trip", "[queue]") { + // The motivating case: WHERE MANDT = '000'. Apostrophes are ordinary in + // JSON; double quotes are the ones that must be escaped and unescaped. + for (const std::string v : {std::string("MANDT = '000'"), + std::string("name = \"quoted\""), + std::string("path\\with\\backslashes"), + std::string("mixed \"a\" and 'b' and \\c")}) { + const std::string j = cmd::BuildParams({{"where", v}}); + CHECK(AbapJstr(j, "where") == v); + } +} + +TEST_CASE("a value cannot inject another key", "[queue]") { + // If the escaping were wrong, this value would close its own string and + // introduce a "target" the caller never asked for. + const std::string evil = R"(x","target":"evil)"; + const std::string j = cmd::BuildParams({{"where", evil}, {"target", "honest"}}); + CHECK(AbapJstr(j, "where") == evil); + CHECK(AbapJstr(j, "target") == "honest"); +} + +TEST_CASE("the JSON also survives being put in a SQL literal", "[queue]") { + // Second hop: the whole document goes into an INSERT as a string literal. + const std::string j = cmd::BuildParams({{"where", "MANDT = '000'"}}); + const std::string lit = dbc::SqlLiteral(j); + CHECK(lit.front() == '\''); + CHECK(lit.back() == '\''); + // Every apostrophe inside is doubled, so the literal cannot end early. + for (size_t i = 1; i + 1 < lit.size(); i++) { + if (lit[i] == '\'') { + REQUIRE(i + 2 < lit.size()); + CHECK(lit[i + 1] == '\''); + i++; + } + } +} + +TEST_CASE("empty and numeric values are represented, not dropped", "[queue]") { + const std::string j = cmd::BuildParams({{"columns", ""}, {"batch", "50000"}}); + CHECK(AbapJstr(j, "columns").empty()); + // Numbers are written as strings deliberately: the ABAP side converts them, + // and one representation is easier to reason about than two. + CHECK(AbapJstr(j, "batch") == "50000"); +} + +TEST_CASE("an empty parameter set is still valid JSON", "[queue]") { + CHECK(cmd::BuildParams({}) == "{}"); +} + +TEST_CASE("a bare scalar is read the way cmd_id arrives", "[queue]") { + // DuckDB returns cmd_id as a JSON number, not a string. The driver read + // only quoted values at first, so it claimed a command and then decided + // there was nothing to run -- leaving the row stuck in RUNNING. + CHECK(AbapJstr(R"({"cmd_id":42,"verb":"replicate"})", "cmd_id") == "42"); + CHECK(AbapJstr(R"({"cmd_id":42,"verb":"replicate"})", "verb") == "replicate"); + CHECK(AbapJstr(R"({"result":null})", "result").empty()); +}