Skip to content

feat: support federated entities creation in sdk - #642

Open
niket0503 wants to merge 13 commits into
mainfrom
feat/datafabric-federated-entities
Open

niket0503 wants to merge 13 commits into
mainfrom
feat/datafabric-federated-entities

Conversation

@niket0503

@niket0503 niket0503 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Adds federated Data Fabric entity support to the SDK, on top of the v3 API migration.

What's in here

1. Migrate entity operations to the v3 API (DS-8953) — record CRUD and schema create/update/metadata move to /api/v3/entities/.... Mostly endpoint-path swaps + request/response type alignment; request bodies, pagination, and batch responses are unchanged. GET_ALL stays on v1 (v3 has no tenant-only listing); getAll() routes folderKey/includeFolderEntities to v3.

2. Federated entity create, update, and query.

  • create accepts entityClass: Federated with externalFields (external-connector or native source + object detail + field mappings) and optional sourceJoinConditionDetails.
  • updateById gains federated source/join deltas: addExternalSources, removeExternalSources (cascades the removed source's joins), addFieldsToSource, removeFieldsFromSource, updateExternalFieldMapping, addSourceJoins, updateSourceJoin. The v3 API is a full-definition upsert, so the SDK reads the current definition, merges the delta, and reposts the whole thing.
  • queryRecordsById now routes to the v3 query endpoint (/api/v3/entities/entity/{id}/query) for every join-less query. This is what lets federated entities be queried — the legacy v1 by-id endpoint blocks them (Cannot access Federated entity …). Queries with multi-entity joins still route to the v1 by-name endpoint (the only one that executes joins).

Notes for reviewers (found validating live against the v3 API)

  • The GET flattens external-source fields into top-level fields (isExternalField=true) as well as under externalFields; carrying those into fields on the repost duplicates them and the upsert fails — the native carry-forward excludes isExternalField.
  • The GET omits fieldDisplayType but the upsert requires it → defaulted to "Basic" (a missing one surfaces as a misleading join-dependency error, not a clear validation error).
  • Read-shape sourceJoinCriterias (object/field IDs) are translated to write-shape sourceJoinConditionDetails (object names + connection ids). joinType passes through (string "LeftJoin" accepted); entityClass maps to entityClassId.
  • Multi-entity joins on the v3 query endpoint now return a 400 instead of being silently dropped (the old v1 by-id behavior) — a correctness improvement.

Verification

  • Unit: create, delta translation, cascade, and query-routing tests pass (npm run test:unit); typecheck + lint clean.
  • Live: end-to-end create / update / read (filter, projection, sort) verified against a Salesforce-backed federated entity. A gated integration test asserts the joins-400 behavior.

Version

Preview version bumped to 1.6.1-entities3 (published). A formal release version bump remains a separate step per the repo release workflow.

🤖 Generated with Claude Code

@niket0503
niket0503 requested a review from a team August 5, 2026 09:50
@niket0503 niket0503 changed the title Feat/datafabric federated entities feat: support federated entities creation in sdk Aug 5, 2026
Comment thread package.json
Comment thread src/models/data-fabric/entities.types.ts Outdated
Comment thread src/models/data-fabric/entities.types.ts
@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Three issues found this run — inline comments posted for each.

@niket0503
niket0503 force-pushed the feat/datafabric-federated-entities branch from 734646b to 9fac1c3 Compare August 5, 2026 10:11
Comment thread tests/integration/shared/data-fabric/entities.integration.test.ts Outdated
@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

One new finding this run — inline comment posted.

@niket0503
niket0503 force-pushed the feat/datafabric-federated-entities branch from 9fac1c3 to ae989f2 Compare August 5, 2026 11:09
Comment thread tests/integration/shared/data-fabric/entities.integration.test.ts Outdated
Comment thread tests/unit/services/data-fabric/entities.test.ts Outdated
Comment thread tests/unit/services/data-fabric/entities.test.ts Outdated
@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Three findings this run — inline comments posted for each.

1. Integration test: wrong field name after SDK transform (line 1240)
entities.constants.ts maps isInsightsEnabled → isAnalyticsEnabled in EntityMap. After getById(), the returned EntityGetResponse exposes isAnalyticsEnabled; the assertion expect(updated.isInsightsEnabled).toBe(true) evaluates expect(undefined).toBe(true) at runtime — the test will always fail against a live API.
Fix: expect(updated.isAnalyticsEnabled).toBe(true)

2. any type violations in new unit tests (line 1906)
let capturedConfig: any and (s: any) callbacks at lines 2953/2975 violate the no-any convention. Use Parameters<typeof PaginationHelpers.getAll>[0] for the captured config, and Record<string, unknown> for the find/map element type.

3. Raw numbers in federatedRaw mock (line 2860)
directionType: 0 (×2) and entityClass: "Federated" use bare literals instead of DataDirectionType.ReadOnly / EntityClass.Federated. Per convention: "Use enums for fixed value sets — NEVER leave raw strings/numbers."

@niket0503
niket0503 force-pushed the feat/datafabric-federated-entities branch from ae989f2 to b99532d Compare August 5, 2026 11:46
Comment thread tests/integration/shared/data-fabric/entities.integration.test.ts Outdated
@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

One new finding this run — inline comment posted.

Misleading integration test description (line 1424)
The test titled 'should remove a source and cascade its join (no dangling join left)' calls removeFieldsFromSource with an empty fieldNames: [] — removing zero fields and leaving the entity unchanged. The cascade path (removeExternalSources dropping dependent joins) is never exercised, and the assertion passes trivially. Per rules.md, test descriptions must match what is being tested.

@niket0503
niket0503 force-pushed the feat/datafabric-federated-entities branch from b99532d to 2dbc1db Compare August 5, 2026 11:58
Comment thread src/services/data-fabric/entities.ts Outdated
Comment thread tests/unit/services/data-fabric/entities.test.ts Outdated
Comment thread tests/unit/services/data-fabric/entities.test.ts Outdated
Comment thread tests/unit/services/data-fabric/entities.test.ts Outdated
Comment thread src/models/data-fabric/entities.types.ts Outdated
@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Five new findings this run — inline comments posted for each.

1. Raw string 'Basic' in carryForwardSource (src/services/data-fabric/entities.ts:119)
FieldDisplayType.Basic exists and is already imported — use it instead of the raw string.

2. logicalOperator: 0 as const in new test (entities.test.ts:1910)
LogicalOperator.And = 0 exists — add LogicalOperator to the import and use the enum.

3. joinType: "LeftJoin" raw string in federatedRaw mock (entities.test.ts:2869)
JoinType.LeftJoin is already imported — use it instead of the raw string.

4. expect(def.entityClassId).toBe(10) raw number (entities.test.ts:2884)
EntityClassId.Federated = 10 is already imported — use the enum constant.

5. EntityCreateExternalSource.externalObjectDetail marked optional (entities.types.ts:958)
This field identifies which external table the source reads from and has no server-side default — it is structurally required per the structured-input-type convention.

@niket0503
niket0503 force-pushed the feat/datafabric-federated-entities branch from 2dbc1db to 6b46c22 Compare August 5, 2026 12:32
Comment thread src/services/data-fabric/entities.ts Outdated
Comment thread tests/unit/services/data-fabric/entities.test.ts Outdated
@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Two new findings this run — inline comments posted for each.

1. Orphaned JSDoc block (src/services/data-fabric/entities.ts:860)
The long JSDoc at lines 860–865 is orphaned — TypeScript only attaches the immediately preceding JSDoc to a declaration, so the short one at 866 wins for buildExternalFieldsPayload. The long block (which describes buildExternalSourcesPayload) floats unattached and documents nothing, while buildExternalSourcesPayload ends up with no JSDoc. Fix: move the long JSDoc to immediately precede buildExternalSourcesPayload.

2. Raw string "Basic" in assertion (entities.test.ts:2889)
.toBe("Basic") should use FieldDisplayType.Basic. Convention: "Use enums for fixed value sets — NEVER leave raw strings/numbers." FieldDisplayType is already imported.

@niket0503
niket0503 force-pushed the feat/datafabric-federated-entities branch from 6b46c22 to 0899020 Compare August 5, 2026 12:43
Comment thread tests/unit/services/data-fabric/entities.test.ts Outdated
Comment thread src/models/data-fabric/entities.types.ts Outdated
…EntityClass.Federated

- Move the duplicated idsEqual GUID compare into tests/integration/utils/helpers.ts
  and import it in the records + schema suites.
- Drop @experimental from the EntityClass.Federated enum member — Native is GA so
  the enum isn't wholly experimental, and the federated feature is already marked
  @experimental on every method/option/type that uses it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread src/utils/constants/endpoints/data-fabric.ts Outdated
CEP's v3 by-name query route now handles multi-entity joins (it only
rejects joins for Federated entities, which can't be joined). So the
v1 EntityService by-name query is no longer needed for joins.

Drops QUERY_BY_NAME_V3 and folds the v1 QUERY_BY_NAME into one v3
by-name query endpoint. Non-join and join by-name queries now use the
same route. By-id join queries still resolve the name and address by
name, since the v3 by-id route rejects joins.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

1 similar comment
@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

CEP now treats IsInsightsEnabled as immutable after entity creation —
the v3 metadata PATCH rejects any change with a 400. So isAnalyticsEnabled
can only be set on create.

Removes isAnalyticsEnabled from EntityUpdateByIdOptions and the updateById
metadata PATCH (create already sends it). Updates the schema integration
test to enable analytics at create time and drops the two updateById
unit tests that asserted the now-rejected PATCH field.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread src/models/data-fabric/entities.types.ts
@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

One new finding this run — inline comment posted.

Missing @experimental on three new Federated-feature types (entities.types.ts:853–877)

SearchabilityOperator, SearchabilityNamedSearch, and Searchability are new publicly-exported interfaces used exclusively by the Federated (experimental) feature. They appear in EntityCreateExternalFieldMapping (@experimental) and on the new searchability field of ExternalFieldMapping. The earlier @experimental propagation pass (resolved thread PRRT_kwDOOpObr86axlVi) covered 11 other Federated types but missed these three — they were likely added in a later commit.

@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

SearchabilityOperator, SearchabilityNamedSearch, and Searchability are
publicly-exported types used only by the Federated feature (via
EntityCreateExternalFieldMapping and ExternalFieldMapping.searchability).
TypeDoc renders a page per interface, so each needs @experimental to
carry the preview warning — matching the other Federated types.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

Comment thread tests/integration/utils/helpers.ts Outdated
Comment on lines +62 to +65
* Compares two GUID strings case-insensitively. v3 endpoints return uppercase
* GUIDs while v1 and write operations return lowercase, so use this instead of
* strict equality when comparing record IDs across API versions.
*/

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This will introduce breaking change. Should we transform the response to lowercase in SDK? CC: @vnaren23

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can this be fixed from DF side? Is there any reason for v3 returning uppercase?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a normalization so that sdk behavior remains consistent.
For DF side, will discuss with team, but sdk normalization should unblock

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Resolved: this is being fixed on the DF/backend side, not in the SDK.

One correction on direction — it is the reverse of the (now-removed) helper note: v1 returned the record Id uppercase; it is v3 that returns it lowercase. Reason: the v3 write/query/list responses wrap the record in EntityWriteResponseV3, which types Id as a System.Guid, and Newtonsoft serialises a Guid lowercase by default. v1 returned the raw DB string (uppercase). So it is an unintended side-effect of the typed envelope, not intentional — and it affects only the single insert/update, query, and list-read responses (batch and single-read return the raw uppercase string, so they are unchanged).

Plan (agreed with DF): the backend will hotfix the v3 response to emit the Id uppercase again (feature-flagged) early next week; the SDK will not normalise. I have dropped the SDK-side normalisation and removed this idsEqual helper — the integration tests now use strict ===, which pass once the hotfix lands. TS release follows the backend fix.

The API serializes directionType as a string ("ReadOnly"/"ReadAndWrite")
on both read and write (Newtonsoft StringEnumConverter; the entity API
host has used Newtonsoft since 2021). Numeric values were accepted on
write but every read came back as the string, so a numeric enum
mistyped the response and broke equality checks. Reverting to the string
values also matches released main, so it's non-breaking.

Verified end-to-end against alpha via the CLI + local SDK: federated
create, get, list, and records query all succeed and directionType
round-trips as "ReadOnly".

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@niket0503
niket0503 force-pushed the feat/datafabric-federated-entities branch from b162f49 to fea7b79 Compare September 22, 2026 08:16
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Stale @param on EntityMethods.update() (entities.models.ts line 1439)

The EntityServiceModel.updateById() @PARAM at line 1075 was correctly updated to list the federated delta fields. However, the parallel @param on the bound entity method EntityMethods.update() was not updated. The two now disagree:

Line 1439 still reads:

At least one of `addFields`, `removeFields`, `updateFields`, `displayName`, `description`, or `isRbacEnabled` must be provided

A developer hovering over entity.update() in their IDE will see this outdated guidance and incorrectly assume that passing only a federated delta (e.g. addExternalSources) throws a ValidationError.

Fix — update line 1439 to match line 1075:

@param options - Changes to apply ({@link EntityUpdateByIdOptions}). At least one of `addFields`, `removeFields`, `updateFields`, `displayName`, `description`, `isRbacEnabled`, or a federated source/join delta (`addExternalSources`, `removeExternalSources`, `addFieldsToSource`, `removeFieldsFromSource`, `updateExternalFieldMapping`, `addSourceJoins`, `updateSourceJoin`) must be provided — calling with no options, `{}`, or only `folderKey` throws a `ValidationError`. Field names passed in `addFields[].name` and `removeFields[].name` must be camelCase — start with a letter, letters and numbers only; the Data Fabric backend rejects underscores in field names. The `folderKey` property is **experimental**.

@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

@niket0503
niket0503 force-pushed the feat/datafabric-federated-entities branch 2 times, most recently from 0e347a0 to 0ea5f5b Compare September 22, 2026 09:59
Comment thread src/services/data-fabric/entities.ts
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Four findings this run — one new inline comment posted and three previously-resolved threads unresolved.

1. isAnalyticsEnabled silently ignored in updateById (entities.ts:483)
isAnalyticsEnabled was added to EntityUpdateByIdOptions in this PR but the service never acts on it: it's not in hasMetadataChanges (so passing it alone still throws ValidationError), not in the metadata PATCH body, and not applied in the schema-upsert path. The integration test comment says it's immutable after creation — if so, it should be removed from EntityUpdateByIdOptions rather than left as a silent no-op.

2. EntityMethods.update() @param is stale (entities.models.ts line 1439 — unchanged line, not in diff)
The EntityServiceModel.updateById() @param at line 1075 was updated to list federated delta fields. The parallel annotation on the bound entity method EntityMethods.update() at line 1439 was not updated — it still reads "At least one of addFields, removeFields, updateFields, displayName, description, or isRbacEnabled must be provided", omitting the federated options. IDE hover on entity.update() inherits this stale text.

3. Unresolved: EntityServiceModel.updateById() @param missing isAnalyticsEnabled (PRRT_kwDOOpObr86jpk8S)
The @param annotation was updated to include federated fields but isAnalyticsEnabled was not added. Depends on resolution of finding #1 above — if isAnalyticsEnabled is removed from the options type, the annotation is already correct; if it's implemented, it needs to be listed.

4. Unresolved: error message missing isAnalyticsEnabled (PRRT_kwDOOpObr86jpyiR)
Same dependency — the error message lists federated delta fields but not isAnalyticsEnabled. Pending design decision on #1.

5. Unresolved: EntityClass.Federated missing @experimental (PRRT_kwDOOpObr86W4064)
The suggestion block included @experimental on the Federated enum value, but the author only applied the two explicitly named fixes (class-level JSDoc and Case value docs). The current code has /** Federated entity — unified read-only view across UiPath and external sources. */ without the tag.

Comment thread src/models/data-fabric/entities.models.ts
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

One new finding this run — inline comment posted.

Stale @param on EntityMethods.update() (entities.models.ts line 1075)

The EntityServiceModel.updateById() @param options was updated by this PR to include the federated delta fields. The parallel @param on EntityMethods.update() (~line 1439) was not updated and still lists only the pre-federated field set. A developer reading the IDE tooltip for the bound method will be incorrectly told that passing only a federated delta (e.g. addExternalSources) throws a ValidationError.

…round

The v1->v3 record-Id casing mismatch (v3 EntityWriteResponseV3 serialises
the Id lowercase, v1 returned uppercase) is being fixed at the source: the
backend hotfixes the v3 response casing (feature-flagged). Per that
decision the SDK does not normalise casing — the earlier SDK-side
normalisation is dropped.

Remove the case-insensitive idsEqual test helper and use strict Id
equality in the integration tests; these pass once the backend hotfix
returns the Id uppercase again.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@niket0503
niket0503 force-pushed the feat/datafabric-federated-entities branch from 0ea5f5b to 2c6228e Compare September 22, 2026 12:44
Comment thread src/models/data-fabric/entities.models.ts Outdated
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

One new finding this run — inline comment posted.

Invalid TypeScript syntax in @example blocks (entities.models.ts line 1027)

elementInstanceId: <elementInstanceId> (without string quotes) is not valid TypeScript — in non-JSX mode <T>expr is the old-style type assertion, and <elementInstanceId> with no following expression is a syntax error. A user copying either the create() example (line 1027) or the updateById() example (line 1120) verbatim would get a compile error. Fix: use a numeric literal 0 as the placeholder since elementInstanceId is typed number.

…ples

- EntityMethods.update() @PARAM now lists the federated source/join deltas,
  matching EntityServiceModel.updateById() (the bound method accepts the
  same EntityUpdateByIdOptions, so the tooltip was under-documenting it).
- @example placeholders `elementInstanceId: <elementInstanceId>` are not
  valid TypeScript (that parses as a type assertion); elementInstanceId is
  numeric, so use `0`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

This branch was successfully deployed

1 active (outdated) deployment
github-packages-dev 0a7d0e93 Deployed Aug 20, 2026 by niket0503 via publish-sdk #92
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants