cyclosproject / ng-openapi-gen

An OpenAPI 3.0 codegen for Angular
MIT License
397 stars 132 forks source link

Readonly modifier is being ignored #193

Open schimini opened 2 years ago

schimini commented 2 years ago

Hi, not sure if this is a bug, it's looking more like a feature request, but I wanted to know if I could use the readonly property modifier to generate different types for creation requests ( POST, PUT) and read requests (GET) using the same schema.

My use case is: My schema has an id, and when creating the entity via post I don't want to provide an id. But also I don't want to duplicate every schema, or use extension just for this.

Could someone look into this? Should I dive into a possible PR?

luisfpg commented 2 years ago

Sorry, but we focus on implementing standard specification. The way to go would be to define different schemes.

schimini commented 2 years ago

@luisfpg thanks for responding.

Isn't readOnly standard since it is described in the openAPI sepcification doc?

luisfpg commented 2 years ago

Sorry, I've misunderstood your question. I think the readonly modifier should be generated for properties marked as readOnly: true in the schema. AFAIK, there's no TypeScript modifier for writeonly so far.

schimini commented 2 years ago

No problem!

I checked and I don't think typescript's readonly will be usefull in this case (at least by itself). openAPI's readOnly is meant to say that objects can't be constructed with a specific property, whilst typescript's readonly is meant to block the property from being updated (implying that it should be initialized).

Basically if something like this was generated:

export interface TestSchema {
  readonly id: number;
  prop: string;
}

typescript complier would throw an error when trying to do this:


//apiTestPost(id: number, body: TestSchema): : Observable<TestSchema>

this.service.apiTestPost(id, {prop: 'test string'})

because id is not present in the body.

TLDR; I think openAPI's readOnly translates into two possibilities:

The first one is to generate an analogous interface without readonly properties.

The other is basically the same thing but using ts type checker by declaring:

type KeysOfType<T, SelectedType> = {
  [key in keyof T]: SelectedType extends T[key] ? key : never;
}[keyof T];

type Optional<T> = Partial<Pick<T, KeysOfType<T, undefined>>>;

type IfEquals<X, Y, A=X, B=never> =
  (<T>() => T extends X ? 1 : 2) extends
  (<T>() => T extends Y ? 1 : 2) ? A : B;

type WritableKeys<T> = {
  [P in keyof T]: IfEquals<{ [Q in P]: T[P] }, { -readonly [Q in P]: T[P] }, P>
}[keyof T];

type WritableObject<T> = {
  [K in WritableKeys<T>]: T[K]
};

type WriteableNoOptional<T> = Omit<WritableObject<T>, KeysOfType<T, undefined>>

// and then generating something like this:
export interface TestSchema {
  readonly id: number;
  prop: string;
}

export type WriteableTestSchema = WriteableNoOptional<TestSchema> & Optional<TestSchema>;

//apiTestPost(id: number, body: WriteableTestSchema ): : Observable<TestSchema>
schimini commented 2 years ago

Any thoughts on this @luisfpg ?

luisfpg commented 2 years ago

Generating 2 interfaces from a single input is out of question, because the way the generator is done. There's another request which always gets blocked by the same fact: generating a proper Enum type for inline enums. So, in this sense, I'd say this would always be a limitation of the library: not supporting readOnly nor writeOnly.

schimini commented 2 years ago

I don't understand the limitations of the generator and I won't debate you on that for sure. Since 2 interfaces are out of question the method I wrote above might work. You only generate one interface (with the readonly modifier on the readOnly properties) and then depending on the http method for an endpoint the body param could use the helpers defined in my other comment in conjunction with:

type WriteableSchema<T> =  WriteableNoOptional<T> & Optional<T>

//method
apiTestPost(id: number, body: WriteableSchema<TestSchema>) : Observable<TestSchema>

I know it looks cumbersome for a low reward, and it might slow the ts compiler / linter a bit , and for that reason a flag could be added ( --experimental or something), but I think it works without compromising any part of the generator.

luisfpg commented 2 years ago

I'm sorry, but I really have no time to work on the generator nowadays... If you could provide a PR, that would be greatly appreciated. What I do think is that for models that don't have any readOnly properties, nothing should change at all in the generation...

schimini commented 2 years ago

No problem. I'll try to make a PR for it.

What I do think is that for models that don't have any readOnly properties, nothing should change at all in the generation...

About this I'll take it into consideration. I'll probably use some flag, since I've seen that pattern before in the generator code.