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

User Proto 업데이트 #1

Merged
merged 2 commits into from
Jan 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .github/workflows/pull-request.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,3 @@ jobs:
- uses: actions/checkout@v4
- uses: bufbuild/buf-setup-action@v1
- uses: bufbuild/buf-lint-action@v1
with:
input: proto
6 changes: 6 additions & 0 deletions buf.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
version: v1
lint:
use:
- DEFAULT
except:
- PACKAGE_VERSION_SUFFIX
53 changes: 30 additions & 23 deletions proto/user.proto
Original file line number Diff line number Diff line change
@@ -1,45 +1,52 @@
syntax = "proto3";
package fluentify;

message CreateUserRequest {
string name = 1;
int age = 2;
string disorderType = 3;
package proto;
option go_package = "github.com/gdsc-ys/fluentify-idl/proto";

message UserDTO {
string id = 1;
string name = 2;
int32 age = 3;
DisorderType disorder_type = 4;
}

message CreateUserResponse {
int id = 1;
enum DisorderType {
DISORDER_TYPE_UNSPECIFIED = 0;
}

// message CreateUserRequest {
// string name = 1;
// int age = 2;
// string disorderType = 3;
// }

// message CreateUserResponse {
// int id = 1;
// }
Copy link
Member Author

Choose a reason for hiding this comment

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

@namooplus @C-limlim Firebase Authentication 에 구글 로그인 사용하면 따로 CreateUser API 는 필요 없을거 같은데 필요한가욤??

Copy link
Member

Choose a reason for hiding this comment

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

필요없을 것 같습니당

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

만약 클라이언트 상에서 firebase authentication을 직접 참조하지 않고 서버 거쳐서 작업하면 필요할지도?

Copy link
Member Author

Choose a reason for hiding this comment

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

보통은 클라에서 firebase auth 진행 -> 나오는 id token 을 서버로 올려줌 -> 서버에서는 firebase admin sdk (모든 firebase 서비스에 접근 가능한 sdk) 로 올라온 token 의 validity 체크 순으로 진행되는 것 같던데

이런 방법도 있긴 하네요.


message GetUserRequest {
int id = 1;
string id = 1;
Copy link
Member

Choose a reason for hiding this comment

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

@jyoo0515 id 다 string으로 할건가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

그 firebase authentication 을 쓰면 거기서 주는 id 가 string 입니당

}

message GetUserResponse {
int32 id = 1;
string name = 2;
int32 age = 3;
string disorderType = 4;
UserDTO user = 1;
}

message UpdateUserRequest {
int32 id = 1;
string name = 2;
int32 age = 3;
string disorderType = 4;
string id = 1;
optional string name = 2;
optional int32 age = 3;
DisorderType disorder_type = 4;
Copy link
Member

Choose a reason for hiding this comment

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

DisorderType도 optional 일 것 같은데 어떻게 생각하시나요

Copy link
Member Author

Choose a reason for hiding this comment

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

따로 message 로 정의한 애들은 자동으로 optional 입니다! (primitive 가 아니고 object 느낌으로다가) 근데 명시적인게 더 좋으면 optional prefix 붙여도 좋을듯

Copy link
Member

Choose a reason for hiding this comment

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

아하 배우고 갑니다~ b 안붙이는걸로 하겠습니당

Copy link
Member

Choose a reason for hiding this comment

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

그럼 id에는 required 붙여야하지 않나여

Copy link
Member

Choose a reason for hiding this comment

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

put의 역할을 할까요 patch의 역할을 할까요

Copy link
Member Author

@jyoo0515 jyoo0515 Jan 27, 2024

Choose a reason for hiding this comment

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

그럼 id에는 required 붙여야하지 않나여

이게 역사가 있는데 처음에는 requiredoptional 키워드가 있다가 -> 둘다 없어졌다가 -> 그중에 optional 만 돌아옴 ㅋㅋㅋㅋ

따라서 기본적으로 optional 안붙어있는 애들은 required 라고 보면 됨! optional 키워드 안쓰고 아까 말한거처럼 따로 message 를 정의한 애들은 기본적으로 nullbale 이라는 점을 이용한 wrapper라는 애들이 있긴 한데 사용성이 안좋아서 (StringValue user_id = 1 이면 userId.value 로 꺼내써야 함) optional 을 붙였습니다

Copy link
Member Author

Choose a reason for hiding this comment

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

put의 역할을 할까요 patch의 역할을 할까요

patch 로 생각하긴 했는데 put 이 편하면 put 으로 가도 됩니다~

Copy link
Member

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ 이런...

Copy link
Member

Choose a reason for hiding this comment

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

patch 좋슴돠

}

message UpdateUserResponse {
int32 id = 1;
string name = 2;
int32 age = 3;
string disorderType = 4;
UserDTO user = 1;
}

message DeleteUserRequest {
int32 id = 1;
string id = 1;
}

message DeleteUserResponse {
int32 id = 1;
}
string id = 1;
}
Loading