Skip to content

Conversation

Capella87
Copy link
Contributor

@Capella87 Capella87 commented Aug 22, 2024

Description

현재 GET handler는 샘플의 EventListEntity를 생성해 반환하는 방식으로 구현되어 있습니다.

Features on this PR

이벤트를 보여주기 위해 필요한 정보를 담는 클래스 두 개를 새로 정의했습니다.

EventListEntity

      "EventListEntity": {
        "required": [
          "endTime",
          "eventId",
          "eventName",
          "isActive",
          "OrganizerEmail",
          "organizerName",
          "registered",
          "startTime",
          "tz"
        ],
        "type": "object",
        "properties": {
          "eventId": {
            "type": "string",
            "description": "Event Id",
            "nullable": true
          },
          "eventName": {
            "type": "string",
            "description": "Event name",
            "nullable": true
          },
          "startTime": {
            "type": "string",
            "description": "Event start time",
            "format": "date-time",
            "nullable": true
          },
          "endTime": {
            "type": "string",
            "description": "Event end time",
            "format": "date-time",
            "nullable": true
          },
          "tz": {
            "type": "string",
            "description": "Event's time zone. It could be serialized to string",
            "nullable": true
          },
          "organizerName": {
            "type": "string",
            "description": "Organizer's name",
            "nullable": true
          },
          "OrganizerEmail": {
            "type": "string",
            "description": "Organizer's email address.",
            "format": "email",
            "nullable": true
          },
          "registered": {
            "type": "integer",
            "description": "The number of registered users",
            "format": "int32",
            "nullable": true
          },
          "resources": {
            "type": "array",
            "items": {
              "$ref": "#/components/schemas/ResourceEntity"
            },
            "description": "List of resources that the organizer deploys",
            "nullable": true
          },
          "isActive": {
            "type": "boolean",
            "description": "Shows whether the event is active or not",
            "nullable": true
          }
        },
        "additionalProperties": false,
        "description": "Data transfer model for showing all events"
      },

해당 클래스는 내부에 리소스 정보를 담기 위해 ResourceEntity라는 서브 클래스 정의를 갖고 있습니다.

ResourceEntity

"ResourceEntity": {
        "required": [
          "id",
          "name"
        ],
        "type": "object",
        "properties": {
          "id": {
            "type": "string",
            "description": "Resource's Id",
            "nullable": true
          },
          "name": {
            "type": "string",
            "description": "Resource's name. e.g. gpt4o",
            "nullable": true
          }
        },
        "additionalProperties": false,
        "description": "Define the list entity of Resources"
      },

Azure 쪽에서는 리소스(예: gpt4o)를 표시하고 있었기 때문에 추가하였습니다. 이 클래스에서는 리소스 ID와 이름만 들어 있습니다. 리소스에 대한 자세한 정보를 담는 클래스는 별도로 정의될 것 같습니다.

Endpoint

이벤트 목록을 반환하는 GET 메소드를 MapGet을 통해 /events 경로 하에 정의하였으며, 해당 메소드를 갖고 있는 endpoint를 RouteHandlerBuilder를 사용하여 EventEndpoint로 정의하였습니다.

"paths": {
    "/events": {
      "get": {
        "tags": [
          "events"
        ],
        "summary": "Get all events",
        "description": "Show all events in Azure OpenAI Proxy services.",
        "operationId": "GetEvents",
        "responses": {
          "200": {
            "description": "OK",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/EventListEntity"
                }
              }
            }
          },
          "401": {
            "description": "Unauthorized",
            "content": {
              "application/problem+json": {
                "schema": {
                  "$ref": "#/components/schemas/ProblemHttpResult"
                }
              }
            }
          },
          "404": {
            "description": "Not Found",
            "content": {
              "application/problem+json": {
                "schema": {
                  "$ref": "#/components/schemas/ProblemHttpResult"
                }
              }
            }
          },
          "500": {
            "description": "Internal Server Error",
            "content": {
              "application/problem+json": {
                "schema": {
                  "$ref": "#/components/schemas/ProblemHttpResult"
                }
              }
            }
          }
        }
      }
    },

아울러, 페이로드를 정의해야 했으나 현재 구현에서는 샘플 객체를 반환하는 방식으로만 구현했고, Authorization 관련 구현이 없어 따로 정의하지는 않았습니다.

events는 TypedResults를 이용해 정상적으로는 200 OK를 반환하며, Authorization 없는 접근의 경우 401번, 찾을 수 없었던 경우면 404번, 서버에 문제가 생긴 경우 500번을 반환하게 정했습니다.

해당 에러가 발생하는 경우 application/problem+json MIME으로 반환하게 됩니다.

기타 변경점

  • Swashbuckle에서 XML comment를 파싱하여 description을 작성하게 Swagger 설정 추가

해당 endpoint에 관련된 테스트도 추가하겠습니다.

감사합니다.

@justinyoo
Copy link
Contributor

@Capella87 머지 컨플릭트 먼저 해결해 주세요!

@Capella87
Copy link
Contributor Author

Capella87 commented Aug 23, 2024

충돌 해결했습니다.

Copy link
Contributor

@justinyoo justinyoo left a comment

Choose a reason for hiding this comment

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

코멘트 남겨뒀습니다. 무언가 새로운 것을 도입하고 싶다면, 별도의 이슈를 생성하고 거기서 논의를 거쳐 그것을 바탕으로 다른 PR을 생성해 주세요. 새로운 것은 이 태스크의 스코프가 아닙니다.

@justinyoo
Copy link
Contributor

@Capella87 로컬에서 빌드가 성공하든가요? 로컬에서 빌드가 성공하는지 확인하시고 커밋해 보세요.

@justinyoo
Copy link
Contributor

@Capella87 거의 비슷한 형태로 다른 엔드포인트만을 구현하는 것이다보니 기존의 코드베이스에 대한 분석을 먼저 하고 비슷하게 적용시켜보는 것이 제일 좋습니다.

@Capella87
Copy link
Contributor Author

Capella87 commented Aug 23, 2024

@Capella87 로컬에서 빌드가 성공하든가요? 로컬에서 빌드가 성공하는지 확인하시고 커밋해 보세요.

확인한 결과 ServiceCollectionExtensions에서 SecretClient 가 정의된 Azure.Security.KeyVault.SecretsDefaultAzureCredential이 정의된 Azure.Identity 네임스페이스가 소스 코드에 누락되어 있어 fb3e867 에서 추가했습니다.

추가한 이후 빌드와 테스트가 성공했습니다.

@Capella87
Copy link
Contributor Author

수정한 점

  • EventListEntityEventDetails로 이름을 변경하고, Admin 클래스에 맞추어 프로퍼티도 수정했습니다. (946c381)
  • EventDetailsEventEndpoint에 있는 주석을 규칙에 맞게 다시 작성했습니다. (bf5c999)
  • 중복되거나 불필요한 설정을 전부 제거했습니다. (8e1e71a, c03c5d1, 99616eb)
  • EventEndpoint의 handler의 HttpRequest 매개변수를 삭제했습니다. (78927ac)
  • EventEndpoint의 handler가 Results.Ok()를 반환하게 수정했습니다. (bf5c999, 2326d7e)
  • Endpoint의 OpenAPI annotation 중 오류 발생시 반환하는 코드와 관련된 부분을 수정했습니다. (86ed5ec)
  • Endpoint의 OpenAPI summary와 description을 '로그인된 사용자가 참여한 이벤트를 보여준다'는 내용을 포함하게 수정했습니다. (343db20)

@justinyoo
Copy link
Contributor

justinyoo commented Aug 23, 2024

확인한 결과 ServiceCollectionExtensions에서 SecretClient 가 정의된 Azure.Security.KeyVault.SecretsDefaultAzureCredential이 정의된 Azure.Identity 네임스페이스가 소스 코드에 누락되어 있어 fb3e867 에서 추가했습니다.

추가한 이후 빌드와 테스트가 성공했습니다.

그렇지 않습니다 최신 코드를 받아보세요. 누락됐다고 하는 코드는 이미 한참전에 추가되어 있었어요. 지금 이상태라면 자신의 코드가 가장 최신을 반영하고 있는지 먼저 검토해 보는 것을 추천합니다.

@justinyoo
Copy link
Contributor

수정한 점

  • EventListEntityEventDetails로 이름을 변경하고, Admin 클래스에 맞추어 프로퍼티도 수정했습니다. (946c381)
  • EventDetailsEventEndpoint에 있는 주석을 규칙에 맞게 다시 작성했습니다. (bf5c999)
  • 중복되거나 불필요한 설정을 전부 제거했습니다. (8e1e71a, c03c5d1, 99616eb)
  • EventEndpoint의 handler의 HttpRequest 매개변수를 삭제했습니다. (78927ac)
  • EventEndpoint의 handler가 Results.Ok()를 반환하게 수정했습니다. (bf5c999, 2326d7e)
  • Endpoint의 OpenAPI annotation 중 오류 발생시 반환하는 코드와 관련된 부분을 수정했습니다. (86ed5ec)
  • Endpoint의 OpenAPI summary와 description을 '로그인된 사용자가 참여한 이벤트를 보여준다'는 내용을 포함하게 수정했습니다. (343db20)

다른 코멘트에서도 언급을 했지만, 현재 변경사항이 최신 코드베이스에서 한 것인지 의구심이 듭니다. 자신의 코드베이스를 최신화 한 후에 이 변경사항을 적용시켜보시고, 그리고 다시 PR을 생성해 주세요.

Remove unused using directives in EventEndpoint.cs, Program.cs

Removed the following unused using directives:
- In EventEndpoint.cs:
  - Microsoft.AspNetCore.Http.HttpResults
  - Microsoft.AspNetCore.Mvc
- In Program.cs:
  - System.Reflection
  - Microsoft.Extensions.DependencyInjection
  - Microsoft.OpenApi.Models
@Capella87
Copy link
Contributor Author

수정한 점

  • AdminEventDetailsEventDetails를 상속하게 리팩토링 (e4c9859)
  • 불필요한 namespace 삭제 (f46d996)

Copy link
Contributor

@justinyoo justinyoo left a comment

Choose a reason for hiding this comment

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

잘하셨습니다! 이제 테스트코드를 작성해 주세요!

@Capella87 Capella87 requested a review from justinyoo August 25, 2024 07:41
@Capella87
Copy link
Contributor Author

Capella87 commented Aug 26, 2024

추가한 점

  • EventEndpoint에서 경로 반환, verb 테스트, 태그 반환, 값 반환, 객체 반환, 응답(response) 반환에 대한 xUnit 테스트 코드 작성 (0baa716, 4a42151)

@justinyoo
Copy link
Contributor

@Capella87 컨플릭 났어요~!

@Capella87
Copy link
Contributor Author

@justinyoo 해결했습니다.

Copy link
Contributor

@justinyoo justinyoo left a comment

Choose a reason for hiding this comment

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

좋습니다! 고생하셨어요!

@justinyoo justinyoo merged commit 30fd8a6 into aliencube:main Aug 28, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OpenAPI] Add endpoint for list of events
2 participants