From 88f6ff0fcc7bc70f69299309dfe8f24e2684392b Mon Sep 17 00:00:00 2001 From: Matthew O'Neill Date: Wed, 18 Mar 2026 12:46:18 +0000 Subject: [PATCH 1/3] update view code params to take constant not constants constants gets passed to the JVM which is then ignored. This then causes any query that uses constants to fail with an 'Unknown variable' error --- lib/R/R/view.R | 8 ++++---- lib/python/pathling/datasource.py | 6 +++--- lib/python/tests/test_view.py | 20 ++++++++++++++++++++ 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/R/R/view.R b/lib/R/R/view.R index 66d9da7d66..4f57e1db85 100644 --- a/lib/R/R/view.R +++ b/lib/R/R/view.R @@ -20,7 +20,7 @@ #' @param ds The DataSource object containing the data to be queried. #' @param resource A string representing the type of FHIR resource that the view is based upon, e.g. 'Patient' or 'Observation'. #' @param select A list of columns and nested selects to include in the view. Each element should be a list with appropriate structure. -#' @param constants An optional list of constants that can be used in FHIRPath expressions. +#' @param constant An optional list of constants that can be used in FHIRPath expressions. #' @param where An optional list of FHIRPath expressions that can be used to filter the view. #' @param json An optional JSON string representing the view definition, as an alternative to providing the parameters as R objects. #' @return A Spark DataFrame containing the results of the view. @@ -66,7 +66,7 @@ #' ) #' ) #' } -ds_view <- function(ds, resource, select = NULL, constants = NULL, where = NULL, json = NULL) { +ds_view <- function(ds, resource, select = NULL, constant = NULL, where = NULL, json = NULL) { jquery <- j_invoke(ds, "view", as.character(resource)) if (!is.null(json)) { @@ -83,8 +83,8 @@ ds_view <- function(ds, resource, select = NULL, constants = NULL, where = NULL, query$select <- select } - if (!is.null(constants)) { - query$constants <- constants + if (!is.null(constant)) { + query$constant <- constant } if (!is.null(where)) { diff --git a/lib/python/pathling/datasource.py b/lib/python/pathling/datasource.py index 795098bedc..0115696132 100644 --- a/lib/python/pathling/datasource.py +++ b/lib/python/pathling/datasource.py @@ -72,7 +72,7 @@ def view( self, resource: Optional[str] = None, select: Optional[Sequence[Dict]] = None, - constants: Optional[Sequence[Dict]] = None, + constant: Optional[Sequence[Dict]] = None, where: Optional[Sequence[Dict]] = None, json: Optional[str] = None, ) -> DataFrame: @@ -82,7 +82,7 @@ def view( :param resource: The FHIR resource that the view is based upon, e.g. 'Patient' or 'Observation'. :param select: A list of columns and nested selects to include in the view. - :param constants: A list of constants that can be used in FHIRPath expressions. + :param constant: A list of constants that can be used in FHIRPath expressions. :param where: A list of FHIRPath expressions that can be used to filter the view. :param json: A JSON string representing the view definition, as an alternative to providing the parameters as Python objects. @@ -96,7 +96,7 @@ def view( args = locals() query = { key: args[key] - for key in ["resource", "select", "constants", "where"] + for key in ["resource", "select", "constant", "where"] if args[key] is not None } query_json = dumps(query) diff --git a/lib/python/tests/test_view.py b/lib/python/tests/test_view.py index 81de389a9f..268374c0df 100644 --- a/lib/python/tests/test_view.py +++ b/lib/python/tests/test_view.py @@ -46,6 +46,26 @@ def test_view_on_ndjson(ndjson_test_data_dir, pathling_ctx): ResultRow("beff242e-580b-47c0-9844-c1a68c36c5bf", "Towne435"), ] +def test_view_with_constants(ndjson_test_data_dir, pathling_ctx): + """Reproduces #2574: view with constants fails to resolve constant references in where clause.""" + + data_source = pathling_ctx.read.ndjson(ndjson_test_data_dir) + result = data_source.view( + resource="Patient", + constant = [{"name": "filter_id", "valueUuid": "8ee183e2-b3c0-4151-be94-b945d6aa8c6d"}], + select=[ + { + "column": [ + {"path": "id", "name": "id"}, + {"value": "name.first().family", "name": "family_name"}, + ] + } + ], + where=[{"path": "id = $filter_id"}], + ) + assert result.collect() == [ + Row("8ee183e2-b3c0-4151-be94-b945d6aa8c6d", "Krajcik437"), + ] ConditionCodingRow = Row("id", "code_text", "system", "code", "display") From 8c9bab786c71c1f2262ff7726b237e1bec846808 Mon Sep 17 00:00:00 2001 From: John Grimes Date: Thu, 19 Mar 2026 14:01:58 +1000 Subject: [PATCH 2/3] fix: Correct Python view constants test and add R equivalent Fix three bugs in test_view_with_constants: use "path" instead of "value" for the column key, use FHIRPath %constant syntax instead of $constant, and use the named ResultRow factory for consistency. Also fix PEP 8 style issues and add a matching R test. --- lib/R/tests/testthat/test-view.R | 32 ++++++++++++++++++++++++++++++++ lib/python/tests/test_view.py | 12 ++++++++---- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/lib/R/tests/testthat/test-view.R b/lib/R/tests/testthat/test-view.R index ccd1be20f9..a0f9e2aabe 100644 --- a/lib/R/tests/testthat/test-view.R +++ b/lib/R/tests/testthat/test-view.R @@ -100,3 +100,35 @@ test_that("test SOF view with components", { expect_equal(colnames(sof_result), colnames(ResultRow)) expect_equal(sof_result %>% sdf_collect(), ResultRow) }) + + +# test SOF view with constants +test_that("test SOF view with constants", { + # expectations + ResultRow <- tibble::tibble( + id = c("8ee183e2-b3c0-4151-be94-b945d6aa8c6d"), + family_name = c("Krajcik437") + ) + + # actuals + sof_result <- ds_view( + test_data_source(), + resource = "Patient", + constant = list( + list(name = "filter_id", valueUuid = "8ee183e2-b3c0-4151-be94-b945d6aa8c6d") + ), + select = list( + list( + column = list( + list(path = "id", name = "id"), + list(path = "name.first().family", name = "family_name") + ) + ) + ), + where = list( + list(path = "id = %filter_id") + ) + ) + expect_equal(colnames(sof_result), colnames(ResultRow)) + expect_equal(sof_result %>% sdf_collect(), ResultRow) +}) diff --git a/lib/python/tests/test_view.py b/lib/python/tests/test_view.py index 268374c0df..90188878a5 100644 --- a/lib/python/tests/test_view.py +++ b/lib/python/tests/test_view.py @@ -46,27 +46,31 @@ def test_view_on_ndjson(ndjson_test_data_dir, pathling_ctx): ResultRow("beff242e-580b-47c0-9844-c1a68c36c5bf", "Towne435"), ] + def test_view_with_constants(ndjson_test_data_dir, pathling_ctx): """Reproduces #2574: view with constants fails to resolve constant references in where clause.""" data_source = pathling_ctx.read.ndjson(ndjson_test_data_dir) result = data_source.view( resource="Patient", - constant = [{"name": "filter_id", "valueUuid": "8ee183e2-b3c0-4151-be94-b945d6aa8c6d"}], + constant=[ + {"name": "filter_id", "valueUuid": "8ee183e2-b3c0-4151-be94-b945d6aa8c6d"} + ], select=[ { "column": [ {"path": "id", "name": "id"}, - {"value": "name.first().family", "name": "family_name"}, + {"path": "name.first().family", "name": "family_name"}, ] } ], - where=[{"path": "id = $filter_id"}], + where=[{"path": "id = %filter_id"}], ) assert result.collect() == [ - Row("8ee183e2-b3c0-4151-be94-b945d6aa8c6d", "Krajcik437"), + ResultRow("8ee183e2-b3c0-4151-be94-b945d6aa8c6d", "Krajcik437"), ] + ConditionCodingRow = Row("id", "code_text", "system", "code", "display") From ef79284b69e27142cf261a1f56780880ac2f5cbb Mon Sep 17 00:00:00 2001 From: John Grimes Date: Fri, 20 Mar 2026 06:54:55 +1000 Subject: [PATCH 3/3] fix: Skip SonarCloud analysis on fork pull requests Secrets are not available for PRs from external contributors, causing the sonar step to fail. Split the install and SonarCloud steps so the analysis is skipped when SONAR_TOKEN is absent. --- .github/workflows/test.yml | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 69b9219313..1b578b1211 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -76,14 +76,23 @@ jobs: - name: Run install goal env: R_KEEP_PKG_SOURCE: yes - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} run: | mvn --batch-mode install \ + -pl '!site,!benchmark' + timeout-minutes: 60 + + - name: Run SonarCloud analysis + # Secrets are not available for pull requests from forks. + if: env.SONAR_TOKEN != '' + env: + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + run: | + mvn --batch-mode \ org.sonarsource.scanner.maven:sonar-maven-plugin:sonar \ -Dsonar.projectKey=aehrc_pathling -Dsonar.organization=aehrc \ -Dsonar.host.url=https://sonarcloud.io \ -pl '!site,!benchmark' - timeout-minutes: 60 + timeout-minutes: 10 - name: Upload test artifacts if: always()