From 9fd8d7c3d16ab0e6b9bda48e30f23d5957bceb77 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Wed, 10 Jun 2026 20:53:32 -0700 Subject: [PATCH 1/4] added a notification for when users don't follow the appropriate template in issues or prs --- .github/template-compliance-warning.txt | 9 + .../workflows/template-compliance-warning.yml | 195 ++++++++++++++++++ 2 files changed, 204 insertions(+) create mode 100644 .github/template-compliance-warning.txt create mode 100644 .github/workflows/template-compliance-warning.yml diff --git a/.github/template-compliance-warning.txt b/.github/template-compliance-warning.txt new file mode 100644 index 00000000000..b0f9272b63e --- /dev/null +++ b/.github/template-compliance-warning.txt @@ -0,0 +1,9 @@ +👋 Thanks for opening this {{kind}}, @{{author}}! + +It looks like the {{kind}} description doesn't quite follow our template yet: + +{{details}} + +Filling out the template helps reviewers understand and triage your contribution faster. Please edit the description to complete it. This message will disappear automatically once the template is followed. + +You can find the template prompts by editing the description, or see [CONTRIBUTING.md](https://github.com/{{owner}}/{{repo}}/blob/main/CONTRIBUTING.md) for the full contribution flow. diff --git a/.github/workflows/template-compliance-warning.yml b/.github/workflows/template-compliance-warning.yml new file mode 100644 index 00000000000..6ca71616b66 --- /dev/null +++ b/.github/workflows/template-compliance-warning.yml @@ -0,0 +1,195 @@ +# 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. + +# Post a non-blocking warning when a pull request (or issue) is opened +# without following our template, and clear it automatically once the +# author fixes the description. +# +# Designed to be cheap on CI: +# * Single `github-script` job, no build, no full checkout (only a +# sparse-checkout of the one message .txt file), so a run is a few +# seconds of an ubuntu-latest runner. +# * Triggers only on `opened` / `edited`, never on `synchronize`, so +# it does NOT run on every push to a PR branch. +# * Skips drafts and bots, so WIP and automation don't get nagged. +# * Posts a single sticky comment (idempotency marker) that is +# UPDATED in place while the template is incomplete and DELETED once +# it is followed, so it never piles up duplicate comments. +# +# Issue templates here are GitHub form (`.yaml`) templates whose +# required fields are already enforced at submission time, so for issues +# this only catches a fully blank body (e.g. a blank issue). PR +# templates cannot be enforced by GitHub, which is the main case this +# covers. +# +# Uses `pull_request_target` so PRs from forks are still checked. +# A `pull_request` run from a fork gets a read-only token and could not +# comment. +name: Template compliance warning +on: + issues: + types: [opened, edited] + pull_request_target: + types: [opened, edited] + +permissions: + issues: write + pull-requests: write + +jobs: + check-template: + if: github.event.sender.type != 'Bot' + runs-on: ubuntu-latest + steps: + # Check out only the warning message template. `pull_request_target` + # and `issues` both resolve to the trusted base branch (never the + # fork head), so reading this file is safe. Keeping the wording in a + # .txt file means editing the message does not touch workflow logic. + - uses: actions/checkout@v5 + with: + persist-credentials: false + sparse-checkout: .github/template-compliance-warning.txt + sparse-checkout-cone-mode: false + - uses: actions/github-script@v8 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const isPR = context.eventName === 'pull_request_target'; + const subject = isPR + ? context.payload.pull_request + : context.payload.issue; + + // Drafts are work-in-progress; don't nag until they're ready. + if (isPR && subject.draft) { + core.info(`#${subject.number} is a draft; skipping.`); + return; + } + + const author = subject.user.login; + const issue_number = subject.number; + const kind = isPR ? 'pull request' : 'issue'; + const { owner, repo } = context.repo; + const body = subject.body || ''; + + // Strip HTML comments (the template's guidance) + // before judging whether a section actually has content. + const stripped = body.replace(//g, ''); + + // Build the list of problems with the description. Each entry + // is a user-facing bullet. An empty list means "compliant". + const problems = []; + + if (stripped.trim().length === 0) { + problems.push( + `The description is empty. Please open the ${kind} using ` + + `the provided template and fill it out.`, + ); + } else if (isPR) { + // PR template required sections (headings copied verbatim + // from .github/PULL_REQUEST_TEMPLATE). For each, capture the + // text from its heading to the next "### " heading (or end) + // and treat whitespace-only as not filled in. + const REQUIRED_SECTIONS = [ + 'What changes were proposed in this PR?', + 'How was this PR tested?', + 'Was this PR authored or co-authored using generative AI tooling?', + ]; + for (const heading of REQUIRED_SECTIONS) { + // Escape regex metacharacters in the heading text. + const esc = heading.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + // The trailing `(?![\s\S])` is the end-of-string case (JS + // regex has no `\Z`); with the `m` flag a bare `$` would + // match every line end, not just the end of the body. + const re = new RegExp( + `^#{1,6}\\s*${esc}\\s*$([\\s\\S]*?)(?=^#{1,6}\\s|(?![\\s\\S]))`, + 'm', + ); + const m = stripped.match(re); + if (!m) { + problems.push( + `The **${heading}** section is missing; please keep ` + + `the template's headings.`, + ); + } else if (m[1].trim().length === 0) { + problems.push( + `The **${heading}** section is empty; please fill it in.`, + ); + } + } + } + + const MARKER = ''; + + // Find a previous warning comment from this workflow. + let existing = null; + try { + const comments = await github.paginate( + github.rest.issues.listComments, + { owner, repo, issue_number, per_page: 100 }, + ); + existing = comments.find((c) => (c.body || '').includes(MARKER)); + } catch (e) { + core.warning(`listComments on #${issue_number} failed: ${e.message}`); + // Without the comment list we can't safely de-dupe; bail to + // avoid posting a duplicate warning. + return; + } + + // Compliant now: remove any stale warning and stop. + if (problems.length === 0) { + core.info(`#${issue_number} follows the template.`); + if (existing) { + try { + await github.rest.issues.deleteComment({ + owner, repo, comment_id: existing.id, + }); + core.info(`Cleared resolved warning on #${issue_number}.`); + } catch (e) { + core.warning(`Failed to delete warning: ${e.message}`); + } + } + return; + } + + // Not compliant: render the message and post/update the sticky + // comment. + const fs = require('fs'); + const template = fs.readFileSync( + '.github/template-compliance-warning.txt', 'utf8', + ); + const details = problems.map((p) => `- ${p}`).join('\n'); + const message = MARKER + '\n' + template + .replaceAll('{{author}}', author) + .replaceAll('{{owner}}', owner) + .replaceAll('{{repo}}', repo) + .replaceAll('{{kind}}', kind) + .replaceAll('{{details}}', details); + + try { + if (existing) { + await github.rest.issues.updateComment({ + owner, repo, comment_id: existing.id, body: message, + }); + core.info(`Updated template warning on #${issue_number}.`); + } else { + await github.rest.issues.createComment({ + owner, repo, issue_number, body: message, + }); + core.info(`Posted template warning on #${issue_number}.`); + } + } catch (e) { + core.warning(`Failed to post warning on #${issue_number}: ${e.message}`); + } From 8f2cfbd2137f7c0a5def943acd6d46d421e80b43 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Thu, 11 Jun 2026 02:15:52 -0700 Subject: [PATCH 2/4] changed issues to cover more templates --- .../workflows/template-compliance-warning.yml | 74 +++++++++++++------ 1 file changed, 53 insertions(+), 21 deletions(-) diff --git a/.github/workflows/template-compliance-warning.yml b/.github/workflows/template-compliance-warning.yml index 6ca71616b66..82b8e87cf26 100644 --- a/.github/workflows/template-compliance-warning.yml +++ b/.github/workflows/template-compliance-warning.yml @@ -29,11 +29,11 @@ # UPDATED in place while the template is incomplete and DELETED once # it is followed, so it never piles up duplicate comments. # -# Issue templates here are GitHub form (`.yaml`) templates whose -# required fields are already enforced at submission time, so for issues -# this only catches a fully blank body (e.g. a blank issue). PR -# templates cannot be enforced by GitHub, which is the main case this -# covers. +# Issues are matched to their template by GitHub issue type +# (Bug/Feature/Task) and checked against that template's `required: true` +# fields. Because every template sets a type, an issue with no recognized +# type is flagged outright as not using a template. PR templates cannot +# be enforced by GitHub, which is the main case this covers. # # Uses `pull_request_target` so PRs from forks are still checked. # A `pull_request` run from a fork gets a read-only token and could not @@ -88,26 +88,55 @@ jobs: // before judging whether a section actually has content. const stripped = body.replace(//g, ''); + // Pick the required sections for whichever template applies. + // PRs use the single PR template. Issues are matched by their + // GitHub issue type (set by the form template the author + // chose), so each issue is checked against the right + // template's fields. Only fields marked `required: true` in + // the templates are listed here. + const PR_SECTIONS = [ + 'What changes were proposed in this PR?', + 'How was this PR tested?', + 'Was this PR authored or co-authored using generative AI tooling?', + ]; + const ISSUE_SECTIONS = { + Bug: ['What happened?', 'How to reproduce?', 'Version/Branch'], + Feature: ['Feature Summary', 'Proposed Solution or Design'], + Task: ['Task Summary'], + }; + let requiredSections = null; + if (isPR) { + requiredSections = PR_SECTIONS; + } else { + const typeName = subject.type && subject.type.name; + requiredSections = ISSUE_SECTIONS[typeName] || null; + } + // Build the list of problems with the description. Each entry // is a user-facing bullet. An empty list means "compliant". const problems = []; - if (stripped.trim().length === 0) { + if (!isPR && !requiredSections) { + // All our issue templates set an issue type, so a missing or + // unrecognized type means no template was used (e.g. a blank + // issue). Flag it outright. + problems.push( + `This ${kind} doesn't appear to use one of our templates ` + + `(Bug, Feature, or Task). Please open it using a template ` + + `so the required details are captured.`, + ); + } else if (stripped.trim().length === 0) { problems.push( `The description is empty. Please open the ${kind} using ` + `the provided template and fill it out.`, ); - } else if (isPR) { - // PR template required sections (headings copied verbatim - // from .github/PULL_REQUEST_TEMPLATE). For each, capture the - // text from its heading to the next "### " heading (or end) - // and treat whitespace-only as not filled in. - const REQUIRED_SECTIONS = [ - 'What changes were proposed in this PR?', - 'How was this PR tested?', - 'Was this PR authored or co-authored using generative AI tooling?', - ]; - for (const heading of REQUIRED_SECTIONS) { + } else { + // PR, or an issue with a recognized type: check each required + // section. Capture the text from its heading to the next + // heading (or end of body), treating a blank value or + // GitHub's "_No response_" placeholder (shown for an empty + // field) as not filled in. + for (const heading of requiredSections) { // Escape regex metacharacters in the heading text. const esc = heading.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // The trailing `(?![\s\S])` is the end-of-string case (JS @@ -123,10 +152,13 @@ jobs: `The **${heading}** section is missing; please keep ` + `the template's headings.`, ); - } else if (m[1].trim().length === 0) { - problems.push( - `The **${heading}** section is empty; please fill it in.`, - ); + } else { + const content = m[1].trim(); + if (content.length === 0 || content === '_No response_') { + problems.push( + `The **${heading}** section is empty; please fill it in.`, + ); + } } } } From fb92119168e52664acf40b33c86e3b57cb09d8f7 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Fri, 12 Jun 2026 12:52:39 -0700 Subject: [PATCH 3/4] =?UTF-8?q?=E2=97=8F=20perf(auth):=20collapse=20N+1=20?= =?UTF-8?q?in=20getComputingUnitAccess=20=20=20into=20a=20single=20LEFT=20?= =?UTF-8?q?JOIN?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the two-DAO lookup (fetch unit, then fetch all of the user's access rows and filter in memory) with one LEFT JOIN returning the unit owner and the caller's privilege for this cuid. Missing unit now resolves to NONE instead of throwing; the sole caller maps both to FORBIDDEN. --- .../auth/util/ComputingUnitAccess.scala | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/common/auth/src/main/scala/org/apache/texera/auth/util/ComputingUnitAccess.scala b/common/auth/src/main/scala/org/apache/texera/auth/util/ComputingUnitAccess.scala index 743ed2e7e6a..cbb9b0c72bd 100644 --- a/common/auth/src/main/scala/org/apache/texera/auth/util/ComputingUnitAccess.scala +++ b/common/auth/src/main/scala/org/apache/texera/auth/util/ComputingUnitAccess.scala @@ -18,15 +18,13 @@ package org.apache.texera.auth.util import org.apache.texera.dao.SqlServer -import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum -import org.apache.texera.dao.jooq.generated.tables.daos.{ - ComputingUnitUserAccessDao, - WorkflowComputingUnitDao +import org.apache.texera.dao.jooq.generated.Tables.{ + COMPUTING_UNIT_USER_ACCESS, + WORKFLOW_COMPUTING_UNIT } +import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum import org.jooq.DSLContext -import scala.jdk.CollectionConverters._ - object ComputingUnitAccess { private def context: DSLContext = SqlServer @@ -34,22 +32,25 @@ object ComputingUnitAccess { .createDSLContext() def getComputingUnitAccess(cuid: Integer, uid: Integer): PrivilegeEnum = { - val workflowComputingUnitDao = new WorkflowComputingUnitDao(context.configuration()) - val unit = workflowComputingUnitDao.fetchOneByCuid(cuid) - - if (unit.getUid.equals(uid)) { - return PrivilegeEnum.WRITE // owner has write access - } - - val computingUnitUserAccessDao = new ComputingUnitUserAccessDao(context.configuration()) - val accessOpt = computingUnitUserAccessDao - .fetchByUid(uid) - .asScala - .find(_.getCuid.equals(cuid)) - - accessOpt match { - case Some(access) => access.getPrivilege - case None => PrivilegeEnum.NONE + // owner uid + caller's access privilege in one query + val record = context + .select(WORKFLOW_COMPUTING_UNIT.UID, COMPUTING_UNIT_USER_ACCESS.PRIVILEGE) + .from(WORKFLOW_COMPUTING_UNIT) + .leftJoin(COMPUTING_UNIT_USER_ACCESS) + .on( + COMPUTING_UNIT_USER_ACCESS.CUID + .eq(WORKFLOW_COMPUTING_UNIT.CUID) + .and(COMPUTING_UNIT_USER_ACCESS.UID.eq(uid)) + ) + .where(WORKFLOW_COMPUTING_UNIT.CUID.eq(cuid)) + .fetchOne() + + if (record == null) { + PrivilegeEnum.NONE // no such unit + } else if (record.value1().equals(uid)) { + PrivilegeEnum.WRITE // owner + } else { + Option(record.value2()).getOrElse(PrivilegeEnum.NONE) } } } From 9e17a98bf7042e0334afa83d8db023d889dc9dda Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Fri, 12 Jun 2026 18:36:19 -0700 Subject: [PATCH 4/4] removed unrelated changes and added test cases --- .github/template-compliance-warning.txt | 9 - .../workflows/template-compliance-warning.yml | 227 ------------------ build.sbt | 2 + .../auth/util/ComputingUnitAccess.scala | 3 +- .../auth/util/ComputingUnitAccessSpec.scala | 105 ++++++++ 5 files changed, 109 insertions(+), 237 deletions(-) delete mode 100644 .github/template-compliance-warning.txt delete mode 100644 .github/workflows/template-compliance-warning.yml create mode 100644 common/auth/src/test/scala/org/apache/texera/auth/util/ComputingUnitAccessSpec.scala diff --git a/.github/template-compliance-warning.txt b/.github/template-compliance-warning.txt deleted file mode 100644 index b0f9272b63e..00000000000 --- a/.github/template-compliance-warning.txt +++ /dev/null @@ -1,9 +0,0 @@ -👋 Thanks for opening this {{kind}}, @{{author}}! - -It looks like the {{kind}} description doesn't quite follow our template yet: - -{{details}} - -Filling out the template helps reviewers understand and triage your contribution faster. Please edit the description to complete it. This message will disappear automatically once the template is followed. - -You can find the template prompts by editing the description, or see [CONTRIBUTING.md](https://github.com/{{owner}}/{{repo}}/blob/main/CONTRIBUTING.md) for the full contribution flow. diff --git a/.github/workflows/template-compliance-warning.yml b/.github/workflows/template-compliance-warning.yml deleted file mode 100644 index 82b8e87cf26..00000000000 --- a/.github/workflows/template-compliance-warning.yml +++ /dev/null @@ -1,227 +0,0 @@ -# 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. - -# Post a non-blocking warning when a pull request (or issue) is opened -# without following our template, and clear it automatically once the -# author fixes the description. -# -# Designed to be cheap on CI: -# * Single `github-script` job, no build, no full checkout (only a -# sparse-checkout of the one message .txt file), so a run is a few -# seconds of an ubuntu-latest runner. -# * Triggers only on `opened` / `edited`, never on `synchronize`, so -# it does NOT run on every push to a PR branch. -# * Skips drafts and bots, so WIP and automation don't get nagged. -# * Posts a single sticky comment (idempotency marker) that is -# UPDATED in place while the template is incomplete and DELETED once -# it is followed, so it never piles up duplicate comments. -# -# Issues are matched to their template by GitHub issue type -# (Bug/Feature/Task) and checked against that template's `required: true` -# fields. Because every template sets a type, an issue with no recognized -# type is flagged outright as not using a template. PR templates cannot -# be enforced by GitHub, which is the main case this covers. -# -# Uses `pull_request_target` so PRs from forks are still checked. -# A `pull_request` run from a fork gets a read-only token and could not -# comment. -name: Template compliance warning -on: - issues: - types: [opened, edited] - pull_request_target: - types: [opened, edited] - -permissions: - issues: write - pull-requests: write - -jobs: - check-template: - if: github.event.sender.type != 'Bot' - runs-on: ubuntu-latest - steps: - # Check out only the warning message template. `pull_request_target` - # and `issues` both resolve to the trusted base branch (never the - # fork head), so reading this file is safe. Keeping the wording in a - # .txt file means editing the message does not touch workflow logic. - - uses: actions/checkout@v5 - with: - persist-credentials: false - sparse-checkout: .github/template-compliance-warning.txt - sparse-checkout-cone-mode: false - - uses: actions/github-script@v8 - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - const isPR = context.eventName === 'pull_request_target'; - const subject = isPR - ? context.payload.pull_request - : context.payload.issue; - - // Drafts are work-in-progress; don't nag until they're ready. - if (isPR && subject.draft) { - core.info(`#${subject.number} is a draft; skipping.`); - return; - } - - const author = subject.user.login; - const issue_number = subject.number; - const kind = isPR ? 'pull request' : 'issue'; - const { owner, repo } = context.repo; - const body = subject.body || ''; - - // Strip HTML comments (the template's guidance) - // before judging whether a section actually has content. - const stripped = body.replace(//g, ''); - - // Pick the required sections for whichever template applies. - // PRs use the single PR template. Issues are matched by their - // GitHub issue type (set by the form template the author - // chose), so each issue is checked against the right - // template's fields. Only fields marked `required: true` in - // the templates are listed here. - const PR_SECTIONS = [ - 'What changes were proposed in this PR?', - 'How was this PR tested?', - 'Was this PR authored or co-authored using generative AI tooling?', - ]; - const ISSUE_SECTIONS = { - Bug: ['What happened?', 'How to reproduce?', 'Version/Branch'], - Feature: ['Feature Summary', 'Proposed Solution or Design'], - Task: ['Task Summary'], - }; - let requiredSections = null; - if (isPR) { - requiredSections = PR_SECTIONS; - } else { - const typeName = subject.type && subject.type.name; - requiredSections = ISSUE_SECTIONS[typeName] || null; - } - - // Build the list of problems with the description. Each entry - // is a user-facing bullet. An empty list means "compliant". - const problems = []; - - if (!isPR && !requiredSections) { - // All our issue templates set an issue type, so a missing or - // unrecognized type means no template was used (e.g. a blank - // issue). Flag it outright. - problems.push( - `This ${kind} doesn't appear to use one of our templates ` + - `(Bug, Feature, or Task). Please open it using a template ` + - `so the required details are captured.`, - ); - } else if (stripped.trim().length === 0) { - problems.push( - `The description is empty. Please open the ${kind} using ` + - `the provided template and fill it out.`, - ); - } else { - // PR, or an issue with a recognized type: check each required - // section. Capture the text from its heading to the next - // heading (or end of body), treating a blank value or - // GitHub's "_No response_" placeholder (shown for an empty - // field) as not filled in. - for (const heading of requiredSections) { - // Escape regex metacharacters in the heading text. - const esc = heading.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - // The trailing `(?![\s\S])` is the end-of-string case (JS - // regex has no `\Z`); with the `m` flag a bare `$` would - // match every line end, not just the end of the body. - const re = new RegExp( - `^#{1,6}\\s*${esc}\\s*$([\\s\\S]*?)(?=^#{1,6}\\s|(?![\\s\\S]))`, - 'm', - ); - const m = stripped.match(re); - if (!m) { - problems.push( - `The **${heading}** section is missing; please keep ` + - `the template's headings.`, - ); - } else { - const content = m[1].trim(); - if (content.length === 0 || content === '_No response_') { - problems.push( - `The **${heading}** section is empty; please fill it in.`, - ); - } - } - } - } - - const MARKER = ''; - - // Find a previous warning comment from this workflow. - let existing = null; - try { - const comments = await github.paginate( - github.rest.issues.listComments, - { owner, repo, issue_number, per_page: 100 }, - ); - existing = comments.find((c) => (c.body || '').includes(MARKER)); - } catch (e) { - core.warning(`listComments on #${issue_number} failed: ${e.message}`); - // Without the comment list we can't safely de-dupe; bail to - // avoid posting a duplicate warning. - return; - } - - // Compliant now: remove any stale warning and stop. - if (problems.length === 0) { - core.info(`#${issue_number} follows the template.`); - if (existing) { - try { - await github.rest.issues.deleteComment({ - owner, repo, comment_id: existing.id, - }); - core.info(`Cleared resolved warning on #${issue_number}.`); - } catch (e) { - core.warning(`Failed to delete warning: ${e.message}`); - } - } - return; - } - - // Not compliant: render the message and post/update the sticky - // comment. - const fs = require('fs'); - const template = fs.readFileSync( - '.github/template-compliance-warning.txt', 'utf8', - ); - const details = problems.map((p) => `- ${p}`).join('\n'); - const message = MARKER + '\n' + template - .replaceAll('{{author}}', author) - .replaceAll('{{owner}}', owner) - .replaceAll('{{repo}}', repo) - .replaceAll('{{kind}}', kind) - .replaceAll('{{details}}', details); - - try { - if (existing) { - await github.rest.issues.updateComment({ - owner, repo, comment_id: existing.id, body: message, - }); - core.info(`Updated template warning on #${issue_number}.`); - } else { - await github.rest.issues.createComment({ - owner, repo, issue_number, body: message, - }); - core.info(`Posted template warning on #${issue_number}.`); - } - } catch (e) { - core.warning(`Failed to post warning on #${issue_number}: ${e.message}`); - } diff --git a/build.sbt b/build.sbt index 43f97d9dfff..3fac5bd0971 100644 --- a/build.sbt +++ b/build.sbt @@ -66,7 +66,9 @@ lazy val DAO = (project in file("common/dao")).settings(asfLicensingSettings) lazy val Config = (project in file("common/config")).settings(asfLicensingSettings) lazy val Auth = (project in file("common/auth")) .settings(asfLicensingSettings) + .configs(Test) .dependsOn(DAO, Config) + .dependsOn(DAO % "test->test") // reuse MockTexeraDB embedded Postgres in tests lazy val ConfigService = (project in file("config-service")) .dependsOn(Auth, Config) .settings(asfLicensingSettings) diff --git a/common/auth/src/main/scala/org/apache/texera/auth/util/ComputingUnitAccess.scala b/common/auth/src/main/scala/org/apache/texera/auth/util/ComputingUnitAccess.scala index cbb9b0c72bd..b999cb48906 100644 --- a/common/auth/src/main/scala/org/apache/texera/auth/util/ComputingUnitAccess.scala +++ b/common/auth/src/main/scala/org/apache/texera/auth/util/ComputingUnitAccess.scala @@ -32,7 +32,8 @@ object ComputingUnitAccess { .createDSLContext() def getComputingUnitAccess(cuid: Integer, uid: Integer): PrivilegeEnum = { - // owner uid + caller's access privilege in one query + // At most one row: cuid is the PK of workflow_computing_unit and (cuid, uid) + // is the PK of computing_unit_user_access, so the left join cannot fan out. val record = context .select(WORKFLOW_COMPUTING_UNIT.UID, COMPUTING_UNIT_USER_ACCESS.PRIVILEGE) .from(WORKFLOW_COMPUTING_UNIT) diff --git a/common/auth/src/test/scala/org/apache/texera/auth/util/ComputingUnitAccessSpec.scala b/common/auth/src/test/scala/org/apache/texera/auth/util/ComputingUnitAccessSpec.scala new file mode 100644 index 00000000000..6a5c519fc0a --- /dev/null +++ b/common/auth/src/test/scala/org/apache/texera/auth/util/ComputingUnitAccessSpec.scala @@ -0,0 +1,105 @@ +/* + * 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.auth.util + +import org.apache.texera.dao.MockTexeraDB +import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum +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.BeforeAndAfterAll +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +// Exercises getComputingUnitAccess against an embedded Postgres seeded with a +// computing unit (cuid=100) owned by uid=1, plus a READ grant (uid=2) and a +// WRITE grant (uid=3). Covers every branch of the single-join resolution: +// missing unit, owner, explicit grant, and a user with no grant. +class ComputingUnitAccessSpec + extends AnyFlatSpec + with Matchers + with BeforeAndAfterAll + with MockTexeraDB { + + private val cuid = 100 + + private def newUser(uid: Int, name: String): User = { + val u = new User + u.setUid(uid) + u.setName(name) + u.setPassword("password") + u + } + + override def beforeAll(): Unit = { + initializeDBAndReplaceDSLContext() + val config = getDSLContext.configuration() + + val userDao = new UserDao(config) + Seq(1 -> "owner", 2 -> "reader", 3 -> "writer", 4 -> "stranger").foreach { + case (uid, name) => userDao.insert(newUser(uid, name)) + } + + val unit = new WorkflowComputingUnit + unit.setUid(1) + unit.setName("cu") + unit.setCuid(cuid) + new WorkflowComputingUnitDao(config).insert(unit) + + val accessDao = new ComputingUnitUserAccessDao(config) + Seq(2 -> PrivilegeEnum.READ, 3 -> PrivilegeEnum.WRITE).foreach { + case (uid, privilege) => + val access = new ComputingUnitUserAccess + access.setCuid(cuid) + access.setUid(uid) + access.setPrivilege(privilege) + accessDao.insert(access) + } + } + + override def afterAll(): Unit = shutdownDB() + + "getComputingUnitAccess" should "return NONE when the computing unit does not exist" in { + ComputingUnitAccess.getComputingUnitAccess(999, 1) shouldBe PrivilegeEnum.NONE + } + + it should "return WRITE for the owner regardless of any access row" in { + ComputingUnitAccess.getComputingUnitAccess(cuid, 1) shouldBe PrivilegeEnum.WRITE + } + + it should "return the granted READ privilege for a non-owner" in { + ComputingUnitAccess.getComputingUnitAccess(cuid, 2) shouldBe PrivilegeEnum.READ + } + + it should "return the granted WRITE privilege for a non-owner" in { + ComputingUnitAccess.getComputingUnitAccess(cuid, 3) shouldBe PrivilegeEnum.WRITE + } + + it should "return NONE for a user with no access row on an existing unit" in { + ComputingUnitAccess.getComputingUnitAccess(cuid, 4) shouldBe PrivilegeEnum.NONE + } +}