You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by "klesh (via GitHub)" <gi...@apache.org> on 2023/09/25 10:36:45 UTC
[GitHub] [incubator-devlake] klesh opened a new pull request, #6146: refactor: unify connection/scope/scopeConfig helpers
klesh opened a new pull request, #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146
### Summary
Previously, the connection/scope/scopeConfig were written at different times by multiple people so the code is kind of messy.
This PR tries to unify them in a more readable and maintainable way:
1. introduce `service helper` and `api helper`, the former is designed to help with business logic, like loading from the database, creating, updating, and validating, while the latter acts as the adapter between Http Requests and Services
2. introduce `model service helper` which implements some basic business logic shared by all models so you may nest the helper within your own service class and use the functionalities in any way you want including exposing them directly or overriding them when needed.
3. connection / scope / scope helpers are organized in a unified way, all are offered both a `service helper` and an `api helper`, the former helper can be used in any package as you see fit, while the latter `api helper` should be used only in the `api/` package.
4. this PR only updates part of the `github` plugins as a PoC, I will work on other after the PR merge
### Does this close any open issues?
Part of #5519
### Screenshots
Note that there is a API change, @mintsweet
<img width="964" alt="sope list" src="https://github.com/apache/incubator-devlake/assets/61080/4debf567-51ca-4ced-8529-43c82d0fa5c4">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338666039
##########
backend/helpers/srvhelper/connection_service_helper.go:
##########
@@ -0,0 +1,82 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/core/plugin"
+)
+
+// ConnectionSrvHelper
+type ConnectionSrvHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelSrvHelper[C]
+ pluginName string
+}
+
+// NewConnectionSrvHelper creates a ConnectionDalHelper for connection management
+func NewConnectionSrvHelper[
+ C plugin.ToolLayerConnection,
+ S plugin.ToolLayerScope,
+ SC plugin.ToolLayerScopeConfig,
+](
+ basicRes context.BasicRes,
+ pluginName string,
+) *ConnectionSrvHelper[C, S, SC] {
+ return &ConnectionSrvHelper[C, S, SC]{
+ ModelSrvHelper: NewModelSrvHelper[C](basicRes),
+ pluginName: pluginName,
+ }
+}
+
+func (connSrv *ConnectionSrvHelper[C, S, SC]) DeleteConnection(connection *C) (refs *DsRefs, err errors.Error) {
+ err = connSrv.ModelSrvHelper.NoRunningPipeline(func(tx dal.Transaction) errors.Error {
+ // make sure no blueprint is using the connection
+ connectionId := (*connection).ConnectionId()
+ refs, err = toDsRefs(connSrv.getAllBlueprinsByConnection(connectionId))
+ if err != nil {
+ return err
+ }
+ scopeCount := errors.Must1(connSrv.db.Count(dal.From(new(S)), dal.Where("connection_id = ?", connectionId)))
Review Comment:
Yes, it should panic because it is **Unrecoverable**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] abeizn commented on pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "abeizn (via GitHub)" <gi...@apache.org>.
abeizn commented on PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#issuecomment-1736638087
By the way, have you tried the compatibility under the pg database?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] abeizn commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "abeizn (via GitHub)" <gi...@apache.org>.
abeizn commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338232056
##########
backend/test/helper/client.go:
##########
@@ -289,12 +289,13 @@ func (d *DevlakeClient) initPlugins(cfg *LocalClientConfig) {
for _, p := range cfg.Plugins {
require.NoError(d.testCtx, plugin.RegisterPlugin(p.Name(), p))
}
- for _, p := range plugin.AllPlugins() {
- if pi, ok := p.(plugin.PluginInit); ok {
- err := pi.Init(d.basicRes)
- require.NoError(d.testCtx, err)
- }
- }
+ // The following code doesn't seem correct, plugin should be initialized by the server, not the client
Review Comment:
Delete it, or keep it
##########
backend/plugins/github/api/scope_api.go:
##########
@@ -106,5 +97,6 @@ func GetScope(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors
// @Failure 500 {object} shared.ApiBody "Internal Error"
// @Router /plugins/github/connections/{connectionId}/scopes/{scopeId} [DELETE]
func DeleteScope(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
- return scopeHelper.Delete(input)
+ println("hello delete scope")
Review Comment:
please detele it
##########
backend/plugins/github/api/scope_config_api.go:
##########
@@ -49,8 +51,10 @@ func CreateScopeConfig(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutpu
// @Failure 400 {object} shared.ApiBody "Bad Request"
// @Failure 500 {object} shared.ApiBody "Internal Error"
// @Router /plugins/github/connections/{connectionId}/scope-configs/{id} [PATCH]
-func UpdateScopeConfig(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
- return scHelper.Update(input)
+func PatchScopeConfig(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ println("api hello")
+ fmt.Printf("api patch scope confg %v\n", input.Params)
Review Comment:
same above
##########
backend/plugins/github/api/scope_config_api.go:
##########
@@ -49,8 +51,10 @@ func CreateScopeConfig(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutpu
// @Failure 400 {object} shared.ApiBody "Bad Request"
// @Failure 500 {object} shared.ApiBody "Internal Error"
// @Router /plugins/github/connections/{connectionId}/scope-configs/{id} [PATCH]
-func UpdateScopeConfig(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
- return scHelper.Update(input)
+func PatchScopeConfig(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ println("api hello")
Review Comment:
please delete it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] d4x1 commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "d4x1 (via GitHub)" <gi...@apache.org>.
d4x1 commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338677329
##########
backend/helpers/pluginhelper/api/model_api_helper.go:
##########
@@ -0,0 +1,160 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "fmt"
+ "net/http"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/helpers/utils"
+)
+
+type ModelApiHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ dalHelper *srvhelper.ModelSrvHelper[M]
+ log log.Logger
+ modelName string
+ pkPathVarNames []string
+}
+
+func NewModelApiHelper[M dal.Tabler](
+ basicRes context.BasicRes,
+ dalHelper *srvhelper.ModelSrvHelper[M],
+ pkPathVarNames []string, // path variable names of primary key
+) *ModelApiHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ return &ModelApiHelper[M]{
+ basicRes: basicRes,
+ dalHelper: dalHelper,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ modelName: modelName,
+ pkPathVarNames: pkPathVarNames,
+ }
+}
+
+func (self *ModelApiHelper[M]) Post(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ model := new(M)
+ err := utils.DecodeMapStruct(input.Body, model, false)
+ if err != nil {
+ return nil, err
+ }
+ err = self.dalHelper.Create(model)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Status: http.StatusCreated,
+ Body: model,
+ }, nil
+}
+
+func (self *ModelApiHelper[M]) FindByPk(input *plugin.ApiResourceInput) (*M, errors.Error) {
+ pkv := make([]interface{}, len(self.pkPathVarNames))
+ for i, pkn := range self.pkPathVarNames {
+ pkv[i] = input.Params[pkn]
+ }
+ return self.dalHelper.FindByPk(pkv...)
+}
+
+func (self *ModelApiHelper[M]) GetDetail(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ model, err := self.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: model,
+ }, nil
+}
+
+func (self *ModelApiHelper[M]) Patch(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ model, err := self.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ err = utils.DecodeMapStruct(input.Body, model, true)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, fmt.Sprintf("faled to patch %s", self.modelName))
+ }
+ err = self.dalHelper.Update(model)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: model,
+ }, nil
+}
+
+func (self *ModelApiHelper[M]) Delete(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ model, err := self.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ err = self.dalHelper.DeleteModel(model)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: model,
+ }, nil
+}
+
+func (self *ModelApiHelper[M]) GetAll(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ all, err := self.dalHelper.GetAll()
+ return &plugin.ApiResourceOutput{
+ Body: all,
+ }, err
+}
+
+func (self *ModelApiHelper[M]) PutMultiple(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ var req struct {
+ Data []M `json:"data"`
+ }
+ err := utils.DecodeMapStruct(input.Body, &req, false)
+ if err != nil {
+ return nil, err
+ }
+ for i, item := range req.Data {
+ err := self.dalHelper.CreateOrUpdate(&item)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, fmt.Sprintf("failed to save item %d", i))
+ }
+ }
+ return &plugin.ApiResourceOutput{
+ Body: req.Data,
+ }, nil
+}
+
+func ParsePagination[P any](input *plugin.ApiResourceInput, query ...dal.Clause) (*P, errors.Error) {
Review Comment:
Maybe the first choice is best and simple.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338294533
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,215 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
+}
+
+type ModelSrvHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ log log.Logger
+ db dal.Dal
+ validator *validator.Validate
+ modelName string
+ pk []dal.ColumnMeta
+ pkWhere string
+ pkCount int
+ searchColumns []string
+}
+
+func NewModelSrvHelper[M dal.Tabler](basicRes context.BasicRes) *ModelSrvHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ db := basicRes.GetDal()
+ if db == nil {
+ db = basicRes.GetDal()
+ }
+ pk := errors.Must1(dal.GetPrimarykeyColumns(db, *m))
+ pkWhere := ""
+ for _, col := range pk {
+ if pkWhere != "" {
+ pkWhere += " AND "
+ }
+ pkWhere += fmt.Sprintf("%s = ? ", col.Name())
+ }
+ return &ModelSrvHelper[M]{
+ basicRes: basicRes,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ db: db,
+ validator: validator.New(),
+ modelName: modelName,
+ pk: pk,
+ pkWhere: pkWhere,
+ pkCount: len(pk),
+ }
+}
+
+func (srv *ModelSrvHelper[M]) NewTx(tx dal.Transaction) *ModelSrvHelper[M] {
+ helper := new(ModelSrvHelper[M])
+ *helper = *srv
+ helper.db = tx
+ return helper
+}
+
+func (srv *ModelSrvHelper[M]) ValidateModel(model *M) errors.Error {
+ // the model can validate itself
+ if customValidator, ok := (interface{}(model)).(CustomValidator); ok {
+ return customValidator.CustomValidate(model, srv.validator)
+ }
+ // basic validator
+ if e := srv.validator.Struct(model); e != nil {
+ return errors.BadInput.Wrap(e, "validation faild")
+ }
+ return nil
+}
+
+// Create validates given model and insert it into database if validation passed
+func (srv *ModelSrvHelper[M]) Create(model *M) errors.Error {
+ println("create model")
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ err = srv.db.Create(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return err
+}
+
+// Update validates given model and update it into database if validation passed
+func (srv *ModelSrvHelper[M]) Update(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
Review Comment:
Yes, absolutely. One may update the model's unique index (e.g. name) causing conflict with other rows. it is not unusal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338292974
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,215 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
Review Comment:
2. No, it is not. For example, the `CustomValidate` may be implemented in a **nested struct** of the `model` and sometimes we need to check the whole `model` instead of the **nested struct** only. https://github.com/apache/incubator-devlake/blob/90cb26bd62d226f5c4ed54c17689dab9bcf30de0/backend/helpers/pluginhelper/api/connection_auths.go#L147
3. Passing the `*validate` can be beneficial in terms of performance because the cached metadata in Validator can be reused.
4. the function is meant for plugins to take over the Validation when needed, if plugins implement this interface, the helper would skip its own implementation and let the plugin do what it wants. I think `CustomValidate` is more specify and reflects its purpose better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] abeizn commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "abeizn (via GitHub)" <gi...@apache.org>.
abeizn commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1339493195
##########
backend/plugins/github/api/init.go:
##########
@@ -18,69 +18,44 @@ limitations under the License.
package api
import (
- "strconv"
-
"github.com/apache/incubator-devlake/core/plugin"
"github.com/apache/incubator-devlake/core/context"
- "github.com/apache/incubator-devlake/core/dal"
- "github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
"github.com/apache/incubator-devlake/plugins/github/models"
"github.com/go-playground/validator/v10"
)
var vld *validator.Validate
+
+// var connSrv *srvhelper.ConnectionSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var connApi *api.DsConnectionApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scopeSrv *srvhelper.ScopeSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scopeApi *api.DsScopeApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scSrv *srvhelper.ScopeConfigSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scApi *api.DsScopeConfigApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
var connectionHelper *api.ConnectionApiHelper
-var scopeHelper *api.ScopeApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
var basicRes context.BasicRes
-var scHelper *api.ScopeConfigHelper[models.GithubScopeConfig]
var remoteHelper *api.RemoteApiHelper[models.GithubConnection, models.GithubRepo, repo, plugin.ApiGroup]
func Init(br context.BasicRes, p plugin.PluginMeta) {
-
basicRes = br
+ _, connApi, scopeSrv, scopeApi, scSrv, scApi = api.NewDataSourceHelpers[
+ models.GithubConnection,
+ models.GithubRepo, models.GithubScopeConfig,
+ ](
+ br,
+ "github",
+ []string{"full_name"},
+ )
+ // TODO: refactor remoteHelper
vld = validator.New()
connectionHelper = api.NewConnectionHelper(
basicRes,
vld,
p.Name(),
)
- params := &api.ReflectionParameters{
- ScopeIdFieldName: "GithubId",
- ScopeIdColumnName: "github_id",
- RawScopeParamName: "Name",
- SearchScopeParamName: "name",
Review Comment:
e..., you are right.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] abeizn commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "abeizn (via GitHub)" <gi...@apache.org>.
abeizn commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338507265
##########
backend/plugins/github/api/init.go:
##########
@@ -18,69 +18,44 @@ limitations under the License.
package api
import (
- "strconv"
-
"github.com/apache/incubator-devlake/core/plugin"
"github.com/apache/incubator-devlake/core/context"
- "github.com/apache/incubator-devlake/core/dal"
- "github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
"github.com/apache/incubator-devlake/plugins/github/models"
"github.com/go-playground/validator/v10"
)
var vld *validator.Validate
+
+// var connSrv *srvhelper.ConnectionSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var connApi *api.DsConnectionApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scopeSrv *srvhelper.ScopeSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scopeApi *api.DsScopeApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scSrv *srvhelper.ScopeConfigSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scApi *api.DsScopeConfigApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
var connectionHelper *api.ConnectionApiHelper
-var scopeHelper *api.ScopeApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
var basicRes context.BasicRes
-var scHelper *api.ScopeConfigHelper[models.GithubScopeConfig]
var remoteHelper *api.RemoteApiHelper[models.GithubConnection, models.GithubRepo, repo, plugin.ApiGroup]
func Init(br context.BasicRes, p plugin.PluginMeta) {
-
basicRes = br
+ _, connApi, scopeSrv, scopeApi, scSrv, scApi = api.NewDataSourceHelpers[
+ models.GithubConnection,
+ models.GithubRepo, models.GithubScopeConfig,
+ ](
+ br,
+ "github",
+ []string{"full_name"},
+ )
+ // TODO: refactor remoteHelper
vld = validator.New()
connectionHelper = api.NewConnectionHelper(
basicRes,
vld,
p.Name(),
)
- params := &api.ReflectionParameters{
- ScopeIdFieldName: "GithubId",
- ScopeIdColumnName: "github_id",
- RawScopeParamName: "Name",
- SearchScopeParamName: "name",
Review Comment:
If these are removed, how to implement the data scope search function. I don't see any assignment operations to these anywhere else?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] d4x1 commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "d4x1 (via GitHub)" <gi...@apache.org>.
d4x1 commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338630384
##########
backend/helpers/pluginhelper/api/ds_scope_api_helper.go:
##########
@@ -0,0 +1,99 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+type DsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelApiHelper[S]
+ *srvhelper.ScopeSrvHelper[C, S, SC]
+}
+
+func NewDsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig](
+ basicRes context.BasicRes,
+ srvHelper *srvhelper.ScopeSrvHelper[C, S, SC],
+) *DsScopeApiHelper[C, S, SC] {
+ return &DsScopeApiHelper[C, S, SC]{
+ ModelApiHelper: NewModelApiHelper[S](basicRes, srvHelper.ModelSrvHelper, []string{"connectionId", "scopeId"}),
+ ScopeSrvHelper: srvHelper,
+ }
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) GetPage(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ pagination, err := ParsePagination[srvhelper.ScopePagination](input)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, "failed to decode pathvars into pagination")
+ }
+ scopes, count, err := connApi.ScopeSrvHelper.GetScopesPage(pagination)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: map[string]interface{}{
+ "count": count,
+ "scopes": scopes,
+ },
+ }, nil
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) PutMultiple(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ // fix data[].connectionId
+ connectionId, err := extractConnectionId(input)
+ if err != nil {
+ return nil, err
+ }
+ data, ok := input.Body["data"].([]interface{})
+ if !ok {
+ return nil, errors.BadInput.New("invalid data")
+ }
+ for _, row := range data {
+ dict, ok := row.(map[string]interface{})
+ if !ok {
+ return nil, errors.BadInput.New("invalid data row")
+ }
+ dict["connectionId"] = connectionId
+ }
+ return connApi.ModelApiHelper.PutMultiple(input)
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) Delete(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ var scope *S
+ scope, err := connApi.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ // time.Sleep(1 * time.Minute) # uncomment this line if you were to verify pipelines get blocked while deleting data
+ // check referencing blueprints
+ refs, err := connApi.ScopeSrvHelper.DeleteScope(scope, input.Query.Get("delete_data_only") == "true")
Review Comment:
> I don't understand, what is a magic variable.
magic variable is not a official expression, in fact I mean `"delete_data_only"` in ths line.
Your reply make me realise that it is not a variable, and after searching the project, I find `delete_data_only ` is a common parameter in plugin's API.
So just ignore this comment and forget it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338071876
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,212 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
+}
+
+type ModelSrvHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ log log.Logger
+ db dal.Dal
+ validator *validator.Validate
+ modelName string
+ pk []dal.ColumnMeta
+ pkWhere string
+ pkCount int
+ searchColumns []string
+}
+
+func NewModelSrvHelper[M dal.Tabler](basicRes context.BasicRes) *ModelSrvHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ db := basicRes.GetDal()
+ if db == nil {
+ db = basicRes.GetDal()
+ }
+ pk := errors.Must1(dal.GetPrimarykeyColumns(db, *m))
+ pkWhere := ""
+ for _, col := range pk {
+ if pkWhere != "" {
+ pkWhere += " AND "
+ }
+ pkWhere += fmt.Sprintf("%s = ? ", col.Name())
+ }
+ return &ModelSrvHelper[M]{
+ basicRes: basicRes,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ db: db,
+ validator: validator.New(),
+ modelName: modelName,
+ pk: pk,
+ pkWhere: pkWhere,
+ pkCount: len(pk),
+ }
+}
+
+func (srv *ModelSrvHelper[M]) NewTx(tx dal.Transaction) *ModelSrvHelper[M] {
+ helper := new(ModelSrvHelper[M])
+ *helper = *srv
+ helper.db = tx
+ return helper
+}
+
+func (srv *ModelSrvHelper[M]) ValidateModel(model *M) errors.Error {
+ // the model can validate itself
+ if customValidator, ok := (interface{}(model)).(CustomValidator); ok {
+ return customValidator.CustomValidate(model, srv.validator)
+ }
+ // basic validator
+ if e := srv.validator.Struct(model); e != nil {
+ return errors.BadInput.Wrap(e, "validation faild")
+ }
+ return nil
+}
+
+// Create validates given model and insert it into database if validation passed
+func (srv *ModelSrvHelper[M]) Create(model *M) errors.Error {
+ println("create model")
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ err = srv.db.Create(model)
+ if err != nil && srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+}
+
+// Update validates given model and update it into database if validation passed
+func (srv *ModelSrvHelper[M]) Update(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ if err != nil && srv.db.IsDuplicationError(err) {
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#issuecomment-1736742573
@abeizn what are referring to? the `e2e-postgres` passed, does it count?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] d4x1 commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "d4x1 (via GitHub)" <gi...@apache.org>.
d4x1 commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338630384
##########
backend/helpers/pluginhelper/api/ds_scope_api_helper.go:
##########
@@ -0,0 +1,99 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+type DsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelApiHelper[S]
+ *srvhelper.ScopeSrvHelper[C, S, SC]
+}
+
+func NewDsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig](
+ basicRes context.BasicRes,
+ srvHelper *srvhelper.ScopeSrvHelper[C, S, SC],
+) *DsScopeApiHelper[C, S, SC] {
+ return &DsScopeApiHelper[C, S, SC]{
+ ModelApiHelper: NewModelApiHelper[S](basicRes, srvHelper.ModelSrvHelper, []string{"connectionId", "scopeId"}),
+ ScopeSrvHelper: srvHelper,
+ }
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) GetPage(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ pagination, err := ParsePagination[srvhelper.ScopePagination](input)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, "failed to decode pathvars into pagination")
+ }
+ scopes, count, err := connApi.ScopeSrvHelper.GetScopesPage(pagination)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: map[string]interface{}{
+ "count": count,
+ "scopes": scopes,
+ },
+ }, nil
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) PutMultiple(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ // fix data[].connectionId
+ connectionId, err := extractConnectionId(input)
+ if err != nil {
+ return nil, err
+ }
+ data, ok := input.Body["data"].([]interface{})
+ if !ok {
+ return nil, errors.BadInput.New("invalid data")
+ }
+ for _, row := range data {
+ dict, ok := row.(map[string]interface{})
+ if !ok {
+ return nil, errors.BadInput.New("invalid data row")
+ }
+ dict["connectionId"] = connectionId
+ }
+ return connApi.ModelApiHelper.PutMultiple(input)
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) Delete(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ var scope *S
+ scope, err := connApi.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ // time.Sleep(1 * time.Minute) # uncomment this line if you were to verify pipelines get blocked while deleting data
+ // check referencing blueprints
+ refs, err := connApi.ScopeSrvHelper.DeleteScope(scope, input.Query.Get("delete_data_only") == "true")
Review Comment:
> I don't understand, what is a magic variable.
magic variable is not an official expression, in fact I mean `"delete_data_only"` in ths line.
Your reply make me realise that it is not a variable, and after searching the project, I find `delete_data_only ` is a common parameter in plugin's API.
So just ignore this comment and forget it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338562683
##########
backend/helpers/pluginhelper/api/ds_scope_api_helper.go:
##########
@@ -0,0 +1,99 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+type DsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelApiHelper[S]
+ *srvhelper.ScopeSrvHelper[C, S, SC]
+}
+
+func NewDsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig](
+ basicRes context.BasicRes,
+ srvHelper *srvhelper.ScopeSrvHelper[C, S, SC],
+) *DsScopeApiHelper[C, S, SC] {
+ return &DsScopeApiHelper[C, S, SC]{
+ ModelApiHelper: NewModelApiHelper[S](basicRes, srvHelper.ModelSrvHelper, []string{"connectionId", "scopeId"}),
+ ScopeSrvHelper: srvHelper,
+ }
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) GetPage(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ pagination, err := ParsePagination[srvhelper.ScopePagination](input)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, "failed to decode pathvars into pagination")
+ }
+ scopes, count, err := connApi.ScopeSrvHelper.GetScopesPage(pagination)
+ if err != nil {
Review Comment:
What additional information do you think is necessary?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] abeizn commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "abeizn (via GitHub)" <gi...@apache.org>.
abeizn commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1339428062
##########
backend/plugins/github/api/init.go:
##########
@@ -18,69 +18,44 @@ limitations under the License.
package api
import (
- "strconv"
-
"github.com/apache/incubator-devlake/core/plugin"
"github.com/apache/incubator-devlake/core/context"
- "github.com/apache/incubator-devlake/core/dal"
- "github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
"github.com/apache/incubator-devlake/plugins/github/models"
"github.com/go-playground/validator/v10"
)
var vld *validator.Validate
+
+// var connSrv *srvhelper.ConnectionSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var connApi *api.DsConnectionApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scopeSrv *srvhelper.ScopeSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scopeApi *api.DsScopeApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scSrv *srvhelper.ScopeConfigSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scApi *api.DsScopeConfigApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
var connectionHelper *api.ConnectionApiHelper
-var scopeHelper *api.ScopeApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
var basicRes context.BasicRes
-var scHelper *api.ScopeConfigHelper[models.GithubScopeConfig]
var remoteHelper *api.RemoteApiHelper[models.GithubConnection, models.GithubRepo, repo, plugin.ApiGroup]
func Init(br context.BasicRes, p plugin.PluginMeta) {
-
basicRes = br
+ _, connApi, scopeSrv, scopeApi, scSrv, scApi = api.NewDataSourceHelpers[
+ models.GithubConnection,
+ models.GithubRepo, models.GithubScopeConfig,
+ ](
+ br,
+ "github",
+ []string{"full_name"},
+ )
+ // TODO: refactor remoteHelper
vld = validator.New()
connectionHelper = api.NewConnectionHelper(
basicRes,
vld,
p.Name(),
)
- params := &api.ReflectionParameters{
- ScopeIdFieldName: "GithubId",
- ScopeIdColumnName: "github_id",
- RawScopeParamName: "Name",
- SearchScopeParamName: "name",
Review Comment:
Strangely, shouldn't this be ‘name’, not ‘full_name’?
##########
backend/plugins/github/api/init.go:
##########
@@ -18,69 +18,44 @@ limitations under the License.
package api
import (
- "strconv"
-
"github.com/apache/incubator-devlake/core/plugin"
"github.com/apache/incubator-devlake/core/context"
- "github.com/apache/incubator-devlake/core/dal"
- "github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
"github.com/apache/incubator-devlake/plugins/github/models"
"github.com/go-playground/validator/v10"
)
var vld *validator.Validate
+
+// var connSrv *srvhelper.ConnectionSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var connApi *api.DsConnectionApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scopeSrv *srvhelper.ScopeSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scopeApi *api.DsScopeApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scSrv *srvhelper.ScopeConfigSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scApi *api.DsScopeConfigApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
var connectionHelper *api.ConnectionApiHelper
-var scopeHelper *api.ScopeApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
var basicRes context.BasicRes
-var scHelper *api.ScopeConfigHelper[models.GithubScopeConfig]
var remoteHelper *api.RemoteApiHelper[models.GithubConnection, models.GithubRepo, repo, plugin.ApiGroup]
func Init(br context.BasicRes, p plugin.PluginMeta) {
-
basicRes = br
+ _, connApi, scopeSrv, scopeApi, scSrv, scApi = api.NewDataSourceHelpers[
+ models.GithubConnection,
+ models.GithubRepo, models.GithubScopeConfig,
+ ](
+ br,
+ "github",
+ []string{"full_name"},
+ )
+ // TODO: refactor remoteHelper
vld = validator.New()
connectionHelper = api.NewConnectionHelper(
basicRes,
vld,
p.Name(),
)
- params := &api.ReflectionParameters{
- ScopeIdFieldName: "GithubId",
- ScopeIdColumnName: "github_id",
- RawScopeParamName: "Name",
- SearchScopeParamName: "name",
Review Comment:
Strangely, shouldn't this be ‘name’, not ‘full_name’?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1339475563
##########
backend/plugins/github/api/init.go:
##########
@@ -18,69 +18,44 @@ limitations under the License.
package api
import (
- "strconv"
-
"github.com/apache/incubator-devlake/core/plugin"
"github.com/apache/incubator-devlake/core/context"
- "github.com/apache/incubator-devlake/core/dal"
- "github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
"github.com/apache/incubator-devlake/plugins/github/models"
"github.com/go-playground/validator/v10"
)
var vld *validator.Validate
+
+// var connSrv *srvhelper.ConnectionSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var connApi *api.DsConnectionApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scopeSrv *srvhelper.ScopeSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scopeApi *api.DsScopeApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scSrv *srvhelper.ScopeConfigSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scApi *api.DsScopeConfigApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
var connectionHelper *api.ConnectionApiHelper
-var scopeHelper *api.ScopeApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
var basicRes context.BasicRes
-var scHelper *api.ScopeConfigHelper[models.GithubScopeConfig]
var remoteHelper *api.RemoteApiHelper[models.GithubConnection, models.GithubRepo, repo, plugin.ApiGroup]
func Init(br context.BasicRes, p plugin.PluginMeta) {
-
basicRes = br
+ _, connApi, scopeSrv, scopeApi, scSrv, scApi = api.NewDataSourceHelpers[
+ models.GithubConnection,
+ models.GithubRepo, models.GithubScopeConfig,
+ ](
+ br,
+ "github",
+ []string{"full_name"},
+ )
+ // TODO: refactor remoteHelper
vld = validator.New()
connectionHelper = api.NewConnectionHelper(
basicRes,
vld,
p.Name(),
)
- params := &api.ReflectionParameters{
- ScopeIdFieldName: "GithubId",
- ScopeIdColumnName: "github_id",
- RawScopeParamName: "Name",
- SearchScopeParamName: "name",
Review Comment:
Why? `full_name` includes `name` so it is so much better the `name`, isn't it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338294533
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,215 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
+}
+
+type ModelSrvHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ log log.Logger
+ db dal.Dal
+ validator *validator.Validate
+ modelName string
+ pk []dal.ColumnMeta
+ pkWhere string
+ pkCount int
+ searchColumns []string
+}
+
+func NewModelSrvHelper[M dal.Tabler](basicRes context.BasicRes) *ModelSrvHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ db := basicRes.GetDal()
+ if db == nil {
+ db = basicRes.GetDal()
+ }
+ pk := errors.Must1(dal.GetPrimarykeyColumns(db, *m))
+ pkWhere := ""
+ for _, col := range pk {
+ if pkWhere != "" {
+ pkWhere += " AND "
+ }
+ pkWhere += fmt.Sprintf("%s = ? ", col.Name())
+ }
+ return &ModelSrvHelper[M]{
+ basicRes: basicRes,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ db: db,
+ validator: validator.New(),
+ modelName: modelName,
+ pk: pk,
+ pkWhere: pkWhere,
+ pkCount: len(pk),
+ }
+}
+
+func (srv *ModelSrvHelper[M]) NewTx(tx dal.Transaction) *ModelSrvHelper[M] {
+ helper := new(ModelSrvHelper[M])
+ *helper = *srv
+ helper.db = tx
+ return helper
+}
+
+func (srv *ModelSrvHelper[M]) ValidateModel(model *M) errors.Error {
+ // the model can validate itself
+ if customValidator, ok := (interface{}(model)).(CustomValidator); ok {
+ return customValidator.CustomValidate(model, srv.validator)
+ }
+ // basic validator
+ if e := srv.validator.Struct(model); e != nil {
+ return errors.BadInput.Wrap(e, "validation faild")
+ }
+ return nil
+}
+
+// Create validates given model and insert it into database if validation passed
+func (srv *ModelSrvHelper[M]) Create(model *M) errors.Error {
+ println("create model")
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ err = srv.db.Create(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return err
+}
+
+// Update validates given model and update it into database if validation passed
+func (srv *ModelSrvHelper[M]) Update(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
Review Comment:
Yes, absolutely. One may update the model's unique index (e.g. name) causing conflict with other rows. it is not unusual.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338564174
##########
backend/helpers/pluginhelper/api/ds_scope_api_helper.go:
##########
@@ -0,0 +1,99 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+type DsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelApiHelper[S]
+ *srvhelper.ScopeSrvHelper[C, S, SC]
+}
+
+func NewDsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig](
+ basicRes context.BasicRes,
+ srvHelper *srvhelper.ScopeSrvHelper[C, S, SC],
+) *DsScopeApiHelper[C, S, SC] {
+ return &DsScopeApiHelper[C, S, SC]{
+ ModelApiHelper: NewModelApiHelper[S](basicRes, srvHelper.ModelSrvHelper, []string{"connectionId", "scopeId"}),
+ ScopeSrvHelper: srvHelper,
+ }
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) GetPage(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ pagination, err := ParsePagination[srvhelper.ScopePagination](input)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, "failed to decode pathvars into pagination")
+ }
+ scopes, count, err := connApi.ScopeSrvHelper.GetScopesPage(pagination)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: map[string]interface{}{
+ "count": count,
+ "scopes": scopes,
+ },
+ }, nil
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) PutMultiple(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ // fix data[].connectionId
Review Comment:
Not, it is not extracting, it is fixing the request body by putting the `connectionId` from the request URL path into the body.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338555888
##########
backend/helpers/pluginhelper/api/ds_helper.go:
##########
@@ -0,0 +1,49 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+)
+
+func NewDataSourceHelpers[
Review Comment:
It is returning 6 helpers, 3 `serviceHelper` and 3 `apiHelper`, the former ones can be used in anywhere in the plugin like `tasks` package or `api` package while the latter ones are only for `api` package. It makes no sense for `tasks` package to use `apiHelper`, that is why they are nested inside a struct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338577434
##########
backend/helpers/pluginhelper/api/ds_scope_api_helper.go:
##########
@@ -0,0 +1,99 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+type DsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelApiHelper[S]
+ *srvhelper.ScopeSrvHelper[C, S, SC]
+}
+
+func NewDsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig](
+ basicRes context.BasicRes,
+ srvHelper *srvhelper.ScopeSrvHelper[C, S, SC],
+) *DsScopeApiHelper[C, S, SC] {
+ return &DsScopeApiHelper[C, S, SC]{
+ ModelApiHelper: NewModelApiHelper[S](basicRes, srvHelper.ModelSrvHelper, []string{"connectionId", "scopeId"}),
+ ScopeSrvHelper: srvHelper,
+ }
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) GetPage(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ pagination, err := ParsePagination[srvhelper.ScopePagination](input)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, "failed to decode pathvars into pagination")
+ }
+ scopes, count, err := connApi.ScopeSrvHelper.GetScopesPage(pagination)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: map[string]interface{}{
+ "count": count,
+ "scopes": scopes,
+ },
+ }, nil
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) PutMultiple(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ // fix data[].connectionId
+ connectionId, err := extractConnectionId(input)
+ if err != nil {
+ return nil, err
+ }
+ data, ok := input.Body["data"].([]interface{})
+ if !ok {
+ return nil, errors.BadInput.New("invalid data")
+ }
+ for _, row := range data {
+ dict, ok := row.(map[string]interface{})
+ if !ok {
+ return nil, errors.BadInput.New("invalid data row")
+ }
+ dict["connectionId"] = connectionId
+ }
+ return connApi.ModelApiHelper.PutMultiple(input)
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) Delete(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ var scope *S
+ scope, err := connApi.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ // time.Sleep(1 * time.Minute) # uncomment this line if you were to verify pipelines get blocked while deleting data
+ // check referencing blueprints
+ refs, err := connApi.ScopeSrvHelper.DeleteScope(scope, input.Query.Get("delete_data_only") == "true")
Review Comment:
I don't understand, what is a magic variable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] d4x1 commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "d4x1 (via GitHub)" <gi...@apache.org>.
d4x1 commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338675651
##########
backend/helpers/pluginhelper/api/ds_connection_api_helper.go:
##########
@@ -0,0 +1,76 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "strconv"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+// DsConnectionApiHelper
+type DsConnectionApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
Review Comment:
I am not good at golang's generics. So my code will be like:
```golang
type DsConnectionApiHelper[Connection plugin.ToolLayerConnection, Scope plugin.ToolLayerScope, ScopeConfig plugin.ToolLayerScopeConfig] struct {
```
But it seems too long and introduces a some new entities.
Maybe the best way is to remain its status.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] d4x1 commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "d4x1 (via GitHub)" <gi...@apache.org>.
d4x1 commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1339574466
##########
backend/helpers/pluginhelper/api/ds_scope_api_helper.go:
##########
@@ -0,0 +1,99 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+type DsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelApiHelper[S]
+ *srvhelper.ScopeSrvHelper[C, S, SC]
+}
+
+func NewDsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig](
+ basicRes context.BasicRes,
+ srvHelper *srvhelper.ScopeSrvHelper[C, S, SC],
+) *DsScopeApiHelper[C, S, SC] {
+ return &DsScopeApiHelper[C, S, SC]{
+ ModelApiHelper: NewModelApiHelper[S](basicRes, srvHelper.ModelSrvHelper, []string{"connectionId", "scopeId"}),
+ ScopeSrvHelper: srvHelper,
+ }
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) GetPage(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ pagination, err := ParsePagination[srvhelper.ScopePagination](input)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, "failed to decode pathvars into pagination")
+ }
+ scopes, count, err := connApi.ScopeSrvHelper.GetScopesPage(pagination)
+ if err != nil {
Review Comment:
You are right. Generally speaking, when logging something, we should log not only the error message but also the context. But as you said, we can get enough context from other place, so that's enough.
Thanks for your reply.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338274441
##########
backend/helpers/pluginhelper/api/connection_auths.go:
##########
@@ -144,6 +144,11 @@ func (ma *MultiAuth) SetupAuthenticationForConnection(connection plugin.ApiConne
return apiAuthenticator.SetupAuthentication(req)
}
+func (ma *MultiAuth) CustomValidate(connection interface{}, v *validator.Validate) errors.Error {
+ return ma.ValidateConnection(connection, v)
+}
+
+// TODO: deprecated, rename to CustomValidate instead
Review Comment:
I would love that if it can work. But, unfortunately, No, it will cause the linting to fail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] d4x1 commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "d4x1 (via GitHub)" <gi...@apache.org>.
d4x1 commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338675651
##########
backend/helpers/pluginhelper/api/ds_connection_api_helper.go:
##########
@@ -0,0 +1,76 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "strconv"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+// DsConnectionApiHelper
+type DsConnectionApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
Review Comment:
I am not good at golang's generics. So my code will be like:
```golang
type DsConnectionApiHelper[Connection plugin.ToolLayerConnection, Scope plugin.ToolLayerScope, ScopeConfig plugin.ToolLayerScopeConfig] struct {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338296521
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,215 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
+}
+
+type ModelSrvHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ log log.Logger
+ db dal.Dal
+ validator *validator.Validate
+ modelName string
+ pk []dal.ColumnMeta
+ pkWhere string
+ pkCount int
+ searchColumns []string
+}
+
+func NewModelSrvHelper[M dal.Tabler](basicRes context.BasicRes) *ModelSrvHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ db := basicRes.GetDal()
+ if db == nil {
+ db = basicRes.GetDal()
+ }
+ pk := errors.Must1(dal.GetPrimarykeyColumns(db, *m))
+ pkWhere := ""
+ for _, col := range pk {
+ if pkWhere != "" {
+ pkWhere += " AND "
+ }
+ pkWhere += fmt.Sprintf("%s = ? ", col.Name())
+ }
+ return &ModelSrvHelper[M]{
+ basicRes: basicRes,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ db: db,
+ validator: validator.New(),
+ modelName: modelName,
+ pk: pk,
+ pkWhere: pkWhere,
+ pkCount: len(pk),
+ }
+}
+
+func (srv *ModelSrvHelper[M]) NewTx(tx dal.Transaction) *ModelSrvHelper[M] {
+ helper := new(ModelSrvHelper[M])
+ *helper = *srv
+ helper.db = tx
+ return helper
+}
+
+func (srv *ModelSrvHelper[M]) ValidateModel(model *M) errors.Error {
+ // the model can validate itself
+ if customValidator, ok := (interface{}(model)).(CustomValidator); ok {
+ return customValidator.CustomValidate(model, srv.validator)
+ }
+ // basic validator
+ if e := srv.validator.Struct(model); e != nil {
+ return errors.BadInput.Wrap(e, "validation faild")
+ }
+ return nil
+}
+
+// Create validates given model and insert it into database if validation passed
+func (srv *ModelSrvHelper[M]) Create(model *M) errors.Error {
+ println("create model")
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ err = srv.db.Create(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return err
+}
+
+// Update validates given model and update it into database if validation passed
+func (srv *ModelSrvHelper[M]) Update(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return srv.db.Update(model)
+}
+
+// CreateOrUpdate validates given model and insert or update it into database if validation passed
+func (srv *ModelSrvHelper[M]) CreateOrUpdate(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ return srv.db.CreateOrUpdate(model)
+}
+
+// DeleteModel deletes given model from database
+func (srv *ModelSrvHelper[M]) DeleteModel(model *M) errors.Error {
+ return srv.db.Delete(model)
+}
+
+// FindByPk returns model with given primary key from database
+func (srv *ModelSrvHelper[M]) FindByPk(pk ...interface{}) (*M, errors.Error) {
+ if len(pk) != srv.pkCount {
+ return nil, errors.BadInput.New("invalid primary key")
+ }
+ model := new(M)
+ err := srv.db.First(model, dal.Where(srv.pkWhere, pk...))
+ if err != nil {
+ if srv.db.IsErrorNotFound(err) {
+ return nil, errors.NotFound.Wrap(err, fmt.Sprintf("%s not found", srv.modelName))
+ }
+ return nil, err
+ }
+ return model, nil
+}
+
+// GetAll returns all models from database
+func (srv *ModelSrvHelper[M]) GetAll() ([]*M, errors.Error) {
+ array := make([]*M, 0)
Review Comment:
Any proof? I thought the `golangci-lint` would complain if this is not a good practice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1339483290
##########
backend/helpers/pluginhelper/api/ds_scope_api_helper.go:
##########
@@ -0,0 +1,99 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+type DsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelApiHelper[S]
+ *srvhelper.ScopeSrvHelper[C, S, SC]
+}
+
+func NewDsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig](
+ basicRes context.BasicRes,
+ srvHelper *srvhelper.ScopeSrvHelper[C, S, SC],
+) *DsScopeApiHelper[C, S, SC] {
+ return &DsScopeApiHelper[C, S, SC]{
+ ModelApiHelper: NewModelApiHelper[S](basicRes, srvHelper.ModelSrvHelper, []string{"connectionId", "scopeId"}),
+ ScopeSrvHelper: srvHelper,
+ }
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) GetPage(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ pagination, err := ParsePagination[srvhelper.ScopePagination](input)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, "failed to decode pathvars into pagination")
+ }
+ scopes, count, err := connApi.ScopeSrvHelper.GetScopesPage(pagination)
+ if err != nil {
Review Comment:
I can't think of any case these information could possibly help when an error occurs:
1. `page size`: it is already in the request query string, either it is illegal which is handled by the `parsePagination`, or Out-Of-Range which should be handled in the `parsePagination` function. In any case, `page size` is a 400 User Input Error. outputting this information doesn't seem to be helpful.
2. `page No`, same as above
3. `types of generics`: it is too obvious by looking at the `URL`.
In general, I don't think outputting any of the above variables would help, would you like to show a concrete example? Thanks.
In the meantime, I would update the `parsePagination` to verify `page size` and `page no`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] abeizn merged pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "abeizn (via GitHub)" <gi...@apache.org>.
abeizn merged PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338664058
##########
backend/helpers/pluginhelper/api/model_api_helper.go:
##########
@@ -0,0 +1,160 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "fmt"
+ "net/http"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/helpers/utils"
+)
+
+type ModelApiHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ dalHelper *srvhelper.ModelSrvHelper[M]
+ log log.Logger
+ modelName string
+ pkPathVarNames []string
+}
+
+func NewModelApiHelper[M dal.Tabler](
+ basicRes context.BasicRes,
+ dalHelper *srvhelper.ModelSrvHelper[M],
+ pkPathVarNames []string, // path variable names of primary key
Review Comment:
1. No, it doesn't, it uses Name
2. No, `primary key` is always singular or it is not **Primary**. A primary key with multiple columns is called a Composite Primary Key (Singular)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] abeizn commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "abeizn (via GitHub)" <gi...@apache.org>.
abeizn commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338014432
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,212 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
+}
+
+type ModelSrvHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ log log.Logger
+ db dal.Dal
+ validator *validator.Validate
+ modelName string
+ pk []dal.ColumnMeta
+ pkWhere string
+ pkCount int
+ searchColumns []string
+}
+
+func NewModelSrvHelper[M dal.Tabler](basicRes context.BasicRes) *ModelSrvHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ db := basicRes.GetDal()
+ if db == nil {
+ db = basicRes.GetDal()
+ }
+ pk := errors.Must1(dal.GetPrimarykeyColumns(db, *m))
+ pkWhere := ""
+ for _, col := range pk {
+ if pkWhere != "" {
+ pkWhere += " AND "
+ }
+ pkWhere += fmt.Sprintf("%s = ? ", col.Name())
+ }
+ return &ModelSrvHelper[M]{
+ basicRes: basicRes,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ db: db,
+ validator: validator.New(),
+ modelName: modelName,
+ pk: pk,
+ pkWhere: pkWhere,
+ pkCount: len(pk),
+ }
+}
+
+func (srv *ModelSrvHelper[M]) NewTx(tx dal.Transaction) *ModelSrvHelper[M] {
+ helper := new(ModelSrvHelper[M])
+ *helper = *srv
+ helper.db = tx
+ return helper
+}
+
+func (srv *ModelSrvHelper[M]) ValidateModel(model *M) errors.Error {
+ // the model can validate itself
+ if customValidator, ok := (interface{}(model)).(CustomValidator); ok {
+ return customValidator.CustomValidate(model, srv.validator)
+ }
+ // basic validator
+ if e := srv.validator.Struct(model); e != nil {
+ return errors.BadInput.Wrap(e, "validation faild")
+ }
+ return nil
+}
+
+// Create validates given model and insert it into database if validation passed
+func (srv *ModelSrvHelper[M]) Create(model *M) errors.Error {
+ println("create model")
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ err = srv.db.Create(model)
+ if err != nil && srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+}
+
+// Update validates given model and update it into database if validation passed
+func (srv *ModelSrvHelper[M]) Update(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ if err != nil && srv.db.IsDuplicationError(err) {
Review Comment:
err != nil may be removed, otherwise the execution will never reach this point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] d4x1 commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "d4x1 (via GitHub)" <gi...@apache.org>.
d4x1 commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338172539
##########
backend/helpers/utils/mapstructure.go:
##########
@@ -38,6 +38,17 @@ func DecodeHook(f reflect.Type, t reflect.Type, data interface{}) (interface{},
if t == reflect.TypeOf(json.RawMessage{}) {
return json.Marshal(data)
}
+ // to support decodng url.Values (quer string) to non-array variables
Review Comment:
typo `decodng` `quer`.
##########
backend/helpers/pluginhelper/api/connection_auths.go:
##########
@@ -144,6 +144,11 @@ func (ma *MultiAuth) SetupAuthenticationForConnection(connection plugin.ApiConne
return apiAuthenticator.SetupAuthentication(req)
}
+func (ma *MultiAuth) CustomValidate(connection interface{}, v *validator.Validate) errors.Error {
+ return ma.ValidateConnection(connection, v)
+}
+
+// TODO: deprecated, rename to CustomValidate instead
Review Comment:
Comment with "Deprecated, xxx" will be parsed by golang tool chain. It's better for us to follow this.
##########
backend/helpers/pluginhelper/api/ds_connection_api_helper.go:
##########
@@ -0,0 +1,76 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "strconv"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+// DsConnectionApiHelper
+type DsConnectionApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
Review Comment:
C stands fro `Connection`, S stands for `Scope`, SC stands for ScopeConfig, the letter C in SC means `Config`.
It's confusing when I see it the first time.
##########
backend/helpers/pluginhelper/api/ds_connection_api_helper.go:
##########
@@ -0,0 +1,76 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "strconv"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+// DsConnectionApiHelper
+type DsConnectionApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelApiHelper[C]
+ *srvhelper.ConnectionSrvHelper[C, S, SC]
+}
+
+func NewDsConnectionApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig](
+ basicRes context.BasicRes,
+ connSrvHelper *srvhelper.ConnectionSrvHelper[C, S, SC],
+) *DsConnectionApiHelper[C, S, SC] {
+ return &DsConnectionApiHelper[C, S, SC]{
+ ModelApiHelper: NewModelApiHelper[C](basicRes, connSrvHelper.ModelSrvHelper, []string{"connectionId"}),
+ ConnectionSrvHelper: connSrvHelper,
+ }
+}
+
+func (connApi *DsConnectionApiHelper[C, S, SC]) Delete(input *plugin.ApiResourceInput) (out *plugin.ApiResourceOutput, err errors.Error) {
+ var conn *C
+ conn, err = connApi.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ refs, err := connApi.ConnectionSrvHelper.DeleteConnection(conn)
+ if err != nil {
+ return &plugin.ApiResourceOutput{Body: &shared.ApiBody{
+ Success: false,
+ Message: err.Error(),
+ Data: refs,
+ }, Status: err.GetType().GetHttpCode()}, nil
+ }
+ return &plugin.ApiResourceOutput{
+ Body: conn,
+ }, nil
+}
+
+// DsScopeApiHelper
Review Comment:
comment doesn't match the code.
##########
backend/helpers/pluginhelper/api/ds_connection_api_helper.go:
##########
@@ -0,0 +1,76 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "strconv"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+// DsConnectionApiHelper
Review Comment:
what does `Ds` mean?
##########
backend/helpers/pluginhelper/api/ds_helper.go:
##########
@@ -0,0 +1,49 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+)
+
+func NewDataSourceHelpers[
Review Comment:
So `NewDataSourceHelpers ` will return five helpers.
Does the caller need five helpers at the same time? If yes, why not wrap them into one struct; if no, why return five helpers?
##########
backend/helpers/pluginhelper/api/model_api_helper.go:
##########
@@ -0,0 +1,160 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "fmt"
+ "net/http"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/helpers/utils"
+)
+
+type ModelApiHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ dalHelper *srvhelper.ModelSrvHelper[M]
+ log log.Logger
+ modelName string
+ pkPathVarNames []string
+}
+
+func NewModelApiHelper[M dal.Tabler](
+ basicRes context.BasicRes,
+ dalHelper *srvhelper.ModelSrvHelper[M],
+ pkPathVarNames []string, // path variable names of primary key
+) *ModelApiHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ return &ModelApiHelper[M]{
+ basicRes: basicRes,
+ dalHelper: dalHelper,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ modelName: modelName,
+ pkPathVarNames: pkPathVarNames,
+ }
+}
+
+func (self *ModelApiHelper[M]) Post(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ model := new(M)
+ err := utils.DecodeMapStruct(input.Body, model, false)
+ if err != nil {
+ return nil, err
+ }
+ err = self.dalHelper.Create(model)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Status: http.StatusCreated,
+ Body: model,
+ }, nil
+}
+
+func (self *ModelApiHelper[M]) FindByPk(input *plugin.ApiResourceInput) (*M, errors.Error) {
+ pkv := make([]interface{}, len(self.pkPathVarNames))
+ for i, pkn := range self.pkPathVarNames {
+ pkv[i] = input.Params[pkn]
+ }
+ return self.dalHelper.FindByPk(pkv...)
+}
+
+func (self *ModelApiHelper[M]) GetDetail(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ model, err := self.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: model,
+ }, nil
+}
+
+func (self *ModelApiHelper[M]) Patch(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ model, err := self.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ err = utils.DecodeMapStruct(input.Body, model, true)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, fmt.Sprintf("faled to patch %s", self.modelName))
+ }
+ err = self.dalHelper.Update(model)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: model,
+ }, nil
+}
+
+func (self *ModelApiHelper[M]) Delete(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ model, err := self.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ err = self.dalHelper.DeleteModel(model)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: model,
+ }, nil
+}
+
+func (self *ModelApiHelper[M]) GetAll(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ all, err := self.dalHelper.GetAll()
+ return &plugin.ApiResourceOutput{
+ Body: all,
+ }, err
+}
+
+func (self *ModelApiHelper[M]) PutMultiple(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ var req struct {
+ Data []M `json:"data"`
+ }
+ err := utils.DecodeMapStruct(input.Body, &req, false)
+ if err != nil {
+ return nil, err
+ }
+ for i, item := range req.Data {
+ err := self.dalHelper.CreateOrUpdate(&item)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, fmt.Sprintf("failed to save item %d", i))
+ }
+ }
+ return &plugin.ApiResourceOutput{
+ Body: req.Data,
+ }, nil
+}
+
+func ParsePagination[P any](input *plugin.ApiResourceInput, query ...dal.Clause) (*P, errors.Error) {
Review Comment:
1. It should be unexported if it is in this package.
2.It's not a part of xxx_helper(beacuse helpers need to be used by `NewXXX`), I think it jus a uitl function.
##########
backend/helpers/pluginhelper/api/ds_scope_api_helper.go:
##########
@@ -0,0 +1,99 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+type DsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelApiHelper[S]
+ *srvhelper.ScopeSrvHelper[C, S, SC]
+}
+
+func NewDsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig](
+ basicRes context.BasicRes,
+ srvHelper *srvhelper.ScopeSrvHelper[C, S, SC],
+) *DsScopeApiHelper[C, S, SC] {
+ return &DsScopeApiHelper[C, S, SC]{
+ ModelApiHelper: NewModelApiHelper[S](basicRes, srvHelper.ModelSrvHelper, []string{"connectionId", "scopeId"}),
+ ScopeSrvHelper: srvHelper,
+ }
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) GetPage(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ pagination, err := ParsePagination[srvhelper.ScopePagination](input)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, "failed to decode pathvars into pagination")
+ }
+ scopes, count, err := connApi.ScopeSrvHelper.GetScopesPage(pagination)
+ if err != nil {
Review Comment:
if something goes wrong here, a log will be helpful. Please consider adding some logs when an error occured, especially in this situation, the error just be passed to the caller without any addtional infomation.
##########
backend/helpers/pluginhelper/api/ds_scope_api_helper.go:
##########
@@ -0,0 +1,99 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+type DsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelApiHelper[S]
+ *srvhelper.ScopeSrvHelper[C, S, SC]
+}
+
+func NewDsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig](
+ basicRes context.BasicRes,
+ srvHelper *srvhelper.ScopeSrvHelper[C, S, SC],
+) *DsScopeApiHelper[C, S, SC] {
+ return &DsScopeApiHelper[C, S, SC]{
+ ModelApiHelper: NewModelApiHelper[S](basicRes, srvHelper.ModelSrvHelper, []string{"connectionId", "scopeId"}),
+ ScopeSrvHelper: srvHelper,
+ }
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) GetPage(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ pagination, err := ParsePagination[srvhelper.ScopePagination](input)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, "failed to decode pathvars into pagination")
+ }
+ scopes, count, err := connApi.ScopeSrvHelper.GetScopesPage(pagination)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: map[string]interface{}{
+ "count": count,
+ "scopes": scopes,
+ },
+ }, nil
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) PutMultiple(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ // fix data[].connectionId
Review Comment:
Maybe this comment should be placed in function extractConnectionId.
##########
backend/helpers/srvhelper/dsrefs.go:
##########
@@ -0,0 +1,46 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/models"
+)
+
+type DsRefs struct {
+ Blueprints []string `json:"blueprints"`
+ Projects []string `json:"projects"`
+}
+
+func toDsRefs(blueprints []*models.Blueprint) (*DsRefs, errors.Error) {
+ if len(blueprints) > 0 {
+ blueprintNames := make([]string, 0, len(blueprints))
+ projectNames := make([]string, 0, len(blueprints))
+ for _, bp := range blueprints {
+ blueprintNames = append(blueprintNames, bp.Name)
+ if bp.ProjectName != "" {
+ projectNames = append(projectNames, bp.ProjectName)
+ }
+ }
+ return &DsRefs{
+ Blueprints: blueprintNames,
+ Projects: projectNames,
+ }, errors.Conflict.New("Cannot delete record because it is referenced by blueprints")
Review Comment:
1. From the error message, we cannot get infomation like: which record is referened by which blueprint.
2. This function seems to convert blueprints to something else, there is no logical about `delete record`.
Please consider to update the error message, or the funtion's name.
##########
backend/helpers/pluginhelper/api/ds_scope_api_helper.go:
##########
@@ -0,0 +1,99 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+type DsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelApiHelper[S]
+ *srvhelper.ScopeSrvHelper[C, S, SC]
+}
+
+func NewDsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig](
+ basicRes context.BasicRes,
+ srvHelper *srvhelper.ScopeSrvHelper[C, S, SC],
+) *DsScopeApiHelper[C, S, SC] {
+ return &DsScopeApiHelper[C, S, SC]{
+ ModelApiHelper: NewModelApiHelper[S](basicRes, srvHelper.ModelSrvHelper, []string{"connectionId", "scopeId"}),
+ ScopeSrvHelper: srvHelper,
+ }
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) GetPage(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ pagination, err := ParsePagination[srvhelper.ScopePagination](input)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, "failed to decode pathvars into pagination")
+ }
+ scopes, count, err := connApi.ScopeSrvHelper.GetScopesPage(pagination)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: map[string]interface{}{
+ "count": count,
+ "scopes": scopes,
+ },
+ }, nil
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) PutMultiple(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ // fix data[].connectionId
+ connectionId, err := extractConnectionId(input)
+ if err != nil {
+ return nil, err
+ }
+ data, ok := input.Body["data"].([]interface{})
+ if !ok {
+ return nil, errors.BadInput.New("invalid data")
+ }
+ for _, row := range data {
+ dict, ok := row.(map[string]interface{})
+ if !ok {
+ return nil, errors.BadInput.New("invalid data row")
+ }
+ dict["connectionId"] = connectionId
+ }
+ return connApi.ModelApiHelper.PutMultiple(input)
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) Delete(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ var scope *S
+ scope, err := connApi.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ // time.Sleep(1 * time.Minute) # uncomment this line if you were to verify pipelines get blocked while deleting data
+ // check referencing blueprints
+ refs, err := connApi.ScopeSrvHelper.DeleteScope(scope, input.Query.Get("delete_data_only") == "true")
Review Comment:
Is `delete_data_only ` a magic variable?
##########
backend/helpers/pluginhelper/api/model_api_helper.go:
##########
@@ -0,0 +1,160 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "fmt"
+ "net/http"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/helpers/utils"
+)
+
+type ModelApiHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ dalHelper *srvhelper.ModelSrvHelper[M]
+ log log.Logger
+ modelName string
+ pkPathVarNames []string
+}
+
+func NewModelApiHelper[M dal.Tabler](
+ basicRes context.BasicRes,
+ dalHelper *srvhelper.ModelSrvHelper[M],
+ pkPathVarNames []string, // path variable names of primary key
Review Comment:
1. Does variables' order matter?
2. `primary key` should be plural form `primary keys`.
##########
backend/helpers/srvhelper/connection_service_helper.go:
##########
@@ -0,0 +1,82 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/core/plugin"
+)
+
+// ConnectionSrvHelper
+type ConnectionSrvHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelSrvHelper[C]
+ pluginName string
+}
+
+// NewConnectionSrvHelper creates a ConnectionDalHelper for connection management
+func NewConnectionSrvHelper[
+ C plugin.ToolLayerConnection,
+ S plugin.ToolLayerScope,
+ SC plugin.ToolLayerScopeConfig,
+](
+ basicRes context.BasicRes,
+ pluginName string,
+) *ConnectionSrvHelper[C, S, SC] {
+ return &ConnectionSrvHelper[C, S, SC]{
+ ModelSrvHelper: NewModelSrvHelper[C](basicRes),
+ pluginName: pluginName,
+ }
+}
+
+func (connSrv *ConnectionSrvHelper[C, S, SC]) DeleteConnection(connection *C) (refs *DsRefs, err errors.Error) {
+ err = connSrv.ModelSrvHelper.NoRunningPipeline(func(tx dal.Transaction) errors.Error {
+ // make sure no blueprint is using the connection
+ connectionId := (*connection).ConnectionId()
+ refs, err = toDsRefs(connSrv.getAllBlueprinsByConnection(connectionId))
+ if err != nil {
+ return err
+ }
+ scopeCount := errors.Must1(connSrv.db.Count(dal.From(new(S)), dal.Where("connection_id = ?", connectionId)))
Review Comment:
I saw many codes use `errors.Must1` or `errors.Must`, which means if something goes wrong, the program will panic. Is it as expected?
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,215 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
+}
+
+type ModelSrvHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ log log.Logger
+ db dal.Dal
+ validator *validator.Validate
+ modelName string
+ pk []dal.ColumnMeta
+ pkWhere string
+ pkCount int
+ searchColumns []string
+}
+
+func NewModelSrvHelper[M dal.Tabler](basicRes context.BasicRes) *ModelSrvHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ db := basicRes.GetDal()
+ if db == nil {
+ db = basicRes.GetDal()
+ }
+ pk := errors.Must1(dal.GetPrimarykeyColumns(db, *m))
+ pkWhere := ""
+ for _, col := range pk {
+ if pkWhere != "" {
+ pkWhere += " AND "
+ }
+ pkWhere += fmt.Sprintf("%s = ? ", col.Name())
+ }
+ return &ModelSrvHelper[M]{
+ basicRes: basicRes,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ db: db,
+ validator: validator.New(),
+ modelName: modelName,
+ pk: pk,
+ pkWhere: pkWhere,
+ pkCount: len(pk),
+ }
+}
+
+func (srv *ModelSrvHelper[M]) NewTx(tx dal.Transaction) *ModelSrvHelper[M] {
+ helper := new(ModelSrvHelper[M])
+ *helper = *srv
+ helper.db = tx
+ return helper
+}
+
+func (srv *ModelSrvHelper[M]) ValidateModel(model *M) errors.Error {
+ // the model can validate itself
+ if customValidator, ok := (interface{}(model)).(CustomValidator); ok {
+ return customValidator.CustomValidate(model, srv.validator)
+ }
+ // basic validator
+ if e := srv.validator.Struct(model); e != nil {
+ return errors.BadInput.Wrap(e, "validation faild")
+ }
+ return nil
+}
+
+// Create validates given model and insert it into database if validation passed
+func (srv *ModelSrvHelper[M]) Create(model *M) errors.Error {
+ println("create model")
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ err = srv.db.Create(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return err
+}
+
+// Update validates given model and update it into database if validation passed
+func (srv *ModelSrvHelper[M]) Update(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
Review Comment:
Can update return db's duplicated error?
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,215 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
Review Comment:
1. typo `entitiy`.
2. Param entitiy is unnecessary, because a struct who implement `CustomValidator` will hold itself and can validate itself.
3. Param validate is unnecessary too, a struct who implement `CustomValidator` can validate without any validator.
4. please consider renaming `CustomValidator` to `Validate`.
##########
backend/helpers/srvhelper/scope_config_service_helper.go:
##########
@@ -0,0 +1,66 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+)
+
+// ScopeConfigSrvHelper
+type ScopeConfigSrvHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelSrvHelper[SC]
+}
+
+func NewScopeConfigSrvHelper[
+ C plugin.ToolLayerConnection,
+ S plugin.ToolLayerScope,
+ SC plugin.ToolLayerScopeConfig,
+](basicRes context.BasicRes) *ScopeConfigSrvHelper[C, S, SC] {
+ return &ScopeConfigSrvHelper[C, S, SC]{
+ ModelSrvHelper: NewModelSrvHelper[SC](basicRes),
+ }
+}
+
+func (scopeConfigSrv *ScopeConfigSrvHelper[C, S, SC]) GetAllByConnectionId(connectionId uint64) ([]*SC, errors.Error) {
+ var scopeConfigs []*SC
+ err := scopeConfigSrv.db.All(&scopeConfigs,
+ dal.Where("connection_id = ?", connectionId),
+ dal.Orderby("id DESC"),
+ )
+ return scopeConfigs, err
+}
+
+func (scopeConfigSrv *ScopeConfigSrvHelper[C, S, SC]) DeleteScopeConfig(scopeConfig *SC) (refs []*S, err errors.Error) {
+ err = scopeConfigSrv.ModelSrvHelper.NoRunningPipeline(func(tx dal.Transaction) errors.Error {
+ // make sure no scope is using the scopeConfig
+ sc := (*scopeConfig)
Review Comment:
`sc := (*scopeConfig)`, why parentheses?
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,215 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
+}
+
+type ModelSrvHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ log log.Logger
+ db dal.Dal
+ validator *validator.Validate
+ modelName string
+ pk []dal.ColumnMeta
+ pkWhere string
+ pkCount int
+ searchColumns []string
+}
+
+func NewModelSrvHelper[M dal.Tabler](basicRes context.BasicRes) *ModelSrvHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ db := basicRes.GetDal()
+ if db == nil {
+ db = basicRes.GetDal()
+ }
+ pk := errors.Must1(dal.GetPrimarykeyColumns(db, *m))
+ pkWhere := ""
+ for _, col := range pk {
+ if pkWhere != "" {
+ pkWhere += " AND "
+ }
+ pkWhere += fmt.Sprintf("%s = ? ", col.Name())
+ }
+ return &ModelSrvHelper[M]{
+ basicRes: basicRes,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ db: db,
+ validator: validator.New(),
+ modelName: modelName,
+ pk: pk,
+ pkWhere: pkWhere,
+ pkCount: len(pk),
+ }
+}
+
+func (srv *ModelSrvHelper[M]) NewTx(tx dal.Transaction) *ModelSrvHelper[M] {
+ helper := new(ModelSrvHelper[M])
+ *helper = *srv
+ helper.db = tx
+ return helper
+}
+
+func (srv *ModelSrvHelper[M]) ValidateModel(model *M) errors.Error {
+ // the model can validate itself
+ if customValidator, ok := (interface{}(model)).(CustomValidator); ok {
+ return customValidator.CustomValidate(model, srv.validator)
+ }
+ // basic validator
+ if e := srv.validator.Struct(model); e != nil {
+ return errors.BadInput.Wrap(e, "validation faild")
+ }
+ return nil
+}
+
+// Create validates given model and insert it into database if validation passed
+func (srv *ModelSrvHelper[M]) Create(model *M) errors.Error {
+ println("create model")
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ err = srv.db.Create(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return err
+}
+
+// Update validates given model and update it into database if validation passed
+func (srv *ModelSrvHelper[M]) Update(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return srv.db.Update(model)
+}
+
+// CreateOrUpdate validates given model and insert or update it into database if validation passed
+func (srv *ModelSrvHelper[M]) CreateOrUpdate(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ return srv.db.CreateOrUpdate(model)
+}
+
+// DeleteModel deletes given model from database
+func (srv *ModelSrvHelper[M]) DeleteModel(model *M) errors.Error {
+ return srv.db.Delete(model)
+}
+
+// FindByPk returns model with given primary key from database
+func (srv *ModelSrvHelper[M]) FindByPk(pk ...interface{}) (*M, errors.Error) {
+ if len(pk) != srv.pkCount {
+ return nil, errors.BadInput.New("invalid primary key")
+ }
+ model := new(M)
+ err := srv.db.First(model, dal.Where(srv.pkWhere, pk...))
+ if err != nil {
+ if srv.db.IsErrorNotFound(err) {
+ return nil, errors.NotFound.Wrap(err, fmt.Sprintf("%s not found", srv.modelName))
+ }
+ return nil, err
+ }
+ return model, nil
+}
+
+// GetAll returns all models from database
+func (srv *ModelSrvHelper[M]) GetAll() ([]*M, errors.Error) {
+ array := make([]*M, 0)
Review Comment:
`var array []*M` is preffered.
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,215 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
+}
+
+type ModelSrvHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ log log.Logger
+ db dal.Dal
+ validator *validator.Validate
+ modelName string
+ pk []dal.ColumnMeta
+ pkWhere string
+ pkCount int
+ searchColumns []string
+}
+
+func NewModelSrvHelper[M dal.Tabler](basicRes context.BasicRes) *ModelSrvHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ db := basicRes.GetDal()
+ if db == nil {
+ db = basicRes.GetDal()
+ }
+ pk := errors.Must1(dal.GetPrimarykeyColumns(db, *m))
+ pkWhere := ""
+ for _, col := range pk {
+ if pkWhere != "" {
+ pkWhere += " AND "
+ }
+ pkWhere += fmt.Sprintf("%s = ? ", col.Name())
+ }
+ return &ModelSrvHelper[M]{
+ basicRes: basicRes,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ db: db,
+ validator: validator.New(),
+ modelName: modelName,
+ pk: pk,
+ pkWhere: pkWhere,
+ pkCount: len(pk),
+ }
+}
+
+func (srv *ModelSrvHelper[M]) NewTx(tx dal.Transaction) *ModelSrvHelper[M] {
+ helper := new(ModelSrvHelper[M])
+ *helper = *srv
+ helper.db = tx
+ return helper
+}
+
+func (srv *ModelSrvHelper[M]) ValidateModel(model *M) errors.Error {
+ // the model can validate itself
+ if customValidator, ok := (interface{}(model)).(CustomValidator); ok {
+ return customValidator.CustomValidate(model, srv.validator)
+ }
+ // basic validator
+ if e := srv.validator.Struct(model); e != nil {
+ return errors.BadInput.Wrap(e, "validation faild")
+ }
+ return nil
+}
+
+// Create validates given model and insert it into database if validation passed
+func (srv *ModelSrvHelper[M]) Create(model *M) errors.Error {
+ println("create model")
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ err = srv.db.Create(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return err
+}
+
+// Update validates given model and update it into database if validation passed
+func (srv *ModelSrvHelper[M]) Update(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return srv.db.Update(model)
+}
+
+// CreateOrUpdate validates given model and insert or update it into database if validation passed
+func (srv *ModelSrvHelper[M]) CreateOrUpdate(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ return srv.db.CreateOrUpdate(model)
+}
+
+// DeleteModel deletes given model from database
+func (srv *ModelSrvHelper[M]) DeleteModel(model *M) errors.Error {
+ return srv.db.Delete(model)
+}
+
+// FindByPk returns model with given primary key from database
+func (srv *ModelSrvHelper[M]) FindByPk(pk ...interface{}) (*M, errors.Error) {
+ if len(pk) != srv.pkCount {
+ return nil, errors.BadInput.New("invalid primary key")
+ }
+ model := new(M)
+ err := srv.db.First(model, dal.Where(srv.pkWhere, pk...))
+ if err != nil {
+ if srv.db.IsErrorNotFound(err) {
+ return nil, errors.NotFound.Wrap(err, fmt.Sprintf("%s not found", srv.modelName))
+ }
+ return nil, err
+ }
+ return model, nil
+}
+
+// GetAll returns all models from database
+func (srv *ModelSrvHelper[M]) GetAll() ([]*M, errors.Error) {
+ array := make([]*M, 0)
+ return array, srv.db.All(&array)
+}
+
+func (srv *ModelSrvHelper[M]) GetPage(pagination *Pagination, query ...dal.Clause) ([]*M, int64, errors.Error) {
+ query = append(query, dal.From(new(M)))
+ // process keyword
+ searchTerm := pagination.SearchTerm
+ if searchTerm != "" && len(srv.searchColumns) > 0 {
Review Comment:
This will cause sql injection by splicing sql mauanlly, especiall searchterm is from client side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338275416
##########
backend/helpers/pluginhelper/api/ds_connection_api_helper.go:
##########
@@ -0,0 +1,76 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "strconv"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+// DsConnectionApiHelper
Review Comment:
short for DataSource
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338300146
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,215 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
+}
+
+type ModelSrvHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ log log.Logger
+ db dal.Dal
+ validator *validator.Validate
+ modelName string
+ pk []dal.ColumnMeta
+ pkWhere string
+ pkCount int
+ searchColumns []string
+}
+
+func NewModelSrvHelper[M dal.Tabler](basicRes context.BasicRes) *ModelSrvHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ db := basicRes.GetDal()
+ if db == nil {
+ db = basicRes.GetDal()
+ }
+ pk := errors.Must1(dal.GetPrimarykeyColumns(db, *m))
+ pkWhere := ""
+ for _, col := range pk {
+ if pkWhere != "" {
+ pkWhere += " AND "
+ }
+ pkWhere += fmt.Sprintf("%s = ? ", col.Name())
+ }
+ return &ModelSrvHelper[M]{
+ basicRes: basicRes,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ db: db,
+ validator: validator.New(),
+ modelName: modelName,
+ pk: pk,
+ pkWhere: pkWhere,
+ pkCount: len(pk),
+ }
+}
+
+func (srv *ModelSrvHelper[M]) NewTx(tx dal.Transaction) *ModelSrvHelper[M] {
+ helper := new(ModelSrvHelper[M])
+ *helper = *srv
+ helper.db = tx
+ return helper
+}
+
+func (srv *ModelSrvHelper[M]) ValidateModel(model *M) errors.Error {
+ // the model can validate itself
+ if customValidator, ok := (interface{}(model)).(CustomValidator); ok {
+ return customValidator.CustomValidate(model, srv.validator)
+ }
+ // basic validator
+ if e := srv.validator.Struct(model); e != nil {
+ return errors.BadInput.Wrap(e, "validation faild")
+ }
+ return nil
+}
+
+// Create validates given model and insert it into database if validation passed
+func (srv *ModelSrvHelper[M]) Create(model *M) errors.Error {
+ println("create model")
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ err = srv.db.Create(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return err
+}
+
+// Update validates given model and update it into database if validation passed
+func (srv *ModelSrvHelper[M]) Update(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return srv.db.Update(model)
+}
+
+// CreateOrUpdate validates given model and insert or update it into database if validation passed
+func (srv *ModelSrvHelper[M]) CreateOrUpdate(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ return srv.db.CreateOrUpdate(model)
+}
+
+// DeleteModel deletes given model from database
+func (srv *ModelSrvHelper[M]) DeleteModel(model *M) errors.Error {
+ return srv.db.Delete(model)
+}
+
+// FindByPk returns model with given primary key from database
+func (srv *ModelSrvHelper[M]) FindByPk(pk ...interface{}) (*M, errors.Error) {
+ if len(pk) != srv.pkCount {
+ return nil, errors.BadInput.New("invalid primary key")
+ }
+ model := new(M)
+ err := srv.db.First(model, dal.Where(srv.pkWhere, pk...))
+ if err != nil {
+ if srv.db.IsErrorNotFound(err) {
+ return nil, errors.NotFound.Wrap(err, fmt.Sprintf("%s not found", srv.modelName))
+ }
+ return nil, err
+ }
+ return model, nil
+}
+
+// GetAll returns all models from database
+func (srv *ModelSrvHelper[M]) GetAll() ([]*M, errors.Error) {
+ array := make([]*M, 0)
+ return array, srv.db.All(&array)
+}
+
+func (srv *ModelSrvHelper[M]) GetPage(pagination *Pagination, query ...dal.Clause) ([]*M, int64, errors.Error) {
+ query = append(query, dal.From(new(M)))
+ // process keyword
+ searchTerm := pagination.SearchTerm
+ if searchTerm != "" && len(srv.searchColumns) > 0 {
Review Comment:
Are you sure? would you like to take another closer look? The `searchTerm` is prefixed and suffixed with the `%` and then passed down to `gorm` as SQL parameters, it should be safe from SQL injection IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] d4x1 commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "d4x1 (via GitHub)" <gi...@apache.org>.
d4x1 commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338685369
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,215 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
+}
+
+type ModelSrvHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ log log.Logger
+ db dal.Dal
+ validator *validator.Validate
+ modelName string
+ pk []dal.ColumnMeta
+ pkWhere string
+ pkCount int
+ searchColumns []string
+}
+
+func NewModelSrvHelper[M dal.Tabler](basicRes context.BasicRes) *ModelSrvHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ db := basicRes.GetDal()
+ if db == nil {
+ db = basicRes.GetDal()
+ }
+ pk := errors.Must1(dal.GetPrimarykeyColumns(db, *m))
+ pkWhere := ""
+ for _, col := range pk {
+ if pkWhere != "" {
+ pkWhere += " AND "
+ }
+ pkWhere += fmt.Sprintf("%s = ? ", col.Name())
+ }
+ return &ModelSrvHelper[M]{
+ basicRes: basicRes,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ db: db,
+ validator: validator.New(),
+ modelName: modelName,
+ pk: pk,
+ pkWhere: pkWhere,
+ pkCount: len(pk),
+ }
+}
+
+func (srv *ModelSrvHelper[M]) NewTx(tx dal.Transaction) *ModelSrvHelper[M] {
+ helper := new(ModelSrvHelper[M])
+ *helper = *srv
+ helper.db = tx
+ return helper
+}
+
+func (srv *ModelSrvHelper[M]) ValidateModel(model *M) errors.Error {
+ // the model can validate itself
+ if customValidator, ok := (interface{}(model)).(CustomValidator); ok {
+ return customValidator.CustomValidate(model, srv.validator)
+ }
+ // basic validator
+ if e := srv.validator.Struct(model); e != nil {
+ return errors.BadInput.Wrap(e, "validation faild")
+ }
+ return nil
+}
+
+// Create validates given model and insert it into database if validation passed
+func (srv *ModelSrvHelper[M]) Create(model *M) errors.Error {
+ println("create model")
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ err = srv.db.Create(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return err
+}
+
+// Update validates given model and update it into database if validation passed
+func (srv *ModelSrvHelper[M]) Update(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return srv.db.Update(model)
+}
+
+// CreateOrUpdate validates given model and insert or update it into database if validation passed
+func (srv *ModelSrvHelper[M]) CreateOrUpdate(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ return srv.db.CreateOrUpdate(model)
+}
+
+// DeleteModel deletes given model from database
+func (srv *ModelSrvHelper[M]) DeleteModel(model *M) errors.Error {
+ return srv.db.Delete(model)
+}
+
+// FindByPk returns model with given primary key from database
+func (srv *ModelSrvHelper[M]) FindByPk(pk ...interface{}) (*M, errors.Error) {
+ if len(pk) != srv.pkCount {
+ return nil, errors.BadInput.New("invalid primary key")
+ }
+ model := new(M)
+ err := srv.db.First(model, dal.Where(srv.pkWhere, pk...))
+ if err != nil {
+ if srv.db.IsErrorNotFound(err) {
+ return nil, errors.NotFound.Wrap(err, fmt.Sprintf("%s not found", srv.modelName))
+ }
+ return nil, err
+ }
+ return model, nil
+}
+
+// GetAll returns all models from database
+func (srv *ModelSrvHelper[M]) GetAll() ([]*M, errors.Error) {
+ array := make([]*M, 0)
Review Comment:
Forget it, it doesn't matter
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] d4x1 commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "d4x1 (via GitHub)" <gi...@apache.org>.
d4x1 commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338680663
##########
backend/helpers/pluginhelper/api/ds_scope_api_helper.go:
##########
@@ -0,0 +1,99 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+type DsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
+ *ModelApiHelper[S]
+ *srvhelper.ScopeSrvHelper[C, S, SC]
+}
+
+func NewDsScopeApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig](
+ basicRes context.BasicRes,
+ srvHelper *srvhelper.ScopeSrvHelper[C, S, SC],
+) *DsScopeApiHelper[C, S, SC] {
+ return &DsScopeApiHelper[C, S, SC]{
+ ModelApiHelper: NewModelApiHelper[S](basicRes, srvHelper.ModelSrvHelper, []string{"connectionId", "scopeId"}),
+ ScopeSrvHelper: srvHelper,
+ }
+}
+
+func (connApi *DsScopeApiHelper[C, S, SC]) GetPage(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ pagination, err := ParsePagination[srvhelper.ScopePagination](input)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, "failed to decode pathvars into pagination")
+ }
+ scopes, count, err := connApi.ScopeSrvHelper.GetScopesPage(pagination)
+ if err != nil {
Review Comment:
In this case, page No, page size and ScopeSrvHelper's info(specific types of generics) are helpful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338300146
##########
backend/helpers/srvhelper/model_service_helper.go:
##########
@@ -0,0 +1,215 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package srvhelper
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/models"
+ "github.com/apache/incubator-devlake/helpers/dbhelper"
+ "github.com/go-playground/validator/v10"
+)
+
+type CustomValidator interface {
+ CustomValidate(entitiy interface{}, validate *validator.Validate) errors.Error
+}
+
+type ModelSrvHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ log log.Logger
+ db dal.Dal
+ validator *validator.Validate
+ modelName string
+ pk []dal.ColumnMeta
+ pkWhere string
+ pkCount int
+ searchColumns []string
+}
+
+func NewModelSrvHelper[M dal.Tabler](basicRes context.BasicRes) *ModelSrvHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ db := basicRes.GetDal()
+ if db == nil {
+ db = basicRes.GetDal()
+ }
+ pk := errors.Must1(dal.GetPrimarykeyColumns(db, *m))
+ pkWhere := ""
+ for _, col := range pk {
+ if pkWhere != "" {
+ pkWhere += " AND "
+ }
+ pkWhere += fmt.Sprintf("%s = ? ", col.Name())
+ }
+ return &ModelSrvHelper[M]{
+ basicRes: basicRes,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ db: db,
+ validator: validator.New(),
+ modelName: modelName,
+ pk: pk,
+ pkWhere: pkWhere,
+ pkCount: len(pk),
+ }
+}
+
+func (srv *ModelSrvHelper[M]) NewTx(tx dal.Transaction) *ModelSrvHelper[M] {
+ helper := new(ModelSrvHelper[M])
+ *helper = *srv
+ helper.db = tx
+ return helper
+}
+
+func (srv *ModelSrvHelper[M]) ValidateModel(model *M) errors.Error {
+ // the model can validate itself
+ if customValidator, ok := (interface{}(model)).(CustomValidator); ok {
+ return customValidator.CustomValidate(model, srv.validator)
+ }
+ // basic validator
+ if e := srv.validator.Struct(model); e != nil {
+ return errors.BadInput.Wrap(e, "validation faild")
+ }
+ return nil
+}
+
+// Create validates given model and insert it into database if validation passed
+func (srv *ModelSrvHelper[M]) Create(model *M) errors.Error {
+ println("create model")
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ err = srv.db.Create(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return err
+}
+
+// Update validates given model and update it into database if validation passed
+func (srv *ModelSrvHelper[M]) Update(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ if srv.db.IsDuplicationError(err) {
+ return errors.Conflict.Wrap(err, fmt.Sprintf("%s already exists", srv.modelName))
+ }
+ return err
+ }
+ return srv.db.Update(model)
+}
+
+// CreateOrUpdate validates given model and insert or update it into database if validation passed
+func (srv *ModelSrvHelper[M]) CreateOrUpdate(model *M) errors.Error {
+ err := srv.ValidateModel(model)
+ if err != nil {
+ return err
+ }
+ return srv.db.CreateOrUpdate(model)
+}
+
+// DeleteModel deletes given model from database
+func (srv *ModelSrvHelper[M]) DeleteModel(model *M) errors.Error {
+ return srv.db.Delete(model)
+}
+
+// FindByPk returns model with given primary key from database
+func (srv *ModelSrvHelper[M]) FindByPk(pk ...interface{}) (*M, errors.Error) {
+ if len(pk) != srv.pkCount {
+ return nil, errors.BadInput.New("invalid primary key")
+ }
+ model := new(M)
+ err := srv.db.First(model, dal.Where(srv.pkWhere, pk...))
+ if err != nil {
+ if srv.db.IsErrorNotFound(err) {
+ return nil, errors.NotFound.Wrap(err, fmt.Sprintf("%s not found", srv.modelName))
+ }
+ return nil, err
+ }
+ return model, nil
+}
+
+// GetAll returns all models from database
+func (srv *ModelSrvHelper[M]) GetAll() ([]*M, errors.Error) {
+ array := make([]*M, 0)
+ return array, srv.db.All(&array)
+}
+
+func (srv *ModelSrvHelper[M]) GetPage(pagination *Pagination, query ...dal.Clause) ([]*M, int64, errors.Error) {
+ query = append(query, dal.From(new(M)))
+ // process keyword
+ searchTerm := pagination.SearchTerm
+ if searchTerm != "" && len(srv.searchColumns) > 0 {
Review Comment:
Are you sure? would you like to take another closer look? The `searchTerm` is prefixed and suffixed with the `%` and then passed down to `gorm` as a SQL parameters, it should be safe from SQL injection IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1339463002
##########
backend/helpers/pluginhelper/api/ds_connection_api_helper.go:
##########
@@ -0,0 +1,76 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "strconv"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+// DsConnectionApiHelper
+type DsConnectionApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
Review Comment:
Alright, lets keep it short for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338275979
##########
backend/helpers/pluginhelper/api/ds_connection_api_helper.go:
##########
@@ -0,0 +1,76 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "strconv"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/server/api/shared"
+)
+
+// DsConnectionApiHelper
+type DsConnectionApiHelper[C plugin.ToolLayerConnection, S plugin.ToolLayerScope, SC plugin.ToolLayerScopeConfig] struct {
Review Comment:
How do you suggest to address that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338559965
##########
backend/helpers/pluginhelper/api/model_api_helper.go:
##########
@@ -0,0 +1,160 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package api
+
+import (
+ "fmt"
+ "net/http"
+
+ "github.com/apache/incubator-devlake/core/context"
+ "github.com/apache/incubator-devlake/core/dal"
+ "github.com/apache/incubator-devlake/core/errors"
+ "github.com/apache/incubator-devlake/core/log"
+ "github.com/apache/incubator-devlake/core/plugin"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
+ "github.com/apache/incubator-devlake/helpers/utils"
+)
+
+type ModelApiHelper[M dal.Tabler] struct {
+ basicRes context.BasicRes
+ dalHelper *srvhelper.ModelSrvHelper[M]
+ log log.Logger
+ modelName string
+ pkPathVarNames []string
+}
+
+func NewModelApiHelper[M dal.Tabler](
+ basicRes context.BasicRes,
+ dalHelper *srvhelper.ModelSrvHelper[M],
+ pkPathVarNames []string, // path variable names of primary key
+) *ModelApiHelper[M] {
+ m := new(M)
+ modelName := fmt.Sprintf("%T", m)
+ return &ModelApiHelper[M]{
+ basicRes: basicRes,
+ dalHelper: dalHelper,
+ log: basicRes.GetLogger().Nested(fmt.Sprintf("%s_dal", modelName)),
+ modelName: modelName,
+ pkPathVarNames: pkPathVarNames,
+ }
+}
+
+func (self *ModelApiHelper[M]) Post(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ model := new(M)
+ err := utils.DecodeMapStruct(input.Body, model, false)
+ if err != nil {
+ return nil, err
+ }
+ err = self.dalHelper.Create(model)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Status: http.StatusCreated,
+ Body: model,
+ }, nil
+}
+
+func (self *ModelApiHelper[M]) FindByPk(input *plugin.ApiResourceInput) (*M, errors.Error) {
+ pkv := make([]interface{}, len(self.pkPathVarNames))
+ for i, pkn := range self.pkPathVarNames {
+ pkv[i] = input.Params[pkn]
+ }
+ return self.dalHelper.FindByPk(pkv...)
+}
+
+func (self *ModelApiHelper[M]) GetDetail(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ model, err := self.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: model,
+ }, nil
+}
+
+func (self *ModelApiHelper[M]) Patch(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ model, err := self.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ err = utils.DecodeMapStruct(input.Body, model, true)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, fmt.Sprintf("faled to patch %s", self.modelName))
+ }
+ err = self.dalHelper.Update(model)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: model,
+ }, nil
+}
+
+func (self *ModelApiHelper[M]) Delete(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ model, err := self.FindByPk(input)
+ if err != nil {
+ return nil, err
+ }
+ err = self.dalHelper.DeleteModel(model)
+ if err != nil {
+ return nil, err
+ }
+ return &plugin.ApiResourceOutput{
+ Body: model,
+ }, nil
+}
+
+func (self *ModelApiHelper[M]) GetAll(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ all, err := self.dalHelper.GetAll()
+ return &plugin.ApiResourceOutput{
+ Body: all,
+ }, err
+}
+
+func (self *ModelApiHelper[M]) PutMultiple(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) {
+ var req struct {
+ Data []M `json:"data"`
+ }
+ err := utils.DecodeMapStruct(input.Body, &req, false)
+ if err != nil {
+ return nil, err
+ }
+ for i, item := range req.Data {
+ err := self.dalHelper.CreateOrUpdate(&item)
+ if err != nil {
+ return nil, errors.BadInput.Wrap(err, fmt.Sprintf("failed to save item %d", i))
+ }
+ }
+ return &plugin.ApiResourceOutput{
+ Body: req.Data,
+ }, nil
+}
+
+func ParsePagination[P any](input *plugin.ApiResourceInput, query ...dal.Clause) (*P, errors.Error) {
Review Comment:
1. Good catch.
2. It is a function, so? What is the point exactly? Please elaborate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #6146: refactor: unify connection/scope/scopeConfig helpers
Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on code in PR #6146:
URL: https://github.com/apache/incubator-devlake/pull/6146#discussion_r1338678946
##########
backend/plugins/github/api/init.go:
##########
@@ -18,69 +18,44 @@ limitations under the License.
package api
import (
- "strconv"
-
"github.com/apache/incubator-devlake/core/plugin"
"github.com/apache/incubator-devlake/core/context"
- "github.com/apache/incubator-devlake/core/dal"
- "github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/helpers/pluginhelper/api"
+ "github.com/apache/incubator-devlake/helpers/srvhelper"
"github.com/apache/incubator-devlake/plugins/github/models"
"github.com/go-playground/validator/v10"
)
var vld *validator.Validate
+
+// var connSrv *srvhelper.ConnectionSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var connApi *api.DsConnectionApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scopeSrv *srvhelper.ScopeSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scopeApi *api.DsScopeApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scSrv *srvhelper.ScopeConfigSrvHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
+var scApi *api.DsScopeConfigApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
var connectionHelper *api.ConnectionApiHelper
-var scopeHelper *api.ScopeApiHelper[models.GithubConnection, models.GithubRepo, models.GithubScopeConfig]
var basicRes context.BasicRes
-var scHelper *api.ScopeConfigHelper[models.GithubScopeConfig]
var remoteHelper *api.RemoteApiHelper[models.GithubConnection, models.GithubRepo, repo, plugin.ApiGroup]
func Init(br context.BasicRes, p plugin.PluginMeta) {
-
basicRes = br
+ _, connApi, scopeSrv, scopeApi, scSrv, scApi = api.NewDataSourceHelpers[
+ models.GithubConnection,
+ models.GithubRepo, models.GithubScopeConfig,
+ ](
+ br,
+ "github",
+ []string{"full_name"},
+ )
+ // TODO: refactor remoteHelper
vld = validator.New()
connectionHelper = api.NewConnectionHelper(
basicRes,
vld,
p.Name(),
)
- params := &api.ReflectionParameters{
- ScopeIdFieldName: "GithubId",
- ScopeIdColumnName: "github_id",
- RawScopeParamName: "Name",
- SearchScopeParamName: "name",
Review Comment:
It is handled here https://github.com/apache/incubator-devlake/pull/6146/files#diff-48658af240a5af910b7bb5d4e3effb2a0a55e4433b57d874921b76ef9f0ce51eR58
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org