dam4rus commented on code in PR #6838: URL: https://github.com/apache/nifi/pull/6838#discussion_r1086312485
########## nifi-toolkit/nifi-toolkit-kafka-migrator/src/main/java/org/apache/nifi/toolkit/kafkamigrator/service/KafkaMigrationService.java: ########## @@ -0,0 +1,71 @@ +/* + * 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.nifi.toolkit.kafkamigrator.service; + +import org.apache.commons.lang3.StringUtils; +import org.apache.nifi.toolkit.kafkamigrator.migrator.Migrator; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; + +import javax.xml.xpath.XPath; +import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathExpressionException; +import javax.xml.xpath.XPathFactory; + +import org.apache.nifi.toolkit.kafkamigrator.MigratorConfiguration.MigratorConfigurationBuilder; + +public interface KafkaMigrationService { + + String REGEX_FOR_REPLACEABLE_PROCESSOR_NAMES = "(Get|Put|Consume|Publish)Kafka(Record)?(_0_1\\d)?"; + boolean IS_VERSION_EIGHT_PROCESSOR = Boolean.TRUE; + boolean IS_NOT_VERSION_EIGHT_PROCESSOR = Boolean.FALSE; + + String getPathForProcessors(); + String getPathForClass(); + Migrator createPublishMigrator(final MigratorConfigurationBuilder configurationBuilder); + Migrator createConsumeMigrator(final MigratorConfigurationBuilder configurationBuilder); + Migrator createVersionEightPublishMigrator(final MigratorConfigurationBuilder configurationBuilder); + Migrator createVersionEightConsumeMigrator(final MigratorConfigurationBuilder configurationBuilder); + + default void replaceKafkaProcessors(final Document document, final MigratorConfigurationBuilder configurationBuilder) throws XPathExpressionException { + Migrator migrator; + final XPath xPath = XPathFactory.newInstance().newXPath(); + + final NodeList processors = (NodeList) xPath.evaluate(getPathForProcessors(), document, XPathConstants.NODESET); + for (int i = 0; i < processors.getLength(); i++) { + final Node processor = processors.item(i); + final Element className = ((Element) xPath.evaluate(getPathForClass(), processor, XPathConstants.NODE)); + final String processorName = StringUtils.substringAfterLast(className.getTextContent(), "."); + + if (processorName.matches(REGEX_FOR_REPLACEABLE_PROCESSOR_NAMES)) { + if (processorName.contains("Publish")) { + migrator = createPublishMigrator(configurationBuilder); Review Comment: Shouldn't `configurationBuilder` be cloned here or inside `createPublishMigrator`? It could lead to confusion if someone would extend this code and there would be some properties set in a previous iteration that shouldn't be set. If this is just some "throwaway" code this is fine but it's "better safe than sorry" ########## nifi-toolkit/nifi-toolkit-kafka-migrator/src/main/java/org/apache/nifi/toolkit/kafkamigrator/migrator/AbstractKafkaMigrator.java: ########## @@ -0,0 +1,195 @@ +/* + * 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.nifi.toolkit.kafkamigrator.migrator; + +import org.apache.commons.lang3.StringUtils; +import org.apache.nifi.toolkit.kafkamigrator.MigratorConfiguration; +import org.apache.nifi.toolkit.kafkamigrator.MigratorConfiguration.MigratorConfigurationBuilder; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; + +import javax.xml.xpath.XPath; +import javax.xml.xpath.XPathConstants; +import javax.xml.xpath.XPathExpressionException; +import javax.xml.xpath.XPathFactory; +import java.util.HashMap; +import java.util.Map; + +public abstract class AbstractKafkaMigrator implements Migrator { + static final XPath XPATH = XPathFactory.newInstance().newXPath(); + private final static String NEW_KAFKA_PROCESSOR_VERSION = "_2_0"; + private final static String ARTIFACT = "nifi-kafka-2-0-nar"; + private final static String PATH_FOR_ARTIFACT = "bundle/artifact"; + + final boolean isVersion8Processor; + final boolean isKafkaBrokersPresent; + + final Map<String, String> kafkaProcessorProperties; + final Map<String, String> propertiesToBeSaved; + final Map<String, String> controllerServices; + + final String xpathForProperties; + final String propertyKeyTagName; + final String propertyTagName; + + final String xpathForTransactionProperty; + final String transactionTagName; + final boolean transaction; + + + public AbstractKafkaMigrator(final MigratorConfigurationBuilder configurationBuilder) { + final MigratorConfiguration configuration = configurationBuilder.build(); Review Comment: Just my opinion, but passing MigrationConfiguration instead of the builder would be a bit cleaner since we don't do anything with it. Looking at the call from the call site could lead you to believe that the builder is modified here. ########## nifi-toolkit/nifi-toolkit-kafka-migrator/src/main/java/org/apache/nifi/toolkit/kafkamigrator/service/KafkaFlowMigrationService.java: ########## @@ -0,0 +1,75 @@ +/* + * 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.nifi.toolkit.kafkamigrator.service; + +import org.apache.nifi.toolkit.kafkamigrator.descriptor.KafkaProcessorDescriptor; +import org.apache.nifi.toolkit.kafkamigrator.migrator.ConsumeKafkaFlowMigrator; +import org.apache.nifi.toolkit.kafkamigrator.migrator.Migrator; +import org.apache.nifi.toolkit.kafkamigrator.migrator.PublishKafkaFlowMigrator; +import org.apache.nifi.toolkit.kafkamigrator.MigratorConfiguration.MigratorConfigurationBuilder; +import org.apache.nifi.toolkit.kafkamigrator.descriptor.FlowPropertyXpathDescriptor; + + +public class KafkaFlowMigrationService implements KafkaMigrationService { + private static final String XPATH_FOR_PROCESSORS_IN_FLOW = ".//processor"; + private static final String CLASS_TAG_NAME = "class"; + + public KafkaFlowMigrationService() { + } + + @Override + public String getPathForProcessors() { + return XPATH_FOR_PROCESSORS_IN_FLOW; + } + + @Override + public String getPathForClass() { + return CLASS_TAG_NAME; + } + + @Override + public Migrator createPublishMigrator(final MigratorConfigurationBuilder configurationBuilder) { + configurationBuilder.setIsVersion8Processor(IS_NOT_VERSION_EIGHT_PROCESSOR) + .setProcessorDescriptor(new KafkaProcessorDescriptor("Publish")) Review Comment: `"Publish"` and `"Consume"` could be extracted to an enum or constants -- 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]
