It was declared but never assigned, and this session treats an uninitialized
variable as an error that stops the step before the assertion runs - so the
macro reported FAIL for a response that did carry the abort object.
%if &failfast=1 and "&state" in ("canceled" "failed") fails to evaluate: %EVAL
reports it needs a numeric operand and stops the macro, which aborted every test
that reached the guard even though the child had completed or aborted cleanly.
Two plain comparisons do the same job and evaluate reliably.
dc_assert_abort inferred the outcome from the status row. The assertion should
prove the service rejected its input, so it now reads the child's response and
requires a sasjsAbort object, and compares that object's MSG value with the
message the service is expected to return:
%mpe_assert_abort(desc=An invalid libds aborts the service,
outref=web1, msg=Invalid libds)
Renamed with the mpe_ prefix to match the other macro-level helpers. getdata
Test 4 asserted completion with the abort macro, which was the wrong tool; it
now checks the status row for state=completed directly.
dictionary.libnames lists the DC libref as well, so it was seeded into the
result by the SELECT and then added a second time by the explicit INSERT. The
duplicate row then collides with pk_mpe_datacatalog_libs when the refresh closes
the bitemporal record:
ERROR: Duplicate values not allowed on index pk_mpe_datacatalog_libs
The DC libref is now excluded from the scan, so it arrives once, from the INSERT
that carries its library name and engine.
@sasjs/core 6.1.0 ships outlogds and outstatus, so the temporary copy of
mx_execute is gone and the abort assertions no longer need a platform branch:
- dc_assert_abort reads the status dataset instead of work.results on Viya and
the response text elsewhere - one table, one code path;
- the abort cases call dc_execute with outstatus and failfast=0, so a cancelled
child is reported rather than aborting the test;
- getdata Test 4 loses its %mf_getplatform() gate for the same reason, and reads
the outlib only when the status says the child completed;
- dc_execute passes viyaresult through, which the abort cases ask for.
A full makedata on every sasjs test run is expensive, and it made the suite
depend on state the tests themselves should not be creating. Move it to where
DC already does this: the npm scripts that send deploy/makeData*.json to
services/admin/makedata.
npm run sas719setup deploy, then seed once
npm run sas719test run the suite as often as wanted
KEEPME in the payload keeps makedata on the server, so a reseed does not need a
redeploy first. Viya's makedata gains the same switch, so nextviyamakedata
behaves the same way.
Tests must now leave the estate as they found it - that is the follow-up, and it
is what makes re-running a single test possible.
Tests inferred whether a service call worked by reading an outlib that a failed
call never touched, so assertions ran against the previous call's tables. This
adds dc_execute, which wraps mx_execute and makes each call observable:
- failfast: abort as soon as the child did not complete, with its message, so
the first failure is the real one
- a status row per call (outstatus) so a test can assert the outcome
- the child's log behind the harness debug flag, quiet otherwise
testsetup reseeds the data baseline before the test library is built, makedata
honours a keepme switch and getdata initialises the variables it later tests
with %length, plus desc= values reworded where a comma or apostrophe unbalanced
the parameter.
sasjs/macros/mx_execute.sas is a TEMPORARY copy from sasjs/core#476, which adds
outlogds/outstatus and stops leaving work.results in the caller WORK. Delete it
and bump the dependency once that PR is released.
The four security test programs proved that a bad input aborted the service by
reading work.results, the job table %mv_jobflow writes in the Viya branch of
%mx_execute. The SASjs Server and SAS 9 branches call %ms_testservice instead,
so the table never exists, the data step fails with "File WORK.RESULTS.DATA
does not exist." and the assertion reported (0=1) - a failure that says
nothing about whether the service validated its input.
The platform difference is now in one macro, dc_assert_abort: on Viya it reads
the job state as before, elsewhere it looks for the sasjsAbort block that
mp_abort leaves in the webout %mx_execute returns through outref. The call to
the service stays outside the platform check, so the payloads are submitted on
every platform and the assertion is real rather than a skip.
Verified on SASjs Server (sas.4gl.io, target sas719, appLoc /Public/app/dc719):
the seven abort assertions PASS, i.e. the services do abort on the macro, SQL
injection and path traversal payloads. Negative control: the same program with
a valid libds reports FAIL (0=1), so the assertion can still fail.
sasjs lint: exit 0, no warnings in the five files.
The review read the cap as a session option that is never restored, so every later
data step - including the derived-column merge into work.cols1 - would be cut to
&maxrows observations, and a table wide enough to hit the cell limit would lose
columns rather than rows.
On this head the reset is there, two lines after the data step (`options obs=max;`
at what was line 360), so the leak described is not live. It is worth removing the
dependency on that line though: the cap is now a data set option on the SET that
builds the payload, so there is no session option to leak and no later step can be
cut by it, whether or not a reset is remembered.
The existing cell-limit test asserted the row count only - 199 rows for a
1004-column table - which would have passed with 805 columns missing. It now also
compares the COLS payload against dictionary.columns, so the metadata being whole
is tested rather than assumed.
The other obs pair in this program (`options obs=10000` around the filtered
selection, restored at line 290) is deliberately left alone: it bounds the input
rows that the WHERE clause reads, which a data set option on the SET cannot do.
VAR_LABEL was the one free-text column with no rule at all, in the hook or in the
MPE_VALIDATIONS seed. The suggestion in review was an allowlist, but a label has
no reason to exclude any character - quotes are doubled by %mp_derivedvars before
the attrib statement is built, and the value never reaches a macro context - so
the rule is a denylist of what cannot be a single-line column label:
* a control character, which covers the line breaks and tabs that would carry
through into the generated DATA step and render as nothing in a header
* a label over 256 characters, which the engine truncates silently on a quote
boundary, so the configured value and the applied one differ with nothing
saying so
Everything else passes: any script, and the semicolon, ampersand, percent and
comment markers that VAR_FMT and VAR_LOGIC refuse. Implemented in the hook
(anycntrl, klength), as a HARDREGEX in all three seed writers (migration,
mpe_makedata.sas, the mock's makedata.js) so the grid refuses it before submit,
and in the mock hook for the deployed mock.
SAS: new stagedata.test.4 cases 8-10 - a label with an embedded LF is rejected, a
label of free text is accepted, and 257 characters is rejected.
Verified against the deployed mock: the tab and 257-character cases abort with the
right message, free text and UTF-8 pass, and a 256-character label passes. The
newline case cannot be driven through `sasjs request` - the mock server splits a
jsdata row at an embedded newline, so the row arrives short - hence the SAS test.
Both come up in review every time these lines are read: dqrules is the
request-local array the filter/map builds, and work.derived_vars is written by
%mpe_derived before it can return, empty when the table has no derived
variables. Said where the code is, rather than in a review reply.
has() returned true for a character outside the allowed set, which every call
site read as the opposite; it is hasInvalidChar now.
The cycle rejection was the one message without the MPE_DERIVED: prefix. The
SAS hook produced the same text, so both are prefixed rather than letting the
mock drift away from the message a real deployment shows.
loadfile names the submitted rows 'rows' and never defined jsDataRows, so
eval'd there the hook threw a ReferenceError - which the caller logged and
ignored. The rows are bound under the name the hook reads, and a hook that
throws now rejects the submission instead of being swallowed: a hook whose job
is to reject cannot fail open.
The hook's current-row test also required a TX_TO in the future, while the
read-time engine (mpeDerived) treats a missing or zero TX_TO as current, so a
dependency on an open-ended row validated here and broke at read time. Both the
graph and the inactive set, and the RLS guard, now use the engine's predicate.
The VAR_FMT message named '//*' where the check is for /* and */; it now names
the markers it rejects, in the hook and in its SAS counterpart.
COLS.CLS_RULE is the only thing that marks a column read-only in the grid,
and the client applies it only when CLS_FLAG is set - which column level
security drives. A table with no CLS rules therefore rendered its derived
columns as ordinary editable cells, although the values are computed at read
time and dropped from the submission.
getdata now unions a READONLY dq rule per derived variable, which the client
honours unconditionally, and the mock emits the same rule. The SAS test
asserts the rule; the cypress spec double-clicks a derived cell and asserts the
value is unchanged.
VAR_NM carries CASE=UPCASE and the post edit hook upcases every non-delete
staged row, so a name typed in the grid reaches submit uppercase: a delete plus
a re-add of it is the same raw primary key and mpe_loader's dup check rejects
that at approval. Test 6's comment claimed the opposite, so it now states the
truth - only a stored name that is not uppercase reaches submit as two PKs, ie
a name written outside the editor.
Test 7 covers the other arm of the exclusion in the same check: setting an
inactive row active again while another active row depends on it. That starting
state is also one the hook refuses to create, so the case is defensive; both
tests prove the guard does not block the submission that repairs the data.
The inactive set behind the bad-dependency check admitted every delete-marked
staged name, so a submission that delete-marks one case variant of a variable
and stages the same name again as a live active row (the two are distinct keys
to the loader's case sensitive duplicate check) matched the dep against the
inactive copy and aborted a configuration the engine resolves fine.
Both arms now exclude a name that this submission also stages as a live active
row - the staged one, and the control row it replaces. stagedata.test.4 gets a
Test 6 for that shape: delete 'Base_X', stage 'BASE_X', with a surviving active
row declaring VAR_DEPS=BASE_X, which must stage successfully.
Verified: sasjs lint (no new warnings), sasjs cb -t nextviya with @sasjs/core
6.0.0, and the built output carries the new predicate and the test. No SAS
estate was reachable from this box (nextviya and sas.4gl.io both time out), so
the test has not been executed on a real engine yet.
@sasjs/core 6.0.0 renamed the session globals to a _SASJS_ prefix and sets
_SASJS_WORK only; SASJSWORK no longer exists (and sasjs/server's pre-program
compat aliases never covered it). testsetup.sas still read &sasjswork in both
the SASJS and SASMETA arms of %update_settings, so under the new pin those
filename statements built unrooted paths and ms_createfile / mm_createstp wrote
outside the session work directory, breaking sasjs test setup on SASJS and SAS9.
mp_init() is called at the top of the script, so _SASJS_WORK is defined here.
The inactive set behind the VAR_DEPS check took staged rows that are not
delete-marked with active ne 1, plus control rows with active ne 1. A staged
DELETE of a relied-on row was in neither arm, so it passed every guard and the
engine aborted every read after approval. Delete-marked staged rows are now in
the inactive set, and stagedata.test.4 covers it.
%if %then %let ...; %else %let ...; is not valid in open code (SAS stops with
"Expected %DO not found"), which is why these sites used %do/%end blocks. One
%let with %sysfunc(ifc()) needs no blocks and reads as what it is: a choice
between two dataset names. The droplist block in getrawdata/viewdata keeps its
%if %then %do; there is no %else to pair.
A hook is %included from %mp_include, so its open-code %let runs in that
macro's local scope: a %global for the same name inside the hook raises
"exists in a local environment", sets SYSCC, and puts the session into syntax
check mode - every check after it silently skipped while the step still
aborts. The derived hook now sets its (renamed) control variables in the
including scope and does not %global them.
The RLS hook also referenced rls_msg before assigning it whenever there was no
derived conflict, which produced an unresolved-symbol WARNING and SYSCC=4 on
every clean rule submission. It is initialised now.
Four cases: a dependency on an inactive row aborts, deactivating a relied-on
row aborts, an inactive row does not block an unrelated variable, and a
delete-marked RLS rule naming a derived variable loads.
The graph now holds only effective active rows, and a declared dependency that
resolves to an inactive row (existing, or one this submission deactivates) is
rejected: the engine reads only active rows and aborts every read of the table
on the unresolved reference.
The derived-variable / RLS guard goes back to rejecting every scope. Exempting
VIEW rules left them to fail a moment later in mp_filtercheck, which validates
against the physical table where the column does not exist, so the pairing is
refused up front with a message that says why.
RLS batches now exclude delete-marked rows, matching the badloadtypes check, so
a rule being removed is not validated against the derived configuration.
The dependency graph built at submit time pulled every open current row of
MPE_DERIVED into the union, while the engine only reads active=1 rows. An
inactive row that is still open could therefore abort a submission over a
cycle or a stale reference the engine never generates. The union now filters
active=1, matching mpe_derived.sas.
Both derived-variable guards also matched any active row level security rule,
whatever its scope. A VIEW rule is applied on the view the EDIT screen reads,
where a derived column exists, so naming one there is legitimate; only EDIT and
ALL rules run against the submitted data, where the column is absent. The
derived hook now filters rls_scope in ('EDIT','ALL'), and the RLS hook carries
rls_scope through work.batches to filter its side of the same guard, matching
the staging-side check in the same service.
dctest.wideboy is 1004 columns (row_id, 500 char, 500 numeric, plus the
field and position loop variables), so the Test 2 banner says 200 x 1004 =
200,800 cells, matching the @details block. The row cap is unchanged.
The doxygen list omitted mpeinit and mpeterm, which the service calls - both
are now declared the way the other services that use mpeinit declare it. The
compiler inlines them, as it does for those services. webout stays out of the
list: it is supplied by the framework rather than a macro in the repo, so that
one warning needs an exception in the linter.
dctest.wideboy has 1004 columns, not 1001 - row_id, char1-char500,
num1-num500, plus the loop variables field and position and the scratch
variable alphabet - so the test header's cell arithmetic is 200 x 1004 =
200,800. The 199 row cap is unchanged, floor(200000/1004) is still 199.
The MPE_X_WIDE mock comment said 200,400 cells for 200 x 1001 columns, which
was the old counting that added the client-owned delete column. The mock now
counts the table's own columns, so it is 200,200.
The message is built in three short assignments rather than one 161
character %let, which .sasjslint rejects (maxLineLength 107). The
resulting text is unchanged.
The getdata comment explained the truncation by contrast with the abort it
replaces. State what the code does and why instead. Same for the two
not-refused phrasings in the mock and the test header.
A selection over DC_MAXOBS_WEBEDIT or DC_MAXCELLS_WEBEDIT was refused with
"Table is too big", leaving the user with no grid and no way to see what the
filter had matched. getdata now serves the first rows that fit and flags it:
- the row cap is min(DC_MAXOBS_WEBEDIT, DC_MAXCELLS_WEBEDIT / columns), so a
wide table is still bounded by the cell budget
- sasparams carries TRUNCATED (0/1) and TRUNC_MSG, naming the limit reached
- the EDIT screen shows a warning triangle next to the table name, with the
message in a tooltip and the advice to filter
The mock mirrors the new behaviour, the Cypress spec asserts the truncated
grid and the tooltip (real hover, opacity checked so a hidden tooltip cannot
pass), and the SAS limit test now asserts the served row counts and message
rather than a cancelled child job.
- the join against MPE_DERIVED compared an upcased var_nm with the rule's
column as stored, so a case variant slipped past the explanatory rejection
and fell through to the generic filter check. Both sides are upcased.
- the invariant was enforced only from the RLS side: configuring a derived
variable for a table whose active rule already names it passed the derived
hook. The same check now runs there, against the incoming libref/dsn/var_nm.
A table's metadata columns are named per table - VAR_TXFROM, VAR_TXTO and
VAR_PROCESSED in MPE_TABLES hold those names, and they can be anything (the
bitemporal demo table uses VER_FROM_DTTM and VER_TO_DTTM). When they exist
they are physical columns of the target, so the engine's collision check
already refuses a VAR_NM that matches one, whatever it is called.
The hard-coded list treated three example names as special: it refused them on
tables that use other names, and told the reader something untrue about what
is reserved.
- getdata.test now checks the child job state before reading its outlib: a
cancelled child returns no webout, so the previous call's members were being
asserted against and the failures described the wrong problem.
- validatefilter.test's inputdatasets call needs a _PROGRAM column, which
mx_execute reads to decide what to run - without it the job stopped at
'The _PROGRAM column must exist'.
- stagedata.test.3 covers the new RLS check: an MPE_ROW_LEVEL_SECURITY rule
naming a derived variable must be rejected at save time.
- getrawdata selected its filter target with a bare '%else %let' in open code,
the shape that stops the step with 'Expected %DO not found'. Both branches
are %do/%end now, so the download service applies a filter at all.
- the conditional drop in the approve.jsdset step and the one against
MPSEARCH.vwsearch are dead: work.jsdata and work.vwsearch are already
rewritten without the derived columns, so the drops only warn.
- the post-edit hook checked that a reference is declared, but not that a
declared dependency is referenced. The engine rejects the stale shape at read
time, so the same check now rejects it at submit time.
- a row level security rule naming a derived variable is rejected when the rule
is saved, with a message that says why, instead of failing later at submit.
- getdata.test's cleanup deleted only DERIVED_NOTE, leaving DERIVED_NOTE2 with
a dependency on a variable that no longer exists, which aborted every call in
the following run - the cause of the repeated no-result runs.
Both services selected the filter target with
'%if cond %then %let x=y; %else %let x=z;'. In open code the %else is not
paired with the %then and the whole step stops with 'Expected %DO not found',
so neither service ever applied a filter: every assertion downstream failed and
the job produced no results at all.
Found on a real estate - the pattern works inside a macro definition, which is
why the same construct elsewhere in the codebase is fine.
- mpe_derived_postedit ran %local at the top level of a file that is included
at execution time, so the hook stopped before validating anything and every
MPE_DERIVED submission failed. The macro language now lives in a macro
definition, errflag and errmsg are declared global, and the file calls it.
- the cycle check aborts after leaving its loop, and the self, undeclared and
field aborts are driven by a count at the point of call rather than by a
wrapping %if, so an abort cannot be outlived. The same shapes are fixed
upstream in sasjs/core#468.
- a case-variant duplicate survived mpe_loader, which compares raw values:
the hook now rejects it after normalisation and names the variable.
- stagedata dropped the derived columns from the output of the security step
rather than from its input, so a crafted submission could still satisfy an
RLS clause through a derived column. work.jsdata itself is rewritten now.
- the VAR_FMT validation rule rejected the bare w.d forms that the hook and
the engine accept.
- the migration carried the same four dangling insert headers as mpe_makedata,
so the v7.19 migration failed for existing installs.
- the filter fixtures in getdata.test and validatefilter.test had been caught
by the variable rename: the filter schema column is VARIABLE_NM.
- the TX_FROM/TX_TO definitions agree across the ddl, the migration and
mpe_makedatamodel.
@sasjs/core bumped to 5.7.1, which carries the where= fix.
Four stray insert lines and a missing one left the MPE_VALIDATIONS block for
MPE_DERIVED malformed, so makedata aborted and the control table was never
created - found by building the library on a real estate.
The format assertions in the test now read the emitted statement, because proc
contents on a view does not report a format in full ('DATE' for DATE9.).
mpe_derived carried its own copy of the derived variable engine. That engine
now ships in @sasjs/core 5.7.0 as mp_derivedvars, so the local copy is gone and
mpe_derived is a wrapper that supplies what is Data Controller's own: the
control table, the bitemporal filter that picks the current active row, and the
list of metadata names that may not be used as a variable.
The macro goes from 511 lines to 102. The contract and the generated statement
format are unchanged, so the callers in the six services need no edit.
A test covers the wrapper's own responsibility: a closed version of a row and
an inactive row are both ignored.
A label at the 256 character limit produces an attrib statement of about 300
characters, which the default record length wraps, splitting the statement and
breaking the view.
Same harness fix as the core test: the setup inserts create the SQL automatic
macro variables, so the snapshot has to come after them, and the framework's
_SASJS_WEBOUT_HEADERS is ignored.
A configured label replaces the default NAME=expression; when it is missing or
blank the default is used, so existing rows are unaffected.
- MPE_DERIVED gains VAR_LABEL varchar(256), in the ddl, mpe_makedatamodel and
the v7.19 migration
- mpe_derived uses it, through the same escaping and 256-char truncation as
the default label
- the hook trims it; the mock mirrors the fallback
- test 1 now sets a label on BAND and asserts it
The label had 256 chars of storage, so the doubled quotes were truncated on
assignment; a cut between the halves of an escape leaves an unterminated
literal and the view fails to build. The label now holds 512 chars, quotes
are doubled, then the 256 boundary is applied, retreating one character when
the cut lands on an odd quote count.
Allowing a derived variable to reuse a physical column's name was wrong: an
intra-row calculation (SUMMER = SUMMER + 1) implies a retained value, which
depends on the order rows are read. The same argument rules out a variable
listing itself as a dependency.
Reverted:
- the name collision is an error again ('VAR_NM collides with a column in the
target table')
- VAR_DEPS may not list the variable itself
- the physical column list is back to mf_getvarlist - the type/length check
only existed to support shadowing
- the droplist guards in stagedata / viewdata / getrawdata, and the duplicate
suppression in getdata, are no longer needed
- the docs and both test suites drop the shadowing case
cols1 already holds the physical column, so appending the derived row for the
same name would send it twice. The derived row is now appended only when the
name is new; a shadowed column keeps the physical label and format, while its
value comes from the view.
With shadowing allowed, a derived name can also be a physical column. The
derived droplist was built from every derived name, so such a column would
have been dropped on submission (losing the user's edit) and from the view
and raw downloads (losing a real column).
stagedata, viewdata and getrawdata now subtract the target's own columns
from the droplist, so only genuinely virtual columns are dropped.
SUMMER = SUMMER + 1 is a legitimate definition: the name matches an existing
column, the expression reads the physical value, and the view carries the
derived one under the same name.
- mpe_derived now takes the physical column list from mp_getcols, so it has
type and length in one pass; a shadowed name must match the column's type,
and its length may not exceed it (numeric lengths must be equal), as attrib
cannot change either on an existing variable
- a self reference is only an error when the name is not a physical column
- self references are not graph edges, so the dependency sort is unaffected
- the post edit hook no longer rejects a self reference, since it cannot see
the target library; it still rejects cycles between distinct names
- mpe_derived now uses mf_getuniquename for its working tables, matching the
core macro, and carries the same placement sequence fix
- test 4 covers shadowing, docs cover the rule
Same defect as the core review found in mp_derivedvars: the final assemble
merged the placement table by name, but that table is built in dependency
rounds and is not name ordered, so the BY-merge failed whenever the rounds
were not alphabetical, and seq was a BY-group counter rather than the
dependency order.
Each round now stamps seq=_n_+nplaced, the merge runs on the name-sorted
tables, and the result is sorted by seq. Also guards the physical-column
in-list for a zero-column table, and checks libds exists before running.
The cycle check was already in the post edit hook and in mpe_derived - the
topological sort places a variable only once its derived inputs are placed,
and if a round places nothing the remainder is a cycle. What it reported was
just the set of names that could not be placed, which does not tell the
administrator what to change.
The message now walks the edges from the first unresolved variable and names
the loop, eg:
VAR_DEPENDENCIES contains a cycle: A -> B -> C -> A
Names are collected in a dotted seen list, so a lead-in to the loop is shown
as well and the walk stops at the first repeat.
dc
allan