Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: swagger组件中,ApiResponse 的 schema 参数的处理 #4078

Closed
wants to merge 1 commit into from

Conversation

7kyun
Copy link

@7kyun 7kyun commented Sep 19, 2024

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
  • @midway/swagger
Description of change

swagger组件中对直接配置了scheme 的写法做了一些兼容

起因是打算封装一个固定响应格式的装饰器,发现以下写法,直接使用schema不生效

import { ApiExtraModel, ApiOkResponse, getSchemaPath, ReferenceObject, SchemaObject, Type } from '@midwayjs/swagger';
import { ResponseVO } from '@/model/view/Basic';

export interface IResponseOptions<T extends Type<any>> {
  type: T;
  description?: string;
  isArray?: boolean;
  isPaginated?: boolean;
}

const baseTypeNames = ['String', 'Number', 'Boolean'];

export const ApiBasicResponse = <T extends Type<any>>(options: IResponseOptions<T>): MethodDecorator => {
  const { type } = options;
  const typeName = type?.name || 'null';
  let responseData: SchemaObject | ReferenceObject;
  if (options.isArray) {
    responseData = { type: 'array', items: { $ref: getSchemaPath(type) } };
  } else if (options.isPaginated) {
    responseData = {
      type: 'object',
      properties: {
        items: { type: 'array', items: { $ref: getSchemaPath(type) } },
        total: { type: 'number' },
        success: { type: 'boolean' },
      },
    };
  } else if (type) {
    if (baseTypeNames.includes(typeName)) {
      responseData = { type: typeName.toLocaleLowerCase() };
    } else {
      responseData = { $ref: getSchemaPath(type) };
    }
  } else {
    responseData = {
      type: 'null',
      nullable: true,
      default: null,
    };
  }

  const decorators: MethodDecorator[] = [
    ApiOkResponse({
      description: options.description || '请求成功响应',
      schema: {
        title: `响应数据 ${typeName}`,
        allOf: [
          {
            $ref: getSchemaPath(ResponseVO),
          },
          {
            properties: {
              data: responseData,
            },
          },
        ],
      },
    }),
  ];

  return (target, propertyKey, descriptor) => {
    ApiExtraModel([ResponseVO, type].filter(Boolean))(target.constructor);
    decorators.forEach(decorator => decorator(target, propertyKey, descriptor));
  };
};

另外还有一个Bug,使用ApiExtraModels绑定到Method也不生效,看源码貌似只对绑定到Class的数据进行了处理,因时间关系没有再行修复,而是直接改一下实现,手动绑定到类中

ApiExtraModel([ResponseVO, type].filter(Boolean))(target.constructor);

@7kyun 7kyun changed the title fix: swagger组件中,ApiResponse 的制定 schema 参数的处理 fix: swagger组件中,ApiResponse 的 schema 参数的处理 Sep 19, 2024
@czy88840616
Copy link
Member

czy88840616 commented Sep 20, 2024

image 测试好像没问题

@7kyun
Copy link
Author

7kyun commented Sep 23, 2024

image 测试好像没问题

这样配置确实是没问题,不会报错

但产物是有问题的

生成的JSON

image

SwaggerUI也是解析不了的

image

期望生成的结果应该是这样的

"responses": {
      "200": {
          "description": "OK",
          "content": {
              "application/json": {
                  "schema": {
                      "title": "SuccessResponseOfLoginResDto",
                      "allOf": [
                          {
                              "$ref": "#/components/schemas/ResponseDto"
                          },
                          {
                              "properties": {
                                  "data": {
                                      "$ref": "#/components/schemas/LoginResDto"
                                  }
                              }
                          }
                      ]
                  }
              }
          }
      }
  }

@7kyun
Copy link
Author

7kyun commented Sep 23, 2024

这是我直接在github修改提交的PR,所以lint有点问题,后面有空了再拉代码改一下,补个单测再提交吧,目前直接写 content 用着先了

ApiOkResponse({
    description: options.description || '请求成功响应',
    content: {
      'application/json': {
        schema: {
          title: `响应数据 ${typeName}`,
          allOf: [
            {
              $ref: getSchemaPath(ResponseVO),
            },
            {
              properties: {
                data: responseData,
              },
            },
          ],
        },
      },
    },
  })

@czy88840616
Copy link
Member

看了眼,schema 的确必须在 content 底下

@7kyun
Copy link
Author

7kyun commented Sep 23, 2024

其实Swagger还有这两个问题

使用ApiExtraModels绑定到Method也不生效,看源码貌似只对绑定到Class的数据进行了处理,因时间关系没有再行修复,而是直接改一下实现,手动绑定到类中

// 无效
ApiExtraModel([ResponseVO, type].filter(Boolean))(target.constructor, propertyKey, descriptor);
// 有效
ApiExtraModel([ResponseVO, type].filter(Boolean))(target.constructor);

ApiProperty 声明值为 Number 的枚举也是无效的

enum StatusEnum {
  Disabled,
  Enabled,
}

ApiProperty({
  type: 'enum',
  enum: StatusEnum,
})

看源码部分的处理是

// packages/swagger/src/common/enum.utils.ts
export function getEnumValues(enumType: SwaggerEnumType): string[] | number[] {
  if (Array.isArray(enumType)) {
    return enumType as string[];
  }
  if (typeof enumType !== 'object') {
    return [];
  }

  const values = [];
  const uniqueValues = {};

  for (const key in enumType) {
    const value = enumType[key];
    /* eslint-disable no-prototype-builtins */
    // filter out cases where enum key also becomes its value (A: B, B: A)
    if (
      !uniqueValues.hasOwnProperty(value) &&
      !uniqueValues.hasOwnProperty(key)
    ) {
      values.push(value);
      uniqueValues[value] = value;
    }
  }
  return values;
}

核心实现可以修改为

const numericValues = Object.values(enumType)
  .filter((value) => typeof value === 'number')
  .map((value) => value.toString());

return Object.keys(enumType)
  .filter((key) => !numericValues.includes(key))
  .map((key) => enumType[key]);

@czy88840616
Copy link
Member

ApiExtraModel 只支持类啊

@7kyun
Copy link
Author

7kyun commented Sep 23, 2024

看ts类型是支持method的
image

@7kyun
Copy link
Author

7kyun commented Sep 23, 2024

应该是统一用 createMixedDecorator 导致的

@czy88840616
Copy link
Member

我修复了,你看下

@7kyun
Copy link
Author

7kyun commented Sep 23, 2024

我修复了,你看下

辛苦大佬,粗看暂时没啥问题,等后面发布的时候再更新下看看 🤩

@czy88840616
Copy link
Member

@midwayjs/swagger@3.18.1

@7kyun
Copy link
Author

7kyun commented Sep 29, 2024

@czy88840616
大佬,这里是不是缺一个 else 枚举数组会被下面这个逻辑覆盖

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants