Backend fix #98

kanade9 · 2020-02-20T09:41:16Z

今回ので直した箇所
swaggerの定義修正(exampleを追加してわかりやすくしました。)
update by made byを統一して edited byに。uidを返却
expressのインスタンスを一つに統一
star rtaing などをDBに突っ込む時に自動的に0にして格納する
class_dataエンドポイント修正
responseのステータスコード返却対応
ファイルの切り分けを行なった
requireをなくす→これは修正するとモジュールがありません等のエラーになるので対応していません。

cors対応はいらないっていう話だったのでやってないです。

swagger定義のfix

reud

コメント返しました〜

reud · 2020-02-20T09:46:07Z

backend/functions/src/api.ts

+
+async function Verification(req: express.Request, resp: express.Response, next: () => void) {
+    // req.headers.authorization のオブジェクトが未定義となるためにts-ignore
+    // @ts-ignore


ここのコメント消しておいてください (ts-ignore使われていた行は消えましたよね？)

いや、最終的には消えませんでした。ので残しています

ここのts-ignoreはどこにかかっているのです？

ああ、21行目か・・・空行消しておいて欲しいかなぁ・・・

reud · 2020-02-20T09:46:18Z

backend/functions/src/api.ts

+    // @ts-ignore
+
+    // AuthorizationヘッダーはBearer <id_token>の形式のため、id_tokenを取り出すために7文字目以降の文字列を切り出している
+    const tokenstr = req.headers.authorization.toString().slice(7);


reud · 2020-02-20T09:47:50Z

backend/functions/src/api.ts

+
+app.use(Verification);
+
+export const WelcomeLog = functions.auth.user().onCreate((user) => {


これ何に使われているんでしたっけ・・・

WelcomeLogとDeleteLogはユーザ登録と削除のデバッグ用に使ってます！

@kanade9
多分また聞いちゃうのでそれがわかる様に変数名をかえていただけると！

reud · 2020-02-20T09:47:59Z

backend/functions/src/api.ts

+    return 0;
+});
+
+export const DeleteLog = functions.auth.user().onDelete((user) => {


これも何でしたっけ・・・

reud · 2020-02-20T09:48:26Z

backend/functions/src/api.ts

+    return 0;
+});
+
+// build multiple CRUD interfaces:


コメント内容と表す行合ってない様な・・・

reud · 2020-02-20T09:57:36Z

backend/functions/src/api.ts

+app.get('/comment', async (req: functions.Request, resp: express.Response) => {
+    console.log('subject_query= ' + req.query['class_name'] + ' uid=' + req.query['uid']);
+    try {
+        const qss = await fdb.collection('ClassSummary').doc(req.query['class_name']).collection('comment').doc(req.query['uid']).get();


こんな感じで縦に並べてかけるので見にくく感じてみたらこんな感じに書いた方が読みやすいと思う！

const qss = await fdb.collection('ClassSummary') .doc(req.query['class_name']) .collection('comment') .doc(req.query['uid']).get();

reud · 2020-02-20T09:57:53Z

backend/functions/src/api.ts

+    }
+
+    const data = {
+        // nameは授業名です。titleはコメントのタイトル ex.　神授業です!!等


reud · 2020-02-20T09:58:11Z

backend/functions/src/api.ts

+        'title': body.title,
+        'comment': body.comment,
+        'created_at': created_time,
+        'updated_at': moment().add(9, 'h').format(),


reud · 2020-02-20T10:02:27Z

backend/functions/src/api.ts

+    let class_created_time = null;
+    const doc = await fdb.collection('ClassSummary').doc(body.name).collection('comment').doc(body.made_by).get();
+    if (doc.exists) {
+        class_created_time = doc.data().created_at;


class_created_time = doc.data().created_at || moment().add(9, 'h').format();

って短くかけるよ！doc.data().created_atに値が入ってたらそれが使われて、がundefinedならmoment().add(9, 'h').format()が入る式になります。

reud · 2020-02-20T10:03:48Z

backend/functions/src/api.ts

+    try {
+        const qss = await fdb.collection('ClassSummary').doc(req.query['class_name']).collection('comment').doc(req.query['uid']).get();
+        if (!qss.data()) {
+            console.log('No comment were found match with ' + req.query['class_name'] + ' and this uid');


文字列内展開がTypescript(JavaScript)に搭載されているので是非使ってください！

console.log(`No comment were found match with ${req.query['class_name']} and this uid`);

kanade9 · 2020-02-23T16:12:43Z

ネスト消す方法でリファクタリングしてみたのですが[POST]api/class_dataで検証したところ、Error: Value for argument "documentPath" is not a valid resource path. Path must be a non-empty string.となり正しいパスを参照することができませんでした。.catch((e: string) => err = e);を削除してリファクタリング前の状態にしたところ正常に処理を完了することができました。

結論として、リファクタリングがうまくいかないみたいです。

reud · 2020-02-24T01:43:43Z

@kanade9

リファクタリングがうまくいかない

了解です。
リファクタリング前とリファクタリング後のコード載せられますか？変更差分を追いたいのですが。
このコードならこうなるはずだけど、こうなってしまうみたいな情報が載ってると状況がすごく追いやすいです。

変更部分が分からないので有効ではないかもしれないですが、
All firestore set(doc) calls fail due to "Error:Argument "documentPath" is not a valid ResourcePath. Path must be a non-empty string" · Issue #320 · firebase/firebase-admin-node
こことかかもしれないですね

kanade9 · 2020-02-24T02:08:08Z

https://github.com/gekko-org/pj-marowd/pull/98/files/6ef7dc229fc20779dd930ae482f1dd07eb4c5bce..82aa73eec7b9c8da0a4d347d855d8b3d9fb61fb4　
めちゃめちゃ比較わかりづらいんですが差分です。変更部分で大きなところはtry~catchをレビューされた方法で消して書き直した部分です。

参考サイトの記事読みました。documentのデータはjson型なのに、エラーでstring型を指定しているところに原因があるかもしれません。

reud · 2020-02-24T13:36:16Z

@kanade9
URLに空白入ってるw (むしろどうやったんだこれ)
確認してみます。

reud · 2020-02-24T13:41:02Z

backend/functions/src/index.ts

-app.post('/class_data', async (req: functions.Request, resp: express.Response) => {
-    console.log('json received');
-    const body = req.body;
-
-    // Check the validity of the token
-    const uid = admin.auth.decodedToken(body.token).uid;
-    if (!uid) {
-        resp.status(401).send('Unauthorized');
-    } else {
-        console.log(uid);
-    }
-
-    let class_created_time = null;
-    const doc = await fdb.collection('ClassSummary').doc(body.name).collection('comment').doc(body.made_by).get();
-    if (doc.exists) {
-        class_created_time = doc.data().created_at;
-    } else {
-        class_created_time = moment().add(9, 'h').format();
-    }
-    const data = {
-        'name': body.name,
-        'faculty': body.faculty,
-        'department': body.department,
-        'fav_amount': body.fav_amount,
-        'grade': body.grade,
-        'professor': body.professor,
-        'is_random': body.is_random,
-        'rating': body.rating,
-        'term': body.term,
-        'edited_by': body.edited_by,
-        'created_at': class_created_time,
-        'updated_at': moment().add(9, 'h').format(),
-    };
-    try {
-        await fdb.collection('ClassSummary').doc(body.name).set(data);
-        console.log(data);
-        resp.status(200).send(JSON.stringify({'status': 'OK'}));
-    } catch (exception) {
-        console.log('An error occurred. Class data cannot add in database');
-        resp.status(500).send('Internal Server Error');
-    }


reud · 2020-02-24T13:42:52Z

backend/functions/src/api.ts

+app.post('/class_data', async (req: functions.Request, resp: express.Response) => {
+    console.log('json received');
+    const body = req.body;
+
+    // Check the validity of the token
+
+    const uid = admin.auth.decodedToken(body.token).uid;
+    if (!uid) {
+        resp.status(401).send('Unauthorized');
+        return 0;
+    } else {
+        console.log(uid);
+    }
+
+    const doc = await fdb.collection('ClassSummary').doc(body.name).collection('comment').doc(body.made_by).get();
+    const class_created_time = doc.data().created_at || moment().add(9, 'h').format();
+
+    const data = {
+        'name': body.name,
+        'faculty': body.faculty,
+        'department': body.department,
+        'fav_amount': 0,
+        'grade': body.grade,
+        'professor': body.professor,
+        'is_random': body.is_random,
+        'rating': 0,
+        'term': body.term,
+        'edited_by': body.edited_by,
+        'created_at': class_created_time,
+        'updated_at': moment().add(9, 'h').format(),
+    };
+    let err = "";
+    await fdb.collection('ClassSummary').doc(body.name).set(data).catch((e: string) => err = e);
+
+    if (err !== "") {
+        console.log('An error occurred. Class data cannot add in database' + err);
+        resp.status(500).send('Internal Server Error');
+        return 0;
+    } else {
+        console.log(data);
+        resp.status(200).send(JSON.stringify({'status': 'OK'}));
+        return 0;
+    }
+});


@kanade9 この辺見てるけど変更箇所として見る部分は合ってる？

参考サイトの記事読みました。documentのデータはjson型なのに、エラーでstring型を指定しているところに原因があるかもしれません。

これresponse送るところ(resp.status(500).send('Internal Server Error');)について原因があるのではないかっていう予想で合ってる？
見た感じ変更前と変更後で変わってない様な・・・・

reud · 2020-02-24T13:44:30Z

@kanade9
なんか勘違いしてたらスマソン。CORSの問題があるからリクエストってどうなるんでしたっけ・・・？うまくいかない様な・・・？

reud · 2020-02-24T13:45:40Z

backend/functions/src/api.ts

+    if (err !== "") {
+        console.log('An error occurred. Class data cannot add in database' + err);
+        resp.status(500).send('Internal Server Error');
+        return 0;


@kanade9
ふと思ったんですけど、ここ何故0返しているんですか？

voidの認識なので　return;で良いかと。原因の切り分けにもなるので変更お願いします

reud · 2020-02-24T13:46:17Z

backend/functions/src/api.ts

+        resp.status(500).send('Internal Server Error');
+        return 0;
+    } else {
+        console.log(data);


early returnしているのでここのelseはいらないと思う！

kanade9 · 2020-02-24T14:00:51Z

corsはローカルで実行するときは意識しなくて大丈夫
#98 (comment)
ここは1箇所でearly returnをすると、全部の箇所にreturnをつけろって怒られたのでそうしてます。

reud · 2020-02-24T14:06:45Z

全部の箇所にreturnつけた方が良いのでは・・・

reud · 2020-02-24T14:07:57Z

corsはローカルで実行するときは意識しなくて大丈夫

CORSはローカルでも意識する必要がある認識
localhostを共有対象に設定しなきゃいけないはず。

reud · 2020-02-24T14:09:48Z

これは予想ですが、
kanadeがchromeを使ってないからlocalではcors対応していないとか・・・？
CORSエラー対策 - Qiita

kanade9 · 2020-02-24T14:14:45Z

ごめん書き方が悪かったです。"ローカルでは意識しなくて大丈夫"じゃなくて、"PostmanとかCLIからHTTPリクエストを送るときには意識しなくても大丈夫"でした。
https://developer.mozilla.org/ja/docs/Web/HTTP/CORS

kanade9 · 2020-02-24T14:16:22Z

"オリジン間リソース共有Cross-Origin Resource Sharing (CORS) は、追加の HTTP ヘッダーを使用して、あるオリジンで動作しているウェブアプリケーションに、異なるオリジンにある選択されたリソースへのアクセス権を与えるようブラウザーに指示するための仕組みです。ウェブアプリケーションは、自分とは異なるオリジン (ドメイン、プロトコル、ポート番号) にあるリソースをリクエストするとき、オリジン間 HTTP リクエストを実行します。"

今回検証している環境はWebアプリケーションを介していない場合なので、CORSが原因ではないと思われます

reud · 2020-02-24T15:58:41Z

ごめん書き方が悪かったです。"ローカルでは意識しなくて大丈夫"じゃなくて、"PostmanとかCLIからHTTPリクエストを送るときには意識しなくても大丈夫"でした。
https://developer.mozilla.org/ja/docs/Web/HTTP/CORS

CLIからってことね了解です

reud · 2020-02-24T16:04:43Z

"オリジン間リソース共有Cross-Origin Resource Sharing (CORS) は、追加の HTTP ヘッダーを使用して、あるオリジンで動作しているウェブアプリケーションに、異なるオリジンにある選択されたリソースへのアクセス権を与えるようブラウザーに指示するための仕組みです。ウェブアプリケーションは、自分とは異なるオリジン (ドメイン、プロトコル、ポート番号) にあるリソースをリクエストするとき、オリジン間 HTTP リクエストを実行します。"

今回検証している環境はWebアプリケーションを介していない場合なので、CORSが原因ではないと思われます

ブラウザ側が弾いているので私もその認識です。

reud · 2020-02-24T16:06:42Z

@kanade9
made_byがswagger.yamlから消えているのにcloudfunctions側には残っているのでその辺りも統一した方が良いかな！
多分全置換(Ctrl + r) でさくっとできると思う！

kanade9 · 2020-02-24T16:08:05Z

#98 (comment)
注目してほしい箇所は103,104行目あたりです
let err = "";
await fdb.collection('ClassSummary').doc(body.name).set(data).catch((e: string) => err = e);

reud · 2020-02-24T16:24:42Z

@kanade9

await fdb.collection('ClassSummary').doc(body.name).set(data)の場合では正常に取得できています

エラーまとめられるしtry-catchにしましょう！理解足りてませんでした！ごめんなさい！

の様にエラー内容もconsole.logに出してくれると助かります！(すでにやってたらすいません)

try {
  const resp = await fetch(url);
  const json = await resp.json();
  console.log(json);
} catch (e: Error) {
  console.log("エラー発生!");
  console.log(e);
}

kanade9 · 2020-02-25T00:55:51Z

ありがとう！！修正します！

kanade9 · 2020-02-28T08:38:31Z

commentのパラメータ comment_uidについて。　uidはバックエンドで管理するため、なんらかの値が設定されていたら自分のコメントを持ってくるように処理を書きました。(値がセットされていない場合は全件のコメントを返す)

reud · 2020-03-01T00:42:14Z

@kanade9
了解です！swaggerの該当部分のdescriptionにその仕様書いてくれるとありがたい！

kanade9 · 2020-03-01T07:06:59Z

リファクタリング完了しました！修正箇所は
・パラメータの設定によって複数個のデータを返す部分の実装を修正
・exception をつけてエラーをわかりやすく！
・文字列展開の内蔵
・early return とreturn;を記載
・swaggerの仕様変更

reud · 2020-03-04T02:48:13Z

@kanade9

リファクタリング完了しました！修正箇所は
・パラメータの設定によって複数個のデータを返す部分の実装を修正
・exception をつけてエラーをわかりやすく！
・文字列展開の内蔵
・early return とreturn;を記載
・swaggerの仕様変更

見逃してた！あざます！

host: "localhost:5000" basePath: "/pj-marowd/us-central1/api/"

…j-marowd into refactor_backend_fix

swaggerを元に戻した。

Refactor backend fix

kanade9 · 2020-03-14T05:11:31Z

ヘッダーとリファクタリング終了したのでマージします

kanade9 and others added 9 commits February 6, 2020 16:30

added cors module and fixed status code

76ffccc

cors対応ムリ〜〜

3d4cfe9

arrowAll

94206da

updated_by,made_byをedited_byに統一(授業,コメント両方)

0cd617b

swagger定義のfix

[WIP] frontにtokenトークン表示

875fffb

index.ts バックエンド 1つのインスタンスに修正

dec9f23

frontend　uid取り出そうとして色々試行錯誤したやつ消しました

76111c4

エンドポイント直してなかったの修正

6ef7dc2

一通りの修正完了!!

e7c23f8

reud reviewed Feb 20, 2020

View reviewed changes

kanade9 added 2 commits February 21, 2020 16:44

[WIP] tryとcatchのネスト除去などのコードレビュー箇所の修正。uidをバックエンドでよしなにやるところの作業中。

2a409c2

[WIP] コードリファクタリング

82aa73e

reud reviewed Feb 24, 2020

View reviewed changes

kanade9 added 3 commits February 25, 2020 16:47

[WIP] GET class_dataの実装完了

cbd51ed

[WIP] GET,POST comment まで完了。残りはdeleteのみ

c505de1

api.ts: api仕様通りの修正完了 swagger: subjectとかいう謎のものがあったので消した。残作業:細かいリファクタリング

ba2552f

kanade9 and others added 3 commits March 1, 2020 16:07

リファクタリングの完了

0e67989

[WIP] いろいろ修正

e989147

[WIP]

5ec6597

reud and others added 16 commits March 4, 2020 12:25

多分デプロイ出来た

609879e

これでこそ本当にデプロイ出来た

4e74168

fix README

ad5b6c2

色々書いた

12f378a

optionsメソッドの考慮など

1bf7b78

Update swagger.yaml

6b13b89

にゃn

fd5ef9e

host: "localhost:5000" basePath: "/pj-marowd/us-central1/api/"

change scheme

5f505f6

add

4942529

Merge branch 'refactor_backend_fix' of https://github.com/gekko-org/p…

e087793

…j-marowd into refactor_backend_fix

修正ミスの修正(語彙力)

18c9cfa

swaggerを元に戻した。

ファイルの切り分け、許可ヘッダの追加

8bbcf40

tokenの切り出し方の修正

7efbd41

レスポンスヘッダのエラーの修正

75c2b1f

レスポンスヘッダのエラーの修正　再

98ce96b

Merge pull request #101 from gekko-org/refactor_backend_fix

72ccc2c

Refactor backend fix

kanade9 merged commit 7ace2ee into feature/84_swagger_update Mar 14, 2020


		app.use(Verification);

		export const WelcomeLog = functions.auth.user().onCreate((user) => {

Backend fix #98

Backend fix #98

Conversation

kanade9 commented Feb 20, 2020

reud left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kanade9 commented Feb 23, 2020

reud commented Feb 24, 2020 • edited Loading

kanade9 commented Feb 24, 2020 • edited Loading

reud commented Feb 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reud commented Feb 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kanade9 commented Feb 24, 2020 • edited Loading

reud commented Feb 24, 2020

reud commented Feb 24, 2020 • edited Loading

reud commented Feb 24, 2020

kanade9 commented Feb 24, 2020

kanade9 commented Feb 24, 2020

reud commented Feb 24, 2020

reud commented Feb 24, 2020

reud commented Feb 24, 2020

kanade9 commented Feb 24, 2020

reud commented Feb 24, 2020 • edited Loading

kanade9 commented Feb 25, 2020

kanade9 commented Feb 28, 2020

reud commented Mar 1, 2020

kanade9 commented Mar 1, 2020

reud commented Mar 4, 2020

kanade9 commented Mar 14, 2020

reud commented Feb 24, 2020 •

edited

Loading

kanade9 commented Feb 24, 2020 •

edited

Loading

reud commented Feb 24, 2020 •

edited

Loading

kanade9 commented Feb 24, 2020 •

edited

Loading

reud commented Feb 24, 2020 •

edited

Loading

reud commented Feb 24, 2020 •

edited

Loading