samber/loの適用

[Hueter57]

#774

lo.Reduceが使えそうといってたところはまだです。
issueでH1ronoさんが言っていたところはどう書けばいいかいまいちわからなかったです。
別のところですがここならlo.Reduce使えそうです。

Jomon/service/webhook.go

Lines 120 to 123 in 4f7fed3

	amount := 0
	for _, target := range resApp.Targets {
	amount += target.Amount
	}

以下変更できなかったappendを示しておきます。

Jomon/router/middleware.go

Lines 26 to 67 in 4f7fed3

	func (h Handlers) AccessLoggingMiddleware(logger *zap.Logger) echo.MiddlewareFunc {
	return func(next echo.HandlerFunc) echo.HandlerFunc {
	return func(c echo.Context) error {
	start := time.Now()
	err := next(c)
	if err != nil {
	c.Error(err)
	}
	stop := time.Now()
	req := c.Request()
	res := c.Response()
	latency := strconv.FormatFloat(stop.Sub(start).Seconds(), 'f', 9, 64) + "s"
	fields := []zapcore.Field{
	zap.String("requestMethod", req.Method),
	zap.Int("status", res.Status),
	zap.String("userAgent", req.UserAgent()),
	zap.String("remoteIp", c.RealIP()),
	zap.String("referer", req.Referer()),
	zap.String("protocol", req.Proto),
	zap.String("requestUrl", req.URL.String()),
	zap.String("requestSize", req.Header.Get(echo.HeaderContentLength)),
	zap.String("responseSize", strconv.FormatInt(res.Size, 10)),
	zap.String("latency", latency),
	}
	httpCode := res.Status
	switch {
	case httpCode >= 500:
	fields = append(fields, zap.Error(err))
	logger.Error("server error", fields...)
	case httpCode >= 400:
	fields = append(fields, zap.Error(err))
	logger.Warn("client error", fields...)
	case httpCode >= 300:
	logger.Info("redirect", fields...)
	default:
	logger.Info("success", fields...)
	}
	return nil
	}
	}
	}

Jomon/router/request.go

Lines 244 to 255 in 4f7fed3

	for _, tagID := range req.Tags {
	tag, err := h.Repository.GetTag(ctx, *tagID)
	if err != nil {
	if ent.IsNotFound(err) {
	h.Logger.Info("could not find tag in repository", zap.String("ID", tagID.String()))
	return echo.NewHTTPError(http.StatusNotFound, err)
	}
	h.Logger.Error("failed to get tag from repository", zap.Error(err))
	return echo.NewHTTPError(http.StatusInternalServerError, err)
	}
	tags = append(tags, tag)
	}

Jomon/router/request.go

Lines 452 to 463 in 4f7fed3

	for _, tagID := range req.Tags {
	tag, err := h.Repository.GetTag(ctx, *tagID)
	if err != nil {
	if ent.IsNotFound(err) {
	h.Logger.Info("could not find tag in repository", zap.String("ID", tagID.String()))
	return echo.NewHTTPError(http.StatusNotFound, err)
	}
	h.Logger.Error("failed to get tag from repository", zap.Error(err))
	return echo.NewHTTPError(http.StatusInternalServerError, err)
	}
	tags = append(tags, tag)
	}

Jomon/router/transaction_test.go

Line 803 in 4f7fed3

res = append(res, &resOverview)

Jomon/router/transaction.go

Lines 185 to 229 in 4f7fed3

	for _, target := range tx.Targets {
	if target == nil {
	h.Logger.Info("target is nil")
	return echo.NewHTTPError(http.StatusBadRequest, "target is nil")
	}
	created, err := h.Repository.CreateTransaction(
	ctx,
	tx.Amount, *target, tx.Tags, tx.Group, tx.Request)
	if err != nil {
	h.Logger.Error("failed to create transaction in repository", zap.Error(err))
	return echo.NewHTTPError(http.StatusInternalServerError, err)
	}
	tags := lo.Map(created.Tags, func(tag *model.Tag, index int) *TagOverview {
	return &TagOverview{
	ID: tag.ID,
	Name: tag.Name,
	CreatedAt: tag.CreatedAt,
	UpdatedAt: tag.UpdatedAt,
	}
	})
	var group *GroupOverview
	if created.Group != nil {
	group = &GroupOverview{
	ID: created.Group.ID,
	Name: created.Group.Name,
	Description: created.Group.Description,
	Budget: created.Group.Budget,
	CreatedAt: created.Group.CreatedAt,
	UpdatedAt: created.Group.UpdatedAt,
	}
	}
	res := Transaction{
	ID: created.ID,
	Amount: created.Amount,
	Target: created.Target,
	Request: created.Request,
	Tags: tags,
	Group: group,
	CreatedAt: created.CreatedAt,
	UpdatedAt: created.UpdatedAt,
	}
	transactions = append(transactions, &res)
	}

[H1rono]

とりあえずCIの対応お願いします

[H1rono]

issueでH1ronoさんが言っていたところはどう書けばいいかいまいちわからなかったです。

&&を使えばいいです

result := lo.Reduce(targets, func(acc bool, target Target, _ int) bool {
    return acc && ...;
}, true)

breakに比べて自明に効率が落ちるけど微々たるものなので

[H1rono]

とりあえず、entフォルダ以下は自動生成されたやつなので戻して欲しいです 🙏

[H1rono]

や、嘘か
別のPRでrebaseした結果ですね、じゃあ戻さなくて大丈夫です

嘘

[codecov]

Codecov Report

Attention: Patch coverage is 85.65891% with 37 lines in your changes missing coverage. Please review.

Project coverage is 25.64%. Comparing base (72b3b86) to head (28c2e09).

Files with missing lines	Patch %	Lines
router/request.go	83.89%	16 Missing and 3 partials ⚠️
service/webhook.go	0.00%	9 Missing ⚠️
testutil/random/ramdom.go	0.00%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##               v2     #789      +/-   ##
==========================================
- Coverage   25.67%   25.64%   -0.04%     
==========================================
  Files         148      148              
  Lines       26892    26878      -14     
==========================================
- Hits         6904     6892      -12     
+ Misses      19112    19110       -2     
  Partials      876      876              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[H1rono]

メモ
https://github.com/traPtitech/Jomon/compare/v2..samber_lo_adapt

[Hueter57]

レビュー出しましたけどlo.Reduceの変更がまだなのでやります

[Hueter57]

これで変更可能そうなappendとforは全部samber/loのものに変更できたと思います

[H1rono]

概ね良さそうです

[H1rono]

LGTM 👍