exceptionfactory commented on code in PR #6994: URL: https://github.com/apache/nifi/pull/6994#discussion_r1122457755
########## nifi-nar-bundles/nifi-standard-services/nifi-distributed-cache-services-bundle/nifi-distributed-cache-server/src/test/java/org/apache/nifi/distributed/cache/server/map/TestMapCacheClient.java: ########## @@ -0,0 +1,146 @@ +/* + * 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.distributed.cache.server.map; + +import org.apache.commons.lang3.SerializationException; +import org.apache.nifi.components.PropertyDescriptor; +import org.apache.nifi.distributed.cache.client.DistributedMapCacheClientService; +import org.apache.nifi.distributed.cache.client.Serializer; +import org.apache.nifi.distributed.cache.operations.MapOperation; +import org.apache.nifi.distributed.cache.protocol.ProtocolVersion; +import org.apache.nifi.distributed.cache.server.EvictionPolicy; +import org.apache.nifi.distributed.cache.server.codec.CacheOperationResultEncoder; +import org.apache.nifi.distributed.cache.server.codec.CacheVersionRequestHandler; +import org.apache.nifi.distributed.cache.server.codec.CacheVersionResponseEncoder; +import org.apache.nifi.distributed.cache.server.codec.MapCacheRequestDecoder; +import org.apache.nifi.distributed.cache.server.codec.MapCacheRequestHandler; +import org.apache.nifi.distributed.cache.server.codec.MapRemoveResponseEncoder; +import org.apache.nifi.distributed.cache.server.codec.MapSizeResponseEncoder; +import org.apache.nifi.distributed.cache.server.codec.MapValueResponseEncoder; +import org.apache.nifi.event.transport.EventServer; +import org.apache.nifi.event.transport.configuration.ShutdownQuietPeriod; +import org.apache.nifi.event.transport.configuration.TransportProtocol; +import org.apache.nifi.event.transport.netty.NettyEventServerFactory; +import org.apache.nifi.logging.ComponentLog; +import org.apache.nifi.processor.Processor; +import org.apache.nifi.remote.StandardVersionNegotiator; +import org.apache.nifi.remote.VersionNegotiator; +import org.apache.nifi.remote.io.socket.NetworkUtils; +import org.apache.nifi.reporting.InitializationException; +import org.apache.nifi.util.MockConfigurationContext; +import org.apache.nifi.util.MockControllerServiceInitializationContext; +import org.apache.nifi.util.TestRunner; +import org.apache.nifi.util.TestRunners; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetAddress; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class TestMapCacheClient { + private int port; + private EventServer server; + private final Serializer<String> stringSerializer = new StringSerializer(); + + @BeforeEach + public void setRunner() { + final TestRunner runner = TestRunners.newTestRunner(Mockito.mock(Processor.class)); + port = NetworkUtils.getAvailableTcpPort(); + + final NettyEventServerFactory serverFactory = new NettyEventServerFactory( + InetAddress.getLoopbackAddress(), port, TransportProtocol.TCP); Review Comment: The `InetAddress.getLoopbackAddress()` call can behave differently in some environments. Instead, recommend using `InetAddress.getByName("127.0.0.1")` and also using `127.0.0.1` as the client address to avoid any name resolution concerns. ########## nifi-nar-bundles/nifi-standard-services/nifi-distributed-cache-services-bundle/nifi-distributed-cache-server/src/test/java/org/apache/nifi/distributed/cache/server/map/TestMapCacheClient.java: ########## @@ -0,0 +1,146 @@ +/* + * 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.distributed.cache.server.map; + +import org.apache.commons.lang3.SerializationException; +import org.apache.nifi.components.PropertyDescriptor; +import org.apache.nifi.distributed.cache.client.DistributedMapCacheClientService; +import org.apache.nifi.distributed.cache.client.Serializer; +import org.apache.nifi.distributed.cache.operations.MapOperation; +import org.apache.nifi.distributed.cache.protocol.ProtocolVersion; +import org.apache.nifi.distributed.cache.server.EvictionPolicy; +import org.apache.nifi.distributed.cache.server.codec.CacheOperationResultEncoder; +import org.apache.nifi.distributed.cache.server.codec.CacheVersionRequestHandler; +import org.apache.nifi.distributed.cache.server.codec.CacheVersionResponseEncoder; +import org.apache.nifi.distributed.cache.server.codec.MapCacheRequestDecoder; +import org.apache.nifi.distributed.cache.server.codec.MapCacheRequestHandler; +import org.apache.nifi.distributed.cache.server.codec.MapRemoveResponseEncoder; +import org.apache.nifi.distributed.cache.server.codec.MapSizeResponseEncoder; +import org.apache.nifi.distributed.cache.server.codec.MapValueResponseEncoder; +import org.apache.nifi.event.transport.EventServer; +import org.apache.nifi.event.transport.configuration.ShutdownQuietPeriod; +import org.apache.nifi.event.transport.configuration.TransportProtocol; +import org.apache.nifi.event.transport.netty.NettyEventServerFactory; +import org.apache.nifi.logging.ComponentLog; +import org.apache.nifi.processor.Processor; +import org.apache.nifi.remote.StandardVersionNegotiator; +import org.apache.nifi.remote.VersionNegotiator; +import org.apache.nifi.remote.io.socket.NetworkUtils; +import org.apache.nifi.reporting.InitializationException; +import org.apache.nifi.util.MockConfigurationContext; +import org.apache.nifi.util.MockControllerServiceInitializationContext; +import org.apache.nifi.util.TestRunner; +import org.apache.nifi.util.TestRunners; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetAddress; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class TestMapCacheClient { + private int port; + private EventServer server; + private final Serializer<String> stringSerializer = new StringSerializer(); + + @BeforeEach + public void setRunner() { + final TestRunner runner = TestRunners.newTestRunner(Mockito.mock(Processor.class)); + port = NetworkUtils.getAvailableTcpPort(); + + final NettyEventServerFactory serverFactory = new NettyEventServerFactory( + InetAddress.getLoopbackAddress(), port, TransportProtocol.TCP); + serverFactory.setShutdownQuietPeriod(ShutdownQuietPeriod.QUICK.getDuration()); + serverFactory.setShutdownTimeout(ShutdownQuietPeriod.QUICK.getDuration()); + final ComponentLog log = runner.getLogger(); + final VersionNegotiator versionNegotiator = new StandardVersionNegotiator( + ProtocolVersion.V3.value(), ProtocolVersion.V2.value(), ProtocolVersion.V1.value()); + + final MapCache mapCache = new SimpleMapCache("serviceIdentifier", 64, EvictionPolicy.FIFO) { + // override a service function that will cause client network timeout to fire + @Override + public MapPutResult put(ByteBuffer key, ByteBuffer value) throws IOException { + try { + Thread.sleep(3000L); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + return super.put(key, value); Review Comment: Is this line necessary? It looks like it should not be invoked on a successful test, and thus seems unnecessary on failures. ########## nifi-nar-bundles/nifi-standard-services/nifi-distributed-cache-services-bundle/nifi-distributed-cache-server/src/test/java/org/apache/nifi/distributed/cache/server/map/TestMapCacheClient.java: ########## @@ -0,0 +1,146 @@ +/* + * 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.distributed.cache.server.map; + +import org.apache.commons.lang3.SerializationException; +import org.apache.nifi.components.PropertyDescriptor; +import org.apache.nifi.distributed.cache.client.DistributedMapCacheClientService; +import org.apache.nifi.distributed.cache.client.Serializer; +import org.apache.nifi.distributed.cache.operations.MapOperation; +import org.apache.nifi.distributed.cache.protocol.ProtocolVersion; +import org.apache.nifi.distributed.cache.server.EvictionPolicy; +import org.apache.nifi.distributed.cache.server.codec.CacheOperationResultEncoder; +import org.apache.nifi.distributed.cache.server.codec.CacheVersionRequestHandler; +import org.apache.nifi.distributed.cache.server.codec.CacheVersionResponseEncoder; +import org.apache.nifi.distributed.cache.server.codec.MapCacheRequestDecoder; +import org.apache.nifi.distributed.cache.server.codec.MapCacheRequestHandler; +import org.apache.nifi.distributed.cache.server.codec.MapRemoveResponseEncoder; +import org.apache.nifi.distributed.cache.server.codec.MapSizeResponseEncoder; +import org.apache.nifi.distributed.cache.server.codec.MapValueResponseEncoder; +import org.apache.nifi.event.transport.EventServer; +import org.apache.nifi.event.transport.configuration.ShutdownQuietPeriod; +import org.apache.nifi.event.transport.configuration.TransportProtocol; +import org.apache.nifi.event.transport.netty.NettyEventServerFactory; +import org.apache.nifi.logging.ComponentLog; +import org.apache.nifi.processor.Processor; +import org.apache.nifi.remote.StandardVersionNegotiator; +import org.apache.nifi.remote.VersionNegotiator; +import org.apache.nifi.remote.io.socket.NetworkUtils; +import org.apache.nifi.reporting.InitializationException; +import org.apache.nifi.util.MockConfigurationContext; +import org.apache.nifi.util.MockControllerServiceInitializationContext; +import org.apache.nifi.util.TestRunner; +import org.apache.nifi.util.TestRunners; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetAddress; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class TestMapCacheClient { + private int port; + private EventServer server; + private final Serializer<String> stringSerializer = new StringSerializer(); + + @BeforeEach + public void setRunner() { + final TestRunner runner = TestRunners.newTestRunner(Mockito.mock(Processor.class)); + port = NetworkUtils.getAvailableTcpPort(); + + final NettyEventServerFactory serverFactory = new NettyEventServerFactory( + InetAddress.getLoopbackAddress(), port, TransportProtocol.TCP); + serverFactory.setShutdownQuietPeriod(ShutdownQuietPeriod.QUICK.getDuration()); + serverFactory.setShutdownTimeout(ShutdownQuietPeriod.QUICK.getDuration()); + final ComponentLog log = runner.getLogger(); + final VersionNegotiator versionNegotiator = new StandardVersionNegotiator( + ProtocolVersion.V3.value(), ProtocolVersion.V2.value(), ProtocolVersion.V1.value()); + + final MapCache mapCache = new SimpleMapCache("serviceIdentifier", 64, EvictionPolicy.FIFO) { + // override a service function that will cause client network timeout to fire + @Override + public MapPutResult put(ByteBuffer key, ByteBuffer value) throws IOException { + try { + Thread.sleep(3000L); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + return super.put(key, value); + } + }; + + serverFactory.setHandlerSupplier(() -> Arrays.asList( + new CacheVersionResponseEncoder(), + new CacheOperationResultEncoder(), + new MapRemoveResponseEncoder(), + new MapSizeResponseEncoder(), + new MapValueResponseEncoder(), + new MapCacheRequestDecoder(log, 64, MapOperation.values()), + new MapCacheRequestHandler(log, mapCache), + new CacheVersionRequestHandler(log, versionNegotiator) + )); + server = serverFactory.getEventServer(); + } + + @AfterEach + public void shutdownServer() { + server.shutdown(); + } + + /** + * Service will hold request long enough for client timeout to be triggered, thus causing the request to fail. + */ + @Test + public void testClientTimeoutOnServerNetworkFailure() throws InitializationException, IOException { + final DistributedMapCacheClientService client = new DistributedMapCacheClientService(); + final MockControllerServiceInitializationContext clientInitContext1 = + new MockControllerServiceInitializationContext(client, "client"); + client.initialize(clientInitContext1); + + final Map<PropertyDescriptor, String> clientProperties = new HashMap<>(); + clientProperties.put(DistributedMapCacheClientService.HOSTNAME, "localhost"); Review Comment: Recommend changing `localhost` to `127.0.0.1` to avoid unnecessary name resolution: ```suggestion clientProperties.put(DistributedMapCacheClientService.HOSTNAME, "127.0.0.1"); ``` ########## nifi-nar-bundles/nifi-standard-services/nifi-distributed-cache-services-bundle/nifi-distributed-cache-server/src/test/java/org/apache/nifi/distributed/cache/server/map/TestMapCacheClient.java: ########## @@ -0,0 +1,146 @@ +/* + * 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.distributed.cache.server.map; + +import org.apache.commons.lang3.SerializationException; +import org.apache.nifi.components.PropertyDescriptor; +import org.apache.nifi.distributed.cache.client.DistributedMapCacheClientService; +import org.apache.nifi.distributed.cache.client.Serializer; +import org.apache.nifi.distributed.cache.operations.MapOperation; +import org.apache.nifi.distributed.cache.protocol.ProtocolVersion; +import org.apache.nifi.distributed.cache.server.EvictionPolicy; +import org.apache.nifi.distributed.cache.server.codec.CacheOperationResultEncoder; +import org.apache.nifi.distributed.cache.server.codec.CacheVersionRequestHandler; +import org.apache.nifi.distributed.cache.server.codec.CacheVersionResponseEncoder; +import org.apache.nifi.distributed.cache.server.codec.MapCacheRequestDecoder; +import org.apache.nifi.distributed.cache.server.codec.MapCacheRequestHandler; +import org.apache.nifi.distributed.cache.server.codec.MapRemoveResponseEncoder; +import org.apache.nifi.distributed.cache.server.codec.MapSizeResponseEncoder; +import org.apache.nifi.distributed.cache.server.codec.MapValueResponseEncoder; +import org.apache.nifi.event.transport.EventServer; +import org.apache.nifi.event.transport.configuration.ShutdownQuietPeriod; +import org.apache.nifi.event.transport.configuration.TransportProtocol; +import org.apache.nifi.event.transport.netty.NettyEventServerFactory; +import org.apache.nifi.logging.ComponentLog; +import org.apache.nifi.processor.Processor; +import org.apache.nifi.remote.StandardVersionNegotiator; +import org.apache.nifi.remote.VersionNegotiator; +import org.apache.nifi.remote.io.socket.NetworkUtils; +import org.apache.nifi.reporting.InitializationException; +import org.apache.nifi.util.MockConfigurationContext; +import org.apache.nifi.util.MockControllerServiceInitializationContext; +import org.apache.nifi.util.TestRunner; +import org.apache.nifi.util.TestRunners; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetAddress; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class TestMapCacheClient { + private int port; + private EventServer server; + private final Serializer<String> stringSerializer = new StringSerializer(); + + @BeforeEach + public void setRunner() { + final TestRunner runner = TestRunners.newTestRunner(Mockito.mock(Processor.class)); + port = NetworkUtils.getAvailableTcpPort(); + + final NettyEventServerFactory serverFactory = new NettyEventServerFactory( + InetAddress.getLoopbackAddress(), port, TransportProtocol.TCP); + serverFactory.setShutdownQuietPeriod(ShutdownQuietPeriod.QUICK.getDuration()); + serverFactory.setShutdownTimeout(ShutdownQuietPeriod.QUICK.getDuration()); + final ComponentLog log = runner.getLogger(); + final VersionNegotiator versionNegotiator = new StandardVersionNegotiator( + ProtocolVersion.V3.value(), ProtocolVersion.V2.value(), ProtocolVersion.V1.value()); + + final MapCache mapCache = new SimpleMapCache("serviceIdentifier", 64, EvictionPolicy.FIFO) { + // override a service function that will cause client network timeout to fire + @Override + public MapPutResult put(ByteBuffer key, ByteBuffer value) throws IOException { + try { + Thread.sleep(3000L); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + return super.put(key, value); + } + }; + + serverFactory.setHandlerSupplier(() -> Arrays.asList( + new CacheVersionResponseEncoder(), + new CacheOperationResultEncoder(), + new MapRemoveResponseEncoder(), + new MapSizeResponseEncoder(), + new MapValueResponseEncoder(), + new MapCacheRequestDecoder(log, 64, MapOperation.values()), + new MapCacheRequestHandler(log, mapCache), Review Comment: Are all of these Encoders required, or can this be scoped down to just the ones necessary for the test operation? -- 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]
