|
1 | 1 | # Naming Audit: abacpolicies |
2 | 2 |
|
3 | | -**Path:** `packages/abacpolicies/src/v1/` |
| 3 | +**Path:** `packages/uc/abacpolicies/src/v1/` |
4 | 4 | **Versions audited:** v1 |
5 | | -**Inferred domain:** Attribute-Based Access Control (ABAC) policies on Unity Catalog securables — create/get/list/update/delete row-filter and column-mask policies. |
6 | | -**Total weird names flagged:** 7 |
| 5 | +**Total weird names flagged:** 2 |
7 | 6 |
|
8 | 7 | ## Summary |
9 | 8 | | Severity | Count | |
10 | 9 | | --- | --- | |
11 | | -| High | 3 | |
12 | | -| Medium | 2 | |
13 | | -| Low | 0 | |
14 | | -| Observation | 2 | |
| 10 | +| High | 2 | |
15 | 11 |
|
16 | 12 | ## High severity |
17 | 13 |
|
18 | | -### 1. `PolicyInfo` — `src/v1/model.ts:137` |
| 14 | +### 1. `PolicyInfo` — `src/v1/model.ts:136` |
19 | 15 | - **Why weird:** `Info` is a generic suffix that adds nothing — every type is "info about something". This is the central domain entity; it should just be called `Policy`. (See rule 1: `Info` is on the vague-suffix list.) |
20 | 16 | - **Category:** 1 (vague suffix `Info`), 8 (redundant type suffix). |
21 | 17 | - **Suggested name:** `Policy`. |
22 | 18 | - **Rationale:** `Policy` is the noun the user actually thinks about. The `Info` suffix is a tic carried over from Go SDK naming conventions that does not apply in TS, where the namespace and import path already disambiguate. |
23 | 19 |
|
24 | | -### 2. `onSecurableType: string` on `DeletePolicyRequest` / `GetPolicyRequest` / `ListPoliciesRequest` / `UpdatePolicyRequest` — `src/v1/model.ts:64,92,101,232` |
25 | | -- **Why weird:** Typed as `string` everywhere on these request DTOs while the same field on `PolicyInfo` (`model.ts:145`) is typed as `SecurableType` enum. The values are the same domain (`CATALOG`, `SCHEMA`, ...) — the inconsistency is a bug. |
26 | | -- **Category:** 6 (misleading — name implies enum, actual type is `string`), 16 (field type contradicts domain). |
27 | | -- **Suggested name:** Keep the name but type it `SecurableType`. |
28 | | -- **Rationale:** Same field name with two different types across four request DTOs forces callers to remember which one is loose. This is almost certainly a generator bug worth flagging upstream. |
29 | | - |
30 | | -### 3. `MatchColumn` — `src/v1/model.ts:130` |
| 20 | +### 2. `MatchColumn` — `src/v1/model.ts:129` |
31 | 21 | - **Why weird:** Reads as a verb (`MatchColumn`) — could be a method or a type. The field that uses it is plural (`matchColumns: MatchColumn[]`), which then reads as "match columns are an array of `MatchColumn`", and a `MatchColumn` is actually a "column matcher / condition + alias pair". |
32 | 22 | - **Category:** 6 (misleading verb-as-noun), 9 (singular noun whose meaning is unclear). |
33 | 23 | - **Suggested name:** `ColumnMatcher` or `ColumnMatchCondition`. |
34 | 24 | - **Rationale:** Type names should be nouns; the verb form misleads. `ColumnMatcher` makes `matchColumns: ColumnMatcher[]` clearly read as "the matchers". |
35 | | - |
36 | | -## Medium severity |
37 | | - |
38 | | -### 4. `SecurableType.STAGING_TABLE` — `src/v1/model.ts:33` |
39 | | -- **Why weird:** Enum value pinned by a comment that says it isn't a real securable yet: "TODO: [UC-2980] Staging tables aren't full-fleged securables yet." Internal TODOs in generated SDK enums leak abstraction. |
40 | | -- **Category:** 18 (questionable enum value). |
41 | | -- **Suggested name:** Remove until it actually is a securable, or mark `@experimental`. |
42 | | -- **Rationale:** Public SDK enums shouldn't contain TODO-tagged speculative values. |
43 | | - |
44 | | -### 5. Inconsistent rename style for `*Options` types — `src/v1/model.ts:36,215` |
45 | | -- **Why weird:** `ColumnMaskOptions` and `RowFilterOptions` — two near-identical types describing variants of policy options. Each is a discriminator member; the `Options` suffix is redundant given the discriminator already says "this is the X options". |
46 | | -- **Category:** 8 (redundant suffix), 12 (duplicate concept across similar types). |
47 | | -- **Suggested name:** Either keep current names but acknowledge as boilerplate, or rename to `RowFilter`, `ColumnMask` (the `$case` discriminator already disambiguates). |
48 | | -- **Rationale:** Generator artefact; flagging because near-identical types is the moment to ask whether the API surface should collapse. |
49 | | - |
50 | | -## Low severity |
51 | | - |
52 | | -_None._ |
53 | | - |
54 | | -## Observations |
55 | | - |
56 | | -### 6. Action-verb conventions in `Client` |
57 | | -The client uses `Create`/`Get`/`List`/`Update`/`Delete` consistently. No mixed `Fetch`/`Retrieve`/`Read`. This is good. (Listed as observation per rule 17 since the audit asked us to flag inconsistencies; here we explicitly note consistency.) |
58 | | - |
59 | | -### 7. Acronym casing for `Http` / `Url` / `Id` across the SDK |
60 | | -The codebase uses `Http` (PascalCase capital-then-lower, e.g. `HttpClient`, `HttpRequest`) alongside the imported `URLSearchParams` (ALLCAPS, Web standard) and lowercase `url` / `userAgent`. Mixing `Http`-style with `URL`-style acronym casing is inconsistent across the SDK surface — common across JS ecosystem and probably not worth changing, but worth noting as a cross-package consistency question. |
61 | | -- **Category:** 3 (acronym casing). |
62 | | - |
63 | | -## Domain glossary |
64 | | -- `abac` — Attribute-Based Access Control (package name only; not referenced in current model code). |
65 | | -- `uc` — Unity Catalog (referenced in comment at model.ts:32 as "UC-2980", and implicitly across all types since policies live on Unity Catalog securables). |
66 | | -- `wkt` — Well-Known Types (import path `@databricks/sdk-core/wkt`). |
67 | | -- `oss` — not encountered in this package. |
68 | | -- `m2m`/`u2m`/`pat` — not encountered in this package. |
69 | | -- `iam` — not encountered, but conceptually overlaps with `Principal` references. |
70 | | - |
71 | | -## File coverage |
72 | | -- `src/v1/model.ts` (497 lines): read fully. |
73 | | -- `src/v1/client.ts` (250 lines): read fully. |
74 | | -- `src/v1/utils.ts` (150 lines): read fully. |
75 | | -- `src/v1/index.ts` (20 lines): read fully. |
0 commit comments