This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/texera.git
commit 0da794d90f0ed5462b6b12aed4238b10996a6b08 Author: Xinyuan Lin <[email protected]> AuthorDate: Sun Aug 16 01:24:17 2026 +0000 test(services): cover the five service entry points' bootstrap (#7694) ### What changes were proposed in this PR? None of the five Dropwizard service entry points had a spec for `initialize()`, which left them the least-covered files in their own modules. Adds one `*RunSpec` per service. | File | Before | After | |---|---|---| | `WorkflowCompilingService.scala` | 48.6% | **29/37 (78.4%)** | | `AccessControlService.scala` | 37.9% | **21/29 (72.4%)** | | `ConfigService.scala` | 36.7% | **22/30 (73.3%)** | | `NotebookMigrationService.scala` | 46.9% | **24/32 (75.0%)** | | `ComputingUnitManagingService.scala` | 53.6% | **20/28 (71.4%)** | Across the five: **~44.9% -> 74.4%**, and 15 -> 35 tests. Nothing binds a port or starts a server -- `new Bootstrap[Configuration](new Service)` only builds an object mapper and a file source provider. Covered: environment-variable substitution including the `:-` default delimiter, the substituting source provider's delegate, the Scala module on Dropwizard's object mapper (with a `readValue` round trip), the connection pool's JDBC URL and the lifecycle phase that opens it, and the request-logging filter's registration, dispatch set, and forward-and-log behaviour. ### Verification The build applied 18 mutations, all red. Review then proposed 15 more; each was **run** before anything was concluded. Outcome: 11 fixed and proven red on a named test, 2 recorded as unpinnable with evidence, 2 refuted. Three are worth spelling out. **An assertion that was carried by an accidental exception.** The substitution test asserted that an unset variable survives as a literal -- but with a *strict* substitutor the throw happens inside `open()` on the preceding setup line, so the matcher was never reached. Proof: under `EnvironmentVariableSubstitutor(false) -> (true)` the substitution test now **passes**, and a separate new test, "leave a variable with neither a value nor a default as a literal", is what goes red. The strictness claim now has its own assertion instead of riding on an exception. **A lifecycle claim that needed the effect pinned, not the phase.** Faithfully relocating the whole `SqlServer.initConnection` block from `initialize()` into `run()` now fails "initialize should open a connection pool against the configured JDBC URL" with `Some(false) was not equal to Some(true)`. `Some(false)` rather than `None` is the point: an earlier `run()` test had already installed a pool, so a URL-only assertion would have passed -- it is the SqlServer-identity check that kills it. **A refuted finding.** `EnvironmentVariableSubstitutor(false) -> (false, true)` survives, but it is an equivalent mutant over every config this repo ships: `StringSubstitutor` only differs on those arguments when a `${` appears *inside* a variable expression, and no nested form exists anywhere in the tree. Adding a `${${...}}` fixture would cement syntax nothing uses, so argument 2 is left deliberately unpinned. Two suggestions were also declined with reasons: asserting `databaseReachable shouldBe true` when CI env vars are set (it makes the spec environment-aware and breaks for developers without local Postgres), and asserting `FAIL_ON_UNKNOWN_PROPERTIES` (`initialize()` never touches it, so it pins a Dropwizard default rather than our code). ### One thing these tests cannot pin `storage.conf:173-178` ships `username == password == "postgres"`, and CI authenticates the default superuser with that same password. Swapping the last two arguments of `SqlServer.initConnection` therefore survives here -- measured, 6/6 green -- and so does hard-coding either to the literal. The test's claim is narrowed accordingly (it now says "against the configured JDBC URL"). Killing it needs a `common/dao` spec with a purpose-built role where user != password; that is out of scope for a test-only PR on these five files. ### Deliberately not included `main()` on all five: `Application.run(String...)` binds a real port, and dropwizard-core's `onFatalError` calls `System.exit(1)`, which would kill the shared sbt test JVM. Two defects are reported rather than pinned: - **Lifecycle inconsistency.** Three of eight entry points open the connection pool in `initialize()` (`AccessControlService:50`, `ConfigService:48`, `NotebookMigrationService:53`) while five do it in `run()`. Since `Application.run` calls `initialize()` before the CLI parses arguments, `check` or `--help` on those three requires a live Postgres and opens a 10-connection pool that nothing ever closes. The spec comment says explicitly that the current phase is recorded, not endorsed. - **`WorkflowCompilingService.scala:75-96` duplicates `RequestLoggingFilter`** from `common/auth`, which the other four services call via `RequestLoggingFilter.register`. The new tests read the filter back out of the captured `FilterHolder` rather than by class, so that refactor would leave them green. No production file is touched. ### Any related issues, documentation, discussions? Closes #7693 ### How was this PR tested? ``` sbt "WorkflowCompilingService/test" "AccessControlService/test" "ConfigService/test" "NotebookMigrationService/test" "ComputingUnitManagingService/test" ``` ``` [info] Tests: succeeded 13, failed 0, canceled 0, ignored 0, pending 0 [info] Tests: succeeded 6, failed 0, canceled 0, ignored 0, pending 0 [info] Tests: succeeded 42, failed 0, canceled 0, ignored 0, pending 0 [info] Tests: succeeded 33, failed 0, canceled 0, ignored 0, pending 0 [info] Tests: succeeded 119, failed 0, canceled 0, ignored 0, pending 0 ``` Zero canceled, which is worth checking explicitly: sbt's JUnit XML does not mark ScalaTest cancellations, so a spec that silently cancels reads as green in the XML. `Test/scalafmtCheck` and `Test/scalafix --check` pass on all five projects. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --- .../service/AccessControlServiceRunSpec.scala | 179 ++++++++++++++++++- .../ComputingUnitManagingServiceRunSpec.scala | 90 +++++++++- .../texera/service/ConfigServiceRunSpec.scala | 182 ++++++++++++++++++- .../service/NotebookMigrationServiceRunSpec.scala | 180 ++++++++++++++++++- .../service/WorkflowCompilingServiceRunSpec.scala | 192 ++++++++++++++++++++- 5 files changed, 813 insertions(+), 10 deletions(-) diff --git a/access-control-service/src/test/scala/org/apache/texera/service/AccessControlServiceRunSpec.scala b/access-control-service/src/test/scala/org/apache/texera/service/AccessControlServiceRunSpec.scala index 04443ab9c6..7f647ab0fc 100644 --- a/access-control-service/src/test/scala/org/apache/texera/service/AccessControlServiceRunSpec.scala +++ b/access-control-service/src/test/scala/org/apache/texera/service/AccessControlServiceRunSpec.scala @@ -19,12 +19,16 @@ package org.apache.texera.service -import io.dropwizard.core.setup.Environment +import io.dropwizard.configuration.ConfigurationSourceProvider +import io.dropwizard.core.setup.{Bootstrap, Environment} import io.dropwizard.jersey.DropwizardResourceConfig import io.dropwizard.jersey.setup.JerseyEnvironment import io.dropwizard.jetty.MutableServletContextHandler import io.dropwizard.jetty.setup.ServletEnvironment import org.apache.texera.auth.{RoleAnnotationEnforcer, UnauthorizedExceptionMapper} +import org.apache.texera.common.config.StorageConfig +import org.apache.texera.dao.SqlServer +import org.apache.texera.service.AccessControlServiceRunSpec.SpecPayload import org.apache.texera.service.activity.UserActivityEventListener import org.apache.texera.service.resource.{ AccessControlResource, @@ -38,6 +42,11 @@ import org.mockito.Mockito.{mock, verify, when} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers +import java.io.{ByteArrayInputStream, FileNotFoundException, InputStream} +import java.nio.charset.StandardCharsets.UTF_8 +import java.sql.{DriverManager, SQLException} +import scala.util.{Try, Using} + class AccessControlServiceRunSpec extends AnyFlatSpec with Matchers { "AccessControlService.run" should "register UserActivityEventListener on the Jersey environment" in { @@ -71,4 +80,172 @@ class AccessControlServiceRunSpec extends AnyFlatSpec with Matchers { ) ) shouldBe empty } + + // --- initialize() --- + + /** + * Registering the driver is explicit because sbt gives each test project a layered classloader + * that DriverManager's own ServiceLoader scan does not always reach; a deployed service has one + * flat classpath and never hits this. Both the probe below and HikariCP resolve the driver + * through DriverManager, so this has to happen before initialize() runs. A missing class throws + * out of here rather than reading as "no database". + */ + private lazy val postgresDriverLoaded: Class[_] = Class.forName("org.postgresql.Driver") + + // Only the pool assertion needs a database. This service, config-service and + // notebook-migration-service are the three entry points that call SqlServer.initConnection from + // initialize(); the other five call it from run(). That inconsistency is not endorsed here: + // if the call moves into run(), the pool test below moves with it (and goes red until it does). + private def databaseReachable: Boolean = + try { + postgresDriverLoaded + DriverManager + .getConnection( + StorageConfig.jdbcUrl, + StorageConfig.jdbcUsername, + StorageConfig.jdbcPassword + ) + .close() + true + } catch { + // Narrowed to SQLException on purpose: a missing driver or a linkage error is a broken test + // classpath and has to fail loudly rather than silently cancel the assertion. + case _: SQLException => false + } + + private val seededConfigPath = "access-control-service-spec.yml" + + // The `:-` default form is the only substitution shape this service actually ships: both + // `${...}` in access-control-service-web-config.yaml (lines 28 and 31) carry a default. + private val seededConfig = + "defined: ${PATH}\ndefaulted: ${TEXERA_ACCESS_CONTROL_SERVICE_SPEC_UNSET:-fallback}\n" + + private val unsetConfigPath = "access-control-service-spec-unset.yml" + private val unsetConfig = "unset: ${TEXERA_ACCESS_CONTROL_SERVICE_SPEC_UNSET}\n" + + /** What one full initialize() left behind. */ + private case class Initialized( + bootstrap: Bootstrap[AccessControlServiceConfiguration], + outcome: Try[Unit], + sqlServerBefore: Option[SqlServer], + sqlServerAfter: Option[SqlServer], + pooledJdbcUrl: Try[String] + ) + + /** + * A single full initialize(), shared by every assertion below so the connection pool is opened + * once. + * + * The bootstrap is seeded with a recognisable in-memory configuration source first: that is what + * makes the wrapping observable, because initialize() is required to wrap the provider already + * installed, and a version that instead built a fresh file-reading provider would not find these + * paths. + * + * initialize() is called inside a Try because opening the pool is its last statement: the + * configuration-source and object-mapper effects have already landed by the time it can fail, so + * the assertions that need no database still run on a host with no Postgres instead of silently + * cancelling. `outcome` carries the failure to the one assertion that does need it. + * + * `SqlServer.initConnection` swaps a JVM-global singleton. The suites of this module share one + * unforked JVM and AccessControlResourceSpec points that singleton at its own MockTexeraDB + * database, so the pool is closed and the previously installed DSLContext is put back the + * instant initialize() returns. Closing our pool is safe for that restored context: it carries + * its own DataSource. + */ + private lazy val initialized: Initialized = { + val application = new AccessControlService + val bootstrap = new Bootstrap[AccessControlServiceConfiguration](application) + bootstrap.setConfigurationSourceProvider(new ConfigurationSourceProvider { + override def open(path: String): InputStream = + path match { + case `seededConfigPath` => new ByteArrayInputStream(seededConfig.getBytes(UTF_8)) + case `unsetConfigPath` => new ByteArrayInputStream(unsetConfig.getBytes(UTF_8)) + case other => throw new FileNotFoundException(other) + } + }) + postgresDriverLoaded + val priorContext = Try(SqlServer.getInstance().createDSLContext()).toOption + val sqlServerBefore = Try(SqlServer.getInstance()).toOption + try { + Initialized( + bootstrap, + Try(application.initialize(bootstrap)), + sqlServerBefore, + Try(SqlServer.getInstance()).toOption, + Try( + SqlServer + .getInstance() + .createDSLContext() + .connectionResult(connection => connection.getMetaData.getURL) + ) + ) + } finally { + Try(SqlServer.getInstance().close()) + priorContext.foreach(context => Try(SqlServer.getInstance().replaceDSLContext(context))) + } + } + + private def resolve(path: String): String = + Using.resource(initialized.bootstrap.getConfigurationSourceProvider.open(path)) { stream => + new String(stream.readAllBytes(), UTF_8) + } + + "AccessControlService.initialize" should "substitute environment variables into the configuration source it was handed" in { + // Whole-string rather than a pair of `include`s, so a substitutor that also mangled the rest + // of the document could not pass. The second line is the shape the service's own YAML uses: + // it only resolves while the substitutor keeps commons-text's `:-` value delimiter, and a + // deployment that lost it would hand logback the literal "${TEXERA_SERVICE_LOG_LEVEL:-INFO}" + // as a level. + resolve(seededConfigPath) shouldBe s"defined: ${System.getenv("PATH")}\ndefaulted: fallback\n" + } + + it should "leave a variable with neither a value nor a default as a literal" in { + // The substitutor is built non-strict. A strict one raises UndefinedEnvironmentVariableException + // out of open(), i.e. refuses to boot a deployment whose config names a variable it does not + // set; asserting the resolved text here makes that a stated claim rather than an incidental + // error thrown from the middle of the test above. + resolve(unsetConfigPath) shouldBe "unset: ${TEXERA_ACCESS_CONTROL_SERVICE_SPEC_UNSET}\n" + } + + it should "register the Scala module on Dropwizard's object mapper" in { + val mapper = initialized.bootstrap.getObjectMapper + // The whole module, not only the Option support that `Some("x")` alone would prove: this is + // the mapper Dropwizard hands to Jersey, so every payload the API returns goes through it. + mapper.getRegisteredModuleIds should contain( + "com.fasterxml.jackson.module.scala.DefaultScalaModule$" + ) + + val payload = SpecPayload("[email protected]", Map("read" -> Some(Seq(1, 2)), "write" -> None)) + // Option unwrapped, Map and Seq emitted as JSON, and the camelCase property names left alone + // (Dropwizard's own naming strategy is annotation-sensitive; a global one would rename them). + val json = mapper.writeValueAsString(payload) + json shouldBe """{"userEmail":"[email protected]","grantedAccess":{"read":[1,2],"write":null}}""" + // Reading, too: this is also the mapper Dropwizard parses the YAML configuration with. + mapper.readValue(json, classOf[SpecPayload]) shouldBe payload + } + + it should "open a connection pool against the configured JDBC URL" in { + assume( + databaseReachable, + "initialize() opens a connection pool against the configured JDBC URL (provided in CI)" + ) + // Rethrown rather than collapsed into a missing URL, so a pool that failed to open reports + // why. + initialized.outcome.get + // A *new* SqlServer, not merely a URL read back off the singleton: without this the assertion + // below also passes for an initialize() that opens nothing at all, off a pool some earlier + // test in this JVM installed. + initialized.sqlServerAfter.map(_ ne initialized.sqlServerBefore.orNull) shouldBe Some(true) + // Not the test-cases database: pointing the running service at it would have every access + // check read a schema that the CI e2e specs truncate underneath it. Only the URL is pinned — + // storage.conf ships username and password as the same string ("postgres"), so nothing here + // can tell the two credential arguments apart. + initialized.pooledJdbcUrl.get shouldBe StorageConfig.jdbcUrl + } +} + +object AccessControlServiceRunSpec { + + /** Stands in for the payloads this service returns: camelCase names, a Map, a Seq and an Option. */ + final case class SpecPayload(userEmail: String, grantedAccess: Map[String, Option[Seq[Int]]]) } diff --git a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/ComputingUnitManagingServiceRunSpec.scala b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/ComputingUnitManagingServiceRunSpec.scala index e4694e2d20..f62b043384 100644 --- a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/ComputingUnitManagingServiceRunSpec.scala +++ b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/ComputingUnitManagingServiceRunSpec.scala @@ -20,13 +20,15 @@ package org.apache.texera.service import io.dropwizard.auth.AuthDynamicFeature -import io.dropwizard.core.setup.Environment +import io.dropwizard.configuration.ConfigurationSourceProvider +import io.dropwizard.core.setup.{Bootstrap, Environment} import io.dropwizard.jersey.DropwizardResourceConfig import io.dropwizard.jersey.setup.JerseyEnvironment import io.dropwizard.jetty.MutableServletContextHandler import org.apache.texera.auth.RoleAnnotationEnforcer import org.apache.texera.common.config.StorageConfig import org.apache.texera.dao.SqlServer +import org.apache.texera.service.ComputingUnitManagingServiceRunSpec.SpecPayload import org.apache.texera.service.resource.{ AdminComputingUnitResource, ComputingUnitAccessResource, @@ -39,7 +41,10 @@ import org.mockito.Mockito.{mock, verify, when} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers +import java.io.{ByteArrayInputStream, FileNotFoundException, InputStream} +import java.nio.charset.StandardCharsets.UTF_8 import java.sql.DriverManager +import scala.util.Using class ComputingUnitManagingServiceRunSpec extends AnyFlatSpec with Matchers { @@ -105,4 +110,87 @@ class ComputingUnitManagingServiceRunSpec extends AnyFlatSpec with Matchers { catch { case _: Throwable => () } } } + + // --- initialize() --- + // Unlike run(), initialize() opens no connection pool: Dropwizard's Bootstrap only allocates + // in-memory scaffolding (an object mapper, a metric registry, a file-reading configuration + // source provider), so these run everywhere, with no database and no port. + + private val seededConfigPath = "computing-unit-managing-service-spec.yml" + + // The `:-` default form is the only substitution shape this service actually ships: both + // `${...}` in computing-unit-managing-service-config.yaml (lines 31 and 33) carry a default. + private val seededConfig = + "defined: ${PATH}\ndefaulted: ${TEXERA_COMPUTING_UNIT_MANAGING_SERVICE_SPEC_UNSET:-fallback}\n" + + private val unsetConfigPath = "computing-unit-managing-service-spec-unset.yml" + private val unsetConfig = "unset: ${TEXERA_COMPUTING_UNIT_MANAGING_SERVICE_SPEC_UNSET}\n" + + /** + * Runs initialize() over a bootstrap that already carries a recognisable in-memory + * configuration source. Seeding it is what makes the wrapping observable: initialize() is + * required to wrap the provider that is already installed, and a version that instead built a + * fresh file-reading provider would fail to find these paths at all. + */ + private def initializedBootstrap(): Bootstrap[ComputingUnitManagingServiceConfiguration] = { + val application = new ComputingUnitManagingService + val bootstrap = new Bootstrap[ComputingUnitManagingServiceConfiguration](application) + bootstrap.setConfigurationSourceProvider(new ConfigurationSourceProvider { + override def open(path: String): InputStream = + path match { + case `seededConfigPath` => new ByteArrayInputStream(seededConfig.getBytes(UTF_8)) + case `unsetConfigPath` => new ByteArrayInputStream(unsetConfig.getBytes(UTF_8)) + case other => throw new FileNotFoundException(other) + } + }) + application.initialize(bootstrap) + bootstrap + } + + private def resolve(path: String): String = + Using.resource(initializedBootstrap().getConfigurationSourceProvider.open(path)) { stream => + new String(stream.readAllBytes(), UTF_8) + } + + "ComputingUnitManagingService.initialize" should "substitute environment variables into the configuration source it was handed" in { + // Whole-string rather than a pair of `include`s, so a substitutor that also mangled the rest + // of the document could not pass. The second line is the shape the service's own YAML uses: + // it only resolves while the substitutor keeps commons-text's `:-` value delimiter, and a + // deployment that lost it would hand logback the literal "${TEXERA_SERVICE_LOG_LEVEL:-INFO}" + // as a level. + resolve(seededConfigPath) shouldBe s"defined: ${System.getenv("PATH")}\ndefaulted: fallback\n" + } + + it should "leave a variable with neither a value nor a default as a literal" in { + // The substitutor is built non-strict. A strict one raises UndefinedEnvironmentVariableException + // out of open(), i.e. refuses to boot a deployment whose config names a variable it does not + // set; asserting the resolved text here makes that a stated claim rather than an incidental + // error thrown from the middle of the test above. + resolve( + unsetConfigPath + ) shouldBe "unset: ${TEXERA_COMPUTING_UNIT_MANAGING_SERVICE_SPEC_UNSET}\n" + } + + it should "register the Scala module on Dropwizard's object mapper" in { + val mapper = initializedBootstrap().getObjectMapper + // The whole module, not only the Option support that `Some("x")` alone would prove: this is + // the mapper Dropwizard hands to Jersey, so every payload the API returns goes through it. + mapper.getRegisteredModuleIds should contain( + "com.fasterxml.jackson.module.scala.DefaultScalaModule$" + ) + + val payload = SpecPayload("cu-1", Map("running" -> Some(Seq(1, 2)), "terminated" -> None)) + // Option unwrapped, Map and Seq emitted as JSON, and the camelCase property names left alone + // (Dropwizard's own naming strategy is annotation-sensitive; a global one would rename them). + val json = mapper.writeValueAsString(payload) + json shouldBe """{"unitName":"cu-1","unitAccess":{"running":[1,2],"terminated":null}}""" + // Reading, too: this is also the mapper Dropwizard parses the YAML configuration with. + mapper.readValue(json, classOf[SpecPayload]) shouldBe payload + } +} + +object ComputingUnitManagingServiceRunSpec { + + /** Stands in for the payloads this service returns: camelCase names, a Map, a Seq and an Option. */ + final case class SpecPayload(unitName: String, unitAccess: Map[String, Option[Seq[Int]]]) } diff --git a/config-service/src/test/scala/org/apache/texera/service/ConfigServiceRunSpec.scala b/config-service/src/test/scala/org/apache/texera/service/ConfigServiceRunSpec.scala index a5c14da61f..5f6cd1238b 100644 --- a/config-service/src/test/scala/org/apache/texera/service/ConfigServiceRunSpec.scala +++ b/config-service/src/test/scala/org/apache/texera/service/ConfigServiceRunSpec.scala @@ -20,15 +20,17 @@ package org.apache.texera.service import io.dropwizard.auth.AuthDynamicFeature -import io.dropwizard.core.setup.Environment +import io.dropwizard.configuration.ConfigurationSourceProvider +import io.dropwizard.core.setup.{Bootstrap, Environment} import io.dropwizard.jersey.DropwizardResourceConfig import io.dropwizard.jersey.setup.JerseyEnvironment import io.dropwizard.jetty.MutableServletContextHandler import io.dropwizard.jetty.setup.ServletEnvironment import org.apache.texera.auth.{RoleAnnotationEnforcer, UnauthorizedExceptionMapper} -import org.apache.texera.common.config.DefaultsConfig +import org.apache.texera.common.config.{DefaultsConfig, StorageConfig} import org.apache.texera.dao.{MockTexeraDB, SqlServer} import org.apache.texera.dao.jooq.generated.Tables.SITE_SETTINGS +import org.apache.texera.service.ConfigServiceRunSpec.SpecPayload import org.apache.texera.service.resource.{ConfigResource, HealthCheckResource} import org.eclipse.jetty.server.session.SessionHandler import org.eclipse.jetty.servlet.FilterHolder @@ -41,7 +43,10 @@ import org.scalatest.BeforeAndAfterAll import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers -import java.sql.{Connection, SQLException} +import java.io.{ByteArrayInputStream, FileNotFoundException, InputStream} +import java.nio.charset.StandardCharsets.UTF_8 +import java.sql.{Connection, DriverManager, SQLException} +import scala.util.{Try, Using} // `run` ends by preloading default.conf into site_settings, so it needs a live // SqlServer: MockTexeraDB gives this suite its own embedded database and points @@ -160,4 +165,175 @@ class ConfigServiceRunSpec Seq(classOf[ConfigResource], classOf[HealthCheckResource]) ) shouldBe empty } + + // --- initialize() --- + + /** + * Registering the driver is explicit because sbt gives each test project a layered classloader + * that DriverManager's own ServiceLoader scan does not always reach; a deployed service has one + * flat classpath and never hits this. Both the probe below and HikariCP resolve the driver + * through DriverManager, so this has to happen before initialize() runs. A missing class throws + * out of here rather than reading as "no database". + */ + private lazy val postgresDriverLoaded: Class[_] = Class.forName("org.postgresql.Driver") + + // Only the pool assertion needs the deployment's own database (this suite's embedded + // MockTexeraDB one is a different URL). This service, access-control-service and + // notebook-migration-service are the three entry points that call SqlServer.initConnection from + // initialize(); the other five call it from run(). That inconsistency is not endorsed here: + // if the call moves into run(), the pool test below moves with it (and goes red until it does, + // along with the two run() tests above, which would then be preloading the deployment's + // site_settings instead of the fixture's). + private def databaseReachable: Boolean = + try { + postgresDriverLoaded + DriverManager + .getConnection( + StorageConfig.jdbcUrl, + StorageConfig.jdbcUsername, + StorageConfig.jdbcPassword + ) + .close() + true + } catch { + // Narrowed to SQLException on purpose: a missing driver or a linkage error is a broken test + // classpath and has to fail loudly rather than silently cancel the assertion. + case _: SQLException => false + } + + private val seededConfigPath = "config-service-spec.yml" + + // The `:-` default form is the only substitution shape this service actually ships: both + // `${...}` in config-service-web-config.yaml (lines 28 and 31) carry a default. + private val seededConfig = + "defined: ${PATH}\ndefaulted: ${TEXERA_CONFIG_SERVICE_SPEC_UNSET:-fallback}\n" + + private val unsetConfigPath = "config-service-spec-unset.yml" + private val unsetConfig = "unset: ${TEXERA_CONFIG_SERVICE_SPEC_UNSET}\n" + + /** What one full initialize() left behind. */ + private case class Initialized( + bootstrap: Bootstrap[ConfigServiceConfiguration], + outcome: Try[Unit], + sqlServerBefore: Option[SqlServer], + sqlServerAfter: Option[SqlServer], + pooledJdbcUrl: Try[String] + ) + + /** + * A single full initialize(), shared by every assertion below so the connection pool is opened + * once. + * + * The bootstrap is seeded with a recognisable in-memory configuration source first: that is what + * makes the wrapping observable, because initialize() is required to wrap the provider already + * installed, and a version that instead built a fresh file-reading provider would not find these + * paths. + * + * initialize() is called inside a Try because opening the pool is its last statement: the + * configuration-source and object-mapper effects have already landed by the time it can fail, so + * the assertions that need no database still run on a host with no Postgres instead of silently + * cancelling. `outcome` carries the failure to the one assertion that does need it. + * + * `SqlServer.initConnection` swaps the same JVM-global singleton this suite's MockTexeraDB + * fixture owns, so the pool is closed and the fixture's DSLContext is put back the instant + * initialize() returns. Closing our pool is safe for that restored context: it carries its own + * DataSource. + */ + private lazy val initialized: Initialized = { + val application = new ConfigService + val bootstrap = new Bootstrap[ConfigServiceConfiguration](application) + bootstrap.setConfigurationSourceProvider(new ConfigurationSourceProvider { + override def open(path: String): InputStream = + path match { + case `seededConfigPath` => new ByteArrayInputStream(seededConfig.getBytes(UTF_8)) + case `unsetConfigPath` => new ByteArrayInputStream(unsetConfig.getBytes(UTF_8)) + case other => throw new FileNotFoundException(other) + } + }) + postgresDriverLoaded + val priorContext = Try(SqlServer.getInstance().createDSLContext()).toOption + val sqlServerBefore = Try(SqlServer.getInstance()).toOption + try { + Initialized( + bootstrap, + Try(application.initialize(bootstrap)), + sqlServerBefore, + Try(SqlServer.getInstance()).toOption, + Try( + SqlServer + .getInstance() + .createDSLContext() + .connectionResult(connection => connection.getMetaData.getURL) + ) + ) + } finally { + Try(SqlServer.getInstance().close()) + priorContext.foreach(context => Try(SqlServer.getInstance().replaceDSLContext(context))) + } + } + + private def resolve(path: String): String = + Using.resource(initialized.bootstrap.getConfigurationSourceProvider.open(path)) { stream => + new String(stream.readAllBytes(), UTF_8) + } + + "ConfigService.initialize" should "substitute environment variables into the configuration source it was handed" in { + // Whole-string rather than a pair of `include`s, so a substitutor that also mangled the rest + // of the document could not pass. The second line is the shape the service's own YAML uses: + // it only resolves while the substitutor keeps commons-text's `:-` value delimiter, and a + // deployment that lost it would hand logback the literal "${TEXERA_SERVICE_LOG_LEVEL:-INFO}" + // as a level. + resolve(seededConfigPath) shouldBe s"defined: ${System.getenv("PATH")}\ndefaulted: fallback\n" + } + + it should "leave a variable with neither a value nor a default as a literal" in { + // The substitutor is built non-strict. A strict one raises UndefinedEnvironmentVariableException + // out of open(), i.e. refuses to boot a deployment whose config names a variable it does not + // set; asserting the resolved text here makes that a stated claim rather than an incidental + // error thrown from the middle of the test above. + resolve(unsetConfigPath) shouldBe "unset: ${TEXERA_CONFIG_SERVICE_SPEC_UNSET}\n" + } + + it should "register the Scala module on Dropwizard's object mapper" in { + val mapper = initialized.bootstrap.getObjectMapper + // The whole module, not only the Option support that `Some("x")` alone would prove: this is + // the mapper Dropwizard hands to Jersey, so every payload the API returns goes through it. + mapper.getRegisteredModuleIds should contain( + "com.fasterxml.jackson.module.scala.DefaultScalaModule$" + ) + + val payload = + SpecPayload("forum_enabled", Map("default" -> Some(Seq(1, 2)), "override" -> None)) + // Option unwrapped, Map and Seq emitted as JSON, and the camelCase property names left alone + // (Dropwizard's own naming strategy is annotation-sensitive; a global one would rename them). + val json = mapper.writeValueAsString(payload) + json shouldBe """{"settingKey":"forum_enabled","storedValues":{"default":[1,2],"override":null}}""" + // Reading, too: this is also the mapper Dropwizard parses the YAML configuration with. + mapper.readValue(json, classOf[SpecPayload]) shouldBe payload + } + + it should "open a connection pool against the configured JDBC URL" in { + assume( + databaseReachable, + "initialize() opens a connection pool against the configured JDBC URL (provided in CI)" + ) + // Rethrown rather than collapsed into a missing URL, so a pool that failed to open reports + // why. + initialized.outcome.get + // A *new* SqlServer, not merely a URL read back off the singleton: without this the assertion + // below also passes for an initialize() that opens nothing at all, off a pool some earlier + // test in this JVM installed. + initialized.sqlServerAfter.map(_ ne initialized.sqlServerBefore.orNull) shouldBe Some(true) + // Not the test-cases database: preloading the defaults into it would leave the deployment's + // own site_settings empty while the CI e2e specs truncate the rows that were written. Only + // the URL is pinned — storage.conf ships username and password as the same string + // ("postgres"), so nothing here can tell the two credential arguments apart. + initialized.pooledJdbcUrl.get shouldBe StorageConfig.jdbcUrl + } +} + +object ConfigServiceRunSpec { + + /** Stands in for the payloads this service returns: camelCase names, a Map, a Seq and an Option. */ + final case class SpecPayload(settingKey: String, storedValues: Map[String, Option[Seq[Int]]]) } diff --git a/notebook-migration-service/src/test/scala/org/apache/texera/service/NotebookMigrationServiceRunSpec.scala b/notebook-migration-service/src/test/scala/org/apache/texera/service/NotebookMigrationServiceRunSpec.scala index 7a11d478a6..64a99c07e2 100644 --- a/notebook-migration-service/src/test/scala/org/apache/texera/service/NotebookMigrationServiceRunSpec.scala +++ b/notebook-migration-service/src/test/scala/org/apache/texera/service/NotebookMigrationServiceRunSpec.scala @@ -20,12 +20,16 @@ package org.apache.texera.service import io.dropwizard.auth.AuthDynamicFeature -import io.dropwizard.core.setup.Environment +import io.dropwizard.configuration.ConfigurationSourceProvider +import io.dropwizard.core.setup.{Bootstrap, Environment} import io.dropwizard.jersey.DropwizardResourceConfig import io.dropwizard.jersey.setup.JerseyEnvironment import io.dropwizard.jetty.MutableServletContextHandler import io.dropwizard.jetty.setup.ServletEnvironment import org.apache.texera.auth.UnauthorizedExceptionMapper +import org.apache.texera.common.config.StorageConfig +import org.apache.texera.dao.SqlServer +import org.apache.texera.service.NotebookMigrationServiceRunSpec.SpecPayload import org.apache.texera.service.resource.{HealthCheckResource, NotebookMigrationResource} import org.glassfish.jersey.server.filter.RolesAllowedDynamicFeature import org.mockito.ArgumentMatchers.isA @@ -33,6 +37,11 @@ import org.mockito.Mockito.{mock, verify, when} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers +import java.io.{ByteArrayInputStream, FileNotFoundException, InputStream} +import java.nio.charset.StandardCharsets.UTF_8 +import java.sql.{DriverManager, SQLException} +import scala.util.{Try, Using} + class NotebookMigrationServiceRunSpec extends AnyFlatSpec with Matchers { "NotebookMigrationService.run" should "register the resources and the JWT auth stack on the Jersey environment" in { @@ -56,4 +65,173 @@ class NotebookMigrationServiceRunSpec extends AnyFlatSpec with Matchers { verify(jersey).register(classOf[UnauthorizedExceptionMapper]) verify(jersey).register(classOf[RolesAllowedDynamicFeature]) } + + // --- initialize() --- + + /** + * Registering the driver is explicit because sbt gives each test project a layered classloader + * that DriverManager's own ServiceLoader scan does not always reach; a deployed service has one + * flat classpath and never hits this. Both the probe below and HikariCP resolve the driver + * through DriverManager, so this has to happen before initialize() runs. A missing class throws + * out of here rather than reading as "no database". + */ + private lazy val postgresDriverLoaded: Class[_] = Class.forName("org.postgresql.Driver") + + // Only the pool assertion needs a database. This service, access-control-service and + // config-service are the three entry points that call SqlServer.initConnection from + // initialize(); the other five call it from run(). That inconsistency is not endorsed here: + // if the call moves into run(), the pool test below moves with it (and goes red until it does). + private def databaseReachable: Boolean = + try { + postgresDriverLoaded + DriverManager + .getConnection( + StorageConfig.jdbcUrl, + StorageConfig.jdbcUsername, + StorageConfig.jdbcPassword + ) + .close() + true + } catch { + // Narrowed to SQLException on purpose: a missing driver or a linkage error is a broken test + // classpath and has to fail loudly rather than silently cancel the assertion. + case _: SQLException => false + } + + private val seededConfigPath = "notebook-migration-service-spec.yml" + + // The `:-` default form is the only substitution shape this service actually ships: both + // `${...}` in notebook-migration-service-web-config.yaml (lines 28 and 31) carry a default. + private val seededConfig = + "defined: ${PATH}\ndefaulted: ${TEXERA_NOTEBOOK_MIGRATION_SERVICE_SPEC_UNSET:-fallback}\n" + + private val unsetConfigPath = "notebook-migration-service-spec-unset.yml" + private val unsetConfig = "unset: ${TEXERA_NOTEBOOK_MIGRATION_SERVICE_SPEC_UNSET}\n" + + /** What one full initialize() left behind. */ + private case class Initialized( + bootstrap: Bootstrap[NotebookMigrationServiceConfiguration], + outcome: Try[Unit], + sqlServerBefore: Option[SqlServer], + sqlServerAfter: Option[SqlServer], + pooledJdbcUrl: Try[String] + ) + + /** + * A single full initialize(), shared by every assertion below so the connection pool is opened + * once. + * + * The bootstrap is seeded with a recognisable in-memory configuration source first: that is what + * makes the wrapping observable, because initialize() is required to wrap the provider already + * installed, and a version that instead built a fresh file-reading provider would not find these + * paths. + * + * initialize() is called inside a Try because opening the pool is its last statement: the + * configuration-source and object-mapper effects have already landed by the time it can fail, so + * the assertions that need no database still run on a host with no Postgres instead of silently + * cancelling. `outcome` carries the failure to the one assertion that does need it. + * + * `SqlServer.initConnection` swaps a JVM-global singleton. The suites of this module share one + * unforked JVM and NotebookMigrationResourceSpec points that singleton at its own MockTexeraDB + * database, so the pool is closed and the previously installed DSLContext is put back the + * instant initialize() returns. Closing our pool is safe for that restored context: it carries + * its own DataSource. + */ + private lazy val initialized: Initialized = { + val application = new NotebookMigrationService + val bootstrap = new Bootstrap[NotebookMigrationServiceConfiguration](application) + bootstrap.setConfigurationSourceProvider(new ConfigurationSourceProvider { + override def open(path: String): InputStream = + path match { + case `seededConfigPath` => new ByteArrayInputStream(seededConfig.getBytes(UTF_8)) + case `unsetConfigPath` => new ByteArrayInputStream(unsetConfig.getBytes(UTF_8)) + case other => throw new FileNotFoundException(other) + } + }) + postgresDriverLoaded + val priorContext = Try(SqlServer.getInstance().createDSLContext()).toOption + val sqlServerBefore = Try(SqlServer.getInstance()).toOption + try { + Initialized( + bootstrap, + Try(application.initialize(bootstrap)), + sqlServerBefore, + Try(SqlServer.getInstance()).toOption, + Try( + SqlServer + .getInstance() + .createDSLContext() + .connectionResult(connection => connection.getMetaData.getURL) + ) + ) + } finally { + Try(SqlServer.getInstance().close()) + priorContext.foreach(context => Try(SqlServer.getInstance().replaceDSLContext(context))) + } + } + + private def resolve(path: String): String = + Using.resource(initialized.bootstrap.getConfigurationSourceProvider.open(path)) { stream => + new String(stream.readAllBytes(), UTF_8) + } + + "NotebookMigrationService.initialize" should "substitute environment variables into the configuration source it was handed" in { + // Whole-string rather than a pair of `include`s, so a substitutor that also mangled the rest + // of the document could not pass. The second line is the shape the service's own YAML uses: + // it only resolves while the substitutor keeps commons-text's `:-` value delimiter, and a + // deployment that lost it would hand logback the literal "${TEXERA_SERVICE_LOG_LEVEL:-INFO}" + // as a level. + resolve(seededConfigPath) shouldBe s"defined: ${System.getenv("PATH")}\ndefaulted: fallback\n" + } + + it should "leave a variable with neither a value nor a default as a literal" in { + // The substitutor is built non-strict. A strict one raises UndefinedEnvironmentVariableException + // out of open(), i.e. refuses to boot a deployment whose config names a variable it does not + // set; asserting the resolved text here makes that a stated claim rather than an incidental + // error thrown from the middle of the test above. + resolve(unsetConfigPath) shouldBe "unset: ${TEXERA_NOTEBOOK_MIGRATION_SERVICE_SPEC_UNSET}\n" + } + + it should "register the Scala module on Dropwizard's object mapper" in { + val mapper = initialized.bootstrap.getObjectMapper + // The whole module, not only the Option support that `Some("x")` alone would prove: this is + // the mapper Dropwizard hands to Jersey, so every payload the API returns goes through it. + mapper.getRegisteredModuleIds should contain( + "com.fasterxml.jackson.module.scala.DefaultScalaModule$" + ) + + val payload = + SpecPayload("notebook.ipynb", Map("migrated" -> Some(Seq(1, 2)), "failed" -> None)) + // Option unwrapped, Map and Seq emitted as JSON, and the camelCase property names left alone + // (Dropwizard's own naming strategy is annotation-sensitive; a global one would rename them). + val json = mapper.writeValueAsString(payload) + json shouldBe """{"notebookName":"notebook.ipynb","cellOutcomes":{"migrated":[1,2],"failed":null}}""" + // Reading, too: this is also the mapper Dropwizard parses the YAML configuration with. + mapper.readValue(json, classOf[SpecPayload]) shouldBe payload + } + + it should "open a connection pool against the configured JDBC URL" in { + assume( + databaseReachable, + "initialize() opens a connection pool against the configured JDBC URL (provided in CI)" + ) + // Rethrown rather than collapsed into a missing URL, so a pool that failed to open reports + // why. + initialized.outcome.get + // A *new* SqlServer, not merely a URL read back off the singleton: without this the assertion + // below also passes for an initialize() that opens nothing at all, off a pool some earlier + // test in this JVM installed. + initialized.sqlServerAfter.map(_ ne initialized.sqlServerBefore.orNull) shouldBe Some(true) + // Not the test-cases database: pointing the running service at it would have every migration + // record land in a schema that the CI e2e specs truncate underneath it. Only the URL is + // pinned — storage.conf ships username and password as the same string ("postgres"), so + // nothing here can tell the two credential arguments apart. + initialized.pooledJdbcUrl.get shouldBe StorageConfig.jdbcUrl + } +} + +object NotebookMigrationServiceRunSpec { + + /** Stands in for the payloads this service returns: camelCase names, a Map, a Seq and an Option. */ + final case class SpecPayload(notebookName: String, cellOutcomes: Map[String, Option[Seq[Int]]]) } diff --git a/workflow-compiling-service/src/test/scala/org/apache/texera/service/WorkflowCompilingServiceRunSpec.scala b/workflow-compiling-service/src/test/scala/org/apache/texera/service/WorkflowCompilingServiceRunSpec.scala index f9b4162247..88b0411667 100644 --- a/workflow-compiling-service/src/test/scala/org/apache/texera/service/WorkflowCompilingServiceRunSpec.scala +++ b/workflow-compiling-service/src/test/scala/org/apache/texera/service/WorkflowCompilingServiceRunSpec.scala @@ -19,19 +19,33 @@ package org.apache.texera.service -import io.dropwizard.core.setup.Environment +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.classic.{Level, Logger => LogbackLogger} +import ch.qos.logback.core.read.ListAppender +import io.dropwizard.configuration.ConfigurationSourceProvider +import io.dropwizard.core.setup.{Bootstrap, Environment} import io.dropwizard.jersey.DropwizardResourceConfig import io.dropwizard.jersey.setup.JerseyEnvironment import io.dropwizard.jetty.MutableServletContextHandler import io.dropwizard.jetty.setup.ServletEnvironment +import jakarta.servlet.{DispatcherType, Filter, FilterChain} +import jakarta.servlet.http.{HttpServletRequest, HttpServletResponse} import org.apache.texera.auth.{RoleAnnotationEnforcer, UnauthorizedExceptionMapper} +import org.apache.texera.service.WorkflowCompilingServiceRunSpec.SpecPayload import org.apache.texera.service.resource.{HealthCheckResource, WorkflowCompilationResource} -import org.eclipse.jetty.servlet.FilterHolder +import org.eclipse.jetty.servlet.{FilterHolder, ServletHandler} import org.glassfish.jersey.server.filter.RolesAllowedDynamicFeature +import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.{any, eq => eqTo, isA} -import org.mockito.Mockito.{mock, verify, when} +import org.mockito.Mockito.{mock, never, verify, when} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers +import org.slf4j.LoggerFactory + +import java.io.{ByteArrayInputStream, FileNotFoundException, InputStream} +import java.nio.charset.StandardCharsets.UTF_8 +import scala.jdk.CollectionConverters._ +import scala.util.Using class WorkflowCompilingServiceRunSpec extends AnyFlatSpec with Matchers { @@ -76,7 +90,95 @@ class WorkflowCompilingServiceRunSpec extends AnyFlatSpec with Matchers { it should "add the request-logging filter to the application context" in { val (_, context) = ranService - verify(context).addFilter(isA(classOf[FilterHolder]), eqTo("/*"), any()) + // Every path and every dispatcher type: a narrower EnumSet registers the filter but leaves + // forwarded, included, async and error dispatches unlogged. + verify(context).addFilter( + isA(classOf[FilterHolder]), + eqTo("/*"), + eqTo(java.util.EnumSet.allOf(classOf[DispatcherType])) + ) + } + + /** + * The filter run() installed, read back out of the holder it registered rather than looked up + * by class name. That keeps these assertions about behaviour: replacing this service's inlined + * copy with the `RequestLoggingFilter.register` the other four services call registers the same + * filter contract and leaves them green. + * + * A FilterHolder only publishes its instance once it has been started and initialised (Jetty + * does that when the context starts), so the holder is walked through that lifecycle here + * instead of binding a server. `wrap` reads the holder's ServletHandler, hence the bare one. + */ + private lazy val requestLogFilter: Filter = { + val (_, context) = ranService + val holder = ArgumentCaptor.forClass(classOf[FilterHolder]) + verify(context).addFilter(holder.capture(), any[String](), any()) + val captured = holder.getValue + captured.setServletHandler(new ServletHandler) + captured.start() + captured.initialize() + captured.getFilter + } + + /** Runs `body` with the request-log logger pinned at `level`, returning what it logged. */ + private def eventsLoggedAt(level: Level)(body: => Unit): Seq[ILoggingEvent] = { + val logger = + LoggerFactory.getLogger("org.eclipse.jetty.server.RequestLog").asInstanceOf[LogbackLogger] + val appender = new ListAppender[ILoggingEvent] + appender.setContext(logger.getLoggerContext) + appender.start() + val previousLevel = logger.getLevel + logger.setLevel(level) + logger.addAppender(appender) + try body + finally { + // Restored: the suites of this module share one JVM and this logger is global to it. + logger.detachAppender(appender) + logger.setLevel(previousLevel) + appender.stop() + } + appender.list.asScala.toSeq + } + + "WorkflowCompilingService's request-logging filter" should "forward the request and log one access line at info" in { + val request = mock(classOf[HttpServletRequest]) + when(request.getRemoteAddr).thenReturn("10.0.0.7") + when(request.getMethod).thenReturn("POST") + when(request.getRequestURI).thenReturn("/api/compile") + when(request.getProtocol).thenReturn("HTTP/1.1") + val response = mock(classOf[HttpServletResponse]) + when(response.getStatus).thenReturn(200) + val chain = mock(classOf[FilterChain]) + + val events = eventsLoggedAt(Level.INFO) { + requestLogFilter.doFilter(request, response, chain) + } + + // Forwarding is the filter's first job: without it every request ends here with an empty + // response instead of reaching the compilation endpoint. + verify(chain).doFilter(request, response) + // The level is asserted with the message: access lines have to stay at info so that raising + // TEXERA_SERVICE_LOG_LEVEL silences them. + events.map(event => (event.getLevel, event.getFormattedMessage)) shouldBe Seq( + (Level.INFO, """10.0.0.7 - "POST /api/compile HTTP/1.1" 200""") + ) + } + + it should "forward the request without reading it when info logging is off" in { + val request = mock(classOf[HttpServletRequest]) + val response = mock(classOf[HttpServletResponse]) + val chain = mock(classOf[FilterChain]) + + val events = eventsLoggedAt(Level.WARN) { + requestLogFilter.doFilter(request, response, chain) + } + + verify(chain).doFilter(request, response) + events shouldBe empty + // Logback would drop the info event on its own, so an absent event proves nothing about the + // isInfoEnabled guard. What the guard buys is not paying to build the line at all, which is + // only observable as the request never being interrogated. + verify(request, never()).getRemoteAddr } // Every endpoint this service registers declares @RolesAllowed/@PermitAll/@DenyAll. @@ -85,4 +187,86 @@ class WorkflowCompilingServiceRunSpec extends AnyFlatSpec with Matchers { Seq(classOf[WorkflowCompilationResource], classOf[HealthCheckResource]) ) shouldBe empty } + + // --- initialize() --- + // Dropwizard's Bootstrap only allocates in-memory scaffolding (an object mapper, a metric + // registry, a file-reading configuration source provider), so initialize() can be driven + // against a real one without binding a port or opening a connection. + + private val seededConfigPath = "workflow-compiling-service-spec.yml" + + // The `:-` default form is the only substitution shape this service actually ships: both + // `${...}` in workflow-compiling-service-config.yaml (lines 28 and 30) carry a default. + private val seededConfig = + "defined: ${PATH}\ndefaulted: ${TEXERA_WORKFLOW_COMPILING_SERVICE_SPEC_UNSET:-fallback}\n" + + private val unsetConfigPath = "workflow-compiling-service-spec-unset.yml" + private val unsetConfig = "unset: ${TEXERA_WORKFLOW_COMPILING_SERVICE_SPEC_UNSET}\n" + + /** + * Runs initialize() over a bootstrap that already carries a recognisable in-memory + * configuration source. Seeding it is what makes the wrapping observable: initialize() is + * required to wrap the provider that is already installed, and a version that instead built a + * fresh file-reading provider would fail to find these paths at all. + */ + private def initializedBootstrap(): Bootstrap[WorkflowCompilingServiceConfiguration] = { + val application = new WorkflowCompilingService + val bootstrap = new Bootstrap[WorkflowCompilingServiceConfiguration](application) + bootstrap.setConfigurationSourceProvider(new ConfigurationSourceProvider { + override def open(path: String): InputStream = + path match { + case `seededConfigPath` => new ByteArrayInputStream(seededConfig.getBytes(UTF_8)) + case `unsetConfigPath` => new ByteArrayInputStream(unsetConfig.getBytes(UTF_8)) + case other => throw new FileNotFoundException(other) + } + }) + application.initialize(bootstrap) + bootstrap + } + + private def resolve(path: String): String = + Using.resource(initializedBootstrap().getConfigurationSourceProvider.open(path)) { stream => + new String(stream.readAllBytes(), UTF_8) + } + + "WorkflowCompilingService.initialize" should "substitute environment variables into the configuration source it was handed" in { + // Whole-string rather than a pair of `include`s, so a substitutor that also mangled the rest + // of the document could not pass. The second line is the shape the service's own YAML uses: + // it only resolves while the substitutor keeps commons-text's `:-` value delimiter, and a + // deployment that lost it would hand logback the literal "${TEXERA_SERVICE_LOG_LEVEL:-INFO}" + // as a level. + resolve(seededConfigPath) shouldBe s"defined: ${System.getenv("PATH")}\ndefaulted: fallback\n" + } + + it should "leave a variable with neither a value nor a default as a literal" in { + // The substitutor is built non-strict. A strict one raises UndefinedEnvironmentVariableException + // out of open(), i.e. refuses to boot a deployment whose config names a variable it does not + // set; asserting the resolved text here makes that a stated claim rather than an incidental + // error thrown from the middle of the test above. + resolve(unsetConfigPath) shouldBe "unset: ${TEXERA_WORKFLOW_COMPILING_SERVICE_SPEC_UNSET}\n" + } + + it should "register the Scala module on Dropwizard's object mapper" in { + val mapper = initializedBootstrap().getObjectMapper + // The whole module, not only the Option support that `Some("x")` alone would prove: this is + // the mapper Dropwizard hands to Jersey, so every payload the API returns goes through it — + // including WorkflowCompilationSuccess's Map of Option-valued output schemas. + mapper.getRegisteredModuleIds should contain( + "com.fasterxml.jackson.module.scala.DefaultScalaModule$" + ) + + val payload = SpecPayload("op-1", Map("port0" -> Some(Seq(1, 2)), "port1" -> None)) + // Option unwrapped, Map and Seq emitted as JSON, and the camelCase property names left alone + // (Dropwizard's own naming strategy is annotation-sensitive; a global one would rename them). + val json = mapper.writeValueAsString(payload) + json shouldBe """{"operatorId":"op-1","outputSchemas":{"port0":[1,2],"port1":null}}""" + // Reading, too: this is also the mapper Dropwizard parses the YAML configuration with. + mapper.readValue(json, classOf[SpecPayload]) shouldBe payload + } +} + +object WorkflowCompilingServiceRunSpec { + + /** Stands in for the payloads this service returns: camelCase names, a Map, a Seq and an Option. */ + final case class SpecPayload(operatorId: String, outputSchemas: Map[String, Option[Seq[Int]]]) }
