diff --git a/.deploy/mta.yaml b/.deploy/mta.yaml index 2dc3b2fb3..59a89cdc7 100644 --- a/.deploy/mta.yaml +++ b/.deploy/mta.yaml @@ -10,7 +10,7 @@ ID: tutorials-ims # Bump this on each release you deploy — it's the version shown by `cf mtas` # and in the mtar filename (tutorials-ims_.mtar). Deploy is manual: # `cd .deploy && mbt build && cf deploy mta_archives/tutorials-ims_.mtar -e ../deploy/.mtaext -f`. -version: 1.31.2 +version: 1.31.3 # Top-level parameters (overridable per-env via deploy/.mtaext). parameters: diff --git a/app/admin-annotations.cds b/app/admin-annotations.cds index 56b4aade3..2fb8c45c1 100644 --- a/app/admin-annotations.cds +++ b/app/admin-annotations.cds @@ -4167,6 +4167,58 @@ annotate AdminService.FeatureFlags with @UI: { ]} }; +// --- Semaphore taxonomy-sync tuning keys (#2477) --- +// String/csv/bool ImsConfig rows surfaced as an editable panel. setValue takes +// a free-text parameter (FE renders a parameter dialog); clearValue removes the +// row so the documented default takes effect again. +annotate AdminService.SemaphoreConfig with @UI: { + HeaderInfo: { + TypeName: 'Semaphore Config Key', TypeNamePlural: 'Semaphore Config Keys', + Title: { Value: label }, + Description: { Value: key } + }, + SelectionFields: [ valueType ], + LineItem: [ + { Value: label }, + { Value: key, Label: 'Config key' }, + { Value: valueType, Label: 'Type' }, + { + $Type: 'UI.DataField', Value: effectiveValue, Label: 'Effective value', + // Neutral (3) when a value is set, Critical (2) when falling back to default. + Criticality: { $edmJson: { $If: [ { $Path: 'isDefault' }, 2, 3 ] } } + }, + { Value: defaultValue, Label: 'Default' }, + { $Type: 'UI.DataFieldForAction', Action: 'AdminService.setValue', Label: 'Set value' }, + { $Type: 'UI.DataFieldForAction', Action: 'AdminService.clearValue', Label: 'Reset to default' } + ], + Identification: [ + { $Type: 'UI.DataFieldForAction', Action: 'AdminService.setValue', Label: 'Set value' }, + { $Type: 'UI.DataFieldForAction', Action: 'AdminService.clearValue', Label: 'Reset to default' } + ], + Facets: [ + { $Type: 'UI.ReferenceFacet', ID: 'General', Label: 'General', Target: '@UI.FieldGroup#General' }, + { $Type: 'UI.ReferenceFacet', ID: 'Value', Label: 'Value', Target: '@UI.FieldGroup#Value' } + ], + FieldGroup#General: { Data: [ + { Value: key }, { Value: label }, { Value: valueType }, { Value: description } + ]}, + FieldGroup#Value: { Data: [ + { Value: effectiveValue, Label: 'Effective value' }, + { Value: rawDbValue, Label: 'Raw DB value' }, + { Value: defaultValue, Label: 'Default value' } + ]} +}; + +annotate AdminService.SemaphoreConfig with { + ![key] @Common.Label: 'Config key'; + label @Common.Label: 'Setting'; + valueType @Common.Label: 'Type'; + effectiveValue @Common.Label: 'Effective value'; + rawDbValue @Common.Label: 'Raw DB value'; + defaultValue @Common.Label: 'Default value'; + description @Common.Label: 'Description'; +}; + // --- Petoberfest admin moderation surface --- // PetSubmissions: moderation queue list report with approve/hide actions. // Blob columns (photoDisplay/photoThumb) are NOT exposed here; thumbnails are diff --git a/app/admin-shell/webapp/controller/Shell.controller.js b/app/admin-shell/webapp/controller/Shell.controller.js index d22d3fb5b..893f6ff64 100644 --- a/app/admin-shell/webapp/controller/Shell.controller.js +++ b/app/admin-shell/webapp/controller/Shell.controller.js @@ -66,7 +66,8 @@ sap.ui.define([ channels: "channels", channelCollections: "channelCollections", channelTopicMap: "channelTopicMap", - channelSubmissions: "channelSubmissions" + channelSubmissions: "channelSubmissions", + semaphoreConfig: "semaphoreConfig" }; var NAV_KEY_TO_TITLE = { @@ -129,7 +130,8 @@ sap.ui.define([ channels: "Channels", channelCollections: "Channel Collections", channelTopicMap: "Channel Topic Map", - channelSubmissions: "Channel Submissions" + channelSubmissions: "Channel Submissions", + semaphoreConfig: "Semaphore Config" }; return Controller.extend("sap.tutorials.admin.shell.controller.Shell", { diff --git a/app/admin-shell/webapp/manifest.json b/app/admin-shell/webapp/manifest.json index 02a9b84f6..692f9a6ea 100644 --- a/app/admin-shell/webapp/manifest.json +++ b/app/admin-shell/webapp/manifest.json @@ -95,6 +95,7 @@ "sap.tutorials.admin.videoRotation": "./components/video-rotation", "sap.tutorials.admin.pats": "./components/pats", "sap.tutorials.admin.featureFlags": "./components/featureFlags", + "sap.tutorials.admin.semaphoreConfig": "./components/semaphoreConfig", "sap.tutorials.admin.petoberfest": "./components/petoberfest", "sap.tutorials.admin.channels": "./components/channels", "sap.tutorials.admin.channelCollections": "./components/channel-collections", @@ -392,6 +393,12 @@ "componentData": {}, "lazy": true }, + "semaphoreConfigComponent": { + "name": "sap.tutorials.admin.semaphoreConfig", + "settings": {}, + "componentData": {}, + "lazy": true + }, "petoberfestComponent": { "name": "sap.tutorials.admin.petoberfest", "settings": {}, @@ -931,6 +938,16 @@ } ] }, + { + "name": "semaphoreConfig", + "pattern": "semaphoreConfig", + "target": [ + { + "name": "semaphoreConfigTarget", + "prefix": "smc" + } + ] + }, { "name": "petoberfest", "pattern": "petoberfest", @@ -1341,6 +1358,13 @@ "viewLevel": 1, "prefix": "ffl" }, + "semaphoreConfigTarget": { + "type": "Component", + "usage": "semaphoreConfigComponent", + "id": "semaphoreConfigTarget", + "viewLevel": 1, + "prefix": "smc" + }, "petoberfestTarget": { "type": "Component", "usage": "petoberfestComponent", diff --git a/app/admin-shell/webapp/model/navigation.json b/app/admin-shell/webapp/model/navigation.json index 475ed49ca..dc4e2ade9 100644 --- a/app/admin-shell/webapp/model/navigation.json +++ b/app/admin-shell/webapp/model/navigation.json @@ -143,7 +143,8 @@ { "key": "display", "title": "Display" }, { "key": "tenant", "title": "Tenant" }, { "key": "ngds", "title": "NGDS Auto-Send", "requiredScope": "Admin" }, - { "key": "featureFlags", "title": "Feature Flags", "requiredScope": "Admin" } + { "key": "featureFlags", "title": "Feature Flags", "requiredScope": "Admin" }, + { "key": "semaphoreConfig", "title": "Semaphore Config", "requiredScope": "Admin" } ] }, { diff --git a/app/admin/semaphoreConfig/webapp/Component.js b/app/admin/semaphoreConfig/webapp/Component.js new file mode 100644 index 000000000..13c5d9458 --- /dev/null +++ b/app/admin/semaphoreConfig/webapp/Component.js @@ -0,0 +1,6 @@ +sap.ui.define(["sap/fe/core/AppComponent"], function (AppComponent) { + "use strict"; + return AppComponent.extend("sap.tutorials.admin.semaphoreConfig.Component", { + metadata: { manifest: "json" } + }); +}); diff --git a/app/admin/semaphoreConfig/webapp/i18n/i18n.properties b/app/admin/semaphoreConfig/webapp/i18n/i18n.properties new file mode 100644 index 000000000..96eabe678 --- /dev/null +++ b/app/admin/semaphoreConfig/webapp/i18n/i18n.properties @@ -0,0 +1,7 @@ +appTitle=Semaphore Config +appSubtitle=Tuning keys for the Semaphore taxonomy-sync job + +# Bound action labels (#2477) — annotations use inline labels; these mirror them +# for i18n-driven references and translation coverage. +setValue=Set value +clearValue=Reset to default diff --git a/app/admin/semaphoreConfig/webapp/manifest.json b/app/admin/semaphoreConfig/webapp/manifest.json new file mode 100644 index 000000000..cb6cfdd75 --- /dev/null +++ b/app/admin/semaphoreConfig/webapp/manifest.json @@ -0,0 +1,71 @@ +{ + "_version": "1.65.0", + "sap.app": { + "id": "sap.tutorials.admin.semaphoreConfig", + "type": "application", + "title": "{{appTitle}}", + "description": "{{appSubtitle}}", + "applicationVersion": { "version": "0.0.1" }, + "i18n": "i18n/i18n.properties", + "dataSources": { + "mainService": { + "uri": "/admin/", + "type": "OData", + "settings": { "odataVersion": "4.0" } + } + } + }, + "sap.ui5": { + "dependencies": { + "minUI5Version": "1.136.0", + "libs": { "sap.fe.templates": {}, "sap.m": {}, "sap.ui.core": {} } + }, + "models": { + "": { + "dataSource": "mainService", + "preload": true, + "settings": { + "synchronizationMode": "None", + "operationMode": "Server", + "autoExpandSelect": true, + "earlyRequests": true + } + }, + "i18n": { + "type": "sap.ui.model.resource.ResourceModel", + "settings": { "bundleName": "sap.tutorials.admin.semaphoreConfig.i18n.i18n" } + } + }, + "routing": { + "routes": [ + { "pattern": ":?query:", "name": "SemaphoreConfigList", "target": "SemaphoreConfigList" }, + { "pattern": "SemaphoreConfig({key}):?query:", "name": "SemaphoreConfigObjectPage", "target": "SemaphoreConfigObjectPage" } + ], + "targets": { + "SemaphoreConfigList": { + "type": "Component", + "id": "SemaphoreConfigList", + "name": "sap.fe.templates.ListReport", + "options": { + "settings": { + "contextPath": "/SemaphoreConfig", + "variantManagement": "Page", + "initialLoad": "Enabled", + "navigation": { + "SemaphoreConfig": { "detail": { "route": "SemaphoreConfigObjectPage" } } + } + } + } + }, + "SemaphoreConfigObjectPage": { + "type": "Component", + "id": "SemaphoreConfigObjectPage", + "name": "sap.fe.templates.ObjectPage", + "options": { + "settings": { "contextPath": "/SemaphoreConfig", "editableHeaderContent": false } + } + } + } + } + } +} diff --git a/docs/.vitepress/config.ts b/docs/.vitepress/config.ts index a051e90d5..eecd159c7 100644 --- a/docs/.vitepress/config.ts +++ b/docs/.vitepress/config.ts @@ -105,6 +105,7 @@ export default defineConfig({ { text: 'Authors', items: [ { text: 'Overview', link: '/authors/' }, { text: 'Writing tutorials', link: '/authors/writing-tutorials' }, + { text: 'Branch protection & PRs', link: '/authors/branch-protection-and-pull-requests' }, { text: 'Meta tutorials (QA)', link: '/authors/meta-tutorials' }, { text: 'Repo / group owners', link: '/authors/repo-group-owners' }, { text: 'Center admin', link: '/authors/center-admin' }, @@ -224,6 +225,7 @@ export default defineConfig({ { text: 'A2A agent consumption guide', link: '/developers/reference/a2a-instructions' }, { text: 'AI Skills Catalog', link: '/developers/reference/ai-skills-catalog' }, { text: 'CAP / CDS gotchas', link: '/developers/reference/cap-cds-gotchas' }, + { text: 'Challenge widget cost sizing', link: '/developers/reference/challenge-widget-cost-sizing' }, { text: 'cds-caching CDS-DB store', link: '/developers/reference/cds-caching-store' }, { text: 'Community blog RSS transport', link: '/developers/reference/community-rss-transport' }, { text: 'Cookie and storage analysis', link: '/developers/reference/cookie-and-storage-analysis' }, diff --git a/docs/authors/README.md b/docs/authors/README.md index 900755deb..2886ecc1e 100644 --- a/docs/authors/README.md +++ b/docs/authors/README.md @@ -7,6 +7,7 @@ This folder is the operational manual for everyone working with the SAP Develope | If you are a... | Read | What you do | | --- | --- | --- | | **Tutorial author** writing markdown | [writing-tutorials.md](writing-tutorials.md) | Write, preview, and publish tutorials | +| **Any author** getting a change merged | [branch-protection-and-pull-requests.md](branch-protection-and-pull-requests.md) | Branches, forks, and PRs under the OSPO branch-protection rules (no more direct pushes to `main`) | | **New tutorial author** learning the ropes | [meta-tutorials.md](meta-tutorials.md) | Follow the platform's own "how to write a tutorial" tutorials (on the QA channel) | | **Repo group owner** in `sap-tutorials` | [repo-group-owners.md](repo-group-owners.md) | Review PRs, plan tutorials, manage your repos | | **Center admin** running the platform | [center-admin.md](center-admin.md) | Catalog, taxonomy, pipeline, access, support | diff --git a/docs/authors/branch-protection-and-pull-requests.md b/docs/authors/branch-protection-and-pull-requests.md new file mode 100644 index 000000000..08494a3c6 --- /dev/null +++ b/docs/authors/branch-protection-and-pull-requests.md @@ -0,0 +1,283 @@ +# Branch Protection & Pull Requests for Authors + +**Why this page exists:** In September 2026, SAP's Open Source Program Office (OSPO) applied an +organization-wide branch-protection ruleset to every repository in the +[`sap-tutorials`](https://github.com/sap-tutorials) GitHub organization. **You can no longer push +directly to the default branch** (`main` or `master`). Every change — even a one-line typo fix — +now goes through a **pull request** that must be **reviewed and approved** before it can be merged. + +If you used to `git push` straight to `main`, this page is the new workflow you must follow. It +explains the concepts (branches, forks, pull requests), the exact rules OSPO enforces, and gives +you step-by-step instructions for both the web UI and the command line. + +> [!IMPORTANT] +> This changes *how* you get content in, not *what* you write or *what happens after merge*. Once +> your PR is merged, the publish pipeline behaves exactly as before — see +> [writing-tutorials.md](writing-tutorials.md) §7 "What happens after merge". + +--- + +## 1. The rules, in one table + +These are the actual rules enforced by the OSPO ruleset **"Protect default branch (org-wide)"**, +applied to the **default branch** of every repo in `sap-tutorials`. Verified live via the GitHub +API on 2026-09-23. + +| Rule | What it means for you | +|------|-----------------------| +| **No direct pushes** | You cannot `git push` to `main`/`master`. The push is rejected. All changes arrive via pull request. | +| **Pull request required** | Every change to the default branch must go through a PR. | +| **≥ 1 approving review** | A PR needs **at least one approval** from someone other than the author before it can merge. | +| **Stale reviews dismissed on push** | If you push new commits *after* getting an approval, that approval is **cleared** and you must be re-approved. | +| **Extra approval for unattributed changes** | If a commit's author can't be matched to a GitHub identity (e.g. a bad `user.email`), the PR needs an **additional** approval. Set your git identity correctly to avoid this — see §7. | +| **No force-pushes** | `git push --force` to the default branch is blocked (`non_fast_forward`). | +| **No branch deletion** | The default branch cannot be deleted. | +| **No bypass** | There is **no bypass list**. Repo admins and org owners follow the same rules — nobody can push directly. | + +**What is NOT required** (so you don't over-think it): code-owner review is *not* mandated, +last-push approval is *not* required, and review-thread resolution is *not* enforced by the rule. +All three merge methods (**merge**, **squash**, **rebase**) are allowed. + +> [!NOTE] +> The rule targets `~DEFAULT_BRANCH`, so it protects whatever the repo's default branch is named. +> Most tutorial repos use `main`; a few older ones (e.g. `Tutorials`) still use `master`. The +> protection applies either way. This guide writes `main` — substitute `master` where your repo +> uses it. Check with `git remote show origin | grep "HEAD branch"`. + +--- + +## 2. Concepts you need (branch, fork, pull request) + +If these terms are already second nature, skip to §4. + +### 2.1 Branch + +A **branch** is a parallel line of work inside a repository. The **default branch** (`main`) is the +"official" version that the tutorial platform publishes from. When you create a branch, you get an +isolated copy of the files where you can commit freely without touching `main`. + +```text +main: A───B───C ← protected, publishes to developers.sap.com + \ +your-branch: D───E ← your work-in-progress, safe to push +``` + +You do your editing on **your branch**, then propose merging it back into `main` via a pull request. + +### 2.2 Fork + +A **fork** is your *personal copy of the entire repository* under your own GitHub account +(`your-username/abap-core-development`). It's a separate repo that remembers where it came from +("upstream"). You push branches to your fork, then open a PR **from your fork into the upstream +`sap-tutorials` repo**. + +**Do you need a fork?** It depends on your access: + +| Your situation | Use | Why | +|----------------|-----|-----| +| You have **write (push) access** to the `sap-tutorials` repo | **Branch directly in the repo** (§4) | Simpler — no fork to keep in sync. You still cannot push to `main`, only to your own branch. | +| You are an **external contributor** with no write access | **Fork** (§5) | You can't create branches in a repo you can't push to. Fork, branch in your fork, PR upstream. | +| Not sure | Try §4; if `git push` to a new branch is rejected with a permissions error, use §5 | — | + +Both paths end at the same place: **a pull request into the protected `main`.** + +### 2.3 Pull request (PR) + +A **pull request** is a proposal: "please merge the commits on *my branch* into *your `main`*." It's +where review happens — a reviewer reads the diff, comments, and clicks **Approve**. Once the PR has +the required approval and passes any checks, it can be **merged**, which is the *only* way changes +now reach `main`. + +--- + +## 3. The new workflow at a glance + +```text +OLD (no longer possible) NEW (required) +──────────────────────── ────────────────────────────────────── +edit on main 1. branch off main (or fork, then branch) +git commit 2. edit + commit on the branch +git push origin main ✗ REJECTED 3. push the branch (to repo or your fork) + 4. open a pull request → main + 5. get ≥ 1 approval + 6. merge the PR ← this reaches main + 7. pipeline publishes (unchanged) +``` + +--- + +## 4. Step by step — you HAVE write access (branch in the repo) + +Use this if you can push to the `sap-tutorials` repo (most SAP-internal authors and repo group +owners). + +### 4.1 Command line + +```bash +# 1. Clone (once) and move into the repo +git clone https://github.com/sap-tutorials/abap-core-development.git +cd abap-core-development + +# 2. Make sure you start from the latest main +git checkout main +git pull origin main + +# 3. Create a branch — name it after your change +git checkout -b fix/typo-in-abap-cloud-ui + +# 4. Edit your .md files / add images, then stage and commit +git add tutorials/abap-cloud-ui-from-interface.md +git commit -m "Fix broken image path in abap-cloud-ui-from-interface" + +# 5. Push YOUR BRANCH (never main) +git push -u origin fix/typo-in-abap-cloud-ui + +# 6. Open the PR (gh CLI). --base main --head is your branch (implied by -u above) +gh pr create --base main \ + --title "Fix broken image path in abap-cloud-ui-from-interface" \ + --body "Corrects a relative image path that 404'd on the published page." +``` + +`gh pr create` prints the PR URL. Share it, or wait for a reviewer to pick it up. + +### 4.2 Web UI (no local git) + +You can do the whole thing in the browser — good for quick text edits: + +1. Browse to the file on `github.com/sap-tutorials/`. +2. Click the **pencil (Edit)** icon. +3. Make your edit. +4. Click **Commit changes…**. GitHub will not let you commit to `main`; it offers + **"Create a new branch for this commit and start a pull request."** Leave that selected, name + the branch, and click **Propose changes**. +5. On the next screen, click **Create pull request**. + +That's the same result as the CLI path — a branch plus a PR into `main`. + +--- + +## 5. Step by step — you do NOT have write access (fork + PR) + +Use this if you're an external contributor or `git push` to a new branch fails with a +`403`/permission error. + +```bash +# 1. Fork via the CLI (or click "Fork" on the repo page in the browser) +gh repo fork sap-tutorials/abap-core-development --clone +cd abap-core-development + +# gh sets up two remotes for you: +# origin → your fork (you can push here) +# upstream → sap-tutorials/... (you open PRs against this; read-only to you) + +# 2. Start from an up-to-date main +git checkout main +git pull upstream main + +# 3. Branch, edit, commit +git checkout -b add/new-hana-tutorial +git add tutorials/my-new-tutorial.md tutorials/my-new-tutorial/ +git commit -m "Add tutorial: my-new-tutorial" + +# 4. Push the branch to YOUR FORK (origin) +git push -u origin add/new-hana-tutorial + +# 5. Open the PR from your fork into upstream main +gh pr create --repo sap-tutorials/abap-core-development \ + --base main --head :add/new-hana-tutorial \ + --title "Add tutorial: my-new-tutorial" \ + --body "New beginner tutorial for SAP HANA Cloud." +``` + +**Keeping your fork current** (do this before starting new work, so you branch from fresh code): + +```bash +git checkout main +git pull upstream main # pull the org's latest +git push origin main # update your fork's main to match +``` + +--- + +## 6. Getting your PR approved and merged + +1. **Request a review.** In the PR page, under **Reviewers**, request your + [repo group owner](repo-group-owners.md), or ask in the platform team channel. A reviewer must + click **Approve** — your own review does not count toward the required approval. +2. **Address feedback.** If the reviewer requests changes, commit them on the **same branch** and + push again. The PR updates automatically. + - ⚠️ **Pushing after an approval clears that approval** (the "dismiss stale reviews" rule). You'll + need the reviewer to approve again. Batch your fixes to minimize round-trips. +3. **Merge.** Once you have ≥ 1 approval (and any checks are green), click **Merge pull request** + (or `gh pr merge --squash`). Squash keeps the history tidy; any of the three methods is allowed. +4. **Delete the branch.** GitHub offers a **Delete branch** button after merge — safe to click; it + only removes your feature branch, never `main`. +5. **Publish is automatic.** Merging to `main` fires the repo-dispatch pipeline. Your tutorial is + live in a minute or two — see [writing-tutorials.md](writing-tutorials.md) §7. + +--- + +## 7. Avoiding the "unattributed changes" extra-approval trap + +The ruleset requires an **extra approval** when a commit's author can't be attributed to a GitHub +account. This almost always means your local git identity doesn't match your GitHub email. Fix it +once: + +```bash +git config --global user.name "Your Name" +git config --global user.email "your-github-email@example.com" # must match a verified GitHub email +``` + +Use the email listed under **GitHub → Settings → Emails** (or your `@users.noreply.github.com` +address). If a PR already shows "unverified"/unattributed commits, re-committing with the correct +identity and re-pushing clears it; otherwise you'll simply need a second approver. + +--- + +## 8. Troubleshooting + +| Symptom | Cause | Fix | +|---------|-------|-----| +| `! [remote rejected] main -> main (protected branch hook declined)` | You tried to push directly to `main`. | Push a **branch** instead (§4/§5) and open a PR. | +| `remote: Permission to sap-tutorials/.git denied` on `git push` | No write access to the repo. | Use the **fork** workflow (§5). | +| PR says **"Merging is blocked — review required"** | No approving review yet. | Request a reviewer (§6.1); your own approval doesn't count. | +| Your approval vanished after you pushed a fix | "Dismiss stale reviews on push" is on. | Re-request approval; batch changes next time. | +| PR wants a **second** approval unexpectedly | Commit is unattributed. | Fix git identity (§7); re-approval or a second approver clears it. | +| `git push --force` rejected | Force-push to a protected branch is blocked. | Don't force-push shared branches; if you must rewrite, do it on your **feature** branch before others pull it. | +| I'm a repo admin — can't I just bypass? | The ruleset has **no bypass list**. | No. Everyone uses PRs, including admins. | + +--- + +## 9. FAQ + +**Do I need this for the `-Contribution` (QA) repos too?** +The ruleset targets every repo's default branch in the org, so treat `*-Contribution` repos the same +way — branch/fork and PR. QA preview then works as described in +[writing-tutorials.md](writing-tutorials.md) §5.1. + +**Can I still make quick typo fixes?** +Yes — the fastest path is the **web-UI edit** (§4.2). GitHub auto-creates the branch and PR for you; +you still need one approval. + +**Who approves my PR?** +Your [repo group owner](repo-group-owners.md) is the default reviewer. For cross-cutting or platform +changes, ask in the platform team channel. + +**Does the merge method matter?** +Not for the rules — merge, squash, and rebase are all allowed. **Squash** is recommended to keep one +clean commit per change. + +**Where do I see the rule itself?** +[github.com/sap-tutorials/<repo>/rules](https://github.com/sap-tutorials) → the repo's +**Settings → Rules** (read-only for non-admins), or the branch-protection banner shown on a blocked +push. + +--- + +## Reference: related docs + +- [writing-tutorials.md](writing-tutorials.md) — authoring workflow, local preview, what happens after merge (§4 there now points here) +- [repo-group-owners.md](repo-group-owners.md) — for the people who review and approve your PRs +- [tutorial-repo-dispatch.yml](tutorial-repo-dispatch.yml) — the Action that triggers a rebuild once your PR is merged +- [GitHub Docs: About pull requests](https://docs.github.com/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests) +- [GitHub Docs: About rulesets](https://docs.github.com/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/about-rulesets) diff --git a/docs/authors/writing-tutorials.md b/docs/authors/writing-tutorials.md index c4499fb2b..d8404d59c 100644 --- a/docs/authors/writing-tutorials.md +++ b/docs/authors/writing-tutorials.md @@ -292,12 +292,17 @@ entity Books : managed { 2. Add or edit a .md file under tutorials/ 3. Commit any new images in the slug-named folder 4. Open a PR against main -5. PR review + merge (current review is informal — see §11) -6. Push to main → repo dispatch event → tutorials-ims CI +5. Get ≥ 1 approving review, then merge (direct pushes to main are blocked — see below) +6. Merge to main → repo dispatch event → tutorials-ims CI 7. CI re-fetches your tutorial, rebuilds Hugo, publishes to HANA 8. Live at developers.sap.com/tutorials/ within a few minutes ``` +> [!IMPORTANT] +> Since September 2026, an org-wide OSPO ruleset **blocks direct pushes to `main`**. Every change — +> even a typo fix — must go through a reviewed pull request. Full rules and step-by-step +> branch/fork/PR instructions: [branch-protection-and-pull-requests.md](branch-protection-and-pull-requests.md). + The notification step (6) is wired by adding a small workflow file to your tutorial repo — see [tutorial-repo-dispatch.yml](tutorial-repo-dispatch.yml). Org admins typically handle this once per repo. --- @@ -380,7 +385,7 @@ If the build fails, the dispatch run will be red in the `tutorials-ims` GitHub A ### Quick edits -Push a change to `main` of the source repo. The pipeline picks it up automatically — no manual step. +Open a reviewed PR against `main` of the source repo (direct pushes to `main` are blocked — see [branch-protection-and-pull-requests.md](branch-protection-and-pull-requests.md)). Once merged, the pipeline picks it up automatically — no manual step. ### Rollback @@ -412,7 +417,7 @@ These items are listed here so authors know what to expect — and what *not* to | Gap | Current state | |-----|---------------| -| Editorial review gate | Informal PR review in source repo; no formal Author QA lane | +| Editorial review gate | PR review now **required** by the OSPO ruleset (≥ 1 approval before merge) — see [branch-protection-and-pull-requests.md](branch-protection-and-pull-requests.md); a formal Author QA lane is still separate | | Approval / sign-off workflow | None — merge equals publish; no reviewer roles | | PR preview deploys | None — preview requires local clone (§5) or QA channel (§5.1) | | VS Code authoring extension | None — live preview, frontmatter validation, link checking are planned | diff --git a/srv/admin-service.cds b/srv/admin-service.cds index fc8bd38a9..5103bb37c 100644 --- a/srv/admin-service.cds +++ b/srv/admin-service.cds @@ -460,6 +460,30 @@ service AdminService { action disable() returns FeatureFlags; } + // Semaphore taxonomy-sync tuning keys (#2477). Like FeatureFlags this is a + // synthesized @cds.persistence.skip viewer over the raw `semaphore.sync.*` + // ImsConfig rows — but the values are string/csv/bool, not booleans, so the + // bound setValue(value) action takes a free-text parameter (FE renders a + // parameter dialog) instead of enable/disable. clearValue() removes the row + // so the documented default takes effect again. Reads live in resolve-time; + // the sync job reads the same rows via srv/lib/semaphore-sync/config-keys.js. + @readonly + @cds.persistence.skip + @Capabilities: { InsertRestrictions: { Insertable: false }, UpdateRestrictions: { Updatable: false }, DeleteRestrictions: { Deletable: false } } + entity SemaphoreConfig { + key ![key] : String(60); // ImsConfig key, e.g. 'semaphore.sync.model' + label : String(60); + valueType : String(10); // 'string' | 'csv' | 'bool' + description : String(500); + effectiveValue : String(500); // DB value if set, else the default + rawDbValue : String(500); // null when the row is absent (→ default) + defaultValue : String(500); + isDefault : Boolean; // true when no ImsConfig row is present + } actions { + action setValue(value : String) returns SemaphoreConfig; + action clearValue() returns SemaphoreConfig; + } + @odata.singleton @requires: 'Admin' entity ChatSettings as projection on ims.ChatSettings actions { diff --git a/srv/admin-service.js b/srv/admin-service.js index 3031ca5a0..d7d6e359d 100644 --- a/srv/admin-service.js +++ b/srv/admin-service.js @@ -60,6 +60,9 @@ import * as mcpAdmin from './lib/mcp-admin-tools.js'; // #1106 Phase 3 (WS2) a import { computeCoverage, resolveThreshold } from './lib/kg-community-coverage.js'; // #1172 import { resolveFeatureFlags } from './lib/feature-flags/resolve.js'; // #feature-flags import { FEATURE_FLAGS } from './lib/feature-flags/registry.js'; // #2060 bound enable/disable +// #2477 — semaphore.sync.* tuning-key panel. Shared descriptor is the single +// source of truth for keys/defaults/valueTypes (also read by the sync job). +import { SEMAPHORE_CONFIG_KEYS, SEMAPHORE_CONFIG_KEY_SET } from './lib/semaphore-sync/config-keys.js'; import { resetFeaturedCache } from './lib/featured-resolve.js'; // #756: max jobName payload length. Matches JobLocks.jobName : String(100) @@ -552,6 +555,97 @@ export default class AdminService extends cds.ApplicationService { this.on('enable', 'FeatureFlags', (req) => setFeatureFlagRow(req, true)); this.on('disable', 'FeatureFlags', (req) => setFeatureFlagRow(req, false)); + // ---- SemaphoreConfig (#2477): synthesized viewer + string setter ------- + // Extract the entity key from a bound-action or object-page READ request. + // CAP surfaces the key on req.params (last segment); a list→detail nav may + // instead arrive as a {key: {'=': …}} equality in the WHERE. Returns the + // key string, or null for a collection read. + const keyFromReq = (req) => { + const params = req.params; + const last = Array.isArray(params) && params.length ? params[params.length - 1] : null; + if (last && typeof last === 'object' && 'key' in last) return last.key; + if (typeof last === 'string') return last; + const where = req.query?.SELECT?.where; + if (Array.isArray(where)) { + // pattern: [ {ref:['key']}, '=', {val:'…'} ] + const i = where.findIndex((t) => t && t.ref && t.ref[0] === 'key'); + if (i >= 0 && where[i + 1] === '=' && where[i + 2]?.val != null) return where[i + 2].val; + } + return null; + }; + // READ synthesizes one row per shared descriptor key, layering the live + // ImsConfig value over the documented default. @cds.persistence.skip, so + // there is nothing to SELECT — we build the rows here and honor $filter on + // key (single-row object-page fetch) plus $count. + const resolveSemaphoreConfig = async () => { + const { ImsConfig } = cds.entities('com.sap.developers.ims'); + let byKey = Object.create(null); + try { + const rows = await SELECT.from(ImsConfig) + .columns('key', 'value') + .where({ key: { in: SEMAPHORE_CONFIG_KEYS.map((k) => k.key) } }); + for (const r of rows || []) byKey[r.key] = r.value == null ? null : String(r.value); + } catch (e) { + cds.log('admin').warn(`SemaphoreConfig ImsConfig read failed, using defaults: ${e.message}`); + } + return SEMAPHORE_CONFIG_KEYS.map((d) => { + const raw = d.key in byKey ? byKey[d.key] : null; + const def = String(d.default ?? ''); + return { + key: d.key, + label: d.label, + valueType: d.valueType, + description: d.description, + rawDbValue: raw, + effectiveValue: raw !== null ? raw : def, + defaultValue: def, + isDefault: raw === null, + }; + }); + }; + + this.on('READ', 'SemaphoreConfig', async (req) => { + let rows = await resolveSemaphoreConfig(); + // Object-page fetch: SemaphoreConfig(key='…') arrives as an equality on key. + const keyEq = keyFromReq(req); + if (keyEq != null) { + const one = rows.find((r) => r.key === keyEq) ?? null; + return one; + } + const wantsCount = req.query?.SELECT?.count; + const total = rows.length; + if (wantsCount) rows.$count = total; + return rows; + }); + + // Upsert a single semaphore.sync.* key. Allowlisted against the shared + // descriptor so an arbitrary ImsConfig key can never be written here. + const writeSemaphoreValue = async (req, value) => { + const key = keyFromReq(req); + if (!key || !SEMAPHORE_CONFIG_KEY_SET.has(key)) { + return req.reject(400, `Unknown semaphore config key: ${key}`); + } + const { ImsConfig } = cds.entities('com.sap.developers.ims'); + if (value === null) { + // clearValue → remove the row so the documented default takes effect. + await DELETE.from(ImsConfig).where({ key }); + } else { + const existing = await SELECT.one.from(ImsConfig).where({ key }); + if (existing) { + await UPDATE(ImsConfig, existing.ID).set({ value: String(value) }); + } else { + await INSERT.into(ImsConfig).entries({ ID: cds.utils.uuid(), key, value: String(value) }); + } + } + // The sync job reads ImsConfig live (its own short cache in readConfig runs + // per-invocation), so no cache bust is needed here. + const all = await resolveSemaphoreConfig(); + return all.find((r) => r.key === key) ?? null; + }; + + this.on('setValue', 'SemaphoreConfig', (req) => writeSemaphoreValue(req, req.data?.value ?? '')); + this.on('clearValue', 'SemaphoreConfig', (req) => writeSemaphoreValue(req, null)); + // Virtual severityCrit element (drives @UI.LineItem Criticality coloring). // Information=3 (Neutral), Success=5 (Positive), Warning=2 (Critical), Error=1 (Negative) this.after('READ', 'Alerts', rows => { diff --git a/srv/jobs/scheduler.js b/srv/jobs/scheduler.js index 0e223da79..ca40188a7 100644 --- a/srv/jobs/scheduler.js +++ b/srv/jobs/scheduler.js @@ -136,6 +136,11 @@ export function _setJobFn(jobName, mockFn) { * triggers (opts.manualTrigger=true), emits a completion SecurityEvent * audit event from the `finally` block (spec §9). * + * #2478: FAILED is recorded for BOTH a thrown exception AND a runner that + * returns a soft-failure object `{ ok:false, error }`. The latter fell + * through to SUCCESS before, masking fail-shut no-ops (e.g. semaphore-tag- + * sync). A returned `{ ok:true }` or any non-`ok:false` value still succeeds. + * * Return shape: {skipped: false, outcome: 'success'|'error', result, * errorMessage} — `skipped` is always false since #958 retired the * lock-held short-circuit; retained in the shape for backward-compat @@ -170,7 +175,26 @@ async function runWithLock(jobName, durationMs, fn, opts = {}) { try { result = await fn(logId); const summary = formatJobSummary(jobName, result); - await logPipelineEnd(logId, 'SUCCESS', summary); + // A job may fail-shut by RETURNING { ok:false, error } rather than throwing + // (e.g. semaphore-tag-sync on a bad fetch). Treat that as a failed run so the + // PipelineLog STATUS and JobLastRun.lastSuccessAt/lastErrorAt reflect reality + // — otherwise a no-op/errored run is mislogged SUCCESS and looks healthy. + if (result && typeof result === 'object' && result.ok === false) { + outcome = 'error'; + errorMessage = result.error ? String(result.error) : `${jobName} returned ok:false`; + LOG.error(`Job ${jobName} returned failure:`, errorMessage); + await logPipelineEnd(logId, 'FAILED', summary, errorMessage); + void alerting.raise({ + eventType: 'ScheduledJobFailed', + severity: 'ERROR', + category: 'ALERT', + subject: `Scheduled job failed: ${jobName}`, + body: errorMessage, + resource: { resourceName: jobName, resourceType: 'job' } + }); // fail-open, non-blocking + } else { + await logPipelineEnd(logId, 'SUCCESS', summary); + } } catch (err) { outcome = 'error'; errorMessage = err.message ?? String(err); diff --git a/srv/jobs/semaphore-tag-sync-job.js b/srv/jobs/semaphore-tag-sync-job.js index 748747c47..fcb2b8acc 100644 --- a/srv/jobs/semaphore-tag-sync-job.js +++ b/srv/jobs/semaphore-tag-sync-job.js @@ -31,18 +31,14 @@ import { checkAndRotateApiKey } from '../lib/semaphore-sync/rotation.js'; import { mapAllTerms } from '../lib/semaphore-sync/mapper.js'; import { applyTerms } from '../lib/semaphore-sync/applier.js'; import { isFlagEnabled } from '../lib/feature-flags/db-flags.js'; +// Key list + defaults live in the shared descriptor so the Admin UI panel +// (AdminService.SemaphoreConfig, #2477) and this reader never drift. +import { SEMAPHORE_CONFIG_KEY_NAMES } from '../lib/semaphore-sync/config-keys.js'; const LOG = cds.log('semaphore-sync'); const NS = 'com.sap.developers.ims'; -const CONFIG_KEYS = [ - 'semaphore.sync.model', - 'semaphore.sync.lang', - 'semaphore.sync.filter', - 'semaphore.sync.actualTagClasses', - 'semaphore.sync.interestItemClasses', - 'semaphore.sync.dryRun', -]; +const CONFIG_KEYS = SEMAPHORE_CONFIG_KEY_NAMES; function splitList(v) { return String(v ?? '') @@ -51,6 +47,34 @@ function splitList(v) { .filter(Boolean); } +// Shorten a class URI to its readable leaf ("…schema#Topic" → "Topic") for the +// histogram; leave short names untouched. +function shortClass(c) { + const s = String(c ?? ''); + if (s.includes('#')) return s.slice(s.lastIndexOf('#') + 1); + if (s.includes('/')) return s.slice(s.lastIndexOf('/') + 1); + return s; +} + +// Distinct SES classes across all mapped rows with a term count each, rendered +// as a compact single-line string. formatJobSummary only renders scalar summary +// fields (number/string/boolean) — an object/array would be silently dropped — +// so the histogram MUST be a pre-formatted string to survive into the SUMMARY. +// This is the whole point of the first dry run: it surfaces the real class +// distribution so semaphore.sync.filter / intakeClasses can be chosen from data. +function classHistogram(rows) { + const counts = new Map(); + for (const r of rows) { + for (const c of r.classes ?? []) { + const k = shortClass(c); + counts.set(k, (counts.get(k) ?? 0) + 1); + } + } + const sorted = [...counts.entries()].sort((a, b) => b[1] - a[1]); + if (sorted.length === 0) return '(no classes on any term)'; + return sorted.map(([k, n]) => `${k}=${n}`).join(', '); +} + // Read the semaphore.sync.* string config from ImsConfig in one SELECT. async function readConfig(db) { const { ImsConfig } = cds.entities(NS); @@ -69,6 +93,10 @@ async function readConfig(db) { filter: map.get('semaphore.sync.filter') || undefined, actualTagClasses: splitList(map.get('semaphore.sync.actualTagClasses')), interestItemClasses: splitList(map.get('semaphore.sync.interestItemClasses')), + // Tier-2 intake allowlist: unmatched terms are INSERTed only if their SES + // class is listed here. Empty ⇒ intake OFF (adopt-only) — the safe default + // until the FILTER/classes are chosen from the dry-run histogram. + intakeClasses: splitList(map.get('semaphore.sync.intakeClasses')), // dryRun defaults to TRUE — first live runs report the plan without writing. dryRun: (map.get('semaphore.sync.dryRun') ?? 'true').toLowerCase() !== 'false', }; @@ -130,9 +158,9 @@ export async function runSemaphoreTagSync(_logId, opts = {}) { interestItemClasses: cfg.interestItemClasses, }); - let applied = { inserted: 0, updated: 0, unchanged: 0, total: rows.length }; + let applied = { inserted: 0, updated: 0, unchanged: 0, skippedIntake: 0, total: rows.length }; try { - applied = await applyTerms(rows, { db, dryRun: cfg.dryRun }); + applied = await applyTerms(rows, { db, dryRun: cfg.dryRun, intakeClasses: cfg.intakeClasses }); } catch (e) { LOG.error(`semaphore-sync upsert failed: ${e.message}`); return { ok: false, error: e.message, mapped: rows.length, skipped: skipped.length }; @@ -146,6 +174,9 @@ export async function runSemaphoreTagSync(_logId, opts = {}) { fetched: Array.isArray(data.terms) ? data.terms.length : 0, mapped: rows.length, skipped: skipped.length, + intakeClasses: cfg.intakeClasses.join(', ') || '(none — adopt-only)', + // Pre-formatted string so formatJobSummary renders it into the SUMMARY. + classHistogram: classHistogram(rows), ...applied, }; LOG.info(`semaphore-sync summary: ${JSON.stringify(summary)}`); diff --git a/srv/lib/semaphore-sync/applier.js b/srv/lib/semaphore-sync/applier.js index 83ece2f15..71faf8244 100644 --- a/srv/lib/semaphore-sync/applier.js +++ b/srv/lib/semaphore-sync/applier.js @@ -18,43 +18,78 @@ // class→flag mapping against the live SAPCore model before the flag is flipped. import cds from '@sap/cds'; +// Tags carry a legacyId (IMS numeric key) auto-assigned by AdminService's +// before(CREATE) hook. This applier writes via cds.db directly, bypassing that +// hook, so it must assign legacyId itself — otherwise new rows land with a NULL +// legacyId (the Admin UI's "ID" column) and self-heal never fires. #2479. +import { getNextLegacyId } from '../legacy-id.js'; -// Fields we consider when deciding whether an existing row needs an UPDATE. -const TRACKED = ['label', 'name', 'titlePath', 'isActualTag', 'isInterestItem', 'semaphoreId']; +// SES SAPCore is the ENTIRE SAP product/topic universe (~21k terms); the Tags +// table is a deliberately curated subset. So the sync is two-tier (#2184): +// +// Tier 1 — ADOPT (existing tags): match by semaphoreId, else by name. Backfill +// ONLY the taxonomy-owned fields (semaphoreId, titlePath, label). The +// isActualTag / isInterestItem flags are EDITORIAL — set per-term by hand, +// not derivable from any SES class — so the sync must never overwrite them +// (verified: 99 tags under the "Software Product" root, only 18/18 flagged). +// +// Tier 2 — INTAKE (new terms): a term with no matching row is INSERTed ONLY +// when it passes the intake allowlist (opts.intakeClasses). New rows land +// INERT (isActualTag=false, isInterestItem=false) awaiting editor curation — +// never active, never flooding the table with all 21k. With no allowlist, +// intake is OFF and unmatched terms are counted `skippedIntake`, not inserted. +// +// Idempotent: a second run with the same payload reports every row `unchanged`. +// dryRun:true computes the same plan without writing. + +// Taxonomy-owned fields the sync may write onto an EXISTING row. The editorial +// flags are deliberately excluded so an adopt/update never clobbers curation. +const ADOPT_FIELDS = ['label', 'name', 'titlePath', 'semaphoreId']; -function differs(existing, row) { - return TRACKED.some((f) => (existing[f] ?? null) !== (row[f] ?? null)); +function adoptDiffers(existing, row) { + return ADOPT_FIELDS.some((f) => (existing[f] ?? null) !== (row[f] ?? null)); +} + +// Case-insensitive membership, tolerant of full class URIs vs short names — +// mirrors mapper.classMatches so intake uses the same class-matching semantics. +function classAllowed(termClasses, allow) { + if (!Array.isArray(allow) || allow.length === 0) return false; + if (!Array.isArray(termClasses) || termClasses.length === 0) return false; + const allowLc = allow.map((w) => String(w).toLowerCase()); + return termClasses.some((c) => { + const cl = String(c ?? '').toLowerCase(); + const short = cl.includes('#') ? cl.slice(cl.lastIndexOf('#') + 1) + : cl.includes('/') ? cl.slice(cl.lastIndexOf('/') + 1) + : cl; + return allowLc.some((w) => w === cl || w === short); + }); } /** - * Upsert mapper rows into Tags. + * Two-tier upsert of mapper rows into Tags. * - * @param {Array} rows Output of mapAllTerms().rows + * @param {Array} rows Output of mapAllTerms().rows (each row also carries + * `classes` for the intake gate) * @param {object} [opts] - * @param {boolean} [opts.dryRun=false] compute the plan without writing - * @param {object} [opts.db] cds db (defaults to cds.db / connect) - * @returns {Promise<{inserted:number, updated:number, unchanged:number, total:number}>} + * @param {boolean} [opts.dryRun=false] compute the plan without writing + * @param {string[]} [opts.intakeClasses=[]] class allowlist for Tier-2 INSERT; + * empty ⇒ intake OFF (adopt-only) + * @param {object} [opts.db] cds db (defaults to cds.db / connect) + * @returns {Promise<{inserted:number, updated:number, unchanged:number, + * skippedIntake:number, total:number}>} */ export async function applyTerms(rows, opts = {}) { - const { dryRun = false } = opts; + const { dryRun = false, intakeClasses = [] } = opts; const db = opts.db ?? cds.db ?? (await cds.connect.to('db')); const { Tags } = cds.entities('com.sap.developers.ims'); let inserted = 0; let updated = 0; let unchanged = 0; + let skippedIntake = 0; const total = Array.isArray(rows) ? rows.length : 0; for (const row of rows ?? []) { - const fields = { - semaphoreId: row.semaphoreId, - label: row.label, - name: row.name, - titlePath: row.titlePath, - isActualTag: !!row.isActualTag, - isInterestItem: !!row.isInterestItem, - }; - // 1. Existing by semaphoreId. let existing = await db.run(SELECT.one.from(Tags).where({ semaphoreId: row.semaphoreId })); // 2. Adopt a legacy/CSV row that matches by name but has no semaphoreId yet. @@ -64,9 +99,16 @@ export async function applyTerms(rows, opts = {}) { ); } + // Tier 1 — ADOPT: update ONLY taxonomy-owned fields; never the editorial flags. if (existing) { - if (differs(existing, fields)) { - if (!dryRun) await db.run(UPDATE(Tags, existing.ID).set(fields)); + const adopt = { + semaphoreId: row.semaphoreId, + label: row.label, + name: row.name, + titlePath: row.titlePath, + }; + if (adoptDiffers(existing, adopt)) { + if (!dryRun) await db.run(UPDATE(Tags, existing.ID).set(adopt)); updated++; } else { unchanged++; @@ -74,12 +116,29 @@ export async function applyTerms(rows, opts = {}) { continue; } - // 3. Insert. Assign the UUID key explicitly for HANA parity. + // Tier 2 — INTAKE: insert a genuinely new term ONLY if class-allowlisted. + // New rows land inert; an editor promotes them by setting the flags later. + if (!classAllowed(row.classes, intakeClasses)) { + skippedIntake++; + continue; + } if (!dryRun) { - await db.run(INSERT.into(Tags).entries({ ID: cds.utils.uuid(), ...fields })); + // Assign legacyId from the Tags HANA sequence (SQLite: in-memory counter). + // Only when actually writing — a dryRun must not burn sequence numbers. + const legacyId = await getNextLegacyId('Tags', db); + await db.run(INSERT.into(Tags).entries({ + ID: cds.utils.uuid(), + legacyId, + semaphoreId: row.semaphoreId, + label: row.label, + name: row.name, + titlePath: row.titlePath, + isActualTag: false, + isInterestItem: false, + })); } inserted++; } - return { inserted, updated, unchanged, total }; + return { inserted, updated, unchanged, skippedIntake, total }; } diff --git a/srv/lib/semaphore-sync/config-keys.js b/srv/lib/semaphore-sync/config-keys.js new file mode 100644 index 000000000..1749acbf2 --- /dev/null +++ b/srv/lib/semaphore-sync/config-keys.js @@ -0,0 +1,87 @@ +// srv/lib/semaphore-sync/config-keys.js +// +// Single source of truth for the `semaphore.sync.*` tuning keys (#2477). +// Both the sync job's readConfig (srv/jobs/semaphore-tag-sync-job.js) and the +// Admin UI panel (AdminService.SemaphoreConfig) import this so the defaults, +// value types, and per-key help live in exactly one place. +// +// These are raw ImsConfig key/value rows (untyped strings on disk). valueType +// records how the job PARSES each string so the UI can validate the same way: +// 'string' — used verbatim (empty → default / undefined). +// 'csv' — comma-separated list, trimmed, blanks dropped (splitList). +// 'bool' — 'true'/'false' string; anything but 'false' is truthy for the +// enable-style keys, but see per-key `parse` for exact semantics. +// +// The Tier-2 intake allowlist (intakeClasses) IS enforced by the applier +// (srv/lib/semaphore-sync/applier.js — a new term is INSERTed only when its SES +// class is allowlisted; empty ⇒ adopt-only, unmatched terms counted +// skippedIntake). The job passes cfg.intakeClasses into applyTerms. + +export const SEMAPHORE_CONFIG_KEYS = [ + { + key: 'semaphore.sync.model', + label: 'SES model name', + valueType: 'string', + default: 'SAPCore', + description: 'Semaphore SES model to pull terms from.', + }, + { + key: 'semaphore.sync.lang', + label: 'Language', + valueType: 'string', + default: 'en', + description: 'Language code for term labels.', + }, + { + key: 'semaphore.sync.filter', + label: 'SES FILTER clause', + valueType: 'string', + default: '', + description: + 'Optional SES FILTER clause (e.g. "CL=Topic") — shrinks the ~61 MB allterms fetch. Empty = no filter (fetch all).', + }, + { + key: 'semaphore.sync.actualTagClasses', + label: 'Actual-tag classes', + valueType: 'csv', + default: '', + description: + 'Comma-separated class names/URIs whose terms are flagged isActualTag=true by the mapper.', + }, + { + key: 'semaphore.sync.interestItemClasses', + label: 'Interest-item classes', + valueType: 'csv', + default: '', + description: + 'Comma-separated class names/URIs whose terms are flagged isInterestItem=true by the mapper.', + }, + { + key: 'semaphore.sync.intakeClasses', + label: 'Tier-2 intake classes', + valueType: 'csv', + default: '', + description: + 'Tier-2 intake allowlist: comma-separated class names/URIs. A genuinely new term is INSERTed only if its SES class is listed here; empty = adopt-only (unmatched terms are skipped, counted skippedIntake). Enforced by the applier.', + }, + { + key: 'semaphore.sync.dryRun', + label: 'Dry run', + valueType: 'bool', + default: 'true', + description: + 'When true (DEFAULT), a run reports the plan without writing tags — validate the class→flag mapping against real data first. Set to "false" to persist.', + }, +]; + +// Fast lookups + validation for the setter action's allowlist. +export const SEMAPHORE_CONFIG_KEY_SET = new Set(SEMAPHORE_CONFIG_KEYS.map((k) => k.key)); + +// Ordered list of just the ImsConfig keys — the job's single-SELECT `in (...)`. +export const SEMAPHORE_CONFIG_KEY_NAMES = SEMAPHORE_CONFIG_KEYS.map((k) => k.key); + +/** Default string value for a key (as it would sit in ImsConfig), or '' . */ +export function defaultFor(key) { + const d = SEMAPHORE_CONFIG_KEYS.find((k) => k.key === key); + return d ? String(d.default ?? '') : ''; +} diff --git a/srv/lib/semaphore-sync/mapper.js b/srv/lib/semaphore-sync/mapper.js index 4aa594d11..27c3cd37c 100644 --- a/srv/lib/semaphore-sync/mapper.js +++ b/srv/lib/semaphore-sync/mapper.js @@ -161,6 +161,9 @@ export function mapAllTerms(data, opts = {}) { titlePath: deriveTitlePath(term), isActualTag, isInterestItem, + // Raw SES classes, carried through for the applier's Tier-2 intake gate + // and the dry-run class histogram (job). Not persisted to Tags. + classes: Array.isArray(term.classes) ? term.classes.map(String) : [], }; // De-dupe on semaphoreId (last write wins) — SES should be unique but be safe. diff --git a/test/unit/admin-semaphore-config.test.js b/test/unit/admin-semaphore-config.test.js new file mode 100644 index 000000000..d87bcea28 --- /dev/null +++ b/test/unit/admin-semaphore-config.test.js @@ -0,0 +1,125 @@ +// test/unit/admin-semaphore-config.test.js +// +// #2477 — AdminService.SemaphoreConfig: a @cds.persistence.skip viewer over the +// semaphore.sync.* ImsConfig tuning keys, with bound setValue(value)/clearValue() +// actions. READ synthesizes one row per shared descriptor key (defaults layered +// under any live ImsConfig value); setValue upserts the raw row; clearValue +// deletes it so the documented default takes effect again. Unknown keys reject. + +import { describe, it, expect, beforeEach } from 'vitest'; +import cds from '@sap/cds'; +import { SEMAPHORE_CONFIG_KEYS } from '../../srv/lib/semaphore-sync/config-keys.js'; + +const NS = 'com.sap.developers.ims'; +const project = cds.test('serve', '--project', '.', '--in-memory'); +const ADMIN_AUTH = { auth: { username: 'admin', password: 'admin' } }; + +let db, ImsConfig; +const ALL_KEYS = SEMAPHORE_CONFIG_KEYS.map((k) => k.key); + +async function readIms(key) { + const row = await db.run(SELECT.one.from(ImsConfig).where({ key })); + return row ? String(row.value) : null; +} + +describe('AdminService.SemaphoreConfig viewer + setValue/clearValue (#2477)', () => { + beforeEach(async () => { + db = await cds.connect.to('db'); + ({ ImsConfig } = cds.entities(NS)); + await db.run(DELETE.from(ImsConfig).where({ key: { in: ALL_KEYS } })); + }); + + it('READ synthesizes every descriptor key with its default when ImsConfig is empty', async () => { + const res = await project.get('/admin/SemaphoreConfig', ADMIN_AUTH); + expect(res.status).toBe(200); + const rows = res.data.value; + expect(rows.map((r) => r.key).sort()).toEqual([...ALL_KEYS].sort()); + for (const d of SEMAPHORE_CONFIG_KEYS) { + const row = rows.find((r) => r.key === d.key); + expect(row.isDefault).toBe(true); + expect(row.rawDbValue).toBeNull(); + expect(row.effectiveValue).toBe(String(d.default ?? '')); + expect(row.defaultValue).toBe(String(d.default ?? '')); + expect(row.valueType).toBe(d.valueType); + } + }); + + it('setValue upserts a string key and the resolved row reflects it', async () => { + const res = await project.post( + `/admin/SemaphoreConfig(key='semaphore.sync.model')/AdminService.setValue`, + { value: 'SAPExtended' }, + ADMIN_AUTH, + ); + expect(res.status).toBe(200); + expect(res.data.key).toBe('semaphore.sync.model'); + expect(res.data.effectiveValue).toBe('SAPExtended'); + expect(res.data.rawDbValue).toBe('SAPExtended'); + expect(res.data.isDefault).toBe(false); + expect(await readIms('semaphore.sync.model')).toBe('SAPExtended'); + }); + + it('setValue round-trips a csv value verbatim (parsing is the job’s concern)', async () => { + const res = await project.post( + `/admin/SemaphoreConfig(key='semaphore.sync.actualTagClasses')/AdminService.setValue`, + { value: 'Topic, Product , Skill' }, + ADMIN_AUTH, + ); + expect(res.status).toBe(200); + expect(res.data.effectiveValue).toBe('Topic, Product , Skill'); + expect(await readIms('semaphore.sync.actualTagClasses')).toBe('Topic, Product , Skill'); + }); + + it('setValue stores a bool key as its string and updates in place on re-set', async () => { + await project.post( + `/admin/SemaphoreConfig(key='semaphore.sync.dryRun')/AdminService.setValue`, + { value: 'false' }, ADMIN_AUTH, + ); + expect(await readIms('semaphore.sync.dryRun')).toBe('false'); + // Re-set updates the same row (no duplicate insert). + const res = await project.post( + `/admin/SemaphoreConfig(key='semaphore.sync.dryRun')/AdminService.setValue`, + { value: 'true' }, ADMIN_AUTH, + ); + expect(res.data.effectiveValue).toBe('true'); + const rows = await db.run(SELECT.from(ImsConfig).where({ key: 'semaphore.sync.dryRun' })); + expect(rows.length).toBe(1); + }); + + it('clearValue removes the row so the default takes effect again', async () => { + await project.post( + `/admin/SemaphoreConfig(key='semaphore.sync.model')/AdminService.setValue`, + { value: 'SAPExtended' }, ADMIN_AUTH, + ); + expect(await readIms('semaphore.sync.model')).toBe('SAPExtended'); + const res = await project.post( + `/admin/SemaphoreConfig(key='semaphore.sync.model')/AdminService.clearValue`, + {}, ADMIN_AUTH, + ); + expect(res.status).toBe(200); + expect(res.data.isDefault).toBe(true); + expect(res.data.effectiveValue).toBe('SAPCore'); + expect(await readIms('semaphore.sync.model')).toBeNull(); + }); + + it('setValue rejects a key outside the semaphore.sync.* allowlist with 400', async () => { + const FOREIGN = 'semaphore.sync.notAKey'; + await db.run(DELETE.from(ImsConfig).where({ key: FOREIGN })); + const res = await project.post( + `/admin/SemaphoreConfig(key='${FOREIGN}')/AdminService.setValue`, + { value: 'true' }, + { ...ADMIN_AUTH, validateStatus: () => true }, + ); + expect(res.status).toBe(400); + // And the non-allowlisted key was NOT written. + expect(await readIms(FOREIGN)).toBeNull(); + }); + + it('rejects unauthenticated callers', async () => { + const res = await project.post( + `/admin/SemaphoreConfig(key='semaphore.sync.model')/AdminService.setValue`, + { value: 'x' }, + { validateStatus: () => true }, + ); + expect(res.status).toBe(401); + }); +}); diff --git a/test/unit/semaphore-sync-applier.test.js b/test/unit/semaphore-sync-applier.test.js index f6c47b49c..1557060d3 100644 --- a/test/unit/semaphore-sync-applier.test.js +++ b/test/unit/semaphore-sync-applier.test.js @@ -5,6 +5,10 @@ import { applyTerms } from '../../srv/lib/semaphore-sync/applier.js'; cds.test('serve', '--project', '.', '--in-memory'); +// A mapper-shaped row. `classes` carries the raw SES classes used by the Tier-2 +// intake gate. `isActualTag`/`isInterestItem` are present on the row but the +// applier deliberately IGNORES them for existing rows (editorial-owned) and +// forces them false on intake inserts. const ROW = (over = {}) => ({ semaphoreId: 's1', label: 'SAP S/4HANA', @@ -12,10 +16,11 @@ const ROW = (over = {}) => ({ titlePath: 'Software Product : SAP S/4HANA', isActualTag: true, isInterestItem: false, + classes: ['Topic'], ...over, }); -describe('semaphore applyTerms', () => { +describe('semaphore applyTerms — two-tier (#2184)', () => { let db; let Tags; @@ -25,51 +30,139 @@ describe('semaphore applyTerms', () => { await DELETE.from(Tags); }); - it('inserts a new term with all Semaphore fields', async () => { + // ── Tier 2: intake ──────────────────────────────────────────────────────── + + it('does NOT insert an unmatched term when intake is off (no allowlist)', async () => { const res = await applyTerms([ROW()], { db }); - expect(res).toEqual({ inserted: 1, updated: 0, unchanged: 0, total: 1 }); + expect(res).toEqual({ inserted: 0, updated: 0, unchanged: 0, skippedIntake: 1, total: 1 }); + expect(await SELECT.from(Tags)).toHaveLength(0); + }); + + it('inserts an unmatched term when its class is allowlisted, flags forced inert', async () => { + const res = await applyTerms([ROW()], { db, intakeClasses: ['Topic'] }); + expect(res).toEqual({ inserted: 1, updated: 0, unchanged: 0, skippedIntake: 0, total: 1 }); const t = await SELECT.one.from(Tags).where({ semaphoreId: 's1' }); expect(t).toMatchObject({ name: 'sap s 4hana', label: 'SAP S/4HANA', titlePath: 'Software Product : SAP S/4HANA', - isActualTag: true, isInterestItem: false, + // New rows land inert regardless of the row's isActualTag:true. + isActualTag: false, isInterestItem: false, }); expect(t.ID).toBeTruthy(); + // Insert path assigns a legacyId (AdminService's before-CREATE hook is + // bypassed by the direct cds.db write, so the applier must set it). #2479. + expect(t.legacyId).toBeTruthy(); }); + it('skips an unmatched term whose class is not in the allowlist', async () => { + const res = await applyTerms([ROW({ classes: ['SomethingElse'] })], { db, intakeClasses: ['Topic'] }); + expect(res).toEqual({ inserted: 0, updated: 0, unchanged: 0, skippedIntake: 1, total: 1 }); + expect(await SELECT.from(Tags)).toHaveLength(0); + }); + + it('matches allowlist against a full class URI by its leaf', async () => { + const res = await applyTerms( + [ROW({ classes: ['http://sap.com/schema#Topic'] })], + { db, intakeClasses: ['Topic'] }, + ); + expect(res.inserted).toBe(1); + }); + + // ── Tier 1: adopt/update existing ────────────────────────────────────────── + it('is idempotent: a second run reports everything unchanged', async () => { - await applyTerms([ROW()], { db }); - const res = await applyTerms([ROW()], { db }); - expect(res).toEqual({ inserted: 0, updated: 0, unchanged: 1, total: 1 }); + await applyTerms([ROW()], { db, intakeClasses: ['Topic'] }); + const res = await applyTerms([ROW()], { db, intakeClasses: ['Topic'] }); + expect(res).toEqual({ inserted: 0, updated: 0, unchanged: 1, skippedIntake: 0, total: 1 }); expect(await SELECT.from(Tags)).toHaveLength(1); }); - it('updates in place when a synced term is renamed (same semaphoreId)', async () => { - await applyTerms([ROW()], { db }); - const res = await applyTerms([ROW({ label: 'SAP S/4HANA Cloud', titlePath: 'Software Product : SAP S/4HANA Cloud' })], { db }); - expect(res).toEqual({ inserted: 0, updated: 1, unchanged: 0, total: 1 }); + it('updates taxonomy fields in place when a synced term is renamed', async () => { + await applyTerms([ROW()], { db, intakeClasses: ['Topic'] }); + const res = await applyTerms( + [ROW({ label: 'SAP S/4HANA Cloud', titlePath: 'Software Product : SAP S/4HANA Cloud' })], + { db }, + ); + expect(res).toEqual({ inserted: 0, updated: 1, unchanged: 0, skippedIntake: 0, total: 1 }); const rows = await SELECT.from(Tags).where({ semaphoreId: 's1' }); expect(rows).toHaveLength(1); // no duplicate expect(rows[0].label).toBe('SAP S/4HANA Cloud'); }); + it('NEVER overwrites editorial flags on an existing row', async () => { + // Existing curated row: hand-set isActualTag=true, isInterestItem=true. + await INSERT.into(Tags).entries({ + ID: 'curated-1', semaphoreId: 's1', name: 'sap s 4hana', + label: 'SAP S/4HANA', titlePath: 'Software Product : SAP S/4HANA', + isActualTag: true, isInterestItem: true, + }); + // Sync row carries the opposite flags — must be ignored. + const res = await applyTerms( + [ROW({ isActualTag: false, isInterestItem: false })], + { db, intakeClasses: ['Topic'] }, + ); + expect(res).toEqual({ inserted: 0, updated: 0, unchanged: 1, skippedIntake: 0, total: 1 }); + const t = await SELECT.one.from(Tags).where({ ID: 'curated-1' }); + expect(t.isActualTag).toBe(true); + expect(t.isInterestItem).toBe(true); + }); + + it('updates taxonomy fields yet preserves flags when both change', async () => { + await INSERT.into(Tags).entries({ + ID: 'curated-2', semaphoreId: 's1', name: 'sap s 4hana', + label: 'Old Label', titlePath: 'old', isActualTag: true, isInterestItem: true, + }); + const res = await applyTerms([ROW({ isActualTag: false, isInterestItem: false })], { db }); + expect(res.updated).toBe(1); + const t = await SELECT.one.from(Tags).where({ ID: 'curated-2' }); + expect(t.label).toBe('SAP S/4HANA'); // taxonomy field adopted + expect(t.titlePath).toBe('Software Product : SAP S/4HANA'); + expect(t.isActualTag).toBe(true); // flags untouched + expect(t.isInterestItem).toBe(true); + }); + it('adopts a legacy row matched by name that lacks a semaphoreId', async () => { - await INSERT.into(Tags).entries({ ID: 'legacy-1', name: 'sap s 4hana', titlePath: 'old', legacyId: 42 }); + await INSERT.into(Tags).entries({ + ID: 'legacy-1', name: 'sap s 4hana', titlePath: 'old', legacyId: 42, + isActualTag: true, isInterestItem: false, + }); const res = await applyTerms([ROW()], { db }); - expect(res).toEqual({ inserted: 0, updated: 1, unchanged: 0, total: 1 }); + expect(res).toEqual({ inserted: 0, updated: 1, unchanged: 0, skippedIntake: 0, total: 1 }); const t = await SELECT.one.from(Tags).where({ ID: 'legacy-1' }); expect(t.semaphoreId).toBe('s1'); expect(t.titlePath).toBe('Software Product : SAP S/4HANA'); + expect(t.isActualTag).toBe(true); // pre-existing flag preserved through adoption expect(await SELECT.from(Tags)).toHaveLength(1); // adopted, not duplicated }); - it('dryRun computes the plan without writing', async () => { - const res = await applyTerms([ROW()], { db, dryRun: true }); - expect(res).toEqual({ inserted: 1, updated: 0, unchanged: 0, total: 1 }); + // ── dry run / empty ──────────────────────────────────────────────────────── + + it('dryRun computes the intake plan without writing', async () => { + const res = await applyTerms([ROW()], { db, dryRun: true, intakeClasses: ['Topic'] }); + expect(res).toEqual({ inserted: 1, updated: 0, unchanged: 0, skippedIntake: 0, total: 1 }); expect(await SELECT.from(Tags)).toHaveLength(0); }); + it('dryRun does NOT assign a legacyId (no sequence burn on a plan)', async () => { + // A real write immediately after must get the first available legacyId — if + // dryRun had consumed one, this row's legacyId would skip a value. + await applyTerms([ROW()], { db, dryRun: true, intakeClasses: ['Topic'] }); + await applyTerms([ROW()], { db, intakeClasses: ['Topic'] }); + const written = await SELECT.one.from(Tags).where({ semaphoreId: 's1' }); + const fresh = await applyTerms( + [ROW({ semaphoreId: 's2', name: 'other' })], + { db, intakeClasses: ['Topic'] }, + ); + expect(fresh.inserted).toBe(1); + const next = await SELECT.one.from(Tags).where({ semaphoreId: 's2' }); + // Consecutive real writes get consecutive legacyIds; the dryRun did not + // consume one in between. + expect(next.legacyId).toBe(written.legacyId + 1); + }); + it('handles an empty payload', async () => { - expect(await applyTerms([], { db })).toEqual({ inserted: 0, updated: 0, unchanged: 0, total: 0 }); + expect(await applyTerms([], { db })).toEqual({ + inserted: 0, updated: 0, unchanged: 0, skippedIntake: 0, total: 0, + }); }); }); diff --git a/test/unit/semaphore-tag-sync-job.test.js b/test/unit/semaphore-tag-sync-job.test.js index 7314675f6..56fc535de 100644 --- a/test/unit/semaphore-tag-sync-job.test.js +++ b/test/unit/semaphore-tag-sync-job.test.js @@ -66,28 +66,39 @@ describe('runSemaphoreTagSync', () => { expect(await SELECT.from(Tags)).toHaveLength(0); }); - it('dryRun default: reports the plan without writing', async () => { + it('dryRun default: reports the plan (with class histogram) without writing', async () => { __setFlagForTest('SEMAPHORE_SYNC_ENABLED', true); - await setConfig(db, { 'semaphore.sync.interestItemClasses': 'IndustryCluster' }); const res = await runSemaphoreTagSync(null, { _deps: okDeps() }); expect(res.ok).toBe(true); expect(res.dryRun).toBe(true); - expect(res).toMatchObject({ fetched: 2, mapped: 2, inserted: 2, updated: 0 }); + // Intake is off by default (no intakeClasses) → both unmatched terms are + // counted skippedIntake, nothing inserted. The histogram surfaces the real + // class distribution so the FILTER/intakeClasses can be chosen from data. + expect(res).toMatchObject({ fetched: 2, mapped: 2, inserted: 0, updated: 0, skippedIntake: 2 }); + expect(res.classHistogram).toContain('SoftwareProduct=1'); + expect(res.classHistogram).toContain('IndustryCluster=1'); + expect(res.intakeClasses).toBe('(none — adopt-only)'); expect(await SELECT.from(Tags)).toHaveLength(0); // dry run wrote nothing }); - it('writes tags when dryRun is disabled', async () => { + it('writes only allowlisted new tags (inert) when dryRun is disabled', async () => { __setFlagForTest('SEMAPHORE_SYNC_ENABLED', true); await setConfig(db, { 'semaphore.sync.dryRun': 'false', - 'semaphore.sync.interestItemClasses': 'IndustryCluster', + // Only SoftwareProduct terms are admitted; IndustryCluster (Retail) is skipped. + 'semaphore.sync.intakeClasses': 'SoftwareProduct', }); const res = await runSemaphoreTagSync(null, { _deps: okDeps() }); - expect(res).toMatchObject({ ok: true, dryRun: false, inserted: 2 }); + expect(res).toMatchObject({ ok: true, dryRun: false, inserted: 1, skippedIntake: 1 }); const tags = await SELECT.from(Tags); - expect(tags).toHaveLength(2); - const retail = tags.find((t) => t.semaphoreId === 's2'); - expect(retail.isInterestItem).toBe(true); + expect(tags).toHaveLength(1); + const s4 = tags.find((t) => t.semaphoreId === 's1'); + expect(s4.name).toBe('sap s 4hana'); + // New intake rows always land inert, awaiting editor curation. + expect(s4.isActualTag).toBe(false); + expect(s4.isInterestItem).toBe(false); + // The non-allowlisted term was not written. + expect(tags.find((t) => t.semaphoreId === 's2')).toBeUndefined(); }); it('fails shut on a fetch error — writes nothing', async () => { diff --git a/test/unit/srv/admin-job-controls.test.js b/test/unit/srv/admin-job-controls.test.js index 561e5b173..c57a8e6b5 100644 --- a/test/unit/srv/admin-job-controls.test.js +++ b/test/unit/srv/admin-job-controls.test.js @@ -163,6 +163,57 @@ describe('AdminService.JobControls', () => { expect(row.lastSuccessAt).toBeTruthy(); }); + // ───────────────────────────────────────────────────────────────── + // #2478 — a job that FAIL-SHUTS by RETURNING { ok:false } (rather than + // throwing) must be recorded as a failed run, not mislogged SUCCESS. + // Regression guard for the semaphore-tag-sync "no new tags, but green" + // symptom: the chassis now inspects the runner's return value. + // ───────────────────────────────────────────────────────────────── + it('records a FAILED run when the fn returns { ok:false } (not a throw)', async () => { + const jobName = nextJobName(); + registerOne(jobName, async () => ({ ok: false, error: 'fetch HTTP 500' })); + await callRunJob(jobName); + await new Promise(resolve => setTimeout(resolve, 200)); + + const { JobLastRun, PipelineLog } = cds.entities('com.sap.developers.ims'); + const row = await SELECT.one.from(JobLastRun).where({ jobName }); + expect(row).toBeTruthy(); + // Failure path: lastErrorAt + message set, lastSuccessAt untouched (null). + expect(row.lastErrorAt).toBeTruthy(); + expect(row.lastSuccessAt).toBeFalsy(); + expect(row.lastErrorMessage).toBe('fetch HTTP 500'); + // The PipelineLog end row for this run is FAILED, not SUCCESS. + const plog = await SELECT.from(PipelineLog) + .where({ pipelineType: 'SCHEDULED_JOB' }); + const mine = plog.filter(p => (p.metadata ?? '').includes(jobName)); + expect(mine.length).toBeGreaterThan(0); + expect(mine.every(p => p.status === 'FAILED')).toBe(true); + }); + + it('falls back to a generic error message when ok:false carries no error', async () => { + const jobName = nextJobName(); + registerOne(jobName, async () => ({ ok: false })); + await callRunJob(jobName); + await new Promise(resolve => setTimeout(resolve, 200)); + + const { JobLastRun } = cds.entities('com.sap.developers.ims'); + const row = await SELECT.one.from(JobLastRun).where({ jobName }); + expect(row.lastErrorAt).toBeTruthy(); + expect(row.lastErrorMessage).toContain('returned ok:false'); + }); + + it('still records SUCCESS when the fn returns { ok:true }', async () => { + const jobName = nextJobName(); + registerOne(jobName, async () => ({ ok: true, inserted: 3 })); + await callRunJob(jobName); + await new Promise(resolve => setTimeout(resolve, 200)); + + const { JobLastRun } = cds.entities('com.sap.developers.ims'); + const row = await SELECT.one.from(JobLastRun).where({ jobName }); + expect(row.lastSuccessAt).toBeTruthy(); + expect(row.lastErrorAt).toBeFalsy(); + }); + // ───────────────────────────────────────────────────────────────── // #750: nextRunsIso (forward-visibility window for the Board tile) // ───────────────────────────────────────────────────────────────── diff --git a/test/unit/srv/run-job-by-name-opts.test.js b/test/unit/srv/run-job-by-name-opts.test.js index e40d6047c..0e2fbdb91 100644 --- a/test/unit/srv/run-job-by-name-opts.test.js +++ b/test/unit/srv/run-job-by-name-opts.test.js @@ -83,3 +83,120 @@ describe('runJobByName opts threading (Phase 4.6)', () => { expect(receivedOpts.user).toBe('tom@example.com'); }); }); + +describe('scheduler soft-failure detection (#2478)', () => { + beforeAll(async () => { + await cds.deploy([ + path.join(process.cwd(), 'db'), + path.join(process.cwd(), 'srv'), + ]).to('sqlite::memory:'); + }); + + beforeEach(() => { + _resetJobRegistry(); + }); + + async function latestLogFor(jobName) { + const { PipelineLog, JobLastRun } = cds.entities('com.sap.developers.ims'); + const rows = await SELECT.from(PipelineLog) + .where({ pipelineType: 'SCHEDULED_JOB' }) + .orderBy({ startedAt: 'desc' }); + const log = rows.find(r => { + try { return JSON.parse(r.metadata || '{}').jobName === jobName; } + catch { return false; } + }); + const lastRun = await SELECT.one.from(JobLastRun).where({ jobName }); + return { log, lastRun }; + } + + it('runner returning {ok:false, error} → FAILED with error in errorDetails (arg 4), failed JobLastRun', async () => { + registerJob({ + jobName: 'soft-fail-cron', + schedule: '0 0 * * *', + ttlMs: 60000, + description: 'test', + fn: () => ({ ok: false, error: 'fetch/mapping failed; nothing written' }), + }); + const res = await runJobByName('soft-fail-cron'); + expect(res.outcome).toBe('error'); + expect(res.errorMessage).toBe('fetch/mapping failed; nothing written'); + + const { log, lastRun } = await latestLogFor('soft-fail-cron'); + expect(log.status).toBe('FAILED'); + expect(log.errorDetails).toBe('fetch/mapping failed; nothing written'); + // error must NOT be smuggled into the summary column + expect(log.summary).not.toBe('fetch/mapping failed; nothing written'); + expect(lastRun.lastErrorAt).toBeTruthy(); + expect(lastRun.lastErrorMessage).toBe('fetch/mapping failed; nothing written'); + expect(lastRun.lastSuccessAt).toBeFalsy(); + }); + + it('runner returning {ok:false} without error → FAILED with synthesized message', async () => { + registerJob({ + jobName: 'soft-fail-noerr-cron', + schedule: '0 0 * * *', + ttlMs: 60000, + description: 'test', + fn: () => ({ ok: false }), + }); + const res = await runJobByName('soft-fail-noerr-cron'); + expect(res.outcome).toBe('error'); + expect(res.errorMessage).toBe('soft-fail-noerr-cron returned ok:false'); + + const { log, lastRun } = await latestLogFor('soft-fail-noerr-cron'); + expect(log.status).toBe('FAILED'); + expect(log.errorDetails).toBe('soft-fail-noerr-cron returned ok:false'); + expect(lastRun.lastErrorMessage).toBe('soft-fail-noerr-cron returned ok:false'); + }); + + it('thrown exception still recorded as FAILED', async () => { + registerJob({ + jobName: 'throw-cron', + schedule: '0 0 * * *', + ttlMs: 60000, + description: 'test', + fn: () => { throw new Error('boom'); }, + }); + const res = await runJobByName('throw-cron'); + expect(res.outcome).toBe('error'); + expect(res.errorMessage).toBe('boom'); + + const { log, lastRun } = await latestLogFor('throw-cron'); + expect(log.status).toBe('FAILED'); + expect(log.errorDetails).toBe('boom'); + expect(lastRun.lastErrorMessage).toBe('boom'); + }); + + it('runner returning {ok:true} still succeeds', async () => { + registerJob({ + jobName: 'ok-true-cron', + schedule: '0 0 * * *', + ttlMs: 60000, + description: 'test', + fn: () => ({ ok: true, processed: 5 }), + }); + const res = await runJobByName('ok-true-cron'); + expect(res.outcome).toBe('success'); + + const { log, lastRun } = await latestLogFor('ok-true-cron'); + expect(log.status).toBe('SUCCESS'); + expect(log.errorDetails).toBeFalsy(); + expect(lastRun.lastSuccessAt).toBeTruthy(); + expect(lastRun.lastErrorAt).toBeFalsy(); + }); + + it('runner returning a plain summary (no ok field) still succeeds', async () => { + registerJob({ + jobName: 'plain-summary-cron', + schedule: '0 0 * * *', + ttlMs: 60000, + description: 'test', + fn: () => ({ processed: 3, skipped: 1 }), + }); + const res = await runJobByName('plain-summary-cron'); + expect(res.outcome).toBe('success'); + + const { log } = await latestLogFor('plain-summary-cron'); + expect(log.status).toBe('SUCCESS'); + }); +});