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
The following commit(s) were added to refs/heads/main by this push:
new d444fe0fc9 test(computing-unit): cover ComputingUnitAccessResource
endpoints, privilege helpers, and the sharing-disabled path (#6714)
d444fe0fc9 is described below
commit d444fe0fc9beec61db17033d4c600a9677fd31db
Author: Prateek Ganigi <[email protected]>
AuthorDate: Tue Jul 21 17:35:21 2026 -0700
test(computing-unit): cover ComputingUnitAccessResource endpoints,
privilege helpers, and the sharing-disabled path (#6714)
### What changes were proposed in this PR?
This PR expands test coverage for ComputingUnitAccessResource. #6446
introduced ComputingUnitAccessResourceSpec but scoped it to
grantAccess/revokeAccess; the rest of the resource was only exercised
incidentally. This adds direct coverage for the remaining surface, all
backed by embedded Postgres via MockTexeraDB:
getComputingUnitAccessList: empty list, and multiple grantees with
correct email / name / privilege mapping.
Privilege helpers (companion object): isOwner (owner / non-owner /
nonexistent cuid), getPrivilege (no grant -> null, granted -> value),
and hasReadAccess / hasWriteAccess across owner, READ-grantee,
WRITE-grantee, and no-grant users.
getOwner: happy path (returns the owner's email). The nonexistent-unit
-> 404 case is already covered by #6475.
Sharing-disabled branch: a new ComputingUnitAccessSharingDisabledSpec
asserting every endpoint returns 403 (ForbiddenException) when sharing
is off.
No production code changes: test-only, plus one build.sbt test-config
change described below.
### Any related issues, documentation, discussions?
Closes #6490. Builds on the test infrastructure added in #6446 and the
getOwner 404 fix in #6475.
### How was this PR tested?
sbt "ComputingUnitManagingService/test": 40 tests across all 6 module
suites pass; no existing suite regressed.
The sharing-disabled path needs special handling:
ComputingUnitConfig.sharingComputingUnitEnabled is resolved once as a
load-time val from the COMPUTING_UNIT_SHARING_ENABLED env var, and the
module's test JVM sets it to true (needed by the sharing-enabled
suites). So the disabled branch can only be exercised in a JVM where
that var is absent. To do that without disturbing the other suites,
build.sbt adds a Test / testGrouping that keeps all sharing-enabled
suites in the existing single forked JVM and isolates any
*SharingDisabledSpec into its own forked JVM with the env var removed.
Each spec asserts the resolved flag (sharingComputingUnitEnabled
shouldBe true / false) as a first case, so a misconfigured grouping
fails loudly instead of silently passing.
### Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.8 in compliance with ASF.
---------
Signed-off-by: Xinyuan Lin <[email protected]>
Co-authored-by: Xinyuan Lin <[email protected]>
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
---
build.sbt | 17 +++-
.../resource/ComputingUnitAccessResourceSpec.scala | 102 +++++++++++++++++++--
.../ComputingUnitAccessSharingDisabledSpec.scala | 99 ++++++++++++++++++++
3 files changed, 208 insertions(+), 10 deletions(-)
diff --git a/build.sbt b/build.sbt
index 479bfe4631..b1ab4b6db3 100644
--- a/build.sbt
+++ b/build.sbt
@@ -183,7 +183,22 @@ lazy val ComputingUnitManagingService = (project in
file("computing-unit-managin
Test / fork := true,
Test / envVars += "COMPUTING_UNIT_SHARING_ENABLED" -> "true",
Test / forkOptions := (Test / forkOptions).value
- .withWorkingDirectory((ThisBuild / baseDirectory).value)
+ .withWorkingDirectory((ThisBuild / baseDirectory).value),
+ // Isolate the sharing-disabled suite into its own forked JVM without
+ // COMPUTING_UNIT_SHARING_ENABLED: the config flag is a load-time val, so
the disabled
+ // branch can only be exercised where the env var is absent (not merely
unset per-test).
+ // All other suites keep running together in one forked JVM (the
pre-grouping default).
+ Test / testGrouping := {
+ val opts = (Test / forkOptions).value
+ val (disabled, enabled) =
+ (Test /
definedTests).value.partition(_.name.endsWith("SharingDisabledSpec"))
+ val enabledGroup = Tests.Group("sharing-enabled", enabled,
Tests.SubProcess(opts))
+ val disabledGroups = disabled.map { suite =>
+ val disabledOpts = opts.withEnvVars(opts.envVars -
"COMPUTING_UNIT_SHARING_ENABLED")
+ Tests.Group(suite.name, Seq(suite), Tests.SubProcess(disabledOpts))
+ }
+ enabledGroup +: disabledGroups
+ }
)
lazy val FileService = (project in file("file-service"))
.settings(commonModuleSettings)
diff --git
a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessResourceSpec.scala
b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessResourceSpec.scala
index 7637c99d26..4b5c78654e 100644
---
a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessResourceSpec.scala
+++
b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessResourceSpec.scala
@@ -29,23 +29,31 @@ import org.apache.texera.dao.jooq.generated.enums.{
UserRoleEnum,
WorkflowComputingUnitTypeEnum
}
-import org.apache.texera.dao.jooq.generated.tables.daos.{UserDao,
WorkflowComputingUnitDao}
-import org.apache.texera.dao.jooq.generated.tables.pojos.{User,
WorkflowComputingUnit}
+import org.apache.texera.dao.jooq.generated.tables.daos.{
+ ComputingUnitUserAccessDao,
+ UserDao,
+ WorkflowComputingUnitDao
+}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{
+ ComputingUnitUserAccess,
+ User,
+ WorkflowComputingUnit
+}
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers
import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
/**
- * Spec for [[ComputingUnitAccessResource]]'s share/revoke endpoints, backed
by an
- * embedded Postgres (via [[MockTexeraDB]]).
+ * Spec for [[ComputingUnitAccessResource]] with sharing ENABLED, backed by
an embedded
+ * Postgres (via [[MockTexeraDB]]). Covers the endpoints — grantAccess,
revokeAccess,
+ * getComputingUnitAccessList, getOwner — and the companion-object privilege
helpers
+ * (isOwner / getPrivilege / hasReadAccess / hasWriteAccess).
*
* The suite runs with COMPUTING_UNIT_SHARING_ENABLED=true (set in
build.sbt), which
- * `ensureSharingIsEnabled()` requires; this is asserted below so a missing
env var
+ * `ensureSharingIsEnabled()` requires; the first case asserts this so a
missing env var
* fails loudly instead of silently short-circuiting every case with a
ForbiddenException.
- *
- * The key regression these tests guard: granting/revoking to an unknown
email must
- * surface a clear IllegalArgumentException, not a NullPointerException
(500), because
- * `userDao.fetchOneByEmail` returns null for an address with no account.
+ * The sharing-DISABLED branch is covered separately by
ComputingUnitAccessSharingDisabledSpec,
+ * which forks without that env var (the flag is a load-time val).
*/
class ComputingUnitAccessResourceSpec
extends AnyFlatSpec
@@ -102,6 +110,15 @@ class ComputingUnitAccessResourceSpec
private def accessEmails(cuid: Integer): List[String] =
accessResource.getComputingUnitAccessList(ownerSession, cuid).map(_.email)
+ /** Inserts an access row directly, bypassing the grant endpoint, to set up
helper tests. */
+ private def grantDirectly(uid: Integer, privilege: PrivilegeEnum): Unit = {
+ val access = new ComputingUnitUserAccess
+ access.setCuid(cuid)
+ access.setUid(uid)
+ access.setPrivilege(privilege)
+ new
ComputingUnitUserAccessDao(getDSLContext.configuration()).insert(access)
+ }
+
override protected def beforeAll(): Unit = {
super.beforeAll()
initializeDBAndReplaceDSLContext()
@@ -203,6 +220,69 @@ class ComputingUnitAccessResourceSpec
ex.getMessage should include("does not have permission to revoke access")
}
+ //
===========================================================================
+ // getComputingUnitAccessList
+ //
===========================================================================
+
+ "getComputingUnitAccessList" should "return an empty list when nothing is
granted" in {
+ accessResource.getComputingUnitAccessList(ownerSession, cuid) shouldBe
empty
+ }
+
+ it should "list every grantee with their email, name, and privilege" in {
+ grantDirectly(granteeUser.getUid, PrivilegeEnum.READ)
+ grantDirectly(strangerUser.getUid, PrivilegeEnum.WRITE)
+
+ val entries = accessResource.getComputingUnitAccessList(ownerSession, cuid)
+ entries should have size 2
+
+ val byEmail = entries.map(entry => entry.email -> entry).toMap
+ byEmail(granteeUser.getEmail).name shouldEqual granteeUser.getName
+ byEmail(granteeUser.getEmail).privilege shouldEqual PrivilegeEnum.READ
+ byEmail(strangerUser.getEmail).privilege shouldEqual PrivilegeEnum.WRITE
+ }
+
+ //
===========================================================================
+ // Privilege helpers (companion object)
+ //
===========================================================================
+
+ "isOwner" should "be true only for the owner of an existing unit" in {
+ ComputingUnitAccessResource.isOwner(cuid, ownerUser.getUid) shouldBe true
+ ComputingUnitAccessResource.isOwner(cuid, strangerUser.getUid) shouldBe
false
+ ComputingUnitAccessResource.isOwner(nonExistentCuid, ownerUser.getUid)
shouldBe false
+ }
+
+ "getPrivilege" should "return null without a grant and the granted privilege
otherwise" in {
+ ComputingUnitAccessResource.getPrivilege(cuid, strangerUser.getUid)
shouldBe null
+
+ grantDirectly(granteeUser.getUid, PrivilegeEnum.READ)
+ ComputingUnitAccessResource.getPrivilege(
+ cuid,
+ granteeUser.getUid
+ ) shouldEqual PrivilegeEnum.READ
+ }
+
+ "the owner" should "have both read and write access" in {
+ ComputingUnitAccessResource.hasReadAccess(cuid, ownerUser.getUid) shouldBe
true
+ ComputingUnitAccessResource.hasWriteAccess(cuid, ownerUser.getUid)
shouldBe true
+ }
+
+ "a READ grantee" should "have read but not write access" in {
+ grantDirectly(granteeUser.getUid, PrivilegeEnum.READ)
+ ComputingUnitAccessResource.hasReadAccess(cuid, granteeUser.getUid)
shouldBe true
+ ComputingUnitAccessResource.hasWriteAccess(cuid, granteeUser.getUid)
shouldBe false
+ }
+
+ "a WRITE grantee" should "have both read and write access" in {
+ grantDirectly(granteeUser.getUid, PrivilegeEnum.WRITE)
+ ComputingUnitAccessResource.hasReadAccess(cuid, granteeUser.getUid)
shouldBe true
+ ComputingUnitAccessResource.hasWriteAccess(cuid, granteeUser.getUid)
shouldBe true
+ }
+
+ "a user with no grant" should "have neither read nor write access" in {
+ ComputingUnitAccessResource.hasReadAccess(cuid, strangerUser.getUid)
shouldBe false
+ ComputingUnitAccessResource.hasWriteAccess(cuid, strangerUser.getUid)
shouldBe false
+ }
+
//
===========================================================================
// getOwner
//
===========================================================================
@@ -214,4 +294,8 @@ class ComputingUnitAccessResourceSpec
ex.getResponse.getStatus shouldEqual 404
ex.getMessage should include(s"Computing unit with cuid=$nonExistentCuid
does not exist")
}
+
+ it should "return the owner's email for an existing unit" in {
+ accessResource.getOwner(ownerSession, cuid) shouldEqual ownerUser.getEmail
+ }
}
diff --git
a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessSharingDisabledSpec.scala
b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessSharingDisabledSpec.scala
new file mode 100644
index 0000000000..675ad5ccdd
--- /dev/null
+++
b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessSharingDisabledSpec.scala
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.service.resource
+
+import jakarta.ws.rs.ForbiddenException
+import org.apache.texera.auth.SessionUser
+import org.apache.texera.common.config.ComputingUnitConfig
+import org.apache.texera.dao.MockTexeraDB
+import org.apache.texera.dao.jooq.generated.enums.{PrivilegeEnum, UserRoleEnum}
+import org.apache.texera.dao.jooq.generated.tables.pojos.User
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+/**
+ * Spec for [[ComputingUnitAccessResource]] when the sharing feature is
DISABLED.
+ *
+ * `ComputingUnitConfig.sharingComputingUnitEnabled` is a load-time val
resolved from the
+ * COMPUTING_UNIT_SHARING_ENABLED env var, so the disabled branch must be
exercised in a
+ * forked JVM where that var resolves to false (build.sbt's `Test /
testGrouping` isolates any
+ * suite whose name ends with "SharingDisabledSpec" and forces the flag off
there).
+ * The first assertion guards that grouping — if it ever stops applying, this
suite fails loudly
+ * rather than silently passing against a sharing-enabled JVM.
+ */
+class ComputingUnitAccessSharingDisabledSpec
+ extends AnyFlatSpec
+ with Matchers
+ with MockTexeraDB
+ with BeforeAndAfterAll {
+
+ private val user: User = {
+ val u = new User
+ u.setName("cu_user")
+ u.setPassword("123")
+ u.setEmail("[email protected]")
+ u.setRole(UserRoleEnum.REGULAR)
+ u
+ }
+
+ // Access resource construction needs an initialized SqlServer; the DB is
otherwise unused
+ // here because ensureSharingIsEnabled() throws before any query runs.
+ lazy val accessResource = new ComputingUnitAccessResource()
+ lazy val session = new SessionUser(user)
+
+ private val cuid: Integer = 1
+
+ override protected def beforeAll(): Unit = {
+ super.beforeAll()
+ initializeDBAndReplaceDSLContext()
+ }
+
+ override protected def afterAll(): Unit = {
+ try shutdownDB()
+ finally super.afterAll()
+ }
+
+ private def expectForbidden(call: => Any): Unit = {
+ val ex = intercept[ForbiddenException](call)
+ ex.getResponse.getStatus shouldEqual 403
+ ex.getMessage should include("sharing feature is disabled")
+ }
+
+ "the test environment" should "have computing-unit sharing disabled" in {
+ ComputingUnitConfig.sharingComputingUnitEnabled shouldBe false
+ }
+
+ "grantAccess" should "be forbidden (403) when sharing is disabled" in {
+ expectForbidden(accessResource.grantAccess(session, cuid, user.getEmail,
PrivilegeEnum.READ))
+ }
+
+ "revokeAccess" should "be forbidden (403) when sharing is disabled" in {
+ expectForbidden(accessResource.revokeAccess(session, cuid, user.getEmail))
+ }
+
+ "getComputingUnitAccessList" should "be forbidden (403) when sharing is
disabled" in {
+ expectForbidden(accessResource.getComputingUnitAccessList(session, cuid))
+ }
+
+ "getOwner" should "be forbidden (403) when sharing is disabled" in {
+ expectForbidden(accessResource.getOwner(session, cuid))
+ }
+}
