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

文字数をutf-32で数える #499

Merged
merged 10 commits into from
Mar 27, 2023
Merged

文字数をutf-32で数える #499

merged 10 commits into from
Mar 27, 2023

Conversation

20jun01
Copy link
Contributor

@20jun01 20jun01 commented Mar 13, 2023

  • close validationの文字数制限をutf-32で数える #423
    validatorのrule lengthをLengthからRuneLengthに変更して数え方を変えています。
    ローカルのテストが80のポートが衝突してできていません。killコマンドで止めても実行するたびに復活するので困ってます。
    おそらく、次の時に聞きます。

@20jun01 20jun01 self-assigned this Mar 13, 2023
vdRuleDisplayNameLength = vd.Length(1, 256) // 外部アカウントのアカウント名文字数上限
vdRuleDescriptionLength = vd.Length(1, 256)
vdRuleResultLength = vd.Length(0, 32)
vdRuleNameLength = vd.RuneLength(1, 32)
Copy link
Member

Choose a reason for hiding this comment

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

vd.RuneLengthとかいうメソッドあったんだ
いい感じにテスト変更してもらえると助かります(全角32文字のコンテスト名とか)

@codecov
Copy link

codecov bot commented Mar 13, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.12 🎉

Comparison is base (dd9ecec) 88.74% compared to head (46e43df) 88.87%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #499      +/-   ##
==========================================
+ Coverage   88.74%   88.87%   +0.12%     
==========================================
  Files          58       58              
  Lines        4673     4673              
==========================================
+ Hits         4147     4153       +6     
+ Misses        473      469       -4     
+ Partials       53       51       -2     
Flag Coverage Δ
integration 81.93% <ø> (+0.08%) ⬆️
unit 88.90% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
interfaces/handler/validator.go 91.89% <ø> (ø)
interfaces/repository/model/user.go 100.00% <ø> (ø)

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ras0q
Copy link
Member

ras0q commented Mar 14, 2023

:80で立ち上げてるとこなさそうな気がしますがどこだろ、、

@20jun01
Copy link
Contributor Author

20jun01 commented Mar 14, 2023

lsof -i -P | grep 80でCOMMANDのところに「go」となっているところが80番ポートを使っているみたいなので、自分の環境が悪そうな気もします

@20jun01
Copy link
Contributor Author

20jun01 commented Mar 14, 2023

lsof -i -P | grep 80でCOMMANDのところに「go」となっているところが80番ポートを使っているみたいなので、自分の環境が悪そうな気もします

goだけでechoライブラリを使った何かが実行され、brew uninstallやhttps://qiita.com/Nekonecode/items/8561bbe27830090bc70cでも消えないので恐ろしいぶっ壊れ方してます。。。

@20jun01
Copy link
Contributor Author

20jun01 commented Mar 14, 2023

oribeさんの多大な協力により環境を直していただいたのでローカルでテストできるようになりました:kansha:

@20jun01
Copy link
Contributor Author

20jun01 commented Mar 21, 2023

make lintしたら変更していないところに新たに文句言われてます:thonk_blob:

@20jun01
Copy link
Contributor Author

20jun01 commented Mar 21, 2023

2023/03/21 15:58:29 /home/runner/work/traPortfolio/traPortfolio/infrastructure/sqlhandler.go:72 Error 1406 (22001): Data too long for column 'name' at row 1
[1.463ms] [rows:0] INSERT INTO accounts (id,type,name,url,user_id,check,created_at,updated_at) VALUES ('5f923b41-be29-4abd-852f-6e9452c34f30',1,'亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜亜','https://huaqFNwEK6L31OW31UKrXyFXHi8X','11111111-1111-4111-8111-111111111111',false,'2023-03-21 15:58:29.229','2023-03-21 15:58:29.229')
もしかして、validationよりも厳しい条件を満たさないと通しちゃいけないAPIありますか?

@ras0q
Copy link
Member

ras0q commented Mar 21, 2023

あーinterfaces/repository/modelのvarchar(32)とかを直す必要がありそう

@20jun01
Copy link
Contributor Author

20jun01 commented Mar 23, 2023

あーinterfaces/repository/modelのvarchar(32)とかを直す必要がありそう

varcharはマルチバイト文字に対応してそうなのでこのままで良さそうな気もします(ちゃんとした出典を持ってくることができませんでした:sad_blob:。)
このnameデータdisplayNameとしてわたしたテストデータなんですが、displaynameと対応したカラムではなくnameと対応したカラムだったりしますか?その場合、テストの呼び方が悪そうなので修正します。

@ras0q
Copy link
Member

ras0q commented Mar 26, 2023

多分勘違いしてたんですが意図的に長いユーザー名を入れてこのエラーが出てるならテスト自体は成功してない、、?

@20jun01
Copy link
Contributor Author

20jun01 commented Mar 27, 2023

desplay nameが256制限で長さ制限に引っかからない256文字のものを通しているのですが、(おそらく)32文字制限のNameに対応しているカラムに入ってしまっているのかな、って思ってました

@ras0q
Copy link
Member

ras0q commented Mar 27, 2023

なるほど
普通にバグでここを256にすればよさそうです

Name string `gorm:"type:varchar(32)"`

@ras0q
Copy link
Member

ras0q commented Mar 27, 2023

あと.DS_storeはgitの管理下から外して欲しい🙏

@20jun01 20jun01 marked this pull request as ready for review March 27, 2023 13:29
@20jun01 20jun01 requested a review from ras0q March 27, 2023 13:30
@20jun01
Copy link
Contributor Author

20jun01 commented Mar 27, 2023

CI / golangci (pull_request) Failing after 1s — reviewdog [golangci] reportのとこも修正しちゃっていいですか?(このPRの変更箇所と関係なさそうですが)
そもそもそんなに直す必要はなさそうでもあります
組み込み関数のnewを再定義してるのだけはややって感じですが、他は書き方の好みの気がします

@ras0q
Copy link
Member

ras0q commented Mar 27, 2023

使ってるlinterに変更が入ったっぽいですね
とりあえずこのPRには関係ないのであとで直せばいいと思います
mgechev/revive#799

Copy link
Member

@ras0q ras0q left a comment

Choose a reason for hiding this comment

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

良さそうです🙌
1回しか使ってないなら変数宣言しなくてもいいかもって思ったんですが好みなのでこのままマージしてくれてOKです

@20jun01
Copy link
Contributor Author

20jun01 commented Mar 27, 2023

ありがとうございます。:kansha:
マージしちゃいます

@20jun01 20jun01 merged commit 2d89c31 into main Mar 27, 2023
@20jun01 20jun01 deleted the improve/validation_count branch March 27, 2023 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

validationの文字数制限をutf-32で数える
2 participants