mengw15 commented on code in PR #7560:
URL: https://github.com/apache/texera/pull/7560#discussion_r3760926138
##########
config-service/src/test/scala/org/apache/texera/service/ConfigServiceRunSpec.scala:
##########
@@ -19,12 +19,135 @@
package org.apache.texera.service
-import org.apache.texera.auth.RoleAnnotationEnforcer
+import io.dropwizard.auth.AuthDynamicFeature
+import io.dropwizard.core.setup.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.dao.{MockTexeraDB, SqlServer}
+import org.apache.texera.dao.jooq.generated.Tables.SITE_SETTINGS
import org.apache.texera.service.resource.{ConfigResource, HealthCheckResource}
+import org.eclipse.jetty.server.session.SessionHandler
+import org.eclipse.jetty.servlet.FilterHolder
+import org.glassfish.jersey.server.filter.RolesAllowedDynamicFeature
+import org.jooq.{ConnectionProvider, SQLDialect}
+import org.jooq.impl.{DSL, DefaultConfiguration}
+import org.mockito.ArgumentMatchers.{any, eq => eqTo, isA}
+import org.mockito.Mockito.{mock, verify, when}
+import org.scalatest.BeforeAndAfterAll
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers
-class ConfigServiceRunSpec extends AnyFlatSpec with Matchers {
+import java.sql.{Connection, SQLException}
+
+// `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
+// SqlServer at it, which lets the whole method — including the request-logging
+// filter installed after the preload — run against mocked Dropwizard wiring.
+class ConfigServiceRunSpec
+ extends AnyFlatSpec
+ with Matchers
+ with BeforeAndAfterAll
+ with MockTexeraDB {
+
+ override protected def beforeAll(): Unit = initializeDBAndReplaceDSLContext()
+
+ override protected def afterAll(): Unit = shutdownDB()
+
+ "ConfigService.run" should "install the API prefix, session handling and its
resources" in {
+ val jersey = mock(classOf[JerseyEnvironment])
+ val servlets = mock(classOf[ServletEnvironment])
+ val context = mock(classOf[MutableServletContextHandler])
+ val env = mock(classOf[Environment])
+ when(env.jersey).thenReturn(jersey)
+ when(env.servlets).thenReturn(servlets)
+ when(env.getApplicationContext).thenReturn(context)
+
when(jersey.getResourceConfig).thenReturn(DropwizardResourceConfig.forTesting())
+
+ new ConfigService().run(mock(classOf[ConfigServiceConfiguration]), env)
+
+ // Everything the service serves lives under /api; losing this silently
moves every
+ // endpoint to the root.
+ verify(jersey).setUrlPattern("/api/*")
+ verify(jersey).register(classOf[SessionHandler])
+ verify(servlets).setSessionHandler(isA(classOf[SessionHandler]))
+ verify(jersey).register(classOf[HealthCheckResource])
+ verify(jersey).register(isA(classOf[ConfigResource]))
+ }
+
+ it should "install the auth stack and the request logging filter" in {
+ val jersey = mock(classOf[JerseyEnvironment])
+ val servlets = mock(classOf[ServletEnvironment])
+ val context = mock(classOf[MutableServletContextHandler])
+ val env = mock(classOf[Environment])
+ when(env.jersey).thenReturn(jersey)
+ when(env.servlets).thenReturn(servlets)
+ when(env.getApplicationContext).thenReturn(context)
+
when(jersey.getResourceConfig).thenReturn(DropwizardResourceConfig.forTesting())
+
+ new ConfigService().run(mock(classOf[ConfigServiceConfiguration]), env)
+
+ // AuthFeatures.register: without these, @Auth parameters do not resolve
and
+ // @RolesAllowed on the settings endpoints is ignored.
+ verify(jersey).register(isA(classOf[AuthDynamicFeature]))
+ verify(jersey).register(classOf[UnauthorizedExceptionMapper])
+ verify(jersey).register(classOf[RolesAllowedDynamicFeature])
+
+ // RequestLoggingFilter.register, which runs only after the preload below
succeeds.
+ verify(context).addFilter(isA(classOf[FilterHolder]), eqTo("/*"), any())
+ }
+
+ it should "preload the default settings into site_settings" in {
+ val jersey = mock(classOf[JerseyEnvironment])
+ val env = mock(classOf[Environment])
+ when(env.jersey).thenReturn(jersey)
+ when(env.servlets).thenReturn(mock(classOf[ServletEnvironment]))
+
when(env.getApplicationContext).thenReturn(mock(classOf[MutableServletContextHandler]))
+
when(jersey.getResourceConfig).thenReturn(DropwizardResourceConfig.forTesting())
+
+ new ConfigService().run(mock(classOf[ConfigServiceConfiguration]), env)
Review Comment:
Confirmed and applied. `MockTexeraDB`'s fixture doesn't truncate (its own
TODO says so) and the two runs above seed the table, so the assertion's
precondition was whatever those left behind. The concrete mutation it missed: a
`run()` that skipped the preload when `site_settings` is non-empty would keep
this green while a fresh deployment got nothing.
The test now clears `SITE_SETTINGS` immediately before the call. Verified
the fix isn't cosmetic: removing the `run()` invocation from this test now
fails it, where before the same mutation still passed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]