Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/commands/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export * from "./tenant/tenant.disable-sensitive-data.ts"
export * from "./tenant/tenant.enable-sensitive-data.ts"
export * from "./tenant/tenant.create.ts"
export * from "./tenant/tenant.fetch.ts"
export * from "./tenant/tenant-instance.fetch.ts"
export * from "./tenant/tenant.list.ts"
export * from "./tenant/tenant.update.ts"
export * from "./tenant/tenant.user-add.ts"
Expand Down
78 changes: 78 additions & 0 deletions src/commands/tenant/tenant-instance.fetch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { Command } from "../../common/command.ts"
import { type TenantInstance, TenantInstanceSchema } from "../../contracts/tenant.ts"
import type { ClientError } from "../../exceptions/client-error.ts"
import { NotFoundException } from "../../exceptions/not-found.ts"
import { parseResponseHelper } from "../../utils/parse-response-helper.ts"

/**
* The input for the tenant instance fetch by id command
*/
export interface TenantInstanceFetchByIdInput {
/** The id of the tenant */
tenantId: string
/** The name of the tenant */
tenant?: never
}

/**
* The input for the tenant instance fetch by name command
*/
export interface TenantInstanceFetchByNameInput {
/** The name of the tenant */
tenant: string
/** The id of the tenant */
tenantId?: never
}

/**
* The input for the tenant instance fetch command
*/
export type TenantInstanceFetchInput = TenantInstanceFetchByIdInput | TenantInstanceFetchByNameInput

/**
* Fetch a tenant instance
*/
export class TenantInstanceFetchCommand extends Command<TenantInstanceFetchInput, TenantInstance> {
/**
* Get the method
*/
protected override getMethod(): string {
return "GET"
}

/**
* Get the base url
*/
protected override getBaseUrl(): string {
return "https://tenant.api.flowcore.io"
}

/**
* Get the path
*/
protected override getPath(): string {
if ("tenantId" in this.input && this.input.tenantId) {
return `/api/v1/tenants/by-id/${this.input.tenantId}/instance`
}
return `/api/v1/tenants/by-name/${this.input.tenant}/instance`
Comment on lines +54 to +57
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid truthy checks for tenantId in routing and error filters.

At Line 54 and Line 73, using this.input.tenantId as a truthy guard can pick the wrong branch for falsy string values, producing incorrect paths/error metadata. Use discriminant presence checks consistently.

Proposed fix
   protected override getPath(): string {
-    if ("tenantId" in this.input && this.input.tenantId) {
+    if ("tenantId" in this.input) {
       return `/api/v1/tenants/by-id/${this.input.tenantId}/instance`
     }
     return `/api/v1/tenants/by-name/${this.input.tenant}/instance`
   }
@@
   protected override handleClientError(error: ClientError): void {
     if (error.status === 404) {
+      const isById = "tenantId" in this.input
       throw new NotFoundException("Tenant", {
-        [this.input.tenantId ? "id" : "name"]: this.input.tenantId ?? this.input.tenant,
+        [isById ? "id" : "name"]: isById ? this.input.tenantId : this.input.tenant,
       })
     }
     throw error
   }

Also applies to: 72-74

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/tenant/tenant-instance.fetch.ts` around lines 54 - 57, The
routing and error-filter branches use a truthy check on this.input.tenantId
which fails for valid falsy strings; change the guards to use a discriminant
presence check (e.g. if ("tenantId" in this.input) rather than if ("tenantId" in
this.input && this.input.tenantId")) in the tenant-instance.fetch code paths
(the branch that returns
`/api/v1/tenants/by-id/${this.input.tenantId}/instance`) and update the
corresponding error/filter logic that currently checks this.input.tenantId
truthiness to also use the `"tenantId" in this.input` presence check so empty or
falsy string IDs route and produce metadata correctly.

}

/**
* Parse the response
*/
protected override parseResponse(rawResponse: unknown): TenantInstance {
return parseResponseHelper(TenantInstanceSchema, rawResponse)
}

/**
* Handle the client error
*/
protected override handleClientError(error: ClientError): void {
if (error.status === 404) {
throw new NotFoundException("Tenant", {
[this.input.tenantId ? "id" : "name"]: this.input.tenantId ?? this.input.tenant,
})
}
throw error
}
}
2 changes: 1 addition & 1 deletion src/commands/tenant/tenant.fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const responseSchema = Type.Object({
configuration: Type.Object({
domain: Type.String(),
configurationRepoUrl: Type.String(),
configurationRepoCredentials: Type.String(),
configurationRepoCredentials: Type.Union([Type.String(), Type.Null()]),
}),
}),
]),
Expand Down
24 changes: 7 additions & 17 deletions src/common/command.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { Tenant } from "../contracts/tenant.ts"
import { ClientError } from "../exceptions/client-error.ts"
import type { ClientError } from "../exceptions/client-error.ts"
import { CommandError } from "../exceptions/command-error.ts"
import type { FlowcoreClient } from "./flowcore-client.ts"
import { tenantCache } from "./tenant.cache.ts"
Expand Down Expand Up @@ -70,31 +69,22 @@ export abstract class Command<Input, Output> {
let tenant = tenantCache.get(inputTenant)

if (!tenant) {
// Dynamically import TenantFetchCommand only when needed
const { TenantFetchCommand } = await import("../commands/tenant/tenant.fetch.ts")

tenant = await client.execute(new TenantFetchCommand({ tenant: inputTenant }))
.catch((error) => {
if (error instanceof ClientError && error.status === 403) {
return {
isDedicated: false,
dedicated: null,
} as unknown as Tenant // Cast needed as we are not returning the full Tenant shape
}
throw error
})
// Dynamically import TenantInstanceFetchCommand only when needed
const { TenantInstanceFetchCommand } = await import("../commands/tenant/tenant-instance.fetch.ts")

tenant = await client.execute(new TenantInstanceFetchCommand({ tenant: inputTenant }))
tenantCache.set(inputTenant, tenant)
}

if (!tenant.isDedicated) {
return null
}

if (!tenant.dedicated?.configuration.domain) {
if (!tenant.instance?.domain) {
throw new CommandError(this.constructor.name, `Tenant ${inputTenant} does not have a dedicated domain configured`)
}

return `https://${this.dedicatedSubdomain}.${tenant.dedicated.configuration.domain}`
return `https://${this.dedicatedSubdomain}.${tenant.instance.domain}`
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/common/tenant.cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { LocalCache } from "../utils/local-cache.ts"

type DedicatedTenant = {
isDedicated: boolean
dedicated: {
status: "ready" | "degraded" | "offline"
configuration: { domain: string }
instance: {
status: string
domain: string
} | null
}

Expand Down
2 changes: 1 addition & 1 deletion src/contracts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export type {
} from "./event-type.ts"
export type { FlowType } from "./flow-type.ts"
export type { FlowcoreEvent, IngestEventInput } from "./event.ts"
export type { Tenant, TenantPreview } from "./tenant.ts"
export type { Tenant, TenantInstance, TenantPreview } from "./tenant.ts"
export type { Permission } from "./permission.ts"
export type {
LegacyScenario,
Expand Down
32 changes: 30 additions & 2 deletions src/contracts/tenant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export const TenantSchema: TObject<{
configuration: TObject<{
domain: TString
configurationRepoUrl: TString
configurationRepoCredentials: TString
configurationRepoCredentials: TUnion<[TString, TNull]>
}>
}>,
]>
Expand All @@ -67,7 +67,7 @@ export const TenantSchema: TObject<{
configuration: Type.Object({
domain: Type.String(),
configurationRepoUrl: Type.String(),
configurationRepoCredentials: Type.String(),
configurationRepoCredentials: Type.Union([Type.String(), Type.Null()]),
}),
}),
]),
Expand Down Expand Up @@ -144,3 +144,31 @@ export const TenantPreviewSchema: TObject = Type.Object({
* The type for a public tenant preview
*/
export type TenantPreview = Static<typeof TenantPreviewSchema>

/**
* The schema for a tenant instance
*/
export const TenantInstanceSchema: TObject<{
isDedicated: TBoolean
instance: TUnion<[
TNull,
TObject<{
status: TString
domain: TString
}>,
]>
}> = Type.Object({
isDedicated: Type.Boolean(),
instance: Type.Union([
Type.Null(),
Type.Object({
status: Type.String(),
domain: Type.String(),
}),
]),
})

/**
* The type for a tenant instance
*/
export type TenantInstance = Static<typeof TenantInstanceSchema>
23 changes: 6 additions & 17 deletions test/tests/commands/data-core.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { assertEquals, assertRejects } from "@std/assert"
import { afterAll, afterEach, describe, it } from "@std/testing/bdd"
import { tenantCache } from "../../../src/common/tenant.cache.ts"
import type { Tenant } from "../../../src/contracts/tenant.ts"
import {
type DataCore,
DataCoreFetchCommand,
Expand Down Expand Up @@ -189,18 +188,13 @@ describe("DataCore", () => {
isFlowcoreManaged: false,
}

fetchMockerBuilderTenant.get(`/api/v1/tenants/by-name/${dataCore.tenant}`)
fetchMockerBuilderTenant.get(`/api/v1/tenants/by-name/${dataCore.tenant}/instance`)
.respondWith(
200,
{
id: dataCore.tenantId,
name: dataCore.tenant,
displayName: dataCore.tenant,
description: dataCore.tenant,
websiteUrl: "https://test.com",
isDedicated: false,
dedicated: null,
} satisfies Tenant,
instance: null,
},
)

fetchMockerBuilderDeleteManager.delete(`/api/v1/data-cores/${dataCore.id}/request-delete`)
Expand Down Expand Up @@ -238,18 +232,13 @@ describe("DataCore", () => {
isFlowcoreManaged: false,
}

fetchMockerBuilderTenant.get(`/api/v1/tenants/by-name/${dataCore.tenant}`)
fetchMockerBuilderTenant.get(`/api/v1/tenants/by-name/${dataCore.tenant}/instance`)
.respondWith(
200,
{
id: dataCore.tenantId,
name: dataCore.tenant,
displayName: dataCore.tenant,
description: dataCore.tenant,
websiteUrl: "https://test.com",
isDedicated: false,
dedicated: null,
} satisfies Tenant,
instance: null,
},
)

fetchMockerBuilderDeleteManager.delete(`/api/v1/data-cores/${dataCore.id}/request-delete`)
Expand Down
61 changes: 61 additions & 0 deletions test/tests/commands/tenant.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import {
FlowcoreClient,
NotFoundException,
type Tenant,
type TenantInstance,
TenantInstanceFetchCommand,
type TenantPreview,
TenantPreviewCommand,
TenantTranslateNameToIdCommand,
Expand Down Expand Up @@ -63,6 +65,65 @@ describe("Tenant", () => {
assertEquals(response, preview)
})

it("should fetch tenant instance by name", async () => {
const instance: TenantInstance = {
isDedicated: true,
instance: { status: "ready", domain: "test.flowcore.io" },
}

fetchMockerBuilder.get(`/api/v1/tenants/by-name/test/instance`)
.respondWith(200, instance)

const command = new TenantInstanceFetchCommand({ tenant: "test" })
const response = await flowcoreClient.execute(command)

assertEquals(response, instance)
})

it("should fetch tenant instance by id", async () => {
const tenantId = crypto.randomUUID()
const instance: TenantInstance = {
isDedicated: true,
instance: { status: "ready", domain: "test.flowcore.io" },
}

fetchMockerBuilder.get(`/api/v1/tenants/by-id/${tenantId}/instance`)
.respondWith(200, instance)

const command = new TenantInstanceFetchCommand({ tenantId })
const response = await flowcoreClient.execute(command)

assertEquals(response, instance)
})

it("should return non-dedicated instance", async () => {
const instance: TenantInstance = {
isDedicated: false,
instance: null,
}

fetchMockerBuilder.get(`/api/v1/tenants/by-name/test/instance`)
.respondWith(200, instance)

const command = new TenantInstanceFetchCommand({ tenant: "test" })
const response = await flowcoreClient.execute(command)

assertEquals(response, instance)
})

it("should throw NotFoundException for missing tenant instance", async () => {
fetchMockerBuilder.get(`/api/v1/tenants/by-name/missing/instance`)
.respondWith(404, { message: "Tenant not found" })

const command = new TenantInstanceFetchCommand({ tenant: "missing" })

await assertRejects(
() => flowcoreClient.execute(command),
NotFoundException,
`Tenant not found: ${JSON.stringify({ name: "missing" })}`,
)
})

it("should throw NotFoundException when preview tenant is not found", async () => {
fetchMockerBuilder.get(`/api/v1/tenants/preview/missing`)
.respondWith(404, { message: "Tenant not found" })
Expand Down
29 changes: 7 additions & 22 deletions test/tests/common/command.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Type } from "@sinclair/typebox"
import { assertEquals, assertRejects } from "@std/assert"
import { afterAll, afterEach, describe, it } from "@std/testing/bdd"
import type { Tenant } from "../../../src/contracts/tenant.ts"
import { Command, CommandError, FlowcoreClient, InvalidResponseException } from "../../../src/mod.ts"
import { parseResponseHelper } from "../../../src/utils/parse-response-helper.ts"
import { FetchMocker } from "../../fixtures/fetch.fixture.ts"
Expand Down Expand Up @@ -66,18 +65,13 @@ describe("FlowcoreCommand", () => {
// act
const command = new CommandWithDedicatedSubdomain({ tenant: "test" })

fetchMockerBuilderTenant.get("/api/v1/tenants/by-name/test")
fetchMockerBuilderTenant.get("/api/v1/tenants/by-name/test/instance")
.respondWith(
200,
{
id: "test",
name: "test",
displayName: "test",
description: "test",
websiteUrl: "https://test.com",
isDedicated: false,
dedicated: null,
} satisfies Tenant,
instance: null,
},
)

fetchMocker.mock("https://test-command.api.flowcore.io").get("/test").respondWith(200, {
Expand All @@ -97,25 +91,16 @@ describe("FlowcoreCommand", () => {
// act
const command = new CommandWithDedicatedSubdomain({ tenant: "test" })

fetchMockerBuilderTenant.get("/api/v1/tenants/by-name/test")
fetchMockerBuilderTenant.get("/api/v1/tenants/by-name/test/instance")
.respondWith(
200,
{
id: "test",
name: "test",
displayName: "test",
description: "test",
websiteUrl: "https://test.com",
isDedicated: true,
dedicated: {
configuration: {
domain: "dedicated.test.com",
configurationRepoUrl: "https://github.com/test/test",
configurationRepoCredentials: "test",
},
instance: {
status: "ready",
domain: "dedicated.test.com",
},
} satisfies Tenant,
},
)

fetchMocker.mock("https://test.dedicated.test.com").get("/test").respondWith(200, {
Expand Down