diff --git a/SW.Bitween.Api/Controllers/GatewayController.cs b/SW.Bitween.Api/Controllers/GatewayController.cs index 83a469b5..9ebbab2b 100644 --- a/SW.Bitween.Api/Controllers/GatewayController.cs +++ b/SW.Bitween.Api/Controllers/GatewayController.cs @@ -15,32 +15,45 @@ namespace SW.Bitween.Controllers; [ApiController] -[Route("api/[controller]")] +[Route("api/gateway")] public class GatewayController( BitweenDbContext dbContext, RequestContext requestContext, IInfolinkCache cache, XchangeService xchangeService) : ControllerBase { - [HttpPost("{gatewayApiName}/sync")] - public Task PostSync([FromRoute] string gatewayApiName) + /// + /// {gateway's url name}/sync or /async. The url name can be several segments, + /// so it is everything before the last one — a route can't put a literal after a catch-all. + /// + /// + /// The literal "gateway" is what keeps this clear of CqApi, which owns the rest of /api/ with + /// templates up to three segments deep: without it, /api/logistics/slim/sync is an admin call. + /// + [HttpPost("{**path}")] + public Task Post([FromRoute] string path) { - return ProcessAsync(gatewayApiName, resultSync: true); - } + var trimmed = (path ?? "").Trim('/').ToLowerInvariant(); + var split = trimmed.LastIndexOf('/'); + if (split <= 0) + return Task.FromResult(NotFound()); - [HttpPost("{gatewayApiName}/async")] - public Task PostAsync([FromRoute] string gatewayApiName) - { - return ProcessAsync(gatewayApiName, resultSync: false); + return trimmed[(split + 1)..] switch + { + "sync" => ProcessAsync(trimmed[..split], resultSync: true), + "async" => ProcessAsync(trimmed[..split], resultSync: false), + _ => Task.FromResult(NotFound()), + }; } - private async Task ProcessAsync([FromRoute] string gatewayApiName, bool resultSync) + private async Task ProcessAsync(string gatewayApiName, bool resultSync) { var globalAdapterValuesSet = await cache.ListGlobalAdapterValuesSetsAsync(); + // Lowered on the column too: rows saved before names had to be lowercase may not be. var apiGateway = await dbContext.Set() .Include(ag => ag.Partners) .ThenInclude(agp => agp.Partner) - .FirstOrDefaultAsync(ag => ag.UrlName == gatewayApiName); + .FirstOrDefaultAsync(ag => ag.UrlName.ToLower() == gatewayApiName); if (apiGateway == null) return NotFound(); @@ -74,7 +87,9 @@ private async Task ProcessAsync([FromRoute] string gatewayApiName var json = await new StreamReader(HttpContext.Request.Body).ReadToEndAsync(); - var xchangeFile = new XchangeFile(json, $"{gatewayApiName}.json"); + // The file name travels with the exchange into handlers, some of which write it to disk — + // a slash there is a directory nobody asked for. + var xchangeFile = new XchangeFile(json, $"{gatewayApiName.Replace('/', '-')}.json"); var validatorProperties = subscription.ValidatorProperties.ToDictionary() .Fill(partner, globalAdapterValuesSet); diff --git a/SW.Bitween.Api/Resources/ApiGateways/GatewayUrlName.cs b/SW.Bitween.Api/Resources/ApiGateways/GatewayUrlName.cs index d9620f83..0250fb0c 100644 --- a/SW.Bitween.Api/Resources/ApiGateways/GatewayUrlName.cs +++ b/SW.Bitween.Api/Resources/ApiGateways/GatewayUrlName.cs @@ -8,14 +8,18 @@ namespace SW.Bitween.Resources.ApiGateways; /// -/// The url name is a path segment — partners call /api/Gateway/{urlName}/sync — so -/// anything needing escaping there makes a gateway that reads as configured and cannot be -/// reached. A space is the one that actually happens: it saves, the endpoint shown on the -/// page is the one the partner copies, and the call 404s with nothing on screen to explain it. +/// The url name is the path partners call — /api/gateway/{urlName}/sync — so anything +/// needing escaping there makes a gateway that reads as configured and cannot be reached. A +/// space is the one that actually happens: it saves, the endpoint shown on the page is the one +/// the partner copies, and the call 404s with nothing on screen to explain it. /// +/// +/// It may run to several segments (logistics/slim/orders): clients lay out their own +/// URL scheme, and the shape differs between them, so no position means anything to us. +/// internal static partial class GatewayUrlName { - [GeneratedRegex("^[a-z0-9]+(?:[-_][a-z0-9]+)*$")] + [GeneratedRegex(@"^[a-z0-9]+(?:[-_][a-z0-9]+)*(?:/[a-z0-9]+(?:[-_][a-z0-9]+)*)*\z")] private static partial Regex Allowed(); public static void Validate(string urlName) @@ -26,7 +30,15 @@ public static void Validate(string urlName) if (!Allowed().IsMatch(urlName)) throw new SWValidationException("GATEWAY_URL_NAME_INVALID", $"'{urlName}' cannot be used in a URL. Use lowercase letters, digits, hyphens " + - "and underscores only — no spaces, and not starting or ending with a separator."); + "and underscores, with / between parts — no spaces, and no part starting or " + + "ending with a separator."); + + // The call ends in /sync or /async, and it is the last segment that says which. A name + // ending in one would make "orders/sync" and "orders" the same address. + var last = urlName[(urlName.LastIndexOf('/') + 1)..]; + if (last is "sync" or "async") + throw new SWValidationException("GATEWAY_URL_NAME_INVALID", + $"'{urlName}' cannot end in '{last}' — partners add /sync or /async after it."); } /// @@ -41,7 +53,9 @@ public static void Validate(string urlName) public static async Task EnsureIsFree(BitweenDbContext dbContext, string urlName, int? existingId = null) { var taken = await dbContext.Set().AsNoTracking() - .Where(gateway => gateway.UrlName == urlName && gateway.Id != existingId) + // Lowered because rows saved before names had to be lowercase may not be, and the + // partner's call is matched without regard to case. + .Where(gateway => gateway.UrlName.ToLower() == urlName && gateway.Id != existingId) .Select(gateway => gateway.Name) .FirstOrDefaultAsync(); diff --git a/SW.Bitween.IntegrationTests/Tests/ApiGatewayTests.cs b/SW.Bitween.IntegrationTests/Tests/ApiGatewayTests.cs index 8aa2c83a..b8276ada 100644 --- a/SW.Bitween.IntegrationTests/Tests/ApiGatewayTests.cs +++ b/SW.Bitween.IntegrationTests/Tests/ApiGatewayTests.cs @@ -67,11 +67,16 @@ private async Task AddPartner(int gatewayId, ApiGatewayPartnerCreate model) [Theory] [InlineData("order sync")] // the one that actually happens — a space [InlineData("Order-Sync")] // upper case, which the route match is not - [InlineData("orders/sync")] // a second path segment [InlineData("-orders")] + [InlineData("/orders")] // an empty part, front or back or middle + [InlineData("orders/")] + [InlineData("logistics//orders")] + [InlineData("orders/sync")] // ends where /sync or /async goes + [InlineData("orders/async")] + [InlineData("orders\n")] // $ would let a final newline through public async Task A_url_name_that_cannot_appear_in_a_path_is_refused(string urlName) { - // Partners call /api/Gateway/{urlName}/sync. Anything needing escaping there produces a + // Partners call /api/gateway/{urlName}/sync. Anything needing escaping there produces a // gateway that reads as configured on every screen and cannot be reached — and the URL // the partner is given to copy is the broken one. var ex = await Assert.ThrowsAsync(() => CreateGateway(urlName)); @@ -83,6 +88,8 @@ public async Task A_url_name_that_cannot_appear_in_a_path_is_refused(string urlN [InlineData("order-sync")] [InlineData("order_sync_v2")] [InlineData("orders2")] + [InlineData("logistics/slim/orders")] // a client's own scheme, as many parts as it has + [InlineData("sync/orders")] // only the last part is reserved public async Task A_usable_url_name_is_accepted(string urlName) { // The guard has to stay narrow: refusing a legitimate name blocks a gateway from @@ -91,6 +98,23 @@ public async Task A_usable_url_name_is_accepted(string urlName) Assert.True(id > 0); } + [Fact] + public async Task A_url_name_matching_an_older_mixed_case_one_is_taken() + { + // Rows saved before names had to be lowercase can still hold capitals, and partner calls + // match without regard to case — so the lowercase twin would answer on the same address. + var legacy = Unique("Legacy-Orders"); + await using (var scope = fixture.CreateScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + db.Set().Add(new ApiGateway { Name = Unique("Legacy"), UrlName = legacy }); + await db.SaveChangesAsync(); + } + + var ex = await Assert.ThrowsAsync(() => CreateGateway(legacy.ToLowerInvariant())); + Assert.StartsWith("GATEWAY_URL_NAME_TAKEN", ex.Message); + } + [Fact] public async Task Attaching_a_partner_demands_an_integration_of_the_gateway_kind() { diff --git a/SW.Bitween.Web/ClientApp/src/lib/__tests__/identifiers.test.ts b/SW.Bitween.Web/ClientApp/src/lib/__tests__/identifiers.test.ts new file mode 100644 index 00000000..076e22fc --- /dev/null +++ b/SW.Bitween.Web/ClientApp/src/lib/__tests__/identifiers.test.ts @@ -0,0 +1,51 @@ +import { describe, it, expect } from 'vitest'; +import { finishUrlName, toUrlName, urlNameProblem } from '../identifiers'; + +describe('toUrlName', () => { + it('keeps / so a url name can run to several parts', () => { + expect(toUrlName('Logistics/Slim/Orders')).toBe('logistics/slim/orders'); + }); + + it('collapses repeated slashes and drops a leading one', () => { + expect(toUrlName('/logistics//slim')).toBe('logistics/slim'); + }); + + it('lets a trailing separator survive mid-typing', () => { + expect(toUrlName('logistics/')).toBe('logistics/'); + expect(toUrlName('orders-')).toBe('orders-'); + }); + + it('turns spaces, newlines and anything else a path cannot hold into a hyphen', () => { + expect(toUrlName('Returns intake\n')).toBe('returns-intake-'); + expect(toUrlName('orders?v=2')).toBe('orders-v-2'); + }); + + it('never lets separators double up or touch a slash', () => { + expect(toUrlName('orders--v2')).toBe('orders-v2'); + expect(toUrlName('orders-_v2')).toBe('orders-v2'); + expect(toUrlName('logistics-/-slim')).toBe('logistics/slim'); + }); +}); + +describe('urlNameProblem', () => { + it('passes a name the API would take', () => { + expect(urlNameProblem('logistics/slim/orders')).toBeNull(); + expect(urlNameProblem('sync/orders')).toBeNull(); + }); + + it('refuses an empty name', () => { + expect(urlNameProblem('')).not.toBeNull(); + expect(urlNameProblem('-/')).not.toBeNull(); + }); + + it('refuses a last part of sync or async', () => { + expect(urlNameProblem('orders/sync')).toMatch(/sync/); + expect(urlNameProblem('orders/async/')).toMatch(/async/); + }); +}); + +describe('finishUrlName', () => { + it('drops separators hugging a slash or ending the name', () => { + expect(finishUrlName('logistics-/-slim/orders_/')).toBe('logistics/slim/orders'); + }); +}); diff --git a/SW.Bitween.Web/ClientApp/src/lib/identifiers.ts b/SW.Bitween.Web/ClientApp/src/lib/identifiers.ts index d1f2b3e5..788ee303 100644 --- a/SW.Bitween.Web/ClientApp/src/lib/identifiers.ts +++ b/SW.Bitween.Web/ClientApp/src/lib/identifiers.ts @@ -18,21 +18,43 @@ export const suggestSlug = (name: string) => .slice(0, 50); /** - * What a person is typing into a URL-name box, kept usable as a path segment. + * What a person is typing into a URL-name box, kept usable as a path. * * Spaces become hyphens as you type rather than being rejected on save: the box * looks like a name field, so people type "returns intake", and the gateway that * saves is one whose endpoint 404s with nothing on screen saying why. * + * `/` splits it into parts ("logistics/slim/orders"), for clients with a URL scheme + * of their own. + * * A trailing separator survives, or "orders-" could never become "orders-inbound". * `finishUrlName` takes it off at save time, which is when it has to be gone. */ export const toUrlName = (typed: string) => typed .toLowerCase() - .replace(/[^a-z0-9_-]+/g, "-") - .replace(/^[-_]+/, "") - .slice(0, 50); + .replace(/[^a-z0-9_/-]+/g, "-") + .replace(/([-_])[-_]+/g, "$1") + .replace(/[-_]*\/[-_]*/g, "/") + .replace(/\/{2,}/g, "/") + .replace(/^[-_/]+/, "") + .slice(0, 200); + +/** `toUrlName` minus the trailing separator that only mattered mid-typing. */ +export const finishUrlName = (typed: string) => toUrlName(typed).replace(/[-_/]+$/, ""); -/** `toUrlName` plus the trailing separator that only mattered mid-typing. */ -export const finishUrlName = (typed: string) => toUrlName(typed).replace(/[-_]+$/, ""); +/** + * The API's url-name rules that typing can't be stopped from breaking, as the message + * to show under the box — or null when it would save. + * + * "sync" can't be refused mid-typing, it may be on its way to "syncs"; so it's said + * here instead, where the save button can wait on it. + */ +export const urlNameProblem = (typed: string): string | null => { + const name = finishUrlName(typed); + if (!name) return "A URL name is required."; + const last = name.slice(name.lastIndexOf("/") + 1); + if (last === "sync" || last === "async") + return `It can't end in "${last}" — partners add /sync or /async after it.`; + return null; +}; diff --git a/SW.Bitween.Web/ClientApp/src/pages/api-gateways/ApiGatewayNewPage.tsx b/SW.Bitween.Web/ClientApp/src/pages/api-gateways/ApiGatewayNewPage.tsx index a68974dd..58c58447 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/api-gateways/ApiGatewayNewPage.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/api-gateways/ApiGatewayNewPage.tsx @@ -4,7 +4,7 @@ import { useMutation, useQueryClient } from "@tanstack/react-query"; import { api } from "../../api"; import { Button, FormError } from "../../components/ui/basics"; import { Field, TextInput } from "../../components/ui/forms"; -import { finishUrlName, suggestSlug, toUrlName } from "../../lib/identifiers"; +import { finishUrlName, suggestSlug, toUrlName, urlNameProblem } from "../../lib/identifiers"; import { BackLink } from "../../components/ui/BackLink"; import { keys } from "../../api/queryKeys"; @@ -24,6 +24,9 @@ export function ApiGatewayNewPage() { }, }); + // Empty is left to `required`, or the page would open on an error. + const urlProblem = urlName ? urlNameProblem(urlName) : null; + const submit = (e: FormEvent) => { e.preventDefault(); create.mutate(); @@ -55,7 +58,8 @@ export function ApiGatewayNewPage() { {create.error?.message}
-
diff --git a/SW.Bitween.Web/ClientApp/src/pages/api-gateways/ApiGatewayPage.tsx b/SW.Bitween.Web/ClientApp/src/pages/api-gateways/ApiGatewayPage.tsx index 0fee0cca..7f4e16d4 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/api-gateways/ApiGatewayPage.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/api-gateways/ApiGatewayPage.tsx @@ -4,7 +4,7 @@ import { keepPreviousData, useMutation, useQuery, useQueryClient } from "@tansta import { Pause, Pencil, Play, Plus, Search, Trash2 } from "lucide-react"; import { api, type ApiGatewayAttachment } from "../../api"; import { Can, useSessionCan } from "../../auth/guards"; -import { finishUrlName, toUrlName } from "../../lib/identifiers"; +import { finishUrlName, toUrlName, urlNameProblem } from "../../lib/identifiers"; import { HistoryCard } from "../../components/config/HistoryCard"; import { Badge, Button, EmptyState, LoadingBlock } from "../../components/ui/basics"; import { Field, TextInput } from "../../components/ui/forms"; @@ -64,6 +64,7 @@ export function ApiGatewayPage() { const [removing, setRemoving] = useState<{ partnerId: number; partnerName: string } | null>(null); const [deleting, setDeleting] = useState(false); const [confirmingActive, setConfirmingActive] = useState(false); + const [confirmingUrl, setConfirmingUrl] = useState(false); const [loaded, setLoaded] = useState(false); useEffect(() => { @@ -106,6 +107,7 @@ export function ApiGatewayPage() { ); const g = gateway.data; + const urlProblem = urlNameProblem(urlName); return (
@@ -145,7 +147,7 @@ export function ApiGatewayPage() {
- + setUrlName(toUrlName(e.target.value))} /> - - + +
{/* @@ -168,7 +170,7 @@ export function ApiGatewayPage() { How a partner calls it 

-              {`POST /api/Gateway/${urlName}/sync\npartnerkey: \n\n`}
+              {`POST /api/gateway/${urlName}/sync\npartnerkey: \n\n`}

The{" "} @@ -290,8 +292,13 @@ export function ApiGatewayPage() { {canEdit && dirty && ( save.mutate()} + error={urlProblem ?? save.error?.message} + onSave={() => { + if (urlProblem) return; + // The URL is what partners hold; changing it cuts every one of them off. + if (finishUrlName(urlName) !== g.urlName) setConfirmingUrl(true); + else save.mutate(); + }} onDiscard={() => setLoaded(false)} /> )} @@ -315,13 +322,32 @@ export function ApiGatewayPage() { /> )} + {confirmingUrl && ( + + Partners calling{" "} + /api/gateway/{g.urlName} will get 404s + until they switch to{" "} + /api/gateway/{finishUrlName(urlName)}. + + } + confirmLabel="Change URL" + onConfirm={async () => { + await save.mutateAsync(); + }} + onClose={() => setConfirmingUrl(false)} + /> + )} + {confirmingActive && ( { diff --git a/SW.Bitween.Web/ClientApp/src/pages/api-gateways/ApiGatewaysPage.tsx b/SW.Bitween.Web/ClientApp/src/pages/api-gateways/ApiGatewaysPage.tsx index 8d17f3c7..503bcaed 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/api-gateways/ApiGatewaysPage.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/api-gateways/ApiGatewaysPage.tsx @@ -137,9 +137,9 @@ export function ApiGatewaysPage() { cell: (g) => ( - /api/Gateway/{g.urlName} + /api/gateway/{g.urlName} ), }, diff --git a/SW.Bitween.Web/ClientApp/src/pages/api-gateways/AttachPartnerPage.tsx b/SW.Bitween.Web/ClientApp/src/pages/api-gateways/AttachPartnerPage.tsx index 28738c11..797286df 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/api-gateways/AttachPartnerPage.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/api-gateways/AttachPartnerPage.tsx @@ -103,7 +103,7 @@ export function AttachPartnerPage() {