Pil0tXia commented on code in PR #98: URL: https://github.com/apache/eventmesh-dashboard/pull/98#discussion_r1564460811
########## eventmesh-dashboard-console/src/main/java/org/apache/eventmesh/dashboard/console/function/metadata/util/convert/ConverterFactory.java: ########## @@ -0,0 +1,70 @@ +/* + * 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 org.apache.eventmesh.dashboard.console.function.metadata.util.convert; + +import org.apache.eventmesh.dashboard.common.model.metadata.ClientMetadata; +import org.apache.eventmesh.dashboard.common.model.metadata.ClusterMetadata; +import org.apache.eventmesh.dashboard.common.model.metadata.ConfigMetadata; +import org.apache.eventmesh.dashboard.common.model.metadata.ConnectorMetadata; +import org.apache.eventmesh.dashboard.common.model.metadata.GroupMetadata; +import org.apache.eventmesh.dashboard.common.model.metadata.RegistryMetadata; +import org.apache.eventmesh.dashboard.common.model.metadata.RuntimeMetadata; +import org.apache.eventmesh.dashboard.common.model.metadata.TopicMetadata; +import org.apache.eventmesh.dashboard.console.entity.meta.MetaEntity; +import org.apache.eventmesh.dashboard.console.entity.runtime.RuntimeEntity; +import org.apache.eventmesh.dashboard.console.entity.topic.TopicEntity; +import org.apache.eventmesh.dashboard.console.function.metadata.util.convert.entity.RegistryEntity2MetadataConverter; +import org.apache.eventmesh.dashboard.console.function.metadata.util.convert.entity.RuntimeEntity2MetadataConverter; +import org.apache.eventmesh.dashboard.console.function.metadata.util.convert.entity.TopicEntity2MetadataConverter; +import org.apache.eventmesh.dashboard.console.function.metadata.util.convert.metadata.ClientMetadata2EntityConverter; +import org.apache.eventmesh.dashboard.console.function.metadata.util.convert.metadata.ClusterMetadata2EntityConverter; +import org.apache.eventmesh.dashboard.console.function.metadata.util.convert.metadata.ConfigMetadata2EntityConverter; +import org.apache.eventmesh.dashboard.console.function.metadata.util.convert.metadata.ConnectionMetadata2EntityConverter; +import org.apache.eventmesh.dashboard.console.function.metadata.util.convert.metadata.RegistryMetadata2EntityConverter; +import org.apache.eventmesh.dashboard.console.function.metadata.util.convert.metadata.RuntimeMetadata2EntityConverter; +import org.apache.eventmesh.dashboard.console.function.metadata.util.convert.metadata.TopicMetadata2EntityConverter; + +public class ConverterFactory { + public static Converter<?, ?> createConverter(Class<?> sourceType) { + if (sourceType == MetaEntity.class) { + return new RegistryMetadata2EntityConverter(); + } else if (sourceType == RegistryMetadata.class) { + return new RegistryEntity2MetadataConverter(); + } else if (sourceType == RuntimeEntity.class) { + return new RuntimeEntity2MetadataConverter(); + } else if (sourceType == RuntimeMetadata.class) { + return new RuntimeMetadata2EntityConverter(); + } else if (sourceType == TopicMetadata.class) { + return new TopicMetadata2EntityConverter(); + } else if (sourceType == TopicEntity.class) { + return new TopicEntity2MetadataConverter(); + } else if (sourceType == ClientMetadata.class) { + return new ClientMetadata2EntityConverter(); + } else if (sourceType == ClusterMetadata.class) { + return new ClusterMetadata2EntityConverter(); + } else if (sourceType == ConfigMetadata.class) { + return new ConfigMetadata2EntityConverter(); + } else if (sourceType == ConnectorMetadata.class) { + return new ConnectionMetadata2EntityConverter(); + } else if (sourceType == GroupMetadata.class) { + return new ConnectionMetadata2EntityConverter(); + } + // Add more conditions here for other types + throw new IllegalArgumentException("No converter found for type " + sourceType); Review Comment: switch-case is definitely better in this case. ########## eventmesh-dashboard-core/src/main/java/org/apache/eventmesh/dashboard/core/meta/runtime/NacosRuntimeCore.java: ########## @@ -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 org.apache.eventmesh.dashboard.core.meta.runtime; + +import org.apache.eventmesh.dashboard.common.model.metadata.RuntimeMetadata; +import org.apache.eventmesh.dashboard.common.model.remoting.runtime.GetRuntimeRequest; +import org.apache.eventmesh.dashboard.common.model.remoting.runtime.GetRuntimeResponse; +import org.apache.eventmesh.dashboard.common.model.remoting.runtime.GetRuntimeResult; +import org.apache.eventmesh.dashboard.core.function.SDK.SDKManager; +import org.apache.eventmesh.dashboard.core.function.SDK.SDKTypeEnum; +import org.apache.eventmesh.dashboard.core.function.SDK.config.CreateNacosConfig; +import org.apache.eventmesh.dashboard.service.remoting.MetaRemotingService; + +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.CompletableFuture; + +import com.alibaba.nacos.api.exception.NacosException; +import com.alibaba.nacos.api.naming.pojo.Instance; +import com.alibaba.nacos.client.naming.NacosNamingService; + +public class NacosRuntimeCore implements MetaRemotingService { + + @Override + public GetRuntimeResult getRuntime(GetRuntimeRequest getRuntimeRequest) { + CreateNacosConfig createNacosConfig = new CreateNacosConfig(); + createNacosConfig.setServerAddress(getRuntimeRequest.getRegistryAddress()); + NacosNamingService nacosNamingService = (NacosNamingService) SDKManager.getInstance() + .createClient(SDKTypeEnum.META_NACOS_NAMING, createNacosConfig).getValue(); + GetRuntimeResult getRuntimeResult = new GetRuntimeResult(); + + CompletableFuture<GetRuntimeResponse> future = CompletableFuture.supplyAsync(() -> { + try { + List<Instance> instances = + nacosNamingService.getAllInstances("EVENTMESH-runtime-GRPC", "GRPC-GROUP"); Review Comment: Why do we only get the gRPC protocol? Is it because one Runtime will register three protocols in Meta? If so, consider what happens if users disable gRPC protocol, the Dashboard may not be able to get Runtime instance by then. I would suggest to unify protocols with the same IP to one Runtime instance. ########## eventmesh-dashboard-console/src/main/java/org/apache/eventmesh/dashboard/console/function/metadata/util/convert/entity/TopicEntity2MetadataConverter.java: ########## @@ -0,0 +1,42 @@ +/* + * 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 org.apache.eventmesh.dashboard.console.function.metadata.util.convert.entity; + +import org.apache.eventmesh.dashboard.common.model.metadata.TopicMetadata; +import org.apache.eventmesh.dashboard.console.entity.topic.TopicEntity; +import org.apache.eventmesh.dashboard.console.function.metadata.util.convert.Converter; + +public class TopicEntity2MetadataConverter implements Converter<TopicEntity, TopicMetadata> { + + @Override + public TopicMetadata convert(TopicEntity source) { + return + TopicMetadata.builder() + .topicName(source.getTopicName()) + .storageId(source.getStorageId()) + .retentionMs(source.getRetentionMs()) + .type(source.getType()) + .description(source.getDescription()) + .build(); + } Review Comment: I found that all field names are same before and after, would it be better to use a `public TopicMetadata(TopicEntity source)` constructor instead to drop these converter classes? ########## eventmesh-dashboard-console/src/main/java/org/apache/eventmesh/dashboard/console/controller/MetaController.java: ########## @@ -0,0 +1,54 @@ +/* + * 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 org.apache.eventmesh.dashboard.console.controller; + +import org.apache.eventmesh.dashboard.console.entity.meta.MetaEntity; +import org.apache.eventmesh.dashboard.console.modle.dto.meta.NewMetaDTO; +import org.apache.eventmesh.dashboard.console.service.registry.RegistryDataService; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.validation.annotation.Validated; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class MetaController { + + @Autowired + private RegistryDataService metaService; + + @PostMapping("/meta/newMeta") + public String addRegistry(@Validated @RequestBody NewMetaDTO newMetaDTO) { + MetaEntity query = new MetaEntity(); + query.setName(""); + query.setType(""); + query.setVersion(""); + query.setClusterId(0L); + query.setHost(newMetaDTO.getMetaHost()); + query.setPort(newMetaDTO.getMetaPort()); + query.setRole(""); + query.setUsername(""); + query.setParams(""); + query.setClusterId(0L); + + metaService.insert(query); + + return "success"; + } Review Comment: I don't think we need to add a Registry on front-end manually. When adding an EventMesh Cluster, the Registry address and namespace are specified and should be stored into `ClusterEntity` as fields. When discovering EventMesh Clusters from a Registry address, the Registry address and namespaces which each contains an EventMesh Cluster should be stored into multiple `ClusterEntity`s as fields either. Because the sum of the registry does not represent the sum of the cluster, it is very difficult to try to store the registry entities and then manage the cluster. ########## eventmesh-dashboard-console/src/main/java/org/apache/eventmesh/dashboard/console/controller/MetaController.java: ########## @@ -0,0 +1,54 @@ +/* + * 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 org.apache.eventmesh.dashboard.console.controller; + +import org.apache.eventmesh.dashboard.console.entity.meta.MetaEntity; +import org.apache.eventmesh.dashboard.console.modle.dto.meta.NewMetaDTO; +import org.apache.eventmesh.dashboard.console.service.registry.RegistryDataService; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.validation.annotation.Validated; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class MetaController { + + @Autowired + private RegistryDataService metaService; Review Comment: Naming like this may be confusing. Better unify the member name, class name, method name and endpoint name together. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
