Autogenerated RPC types - #1136
Conversation
Introduce shared aliases for FreeIPA wire types used by generated request and metadata definitions. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: David Hanina <dhanina@redhat.com>
Define ParamType, ParamClass, and per-class parameter shapes derived from IPA json_metadata output. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: David Hanina <dhanina@redhat.com>
Model IPA object relationships, methods, and attribute metadata for type-safe API usage. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: David Hanina <dhanina@redhat.com>
Introduce ValidResponse, ErrorResponse, and batch response types for IPA JSON-RPC calls. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: David Hanina <dhanina@redhat.com>
Provide Python tooling to produce param and request type definitions from json_metadata responses. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: David Hanina <dhanina@redhat.com>
Pass methodname and command filters and call the JSON endpoint directly so metadata generation receives complete object definitions. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: David Hanina <dhanina@redhat.com>
Introduce per-object Args and Options types plus a RequestMap index for all IPA API methods, with a typed request method helper. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: David Hanina <dhanina@redhat.com>
Adds requests, which adds a RequestMethod type that forces the developer to use the correct types for args and options. Assisted-by: Cursor cursoragent@cursor.com Signed-off-by: David Hanina <dhanina@redhat.com>
There was a problem hiding this comment.
Sorry @duzda, your pull request is larger than the review limit of 150000 diff characters
|
This includes only the initial types for requests, responses will come later |
| export type Decimal = number; | ||
| export type Principal = string; | ||
| export type Bytes = string; | ||
| export type DateTime = string; |
There was a problem hiding this comment.
Dont we want to check? maybe
type ISODateString = ${number}-${number}-${number};
There was a problem hiding this comment.
This code doesn't check, but good idea to dig into the exact type, my idea is to use this alias to know that the value is datetime convertible.
| rdn_attribute: string; | ||
| bindable: boolean; | ||
| relationships: Relationships; | ||
| primary_key: string; |
There was a problem hiding this comment.
But you are already adding the PrimaryKeyObjectMetadata, so why have the primary_key here?
There was a problem hiding this comment.
This is an oversight, thank you.
| ObjectMetadata<T>; | ||
|
|
||
| type ObjectRecord<T extends string> = { | ||
| [K in T]: ObjectMetadata<T> | ComplexObjectMetadata<T>; |
There was a problem hiding this comment.
You are not using the K here. Did you mean ObjectMetadata<K> ?
|
|
||
| def convert_name(name: str) -> str: | ||
| """ | ||
| Convert from snake_case to PascalCase. |
There was a problem hiding this comment.
isnt there some library to do this 😢
There was a problem hiding this comment.
return name.title().replace("_", "")
| if p not in classes[param["class"]]: | ||
| classes[param["class"]].add(p) | ||
|
|
||
| intersection = set.intersection(*classes.values()) |
There was a problem hiding this comment.
why are we interested in the intersections?
There was a problem hiding this comment.
The intersections model ParamMetadataBase, those variables are repeated throughout every type, therefore to save a bit of lines and complexity all the further types extend this one.
Intersections:
['alwaysask', 'attribute', 'autofill', 'class', 'cli_metavar', 'cli_name', 'confirm', 'deprecated', 'deprecated_cli_aliases', 'doc', 'flags', 'label', 'multivalue', 'name', 'no_convert', 'primary_key', 'query', 'required', 'sortorder', 'type']
Classes without intersection:
Str: ['default', 'exclude', 'maxlength', 'minlength', 'noextrawhitespace', 'option_group', 'pattern', 'pattern_errmsg']
StrEnum: ['default', 'exclude', 'option_group', 'values']
Flag: ['default', 'falsehoods', 'option_group', 'truths']
DNParam: []
IA5Str: ['noextrawhitespace']
Bytes: []
...
| }; | ||
| }, | ||
| transformResponse: (response: ShowRPCResponse): Metadata => { | ||
| console.log(response); |
There was a problem hiding this comment.
This change will be later reverted :)
| [], | ||
| { | ||
| object: "all", | ||
| methodname: "all", |
There was a problem hiding this comment.
why the addition of methodname and command ?
There was a problem hiding this comment.
This change will be later reverted :)
| @@ -0,0 +1,17 @@ | |||
| import { RequestMap } from "./requests"; | |||
|
|
|||
| type ObjectValueTuple<T> = { | |||
There was a problem hiding this comment.
Could you give me an example of this type please?
carma12
left a comment
There was a problem hiding this comment.
I will probably come back to this PR to do further review, but please find some initial thoughts in my comments below.
| ? [] | ||
| : ObjectValueTuple<T>; | ||
|
|
||
| type RequestMethod = <const T extends keyof RequestMap>( |
There was a problem hiding this comment.
This function seems to be declared but never used?
There was a problem hiding this comment.
Yes, there will be many more unused types and functions, this will be fixed later, just trying to get some initial ideas and issues.
There was a problem hiding this comment.
Not sure of the clarity of this file's name. It seems to be a diagnostic/analysis tool, not a generator despite its name. Consider renaming it to analyze-param-classes.py or similar.
There was a problem hiding this comment.
Good idea, such name will be much more fitting.
| type ObjectValueTuple<T> = { | ||
| [K in keyof T]-?: (x: T[K], ...args: ObjectValueTuple<Omit<T, K>>) => void; | ||
| } extends (x: infer V, ...args: infer R) => void | ||
| ? [V, ...R] | ||
| : []; |
There was a problem hiding this comment.
The IPA server expects commands in this shape: params: [[positional_args], {named_options}]. The positional args are an ordered array: the first item is always the first argument, the second is the second, etc.
IINM, the ObjectValueTuple function takes a TypeScript object like { uid: string; krbprincipalname: Principal } and tries to convert it into a tuple like [string, Principal]. The problem I see with this is: TypeScript objects don't have a guaranteed order, so the data might be unsorted or not adjusted to what the API calls are expecting. But I'm not 100% if I'm mistaking this, so it might be checked...
There was a problem hiding this comment.
Yes, as of TypeScript 5, the order is not guaranteed, however TypeScript 6 and 7 have this guarantee https://www.typescriptlang.org/docs/handbook/release-notes/typescript-6-0.html#the---stabletypeordering-flag
| type ValidBatch<T> = { | ||
| error: null; | ||
| result: T; | ||
| truncated: boolean; | ||
| summary?: string; | ||
| }; | ||
|
|
||
| type ErrorBatch = { | ||
| error: string; | ||
| error_code: number; | ||
| error_kw: { | ||
| reason: string; | ||
| }; | ||
| error_name: string; | ||
| }; | ||
|
|
||
| type RequestBatch = { | ||
| requests: Request[]; | ||
| }; | ||
|
|
||
| type RequestBatchResponse = { | ||
| responses: Response[]; | ||
| }; |
There was a problem hiding this comment.
I'm still trying to figure out what to do with these types, on top of that they are incorrect at the moment.
|
|
||
| export type ParamFindOptions = { | ||
| all: boolean; | ||
| raw: boolean; |
There was a problem hiding this comment.
Not sure if this parameter should be optional as well (depending on some API calls)?
There was a problem hiding this comment.
For some reason this type is described as mandatory, even though it is not true...
No description provided.