You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2021/08/10 07:33:03 UTC

[GitHub] [dubbo-go] ChangedenCZD opened a new pull request #1372: Ftr: Add config loader hooks for 1.5.

ChangedenCZD opened a new pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372


   <!--  Thanks for sending a pull request!
   Read https://github.com/apache/dubbo-go/blob/master/CONTRIBUTING.md before commit pull request.
   -->
   
   **What this PR does**:
   ```go
   
   func TestLoadWithLoaderHooks(t *testing.T) {
   	extension.SetFilter(constant.GracefulShutdownConsumerFilterKey, func() filter.Filter {
   		return &mockGracefulShutdownFilter{}
   	})
   	extension.SetFilter(constant.GracefulShutdownProviderFilterKey, func() filter.Filter {
   		return &mockGracefulShutdownFilter{}
   	})
   
   	doInitConsumer()
   	doInitProvider()
   
   	ms := &MockService{}
   	SetConsumerService(ms)
   	SetProviderService(ms)
   
   	extension.SetProtocol("registry", GetProtocol)
   	extension.SetCluster(constant.ZONEAWARE_CLUSTER_NAME, cluster_impl.NewZoneAwareCluster)
   	extension.SetProxyFactory("default", proxy_factory.NewDefaultProxyFactory)
   	GetApplicationConfig().MetadataType = "mock"
   	var mm *mockMetadataService
   	extension.SetLocalMetadataService("mock", func() (metadataService service.MetadataService, err error) {
   		if mm == nil {
   			mm = &mockMetadataService{
   				exportedServiceURLs: new(sync.Map),
   				lock:                new(sync.RWMutex),
   			}
   		}
   		return mm, nil
   	})
   
   	// create consumer loader hooks
   	beforeConsumerConnectHook := NewBeforeConsumerConnectHook(func(info *ConsumerConnectInfo) {
   		logger.Debug("BeforeConsumerConnectHook", info)
   		assert.NotNil(t, info)
   	})
   	consumerConnectSuccessHook := NewConsumerConnectSuccessHook(func(info *ConsumerConnectInfo) {
   		logger.Debug("ConsumerConnectSuccessHook", info)
   		assert.NotNil(t, info)
   	})
   	consumerConnectFailHook := NewConsumerConnectFailHook(func(info *ConsumerConnectFailInfo) {
   		logger.Debug("ConsumerConnectFailHook", info)
   		assert.NotNil(t, info)
   	})
   	allConsumersConnectCompleteHook := NewAllConsumersConnectCompleteHook(func() {
   		logger.Debug("AllConsumersConnectCompleteHook")
   		RemoveLoaderHooks( // Remove some consumer loader hooks
   			beforeConsumerConnectHook,
   			consumerConnectSuccessHook,
   			consumerConnectFailHook,
   		)
   		logger.Debug("LoaderHooks length = ", len(loaderHooks), " after AllConsumersConnectCompleteHook")
   		// beforeConsumerConnectHook, consumerConnectSuccessHook 
                   // and consumerConnectFailHook have been removed
   		assert.Equal(t, len(loaderHooks), 6)
   	})
   	// create provider loader hooks
   	beforeProviderConnectHook := NewBeforeProviderConnectHook(func(info *ProviderConnectInfo) {
   		logger.Debug("BeforeProviderConnectHook", info)
   		assert.NotNil(t, info)
   	})
   	providerConnectSuccessHook := NewProviderConnectSuccessHook(func(info *ProviderConnectInfo) {
   		logger.Debug("ProviderConnectSuccessHook", info)
   		assert.NotNil(t, info)
   	})
   	providerConnectFailHook := NewProviderConnectFailHook(func(info *ProviderConnectFailInfo) {
   		logger.Debug("ProviderConnectFailHook", info)
   		assert.NotNil(t, info)
   	})
   	allProvidersConnectCompleteHook := NewAllProvidersConnectCompleteHook(func() {
   		logger.Debug("AllProvidersConnectCompleteHook")
   		RemoveLoaderHooks( // Remove some provider loader hooks
   			beforeProviderConnectHook,
   			providerConnectSuccessHook,
   			providerConnectFailHook,
   		)
   		logger.Debug("LoaderHooks length = ", len(loaderHooks), " after AllProvidersConnectCompleteHook")
   		// beforeProviderConnectHook, providerConnectSuccessHook
                   // and providerConnectFailHook have been removed
   		assert.Equal(t, len(loaderHooks), 3)
   	})
   
   	beforeShutdownHook := NewBeforeShutdownHook(func() {
   		logger.Debug("BeforeShutDownHook")
   		RemoveLoaderHooks( // Remove some consumer and provider loader hooks
   			// Consumer Hooks
   			beforeConsumerConnectHook,
   			consumerConnectSuccessHook,
   			consumerConnectFailHook,
   			allConsumersConnectCompleteHook,
   			// Provider Hooks
   			beforeProviderConnectHook,
   			providerConnectSuccessHook,
   			providerConnectFailHook,
   			allProvidersConnectCompleteHook,
   		)
   		assert.Equal(t, len(loaderHooks), 1)
   	})
   	// add loader hooks before config.Load
   	AddLoaderHooks(
   		// Consumer Hooks
   		beforeConsumerConnectHook,
   		consumerConnectSuccessHook,
   		consumerConnectFailHook,
   		allConsumersConnectCompleteHook,
   		// Provider Hooks
   		beforeProviderConnectHook,
   		providerConnectSuccessHook,
   		providerConnectFailHook,
   		allProvidersConnectCompleteHook,
   		// Shut Down hook
   		beforeShutdownHook,
   	)
   	// check loaderHooks length
   	assert.Equal(t, len(loaderHooks), 9)
   
   	Load()
   
   	assert.Equal(t, ms, GetRPCService(ms.Reference()))
   	ms2 := &struct {
   		MockService
   	}{}
   	RPCService(ms2)
   	assert.NotEqual(t, ms2, GetRPCService(ms2.Reference()))
   
   	conServices = map[string]common.RPCService{}
   	proServices = map[string]common.RPCService{}
   	err := common.ServiceMap.UnRegister("com.MockService", "mock",
   		common.ServiceKey("com.MockService", "huadong_idc", "1.0.0"))
   	assert.Nil(t, err)
   	consumerConfig = nil
   	providerConfig = nil
   }
   ```
   
   **Which issue(s) this PR fixes**:
   <!--
   *Automatically closes linked issue when PR is merged.
   Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
   _If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
   -->
   Fixes #1364
   
   **Special notes for your reviewer**:
   
   **Does this PR introduce a user-facing change?**:
   <!--
   If no, just write "NONE" in the release-note block below.
   If yes, a release note is required:
   Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
   -->
   ```release-note
   None
   ```


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo-go] ChangedenCZD commented on pull request #1372: Ftr: Add config loader hooks for 1.5

Posted by GitBox <gi...@apache.org>.
ChangedenCZD commented on pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372#issuecomment-904281231


   @wangxw666 1.5版本的已改为 #1405 这个解决方案,3.0的建议废除原有的分发机制,改为Hook


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo-go] ChangedenCZD commented on a change in pull request #1372: Ftr: Add config loader hooks for 1.5

Posted by GitBox <gi...@apache.org>.
ChangedenCZD commented on a change in pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372#discussion_r695667238



##########
File path: config/config_loader.go
##########
@@ -56,6 +56,8 @@ var (
 
 	maxWait        = 3
 	confRouterFile string
+
+	loaderHooks map[string][]LoaderHook

Review comment:
       1.5版本的已改为 #1405 这个解决方案,3.0的建议废除原有的分发机制,改为Hook




-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo-go] Mulavar commented on a change in pull request #1372: Ftr: Add config loader hooks for 1.5

Posted by GitBox <gi...@apache.org>.
Mulavar commented on a change in pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372#discussion_r694948519



##########
File path: config/config_loader_hook.go
##########
@@ -0,0 +1,367 @@
+/*
+ * 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 config
+
+import (
+	"fmt"
+	"reflect"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/constant"
+)
+
+const (
+	hookParams = constant.HookParams
+)
+
+type LoaderHook interface {
+	emit() bool
+	// if you want to use emitWithParams, you must add HookParams field and make emit return false
+	emitWithParams(params interface{}) bool
+}
+
+func getLoaderHookType(t LoaderHook) reflect.Type {
+	hookType := reflect.TypeOf(t)
+	if reflect.Ptr == hookType.Kind() {
+		hookType = hookType.Elem()
+	}
+	return hookType
+}
+
+func getLoaderHookTypeString(t LoaderHook) string {
+	return getLoaderHookType(t).String()
+}
+
+func getLoaderHookInfo(v reflect.Value) reflect.Value {
+	if reflect.Ptr == v.Kind() {
+		v = v.Elem()
+	}
+	return v
+}
+
+func getLoaderHookParams(t LoaderHook) interface{} {
+	hookType := getLoaderHookType(t)
+	_, existed := hookType.FieldByName(hookParams)
+	if !existed {
+		panic(fmt.Sprintf("Hook field [%s] not existed for %s", hookParams, hookType.Name()))
+	}
+	hookParams := getLoaderHookInfo(reflect.ValueOf(t)).FieldByName(hookParams)
+	return getLoaderHookInfo(hookParams).Interface()
+}
+
+func AddLoaderHooks(hooks ...LoaderHook) {
+	if len(hooks) > 0 {
+		if loaderHooks == nil {
+			loaderHooks = map[string][]LoaderHook{}
+		}
+		for _, hook := range hooks {
+			hookType := getLoaderHookTypeString(hook)
+			var hooks []LoaderHook
+			if hs, ok := loaderHooks[hookType]; ok {
+				hooks = hs
+			}
+			if hooks == nil {
+				hooks = []LoaderHook{}
+			}
+			loaderHooks[hookType] = append(hooks, hook)
+		}
+	}
+}
+
+func RemoveLoaderHooks(hooks ...LoaderHook) {
+	if loaderHooks != nil && len(loaderHooks) > 0 {
+		for _, rmHook := range hooks {
+			hookType := getLoaderHookTypeString(rmHook)
+			if hooks, ok := loaderHooks[hookType]; ok {
+				for index, targetHook := range hooks {
+					if rmHook == targetHook {
+						newHooks := append(hooks[:index], hooks[index+1:]...)
+						if len(newHooks) == 0 {
+							delete(loaderHooks, hookType)
+						} else {
+							loaderHooks[hookType] = newHooks
+						}
+						break
+					}
+				}
+			}
+		}
+	}
+}
+
+func emitHook(t LoaderHook) {
+	if loaderHooks != nil && len(loaderHooks) > 0 {
+		hookType := getLoaderHookTypeString(t)
+		if hooks, ok := loaderHooks[hookType]; ok {
+			for _, hook := range hooks {
+				if !hook.emit() {
+					hook.emitWithParams(getLoaderHookParams(t))
+				}
+			}
+		}
+	}

Review comment:
       同上,如果没有必要的复杂逻辑,个人建议嵌套不要超过三层。




-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo-go] codecov-commenter edited a comment on pull request #1372: Ftr: Add config loader hooks for 1.5

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372#issuecomment-905421427


   # [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1372](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b5ab3be) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/467c867e04f7d28308305b7601e391080fa73340?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (467c867) will **decrease** coverage by `2.34%`.
   > The diff coverage is `68.88%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/dubbo-go/pull/1372/graphs/tree.svg?width=650&height=150&src=pr&token=dcPE6RyFAL&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##              1.5    #1372      +/-   ##
   ==========================================
   - Coverage   58.07%   55.72%   -2.35%     
   ==========================================
     Files         275      277       +2     
     Lines       13381    16119    +2738     
   ==========================================
   + Hits         7771     8983    +1212     
   - Misses       4650     6197    +1547     
   + Partials      960      939      -21     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [config/graceful\_shutdown.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29uZmlnL2dyYWNlZnVsX3NodXRkb3duLmdv) | `57.30% <0.00%> (+3.01%)` | :arrow_up: |
   | [config/provider\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29uZmlnL3Byb3ZpZGVyX2NvbmZpZy5nbw==) | `62.06% <0.00%> (+7.52%)` | :arrow_up: |
   | [config/reference\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29uZmlnL3JlZmVyZW5jZV9jb25maWcuZ28=) | `80.00% <0.00%> (-1.67%)` | :arrow_down: |
   | [config\_center/nacos/facade.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29uZmlnX2NlbnRlci9uYWNvcy9mYWNhZGUuZ28=) | `62.50% <0.00%> (-27.16%)` | :arrow_down: |
   | [registry/consul/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVnaXN0cnkvY29uc3VsL3JlZ2lzdHJ5Lmdv) | `51.64% <0.00%> (+2.31%)` | :arrow_up: |
   | [...gistry/event/protocol\_ports\_metadata\_customizer.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVnaXN0cnkvZXZlbnQvcHJvdG9jb2xfcG9ydHNfbWV0YWRhdGFfY3VzdG9taXplci5nbw==) | `65.78% <ø> (+6.41%)` | :arrow_up: |
   | [registry/kubernetes/registry.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9yZWdpc3RyeS5nbw==) | `64.15% <0.00%> (+13.03%)` | :arrow_up: |
   | [registry/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVnaXN0cnkvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `26.58% <0.00%> (-42.65%)` | :arrow_down: |
   | [remoting/etcdv3/client.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3RpbmcvZXRjZHYzL2NsaWVudC5nbw==) | `55.69% <0.00%> (+2.69%)` | :arrow_up: |
   | [remoting/etcdv3/facade.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3RpbmcvZXRjZHYzL2ZhY2FkZS5nbw==) | `0.00% <0.00%> (ø)` | |
   | ... and [311 more](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [81c2e23...b5ab3be](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo-go] AlexStocks closed pull request #1372: Ftr: Add config loader hooks for 1.5

Posted by GitBox <gi...@apache.org>.
AlexStocks closed pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372


   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo-go] cjphaha commented on a change in pull request #1372: Ftr: Add config loader hooks for 1.5

Posted by GitBox <gi...@apache.org>.
cjphaha commented on a change in pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372#discussion_r695098961



##########
File path: config/config_loader.go
##########
@@ -56,6 +56,8 @@ var (
 
 	maxWait        = 3
 	confRouterFile string
+
+	loaderHooks map[string][]LoaderHook

Review comment:
       看代码中只有 AddLoaderHooks 函数对loaderHooks 进行初始化,但是 AddLoaderHooks 只被写在了单测里,是否需要在这里进行初始化会更好一些?




-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo-go] ChangedenCZD commented on pull request #1372: Ftr: Add config loader hooks for 1.5

Posted by GitBox <gi...@apache.org>.
ChangedenCZD commented on pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372#issuecomment-902519615


   使用事件分发机制实现 #1405 


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo-go] codecov-commenter commented on pull request #1372: Ftr: Add config loader hooks for 1.5

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372#issuecomment-905421427


   # [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1372](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (96c53b5) into [1.5](https://codecov.io/gh/apache/dubbo-go/commit/467c867e04f7d28308305b7601e391080fa73340?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (467c867) will **decrease** coverage by `2.36%`.
   > The diff coverage is `70.56%`.
   
   > :exclamation: Current head 96c53b5 differs from pull request most recent head b5ab3be. Consider uploading reports for the commit b5ab3be to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/dubbo-go/pull/1372/graphs/tree.svg?width=650&height=150&src=pr&token=dcPE6RyFAL&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##              1.5    #1372      +/-   ##
   ==========================================
   - Coverage   58.07%   55.70%   -2.37%     
   ==========================================
     Files         275      277       +2     
     Lines       13381    16121    +2740     
   ==========================================
   + Hits         7771     8981    +1210     
   - Misses       4650     6195    +1545     
   + Partials      960      945      -15     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [config/graceful\_shutdown.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29uZmlnL2dyYWNlZnVsX3NodXRkb3duLmdv) | `57.30% <0.00%> (+3.01%)` | :arrow_up: |
   | [config/provider\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29uZmlnL3Byb3ZpZGVyX2NvbmZpZy5nbw==) | `62.06% <0.00%> (+7.52%)` | :arrow_up: |
   | [config/reference\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29uZmlnL3JlZmVyZW5jZV9jb25maWcuZ28=) | `80.00% <0.00%> (-1.67%)` | :arrow_down: |
   | [registry/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVnaXN0cnkvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `26.58% <0.00%> (-42.65%)` | :arrow_down: |
   | [remoting/zookeeper/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvem9va2VlcGVyL2xpc3RlbmVyLmdv) | `0.00% <0.00%> (-41.29%)` | :arrow_down: |
   | [config/config\_center\_config.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29uZmlnL2NvbmZpZ19jZW50ZXJfY29uZmlnLmdv) | `66.21% <33.33%> (+2.03%)` | :arrow_up: |
   | [registry/directory/directory.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVnaXN0cnkvZGlyZWN0b3J5L2RpcmVjdG9yeS5nbw==) | `77.89% <58.33%> (+2.14%)` | :arrow_up: |
   | [protocol/dubbo/dubbo\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHJvdG9jb2wvZHViYm8vZHViYm9faW52b2tlci5nbw==) | `70.29% <60.00%> (+0.97%)` | :arrow_up: |
   | [config/config\_loader.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29uZmlnL2NvbmZpZ19sb2FkZXIuZ28=) | `47.12% <64.44%> (+8.28%)` | :arrow_up: |
   | [filter/filter\_impl/generic\_filter.go](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmlsdGVyL2ZpbHRlcl9pbXBsL2dlbmVyaWNfZmlsdGVyLmdv) | `62.50% <65.38%> (-8.72%)` | :arrow_down: |
   | ... and [294 more](https://codecov.io/gh/apache/dubbo-go/pull/1372/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [81c2e23...b5ab3be](https://codecov.io/gh/apache/dubbo-go/pull/1372?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo-go] wangxw666 commented on a change in pull request #1372: Ftr: Add config loader hooks for 1.5

Posted by GitBox <gi...@apache.org>.
wangxw666 commented on a change in pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372#discussion_r694451216



##########
File path: config/config_loader_hook.go
##########
@@ -0,0 +1,367 @@
+/*
+ * 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 config
+
+import (
+	"fmt"
+	"reflect"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/constant"
+)
+
+const (
+	hookParams = constant.HookParams
+)
+
+type LoaderHook interface {
+	emit() bool
+	// if you want to use emitWithParams, you must add HookParams field and make emit return false
+	emitWithParams(params interface{}) bool
+}
+
+func getLoaderHookType(t LoaderHook) reflect.Type {
+	hookType := reflect.TypeOf(t)
+	if reflect.Ptr == hookType.Kind() {
+		hookType = hookType.Elem()
+	}
+	return hookType
+}
+
+func getLoaderHookTypeString(t LoaderHook) string {
+	return getLoaderHookType(t).String()
+}
+
+func getLoaderHookInfo(v reflect.Value) reflect.Value {
+	if reflect.Ptr == v.Kind() {
+		v = v.Elem()
+	}
+	return v
+}
+
+func getLoaderHookParams(t LoaderHook) interface{} {
+	hookType := getLoaderHookType(t)
+	_, existed := hookType.FieldByName(hookParams)
+	if !existed {
+		panic(fmt.Sprintf("Hook field [%s] not existed for %s", hookParams, hookType.Name()))
+	}
+	hookParams := getLoaderHookInfo(reflect.ValueOf(t)).FieldByName(hookParams)
+	return getLoaderHookInfo(hookParams).Interface()
+}
+
+func AddLoaderHooks(hooks ...LoaderHook) {
+	if len(hooks) > 0 {
+		if loaderHooks == nil {
+			loaderHooks = map[string][]LoaderHook{}
+		}
+		for _, hook := range hooks {
+			hookType := getLoaderHookTypeString(hook)
+			var hooks []LoaderHook
+			if hs, ok := loaderHooks[hookType]; ok {
+				hooks = hs
+			}
+			if hooks == nil {
+				hooks = []LoaderHook{}
+			}
+			loaderHooks[hookType] = append(hooks, hook)
+		}
+	}
+}
+
+func RemoveLoaderHooks(hooks ...LoaderHook) {
+	if loaderHooks != nil && len(loaderHooks) > 0 {
+		for _, rmHook := range hooks {
+			hookType := getLoaderHookTypeString(rmHook)
+			if hooks, ok := loaderHooks[hookType]; ok {
+				for index, targetHook := range hooks {
+					if rmHook == targetHook {
+						newHooks := append(hooks[:index], hooks[index+1:]...)
+						if len(newHooks) == 0 {
+							delete(loaderHooks, hookType)
+						} else {
+							loaderHooks[hookType] = newHooks
+						}
+						break
+					}
+				}
+			}
+		}
+	}
+}
+
+func emitHook(t LoaderHook) {
+	if loaderHooks != nil && len(loaderHooks) > 0 {
+		hookType := getLoaderHookTypeString(t)
+		if hooks, ok := loaderHooks[hookType]; ok {
+			for _, hook := range hooks {
+				if !hook.emit() {
+					hook.emitWithParams(getLoaderHookParams(t))
+				}
+			}
+		}
+	}
+}
+
+// emit on before shutdown
+type BeforeShutdownHookEvent func()
+
+type beforeShutdownHook struct {
+	onEvent func()
+}
+
+func (h *beforeShutdownHook) emit() bool {
+	h.onEvent()
+	return true
+}
+
+func (h *beforeShutdownHook) emitWithParams(params interface{}) bool {
+	return params == nil
+}
+
+func NewBeforeShutdownHook(onEvent BeforeShutdownHookEvent) *beforeShutdownHook {
+	return &beforeShutdownHook{
+		onEvent,
+	}
+}
+
+// Consumer Hooks
+// emit on before export consumer
+type BeforeConsumerConnectHookEvent func(info *ConsumerConnectInfo)
+
+// emit on consumer export success
+type ConsumerConnectSuccessHookEvent func(info *ConsumerConnectInfo)
+
+// emit on consumer export fail
+type ConsumerConnectFailHookEvent func(info *ConsumerConnectFailInfo)
+
+// emit on all consumers export complete
+type AllConsumersConnectCompleteHookEvent func()

Review comment:
       AllConsumersConnectCompleteHookEvent -> ConsumerCompleteHookEvent




-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo-go] Mulavar commented on a change in pull request #1372: Ftr: Add config loader hooks for 1.5

Posted by GitBox <gi...@apache.org>.
Mulavar commented on a change in pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372#discussion_r694947646



##########
File path: config/config_loader_hook.go
##########
@@ -0,0 +1,367 @@
+/*
+ * 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 config
+
+import (
+	"fmt"
+	"reflect"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/constant"
+)
+
+const (
+	hookParams = constant.HookParams
+)
+
+type LoaderHook interface {
+	emit() bool
+	// if you want to use emitWithParams, you must add HookParams field and make emit return false
+	emitWithParams(params interface{}) bool
+}
+
+func getLoaderHookType(t LoaderHook) reflect.Type {
+	hookType := reflect.TypeOf(t)
+	if reflect.Ptr == hookType.Kind() {
+		hookType = hookType.Elem()
+	}
+	return hookType
+}
+
+func getLoaderHookTypeString(t LoaderHook) string {
+	return getLoaderHookType(t).String()
+}
+
+func getLoaderHookInfo(v reflect.Value) reflect.Value {
+	if reflect.Ptr == v.Kind() {
+		v = v.Elem()
+	}
+	return v
+}
+
+func getLoaderHookParams(t LoaderHook) interface{} {
+	hookType := getLoaderHookType(t)
+	_, existed := hookType.FieldByName(hookParams)
+	if !existed {
+		panic(fmt.Sprintf("Hook field [%s] not existed for %s", hookParams, hookType.Name()))
+	}
+	hookParams := getLoaderHookInfo(reflect.ValueOf(t)).FieldByName(hookParams)
+	return getLoaderHookInfo(hookParams).Interface()
+}
+
+func AddLoaderHooks(hooks ...LoaderHook) {
+	if len(hooks) > 0 {
+		if loaderHooks == nil {
+			loaderHooks = map[string][]LoaderHook{}
+		}
+		for _, hook := range hooks {
+			hookType := getLoaderHookTypeString(hook)
+			var hooks []LoaderHook
+			if hs, ok := loaderHooks[hookType]; ok {
+				hooks = hs
+			}
+			if hooks == nil {
+				hooks = []LoaderHook{}
+			}
+			loaderHooks[hookType] = append(hooks, hook)
+		}
+	}
+}
+
+func RemoveLoaderHooks(hooks ...LoaderHook) {
+	if loaderHooks != nil && len(loaderHooks) > 0 {
+		for _, rmHook := range hooks {
+			hookType := getLoaderHookTypeString(rmHook)
+			if hooks, ok := loaderHooks[hookType]; ok {
+				for index, targetHook := range hooks {
+					if rmHook == targetHook {
+						newHooks := append(hooks[:index], hooks[index+1:]...)
+						if len(newHooks) == 0 {
+							delete(loaderHooks, hookType)
+						} else {
+							loaderHooks[hookType] = newHooks
+						}
+						break
+					}

Review comment:
       1. 嵌套过深,建议将提前终止的分支代码放前面,比如最外层的if
   if loaderHooks == nil || len(loaderHooks) <= 0 {
       return
   }
   for _,  rmHook := range hooks { 
   // ...
   这样阅读上清晰一些
   2. 好像缩进有点问题,目前dubbogo都是4格。




-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo-go] wangxw666 commented on a change in pull request #1372: Ftr: Add config loader hooks for 1.5

Posted by GitBox <gi...@apache.org>.
wangxw666 commented on a change in pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372#discussion_r694450364



##########
File path: config/config_loader_hook.go
##########
@@ -0,0 +1,367 @@
+/*
+ * 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 config
+
+import (
+	"fmt"
+	"reflect"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/constant"
+)
+
+const (
+	hookParams = constant.HookParams
+)
+
+type LoaderHook interface {
+	emit() bool
+	// if you want to use emitWithParams, you must add HookParams field and make emit return false
+	emitWithParams(params interface{}) bool
+}
+
+func getLoaderHookType(t LoaderHook) reflect.Type {
+	hookType := reflect.TypeOf(t)
+	if reflect.Ptr == hookType.Kind() {
+		hookType = hookType.Elem()
+	}
+	return hookType
+}
+
+func getLoaderHookTypeString(t LoaderHook) string {
+	return getLoaderHookType(t).String()
+}
+
+func getLoaderHookInfo(v reflect.Value) reflect.Value {
+	if reflect.Ptr == v.Kind() {
+		v = v.Elem()
+	}
+	return v
+}
+
+func getLoaderHookParams(t LoaderHook) interface{} {
+	hookType := getLoaderHookType(t)
+	_, existed := hookType.FieldByName(hookParams)
+	if !existed {
+		panic(fmt.Sprintf("Hook field [%s] not existed for %s", hookParams, hookType.Name()))
+	}
+	hookParams := getLoaderHookInfo(reflect.ValueOf(t)).FieldByName(hookParams)
+	return getLoaderHookInfo(hookParams).Interface()
+}
+
+func AddLoaderHooks(hooks ...LoaderHook) {
+	if len(hooks) > 0 {
+		if loaderHooks == nil {
+			loaderHooks = map[string][]LoaderHook{}
+		}
+		for _, hook := range hooks {
+			hookType := getLoaderHookTypeString(hook)
+			var hooks []LoaderHook
+			if hs, ok := loaderHooks[hookType]; ok {
+				hooks = hs
+			}
+			if hooks == nil {
+				hooks = []LoaderHook{}
+			}
+			loaderHooks[hookType] = append(hooks, hook)
+		}
+	}
+}
+
+func RemoveLoaderHooks(hooks ...LoaderHook) {
+	if loaderHooks != nil && len(loaderHooks) > 0 {
+		for _, rmHook := range hooks {
+			hookType := getLoaderHookTypeString(rmHook)
+			if hooks, ok := loaderHooks[hookType]; ok {
+				for index, targetHook := range hooks {
+					if rmHook == targetHook {
+						newHooks := append(hooks[:index], hooks[index+1:]...)
+						if len(newHooks) == 0 {
+							delete(loaderHooks, hookType)
+						} else {
+							loaderHooks[hookType] = newHooks
+						}
+						break
+					}
+				}
+			}
+		}
+	}
+}
+
+func emitHook(t LoaderHook) {
+	if loaderHooks != nil && len(loaderHooks) > 0 {
+		hookType := getLoaderHookTypeString(t)
+		if hooks, ok := loaderHooks[hookType]; ok {
+			for _, hook := range hooks {
+				if !hook.emit() {
+					hook.emitWithParams(getLoaderHookParams(t))
+				}
+			}
+		}
+	}
+}
+
+// emit on before shutdown
+type BeforeShutdownHookEvent func()
+
+type beforeShutdownHook struct {
+	onEvent func()
+}
+
+func (h *beforeShutdownHook) emit() bool {
+	h.onEvent()
+	return true
+}
+
+func (h *beforeShutdownHook) emitWithParams(params interface{}) bool {
+	return params == nil
+}
+
+func NewBeforeShutdownHook(onEvent BeforeShutdownHookEvent) *beforeShutdownHook {
+	return &beforeShutdownHook{
+		onEvent,
+	}
+}
+
+// Consumer Hooks
+// emit on before export consumer
+type BeforeConsumerConnectHookEvent func(info *ConsumerConnectInfo)
+
+// emit on consumer export success
+type ConsumerConnectSuccessHookEvent func(info *ConsumerConnectInfo)
+
+// emit on consumer export fail

Review comment:
       consumer no "export"  ,  consumer is "reference"




-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org


[GitHub] [dubbo-go] wangxw666 commented on a change in pull request #1372: Ftr: Add config loader hooks for 1.5

Posted by GitBox <gi...@apache.org>.
wangxw666 commented on a change in pull request #1372:
URL: https://github.com/apache/dubbo-go/pull/1372#discussion_r694450945



##########
File path: config/config_loader_hook.go
##########
@@ -0,0 +1,367 @@
+/*
+ * 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 config
+
+import (
+	"fmt"
+	"reflect"
+)
+
+import (
+	"github.com/apache/dubbo-go/common/constant"
+)
+
+const (
+	hookParams = constant.HookParams
+)
+
+type LoaderHook interface {
+	emit() bool
+	// if you want to use emitWithParams, you must add HookParams field and make emit return false
+	emitWithParams(params interface{}) bool
+}
+
+func getLoaderHookType(t LoaderHook) reflect.Type {
+	hookType := reflect.TypeOf(t)
+	if reflect.Ptr == hookType.Kind() {
+		hookType = hookType.Elem()
+	}
+	return hookType
+}
+
+func getLoaderHookTypeString(t LoaderHook) string {
+	return getLoaderHookType(t).String()
+}
+
+func getLoaderHookInfo(v reflect.Value) reflect.Value {
+	if reflect.Ptr == v.Kind() {
+		v = v.Elem()
+	}
+	return v
+}
+
+func getLoaderHookParams(t LoaderHook) interface{} {
+	hookType := getLoaderHookType(t)
+	_, existed := hookType.FieldByName(hookParams)
+	if !existed {
+		panic(fmt.Sprintf("Hook field [%s] not existed for %s", hookParams, hookType.Name()))
+	}
+	hookParams := getLoaderHookInfo(reflect.ValueOf(t)).FieldByName(hookParams)
+	return getLoaderHookInfo(hookParams).Interface()
+}
+
+func AddLoaderHooks(hooks ...LoaderHook) {
+	if len(hooks) > 0 {
+		if loaderHooks == nil {
+			loaderHooks = map[string][]LoaderHook{}
+		}
+		for _, hook := range hooks {
+			hookType := getLoaderHookTypeString(hook)
+			var hooks []LoaderHook
+			if hs, ok := loaderHooks[hookType]; ok {
+				hooks = hs
+			}
+			if hooks == nil {
+				hooks = []LoaderHook{}
+			}
+			loaderHooks[hookType] = append(hooks, hook)
+		}
+	}
+}
+
+func RemoveLoaderHooks(hooks ...LoaderHook) {
+	if loaderHooks != nil && len(loaderHooks) > 0 {
+		for _, rmHook := range hooks {
+			hookType := getLoaderHookTypeString(rmHook)
+			if hooks, ok := loaderHooks[hookType]; ok {
+				for index, targetHook := range hooks {
+					if rmHook == targetHook {
+						newHooks := append(hooks[:index], hooks[index+1:]...)
+						if len(newHooks) == 0 {
+							delete(loaderHooks, hookType)
+						} else {
+							loaderHooks[hookType] = newHooks
+						}
+						break
+					}
+				}
+			}
+		}
+	}
+}
+
+func emitHook(t LoaderHook) {
+	if loaderHooks != nil && len(loaderHooks) > 0 {
+		hookType := getLoaderHookTypeString(t)
+		if hooks, ok := loaderHooks[hookType]; ok {
+			for _, hook := range hooks {
+				if !hook.emit() {
+					hook.emitWithParams(getLoaderHookParams(t))
+				}
+			}
+		}
+	}
+}
+
+// emit on before shutdown
+type BeforeShutdownHookEvent func()
+
+type beforeShutdownHook struct {
+	onEvent func()
+}
+
+func (h *beforeShutdownHook) emit() bool {
+	h.onEvent()
+	return true
+}
+
+func (h *beforeShutdownHook) emitWithParams(params interface{}) bool {
+	return params == nil
+}
+
+func NewBeforeShutdownHook(onEvent BeforeShutdownHookEvent) *beforeShutdownHook {
+	return &beforeShutdownHook{
+		onEvent,
+	}
+}
+
+// Consumer Hooks
+// emit on before export consumer
+type BeforeConsumerConnectHookEvent func(info *ConsumerConnectInfo)
+
+// emit on consumer export success
+type ConsumerConnectSuccessHookEvent func(info *ConsumerConnectInfo)

Review comment:
       我觉得ConsumerConnectSuccessHookEvent 叫做 ServiceReferenceSuccessHookEvent 比较合适




-- 
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: notifications-unsubscribe@dubbo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org