exceptionfactory commented on code in PR #10849: URL: https://github.com/apache/nifi/pull/10849#discussion_r2772341726
########## nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/pg/ImportFlowWithIncompatibleBundleIT.java: ########## @@ -0,0 +1,190 @@ +/* + * 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.tests.system.pg; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.nifi.flow.Bundle; +import org.apache.nifi.flow.ParameterProviderReference; +import org.apache.nifi.flow.VersionedParameter; +import org.apache.nifi.flow.VersionedParameterContext; +import org.apache.nifi.flow.VersionedProcessGroup; +import org.apache.nifi.registry.flow.RegisteredFlowSnapshot; +import org.apache.nifi.tests.system.NiFiSystemIT; +import org.apache.nifi.toolkit.client.NiFiClientException; +import org.apache.nifi.web.api.dto.ParameterProviderDTO; +import org.apache.nifi.web.api.entity.ParameterProviderEntity; +import org.apache.nifi.web.api.entity.ParameterProvidersEntity; +import org.apache.nifi.web.api.entity.ProcessGroupEntity; +import org.junit.jupiter.api.Test; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +/** + * System tests to verify that importing/uploading a flow with components that have incompatible bundle versions + * properly falls back to using available bundle versions instead of creating Ghost components. + */ +public class ImportFlowWithIncompatibleBundleIT extends NiFiSystemIT { + + private static final String PROPERTIES_PARAMETER_PROVIDER_TYPE = TEST_PARAM_PROVIDERS_PACKAGE + ".PropertiesParameterProvider"; + private static final String INCOMPATIBLE_VERSION = "0.0.0-DOES-NOT-EXIST"; + + /** + * Tests that when uploading a flow with a ParameterContext that references a ParameterProvider with an + * incompatible bundle version, NiFi falls back to using the only available bundle version instead of + * creating a Ghost component. + * + * This test verifies the fix for a bug where the bundle version fallback logic was not applied to + * ParameterProviders during flow upload, causing them to be ghosted even when only one compatible + * version was available. + * + * This test specifically uses the upload endpoint (POST /process-groups/{id}/process-groups/upload) + * which exercises the code path in ProcessGroupResource that requires the fix. + */ + @Test + public void testUploadFlowWithParameterProviderIncompatibleBundleVersion() throws NiFiClientException, IOException, InterruptedException { + // Build a versioned flow snapshot with a parameter provider that has an incompatible bundle version + final RegisteredFlowSnapshot snapshot = createSnapshotWithParameterProviderIncompatibleBundle(); + + // Serialize the snapshot to a temporary JSON file + final File tempFile = Files.createTempFile("flow-snapshot", ".json").toFile(); + tempFile.deleteOnExit(); Review Comment: This should be replaced with using the `TempDir` annotation so that JUnit manages the file lifecycle. ########## nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/pg/ImportFlowWithIncompatibleBundleIT.java: ########## @@ -0,0 +1,190 @@ +/* + * 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.tests.system.pg; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.nifi.flow.Bundle; +import org.apache.nifi.flow.ParameterProviderReference; +import org.apache.nifi.flow.VersionedParameter; +import org.apache.nifi.flow.VersionedParameterContext; +import org.apache.nifi.flow.VersionedProcessGroup; +import org.apache.nifi.registry.flow.RegisteredFlowSnapshot; +import org.apache.nifi.tests.system.NiFiSystemIT; +import org.apache.nifi.toolkit.client.NiFiClientException; +import org.apache.nifi.web.api.dto.ParameterProviderDTO; +import org.apache.nifi.web.api.entity.ParameterProviderEntity; +import org.apache.nifi.web.api.entity.ParameterProvidersEntity; +import org.apache.nifi.web.api.entity.ProcessGroupEntity; +import org.junit.jupiter.api.Test; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +/** + * System tests to verify that importing/uploading a flow with components that have incompatible bundle versions + * properly falls back to using available bundle versions instead of creating Ghost components. + */ +public class ImportFlowWithIncompatibleBundleIT extends NiFiSystemIT { + + private static final String PROPERTIES_PARAMETER_PROVIDER_TYPE = TEST_PARAM_PROVIDERS_PACKAGE + ".PropertiesParameterProvider"; + private static final String INCOMPATIBLE_VERSION = "0.0.0-DOES-NOT-EXIST"; + + /** + * Tests that when uploading a flow with a ParameterContext that references a ParameterProvider with an + * incompatible bundle version, NiFi falls back to using the only available bundle version instead of + * creating a Ghost component. + * + * This test verifies the fix for a bug where the bundle version fallback logic was not applied to + * ParameterProviders during flow upload, causing them to be ghosted even when only one compatible + * version was available. + * + * This test specifically uses the upload endpoint (POST /process-groups/{id}/process-groups/upload) + * which exercises the code path in ProcessGroupResource that requires the fix. + */ + @Test + public void testUploadFlowWithParameterProviderIncompatibleBundleVersion() throws NiFiClientException, IOException, InterruptedException { + // Build a versioned flow snapshot with a parameter provider that has an incompatible bundle version + final RegisteredFlowSnapshot snapshot = createSnapshotWithParameterProviderIncompatibleBundle(); + + // Serialize the snapshot to a temporary JSON file + final File tempFile = Files.createTempFile("flow-snapshot", ".json").toFile(); + tempFile.deleteOnExit(); + final ObjectMapper objectMapper = new ObjectMapper(); + objectMapper.writeValue(tempFile, snapshot); + + // Upload the flow using the upload endpoint (this goes through ProcessGroupResource, not FlowUpdateResource) + final ProcessGroupEntity uploadedGroup = getNifiClient().getProcessGroupClient().upload( + "root", + tempFile, + "Uploaded Test Group", + 100.0, + 100.0 + ); + + assertNotNull(uploadedGroup, "Uploaded process group should not be null"); + assertNotNull(uploadedGroup.getId(), "Uploaded process group should have an ID"); + + // Verify that the parameter provider was created and is NOT a ghost component + final ParameterProvidersEntity parameterProviders = getNifiClient().getFlowClient().getParamProviders(); + assertNotNull(parameterProviders); + assertNotNull(parameterProviders.getParameterProviders()); + assertFalse(parameterProviders.getParameterProviders().isEmpty(), + "Expected at least one parameter provider to be created"); + + // Find the parameter provider that was created + final ParameterProviderEntity createdProvider = parameterProviders.getParameterProviders().stream() + .filter(pp -> pp.getComponent().getType().equals(PROPERTIES_PARAMETER_PROVIDER_TYPE)) + .findFirst() + .orElse(null); + + assertNotNull(createdProvider, "Expected PropertiesParameterProvider to be created"); + + final ParameterProviderDTO providerDto = createdProvider.getComponent(); + assertNotNull(providerDto); + + // The key assertion: the bundle version should be the actual NiFi version (fallback), + // NOT the incompatible version we specified in the flow + assertNotNull(providerDto.getBundle(), "Bundle should not be null"); + assertFalse(INCOMPATIBLE_VERSION.equals(providerDto.getBundle().getVersion()), + "Bundle version should NOT be the incompatible version - should have fallen back to available version"); + + // The bundle version should be the actual NiFi version + assertEquals(getNiFiVersion(), providerDto.getBundle().getVersion(), + "Bundle version should be the NiFi framework version (fallback)"); + + // Verify the component is not a ghost by checking that it's valid or has actual validation errors + // (Ghost components have a specific type pattern like "(Missing) ClassName") + assertFalse(providerDto.getType().startsWith("(Missing)"), + "Parameter provider should not be a Ghost component - type should not start with '(Missing)'"); + + // Also verify the type is exactly what we expected Review Comment: These comments seem unnecessary in light of the assertion messages. -- 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]
