From: Robert Varga Date: Fri, 28 Mar 2025 16:36:52 +0000 (+0100) Subject: Use Path instead of String for temp directory X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=ac1d3f7c856ba8c027c337896d5553be58112309;p=controller.git Use Path instead of String for temp directory Let's be type-safe, so that things do not get mixed up. JIRA: CONTROLLER-2134 Change-Id: I8f5400213fc3cf424d61d83379d90fabb192d58e Signed-off-by: Robert Varga --- diff --git a/opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorConfig.java b/opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorConfig.java index 6e7df7e278..61c7ae7679 100644 --- a/opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorConfig.java +++ b/opendaylight/md-sal/cds-access-client/src/main/java/org/opendaylight/controller/cluster/access/client/ClientActorConfig.java @@ -7,6 +7,9 @@ */ package org.opendaylight.controller.cluster.access.client; +import java.nio.file.Path; +import org.eclipse.jdt.annotation.NonNull; + /** * Interface for client actor configuration parameters. * @@ -33,7 +36,7 @@ public interface ClientActorConfig { * * @return the directory name */ - String getTempFileDirectory(); + @NonNull Path getTempFileDirectory(); /** * Returns the timer interval whereby, on expiration after response inactivity from the back-end, the connection to diff --git a/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/messaging/MessageSlicingIntegrationTest.java b/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/messaging/MessageSlicingIntegrationTest.java index e5408b3568..d00906c3f8 100644 --- a/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/messaging/MessageSlicingIntegrationTest.java +++ b/opendaylight/md-sal/sal-clustering-commons/src/test/java/org/opendaylight/controller/cluster/messaging/MessageSlicingIntegrationTest.java @@ -7,12 +7,11 @@ */ package org.opendaylight.controller.cluster.messaging; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; import static org.opendaylight.controller.cluster.messaging.MessageSlicerTest.slice; @@ -20,6 +19,7 @@ import static org.opendaylight.controller.cluster.messaging.MessageSlicerTest.sl import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.ObjectOutputStream; +import java.nio.file.Path; import java.util.Arrays; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -28,12 +28,15 @@ import org.apache.pekko.actor.ActorRef; import org.apache.pekko.actor.ActorSystem; import org.apache.pekko.testkit.TestProbe; import org.apache.pekko.testkit.javadsl.TestKit; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; import org.mockito.ArgumentCaptor; -import org.opendaylight.raft.spi.FileBackedOutputStream; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; import org.opendaylight.raft.spi.FileBackedOutputStreamFactory; import org.opendaylight.yangtools.concepts.Identifier; import org.slf4j.Logger; @@ -44,54 +47,59 @@ import org.slf4j.LoggerFactory; * * @author Thomas Pantelis */ -public class MessageSlicingIntegrationTest { +@ExtendWith(MockitoExtension.class) +class MessageSlicingIntegrationTest { private static final Logger LOG = LoggerFactory.getLogger(MessageSlicingIntegrationTest.class); private static final ActorSystem ACTOR_SYSTEM = ActorSystem.create("test"); - private static final FileBackedOutputStreamFactory FILE_BACKED_STREAM_FACTORY = - new FileBackedOutputStreamFactory(1000000000, "target"); private static final Identifier IDENTIFIER = new StringIdentifier("stringId"); private static final int DONT_CARE = -1; private final TestProbe sendToProbe = TestProbe.apply(ACTOR_SYSTEM); private final TestProbe replyToProbe = TestProbe.apply(ACTOR_SYSTEM); - @SuppressWarnings("unchecked") - private final Consumer mockOnFailureCallback = mock(Consumer.class); - - @SuppressWarnings("unchecked") - private final BiConsumer mockAssembledMessageCallback = mock(BiConsumer.class); - - private final MessageAssembler assembler = MessageAssembler.builder() - .assembledMessageCallback(mockAssembledMessageCallback).logContext("test") - .fileBackedStreamFactory(FILE_BACKED_STREAM_FACTORY).build(); - - @Before - public void setup() { - doNothing().when(mockOnFailureCallback).accept(any(Throwable.class)); - doNothing().when(mockAssembledMessageCallback).accept(any(Object.class), any(ActorRef.class)); + @TempDir + private Path tempDir; + @Mock + private Consumer mockOnFailureCallback; + @Mock + private BiConsumer mockAssembledMessageCallback; + + private FileBackedOutputStreamFactory streamFactory; + private MessageAssembler assembler; + + @BeforeEach + void beforeEach() { + streamFactory = new FileBackedOutputStreamFactory(1000000000, tempDir); + assembler = MessageAssembler.builder() + .assembledMessageCallback(mockAssembledMessageCallback) + .fileBackedStreamFactory(streamFactory) + .logContext("test") + .build(); } - @After - public void tearDown() { + @AfterEach + void tearDown() { assembler.close(); } - @AfterClass - public static void staticTearDown() { + @AfterAll + static void staticTearDown() { TestKit.shutdownActorSystem(ACTOR_SYSTEM, true); } @Test - public void testSlicingWithChunks() throws IOException { + void testSlicingWithChunks() throws Exception { LOG.info("testSlicingWithChunks starting"); + doNothing().when(mockAssembledMessageCallback).accept(any(Object.class), any(ActorRef.class)); + // First slice a message where the messageSliceSize divides evenly into the serialized size. byte[] emptyMessageBytes = SerializationUtils.serialize(new BytesMessage(new byte[]{})); int messageSliceSize = 10; int expTotalSlices = emptyMessageBytes.length / messageSliceSize; - ByteArrayOutputStream byteStream = new ByteArrayOutputStream(); + final var byteStream = new ByteArrayOutputStream(); if (emptyMessageBytes.length % messageSliceSize > 0) { expTotalSlices++; int padding = messageSliceSize - emptyMessageBytes.length % messageSliceSize; @@ -105,49 +113,50 @@ public class MessageSlicingIntegrationTest { // Now slice a message where the messageSliceSize doesn't divide evenly. - byteStream.write(new byte[]{100, 101, 102}); + byteStream.write(new byte[] { 100, 101, 102 }); testSlicing("testSlicingWithChunks", messageSliceSize, expTotalSlices + 1, byteStream.toByteArray()); LOG.info("testSlicingWithChunks ending"); } @Test - public void testSingleSlice() { + void testSingleSlice() { LOG.info("testSingleSlice starting"); // Slice a message where the serialized size is equal to the messageSliceSize. In this case it should // just send the original message. - final BytesMessage message = new BytesMessage(new byte[]{1, 2, 3}); - try (MessageSlicer slicer = newMessageSlicer("testSingleSlice", SerializationUtils.serialize(message).length)) { + final var message = new BytesMessage(new byte[] { 1, 2, 3 }); + try (var slicer = newMessageSlicer("testSingleSlice", SerializationUtils.serialize(message).length)) { final boolean wasSliced = slice(slicer, IDENTIFIER, message, sendToProbe.ref(), replyToProbe.ref(), mockOnFailureCallback); assertFalse(wasSliced); - final BytesMessage sentMessage = sendToProbe.expectMsgClass(BytesMessage.class); - assertEquals("Sent message", message, sentMessage); + assertEquals(message, sendToProbe.expectMsgClass(BytesMessage.class)); } LOG.info("testSingleSlice ending"); } @Test - public void testSlicingWithRetry() { + void testSlicingWithRetry() { LOG.info("testSlicingWithRetry starting"); - final BytesMessage message = new BytesMessage(new byte[]{1, 2, 3}); + doNothing().when(mockAssembledMessageCallback).accept(any(Object.class), any(ActorRef.class)); + + final var message = new BytesMessage(new byte[] { 1, 2, 3 }); final int messageSliceSize = SerializationUtils.serialize(message).length / 2; - try (MessageSlicer slicer = newMessageSlicer("testSlicingWithRetry", messageSliceSize)) { + try (var slicer = newMessageSlicer("testSlicingWithRetry", messageSliceSize)) { slice(slicer, IDENTIFIER, message, sendToProbe.ref(), replyToProbe.ref(), mockOnFailureCallback); - MessageSlice sliceMessage = sendToProbe.expectMsgClass(MessageSlice.class); + var sliceMessage = sendToProbe.expectMsgClass(MessageSlice.class); assembler.handleMessage(sliceMessage, sendToProbe.ref()); // Swallow the reply and send the MessageSlice again - it should return a failed reply. replyToProbe.expectMsgClass(MessageSliceReply.class); assembler.handleMessage(sliceMessage, sendToProbe.ref()); - final MessageSliceReply failedReply = replyToProbe.expectMsgClass(MessageSliceReply.class); + final var failedReply = replyToProbe.expectMsgClass(MessageSliceReply.class); assertFailedMessageSliceReply(failedReply, IDENTIFIER, true); // Send the failed reply - slicing should be retried from the beginning. @@ -157,7 +166,7 @@ public class MessageSlicingIntegrationTest { sliceMessage = sendToProbe.expectMsgClass(MessageSlice.class); assembler.handleMessage(sliceMessage, sendToProbe.ref()); - final MessageSliceReply reply = replyToProbe.expectMsgClass(MessageSliceReply.class); + final var reply = replyToProbe.expectMsgClass(MessageSliceReply.class); assertSuccessfulMessageSliceReply(reply, IDENTIFIER, sliceMessage.getSliceIndex()); slicer.handleMessage(reply); @@ -173,29 +182,31 @@ public class MessageSlicingIntegrationTest { } @Test - public void testSlicingWithMaxRetriesReached() { + void testSlicingWithMaxRetriesReached() { LOG.info("testSlicingWithMaxRetriesReached starting"); - final BytesMessage message = new BytesMessage(new byte[]{1, 2, 3}); + doNothing().when(mockOnFailureCallback).accept(any(Throwable.class)); + + final var message = new BytesMessage(new byte[] { 1, 2, 3 }); final int messageSliceSize = SerializationUtils.serialize(message).length / 2; - try (MessageSlicer slicer = newMessageSlicer("testSlicingWithMaxRetriesReached", messageSliceSize)) { + try (var slicer = newMessageSlicer("testSlicingWithMaxRetriesReached", messageSliceSize)) { slice(slicer, IDENTIFIER, message, sendToProbe.ref(), replyToProbe.ref(), mockOnFailureCallback); Identifier slicingId = null; for (int i = 0; i < MessageSlicer.DEFAULT_MAX_SLICING_TRIES; i++) { - MessageSlice sliceMessage = sendToProbe.expectMsgClass(MessageSlice.class); + var sliceMessage = sendToProbe.expectMsgClass(MessageSlice.class); slicingId = sliceMessage.getIdentifier(); assertMessageSlice(sliceMessage, IDENTIFIER, 1, DONT_CARE, SlicedMessageState.INITIAL_SLICE_HASH_CODE, replyToProbe.ref()); assembler.handleMessage(sliceMessage, sendToProbe.ref()); // Swallow the reply and send the MessageSlicer a reply with an invalid index. - final MessageSliceReply reply = replyToProbe.expectMsgClass(MessageSliceReply.class); + final var reply = replyToProbe.expectMsgClass(MessageSliceReply.class); assertSuccessfulMessageSliceReply(reply, IDENTIFIER, sliceMessage.getSliceIndex()); slicer.handleMessage(MessageSliceReply.success(reply.getIdentifier(), 100000, reply.getSendTo())); - final AbortSlicing abortSlicing = sendToProbe.expectMsgClass(AbortSlicing.class); - assertEquals("Identifier", slicingId, abortSlicing.getIdentifier()); + final var abortSlicing = sendToProbe.expectMsgClass(AbortSlicing.class); + assertEquals(slicingId, abortSlicing.getIdentifier()); assembler.handleMessage(abortSlicing, sendToProbe.ref()); } @@ -203,56 +214,63 @@ public class MessageSlicingIntegrationTest { assertFailureCallback(RuntimeException.class); - assertFalse("MessageSlicer did not remove state for " + slicingId, slicer.hasState(slicingId)); - assertFalse("MessageAssembler did not remove state for " + slicingId, assembler.hasState(slicingId)); + assertFalse(slicer.hasState(slicingId), "MessageSlicer did not remove state for " + slicingId); + assertFalse(assembler.hasState(slicingId), "MessageAssembler did not remove state for " + slicingId); } LOG.info("testSlicingWithMaxRetriesReached ending"); } @Test - public void testSlicingWithFailure() { + void testSlicingWithFailure() { LOG.info("testSlicingWithFailure starting"); - final BytesMessage message = new BytesMessage(new byte[]{1, 2, 3}); + doNothing().when(mockOnFailureCallback).accept(any(Throwable.class)); + + final var message = new BytesMessage(new byte[] { 1, 2, 3 }); final int messageSliceSize = SerializationUtils.serialize(message).length / 2; - try (MessageSlicer slicer = newMessageSlicer("testSlicingWithFailure", messageSliceSize)) { + try (var slicer = newMessageSlicer("testSlicingWithFailure", messageSliceSize)) { final boolean wasSliced = slice(slicer, IDENTIFIER, message, sendToProbe.ref(), replyToProbe.ref(), mockOnFailureCallback); assertTrue(wasSliced); - MessageSlice sliceMessage = sendToProbe.expectMsgClass(MessageSlice.class); + var sliceMessage = sendToProbe.expectMsgClass(MessageSlice.class); - MessageSliceException failure = new MessageSliceException("mock failure", - new IOException("mock IOException")); + var failure = new MessageSliceException("mock failure", new IOException("mock IOException")); slicer.handleMessage(MessageSliceReply.failed(sliceMessage.getIdentifier(), failure, sendToProbe.ref())); assertFailureCallback(IOException.class); - assertFalse("MessageSlicer did not remove state for " + sliceMessage.getIdentifier(), - slicer.hasState(sliceMessage.getIdentifier())); + assertFalse(slicer.hasState(sliceMessage.getIdentifier()), + "MessageSlicer did not remove state for " + sliceMessage.getIdentifier()); } LOG.info("testSlicingWithFailure ending"); } @Test - public void testSliceWithFileBackedOutputStream() throws IOException { + void testSliceWithFileBackedOutputStream() throws Exception { LOG.info("testSliceWithFileBackedOutputStream starting"); - final BytesMessage message = new BytesMessage(new byte[]{1, 2, 3}); - FileBackedOutputStream fileBackedStream = FILE_BACKED_STREAM_FACTORY.newInstance(); - try (ObjectOutputStream out = new ObjectOutputStream(fileBackedStream)) { + doNothing().when(mockAssembledMessageCallback).accept(any(Object.class), any(ActorRef.class)); + + final var message = new BytesMessage(new byte[] { 1, 2, 3 }); + final var fileBackedStream = streamFactory.newInstance(); + try (var out = new ObjectOutputStream(fileBackedStream)) { out.writeObject(message); } - try (MessageSlicer slicer = newMessageSlicer("testSliceWithFileBackedOutputStream", + try (var slicer = newMessageSlicer("testSliceWithFileBackedOutputStream", SerializationUtils.serialize(message).length)) { - slicer.slice(SliceOptions.builder().identifier(IDENTIFIER).fileBackedOutputStream(fileBackedStream) - .sendTo(ACTOR_SYSTEM.actorSelection(sendToProbe.ref().path())).replyTo(replyToProbe.ref()) - .onFailureCallback(mockOnFailureCallback).build()); - - final MessageSlice sliceMessage = sendToProbe.expectMsgClass(MessageSlice.class); + slicer.slice(SliceOptions.builder() + .identifier(IDENTIFIER) + .fileBackedOutputStream(fileBackedStream) + .sendTo(ACTOR_SYSTEM.actorSelection(sendToProbe.ref().path())) + .replyTo(replyToProbe.ref()) + .onFailureCallback(mockOnFailureCallback) + .build()); + + final var sliceMessage = sendToProbe.expectMsgClass(MessageSlice.class); assembler.handleMessage(sliceMessage, sendToProbe.ref()); assertAssembledMessage(message, replyToProbe.ref()); } @@ -265,9 +283,9 @@ public class MessageSlicingIntegrationTest { final byte[] messageData) { reset(mockAssembledMessageCallback); - final BytesMessage message = new BytesMessage(messageData); + final var message = new BytesMessage(messageData); - try (MessageSlicer slicer = newMessageSlicer(logContext, messageSliceSize)) { + try (var slicer = newMessageSlicer(logContext, messageSliceSize)) { final boolean wasSliced = slice(slicer, IDENTIFIER, message, sendToProbe.ref(), replyToProbe.ref(), mockOnFailureCallback); assertTrue(wasSliced); @@ -275,14 +293,14 @@ public class MessageSlicingIntegrationTest { Identifier slicingId = null; int expLastSliceHashCode = SlicedMessageState.INITIAL_SLICE_HASH_CODE; for (int sliceIndex = 1; sliceIndex <= expTotalSlices; sliceIndex++) { - final MessageSlice sliceMessage = sendToProbe.expectMsgClass(MessageSlice.class); + final var sliceMessage = sendToProbe.expectMsgClass(MessageSlice.class); slicingId = sliceMessage.getIdentifier(); assertMessageSlice(sliceMessage, IDENTIFIER, sliceIndex, expTotalSlices, expLastSliceHashCode, replyToProbe.ref()); assembler.handleMessage(sliceMessage, sendToProbe.ref()); - final MessageSliceReply reply = replyToProbe.expectMsgClass(MessageSliceReply.class); + final var reply = replyToProbe.expectMsgClass(MessageSliceReply.class); assertSuccessfulMessageSliceReply(reply, IDENTIFIER, sliceIndex); expLastSliceHashCode = Arrays.hashCode(sliceMessage.getData()); @@ -292,15 +310,15 @@ public class MessageSlicingIntegrationTest { assertAssembledMessage(message, replyToProbe.ref()); - assertFalse("MessageSlicer did not remove state for " + slicingId, slicer.hasState(slicingId)); - assertFalse("MessageAssembler did not remove state for " + slicingId, assembler.hasState(slicingId)); + assertFalse(slicer.hasState(slicingId), "MessageSlicer did not remove state for " + slicingId); + assertFalse(assembler.hasState(slicingId), "MessageAssembler did not remove state for " + slicingId); } } private void assertFailureCallback(final Class exceptionType) { - ArgumentCaptor exceptionCaptor = ArgumentCaptor.forClass(Throwable.class); + final var exceptionCaptor = ArgumentCaptor.forClass(Throwable.class); verify(mockOnFailureCallback).accept(exceptionCaptor.capture()); - assertEquals("Exception type", exceptionType, exceptionCaptor.getValue().getClass()); + assertEquals(exceptionType, exceptionCaptor.getValue().getClass()); } private void assertAssembledMessage(final BytesMessage message, final ActorRef sender) { @@ -309,43 +327,43 @@ public class MessageSlicingIntegrationTest { static void assertAssembledMessage(final BiConsumer mockAssembledMessageCallback, final BytesMessage message, final ActorRef sender) { - ArgumentCaptor assembledMessageCaptor = ArgumentCaptor.forClass(Object.class); - ArgumentCaptor senderActorRefCaptor = ArgumentCaptor.forClass(ActorRef.class); + final var assembledMessageCaptor = ArgumentCaptor.forClass(Object.class); + final var senderActorRefCaptor = ArgumentCaptor.forClass(ActorRef.class); verify(mockAssembledMessageCallback).accept(assembledMessageCaptor.capture(), senderActorRefCaptor.capture()); - assertEquals("Assembled message", message, assembledMessageCaptor.getValue()); - assertEquals("Sender ActorRef", sender, senderActorRefCaptor.getValue()); + assertEquals(message, assembledMessageCaptor.getValue()); + assertEquals(sender, senderActorRefCaptor.getValue()); } static void assertSuccessfulMessageSliceReply(final MessageSliceReply reply, final Identifier identifier, final int sliceIndex) { - assertEquals("Identifier", identifier, ((MessageSliceIdentifier)reply.getIdentifier()) - .getClientIdentifier()); - assertEquals("SliceIndex", sliceIndex, reply.getSliceIndex()); + assertEquals(identifier, ((MessageSliceIdentifier) reply.getIdentifier()).getClientIdentifier()); + assertEquals(sliceIndex, reply.getSliceIndex()); } static void assertFailedMessageSliceReply(final MessageSliceReply reply, final Identifier identifier, final boolean isRetriable) { - assertEquals("Identifier", identifier, ((MessageSliceIdentifier)reply.getIdentifier()) - .getClientIdentifier()); - assertEquals("Failure present", Boolean.TRUE, reply.getFailure().isPresent()); - assertEquals("isRetriable", isRetriable, reply.getFailure().orElseThrow().isRetriable()); + assertEquals(identifier, ((MessageSliceIdentifier) reply.getIdentifier()).getClientIdentifier()); + assertTrue(reply.getFailure().isPresent()); + assertEquals(isRetriable, reply.getFailure().orElseThrow().isRetriable()); } static void assertMessageSlice(final MessageSlice sliceMessage, final Identifier identifier, final int sliceIndex, final int totalSlices, final int lastSliceHashCode, final ActorRef replyTo) { - assertEquals("Identifier", identifier, ((MessageSliceIdentifier)sliceMessage.getIdentifier()) - .getClientIdentifier()); - assertEquals("SliceIndex", sliceIndex, sliceMessage.getSliceIndex()); - assertEquals("LastSliceHashCode", lastSliceHashCode, sliceMessage.getLastSliceHashCode()); - assertEquals("ReplyTo", replyTo, sliceMessage.getReplyTo()); + assertEquals(identifier, ((MessageSliceIdentifier) sliceMessage.getIdentifier()).getClientIdentifier()); + assertEquals(sliceIndex, sliceMessage.getSliceIndex()); + assertEquals(lastSliceHashCode, sliceMessage.getLastSliceHashCode()); + assertEquals(replyTo, sliceMessage.getReplyTo()); if (totalSlices != DONT_CARE) { - assertEquals("TotalSlices", totalSlices, sliceMessage.getTotalSlices()); + assertEquals(totalSlices, sliceMessage.getTotalSlices()); } } - private static MessageSlicer newMessageSlicer(final String logContext, final int messageSliceSize) { - return MessageSlicer.builder().messageSliceSize(messageSliceSize).logContext(logContext) - .fileBackedStreamFactory(FILE_BACKED_STREAM_FACTORY).build(); + private MessageSlicer newMessageSlicer(final String logContext, final int messageSliceSize) { + return MessageSlicer.builder() + .messageSliceSize(messageSliceSize) + .logContext(logContext) + .fileBackedStreamFactory(streamFactory) + .build(); } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DatastoreContext.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DatastoreContext.java index 197ae77c78..0ca613325a 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DatastoreContext.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DatastoreContext.java @@ -11,6 +11,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; import com.google.common.annotations.VisibleForTesting; +import java.nio.file.Path; import java.time.Duration; import java.util.concurrent.TimeUnit; import org.apache.commons.text.WordUtils; @@ -242,11 +243,11 @@ public class DatastoreContext implements ClientActorConfig { } @Override - public String getTempFileDirectory() { + public Path getTempFileDirectory() { return raftConfig.getTempFileDirectory(); } - private void setTempFileDirectory(final String tempFileDirectory) { + private void setTempFileDirectory(final Path tempFileDirectory) { raftConfig.setTempFileDirectory(tempFileDirectory); } @@ -604,7 +605,12 @@ public class DatastoreContext implements ClientActorConfig { return this; } + @Deprecated public Builder tempFileDirectory(final String tempFileDirectory) { + return tempFileDirectory(Path.of(tempFileDirectory)); + } + + public Builder tempFileDirectory(final Path tempFileDirectory) { datastoreContext.setTempFileDirectory(tempFileDirectory); return this; } diff --git a/raft/raft-spi/src/main/java/org/opendaylight/raft/spi/FileBackedOutputStream.java b/raft/raft-spi/src/main/java/org/opendaylight/raft/spi/FileBackedOutputStream.java index b8174c6d4c..9874865f5f 100644 --- a/raft/raft-spi/src/main/java/org/opendaylight/raft/spi/FileBackedOutputStream.java +++ b/raft/raft-spi/src/main/java/org/opendaylight/raft/spi/FileBackedOutputStream.java @@ -69,10 +69,9 @@ public class FileBackedOutputStream extends OutputStream { * @param fileDirectory the directory in which to create the file if needed. If {@code null}, the default temp file * location is used. */ - // FIXME: java.io.Path - public FileBackedOutputStream(final int fileThreshold, final @Nullable String fileDirectory) { + public FileBackedOutputStream(final int fileThreshold, final @Nullable Path fileDirectory) { this.fileThreshold = fileThreshold; - this.fileDirectory = fileDirectory != null ? Path.of(fileDirectory) : null; + this.fileDirectory = fileDirectory; } /** diff --git a/raft/raft-spi/src/main/java/org/opendaylight/raft/spi/FileBackedOutputStreamFactory.java b/raft/raft-spi/src/main/java/org/opendaylight/raft/spi/FileBackedOutputStreamFactory.java index 375dbe7ad9..5ab51cc03b 100644 --- a/raft/raft-spi/src/main/java/org/opendaylight/raft/spi/FileBackedOutputStreamFactory.java +++ b/raft/raft-spi/src/main/java/org/opendaylight/raft/spi/FileBackedOutputStreamFactory.java @@ -7,6 +7,7 @@ */ package org.opendaylight.raft.spi; +import java.nio.file.Path; import org.eclipse.jdt.annotation.Nullable; /** @@ -17,16 +18,16 @@ import org.eclipse.jdt.annotation.Nullable; */ public class FileBackedOutputStreamFactory { private final int fileThreshold; - private final String fileDirectory; + private final Path fileDirectory; /** - * Constructor. + * Default constructor. * * @param fileThreshold the number of bytes before streams should switch to buffering to a file - * @param fileDirectory the directory in which to create files if needed. If null, the default temp file + * @param fileDirectory the directory in which to create files if needed. If {@code null}, the default temp file * location is used. */ - public FileBackedOutputStreamFactory(final int fileThreshold, final @Nullable String fileDirectory) { + public FileBackedOutputStreamFactory(final int fileThreshold, final @Nullable Path fileDirectory) { this.fileThreshold = fileThreshold; this.fileDirectory = fileDirectory; } diff --git a/raft/raft-spi/src/main/java/org/opendaylight/raft/spi/SharedFileBackedOutputStream.java b/raft/raft-spi/src/main/java/org/opendaylight/raft/spi/SharedFileBackedOutputStream.java index fc8cda49c7..b5154a4849 100644 --- a/raft/raft-spi/src/main/java/org/opendaylight/raft/spi/SharedFileBackedOutputStream.java +++ b/raft/raft-spi/src/main/java/org/opendaylight/raft/spi/SharedFileBackedOutputStream.java @@ -9,6 +9,7 @@ package org.opendaylight.raft.spi; import com.google.common.base.Preconditions; import com.google.common.io.ByteSource; +import java.nio.file.Path; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; @@ -36,8 +37,7 @@ public final class SharedFileBackedOutputStream extends FileBackedOutputStream { * @param fileDirectory the directory in which to create the file if needed. If {@code null}, the default temp file * location is used. */ - // FIXME: java.nio.file.Path - public SharedFileBackedOutputStream(final int fileThreshold, final String fileDirectory) { + public SharedFileBackedOutputStream(final int fileThreshold, final Path fileDirectory) { super(fileThreshold, fileDirectory); } diff --git a/raft/raft-spi/src/test/java/org/opendaylight/raft/spi/FileBackedOutputStreamTest.java b/raft/raft-spi/src/test/java/org/opendaylight/raft/spi/FileBackedOutputStreamTest.java index 8dc7d634ac..e6d42880db 100644 --- a/raft/raft-spi/src/test/java/org/opendaylight/raft/spi/FileBackedOutputStreamTest.java +++ b/raft/raft-spi/src/test/java/org/opendaylight/raft/spi/FileBackedOutputStreamTest.java @@ -40,7 +40,7 @@ class FileBackedOutputStreamTest { @Test void testFileThresholdNotReached() throws Exception { LOG.info("testFileThresholdNotReached starting"); - try (var fbos = new FileBackedOutputStream(10, tempDir.toString())) { + try (var fbos = new FileBackedOutputStream(10, tempDir)) { final var expected = new byte[]{ 1, 2, 3, 4, 5, 6, 7, 8, 9 }; fbos.write(expected[0]); fbos.write(expected, 1, expected.length - 1); @@ -62,8 +62,8 @@ class FileBackedOutputStreamTest { @Test void testFileThresholdReachedWithWriteBytes() throws Exception { LOG.info("testFileThresholdReachedWithWriteBytes starting"); - try (var fbos = new FileBackedOutputStream(10, tempDir.toString())) { - byte[] bytes = new byte[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}; + try (var fbos = new FileBackedOutputStream(10, tempDir)) { + final var bytes = new byte[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}; fbos.write(bytes[0]); fbos.write(bytes, 1, 11); @@ -97,7 +97,7 @@ class FileBackedOutputStreamTest { @Test void testFileThresholdReachedWithWriteByte() throws Exception { LOG.info("testFileThresholdReachedWithWriteByte starting"); - try (var fbos = new FileBackedOutputStream(2, tempDir.toString())) { + try (var fbos = new FileBackedOutputStream(2, tempDir)) { final var bytes = new byte[]{0, 1, 2}; fbos.write(bytes[0]); fbos.write(bytes[1]); @@ -117,9 +117,9 @@ class FileBackedOutputStreamTest { } @Test - void testWriteAfterAsByteSource() throws IOException { + void testWriteAfterAsByteSource() throws Exception { LOG.info("testWriteAfterAsByteSource starting"); - try (var fbos = new FileBackedOutputStream(3, tempDir.toString())) { + try (var fbos = new FileBackedOutputStream(3, tempDir)) { final var bytes = new byte[]{0, 1, 2}; fbos.write(bytes); @@ -132,10 +132,10 @@ class FileBackedOutputStreamTest { } @Test - void testTempFileDeletedOnGC() throws IOException { + void testTempFileDeletedOnGC() throws Exception { LOG.info("testTempFileDeletedOnGC starting"); - try (var fbos = new FileBackedOutputStream(1, tempDir.toString())) { + try (var fbos = new FileBackedOutputStream(1, tempDir)) { fbos.write(new byte[] { 0, 1 }); assertNotNull(findTempFileName(tempDir)); } diff --git a/raft/raft-spi/src/test/java/org/opendaylight/raft/spi/SharedFileBackedOutputStreamTest.java b/raft/raft-spi/src/test/java/org/opendaylight/raft/spi/SharedFileBackedOutputStreamTest.java index 80754efe89..a2974430ed 100644 --- a/raft/raft-spi/src/test/java/org/opendaylight/raft/spi/SharedFileBackedOutputStreamTest.java +++ b/raft/raft-spi/src/test/java/org/opendaylight/raft/spi/SharedFileBackedOutputStreamTest.java @@ -39,7 +39,7 @@ class SharedFileBackedOutputStreamTest { @Test void testSingleUsage() throws Exception { LOG.info("testSingleUsage starting"); - try (var fbos = new SharedFileBackedOutputStream(5, tempDir.toString())) { + try (var fbos = new SharedFileBackedOutputStream(5, tempDir)) { final var bytes = new byte[] { 0, 1, 2, 3, 4, 5, 6 }; fbos.write(bytes); @@ -54,9 +54,9 @@ class SharedFileBackedOutputStreamTest { @Test void testSharing() throws Exception { LOG.info("testSharing starting"); - try (var fbos = new SharedFileBackedOutputStream(5, tempDir.toString())) { + try (var fbos = new SharedFileBackedOutputStream(5, tempDir)) { String context = "context"; - fbos.setOnCleanupCallback(mockCallback , context); + fbos.setOnCleanupCallback(mockCallback, context); final var bytes = new byte[] { 0, 1, 2, 3, 4, 5, 6 }; fbos.write(bytes); diff --git a/raft/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ConfigParams.java b/raft/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ConfigParams.java index 848891795a..990db0dc99 100644 --- a/raft/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ConfigParams.java +++ b/raft/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/ConfigParams.java @@ -7,6 +7,7 @@ */ package org.opendaylight.controller.cluster.raft; +import java.nio.file.Path; import java.time.Duration; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.controller.cluster.raft.policy.RaftPolicy; @@ -136,7 +137,7 @@ public interface ConfigParams { * * @return the directory in which to create temp files. */ - @NonNull String getTempFileDirectory(); + @NonNull Path getTempFileDirectory(); /** * Returns the threshold in terms of number of bytes when streaming data before it should switch from storing in diff --git a/raft/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/DefaultConfigParamsImpl.java b/raft/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/DefaultConfigParamsImpl.java index d90342352b..8cb1820b6f 100644 --- a/raft/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/DefaultConfigParamsImpl.java +++ b/raft/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/DefaultConfigParamsImpl.java @@ -12,6 +12,7 @@ import static java.util.Objects.requireNonNull; import com.google.common.base.Strings; import com.google.common.base.Suppliers; +import java.nio.file.Path; import java.time.Duration; import java.util.function.Supplier; import org.eclipse.jdt.annotation.NonNull; @@ -25,8 +26,8 @@ import org.slf4j.LoggerFactory; * Default implementation of the ConfigParams. */ public class DefaultConfigParamsImpl implements ConfigParams { - private static final Logger LOG = LoggerFactory.getLogger(DefaultConfigParamsImpl.class); + private static final Path EMPTY_PATH = Path.of(""); private static final int SNAPSHOT_BATCH_COUNT = 20000; /** @@ -76,7 +77,7 @@ public class DefaultConfigParamsImpl implements ConfigParams { private PeerAddressResolver peerAddressResolver = NoopPeerAddressResolver.INSTANCE; - private String tempFileDirectory = ""; + private Path tempFileDirectory = EMPTY_PATH; private int fileBackedStreamingThreshold = 128 * 1_048_576; @@ -127,8 +128,8 @@ public class DefaultConfigParamsImpl implements ConfigParams { this.candidateElectionTimeoutDivisor = candidateElectionTimeoutDivisor; } - public void setTempFileDirectory(final String tempFileDirectory) { - this.tempFileDirectory = tempFileDirectory; + public void setTempFileDirectory(final Path tempFileDirectory) { + this.tempFileDirectory = requireNonNull(tempFileDirectory); } public void setFileBackedStreamingThreshold(final int fileBackedStreamingThreshold) { @@ -213,7 +214,7 @@ public class DefaultConfigParamsImpl implements ConfigParams { } @Override - public String getTempFileDirectory() { + public Path getTempFileDirectory() { return tempFileDirectory; } diff --git a/raft/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/SnapshotManagerTest.java b/raft/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/SnapshotManagerTest.java index 14dbd163d3..04bdc9728a 100644 --- a/raft/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/SnapshotManagerTest.java +++ b/raft/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/SnapshotManagerTest.java @@ -32,7 +32,9 @@ import org.apache.pekko.actor.ActorRef; import org.apache.pekko.persistence.SnapshotSelectionCriteria; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.Captor; @@ -52,6 +54,9 @@ import org.opendaylight.raft.spi.FileBackedOutputStreamFactory; @RunWith(MockitoJUnitRunner.StrictStubs.class) public class SnapshotManagerTest extends AbstractActorTest { + @Rule + public final TemporaryFolder tempDir = TemporaryFolder.builder().assureDeletion().build(); + @Mock private RaftActorContext mockRaftActorContext; @Mock @@ -90,7 +95,7 @@ public class SnapshotManagerTest extends AbstractActorTest { doReturn(mockRaftActorBehavior).when(mockRaftActorContext).getCurrentBehavior(); doReturn(mockTermInfo).when(mockRaftActorContext).termInfo(); - doReturn(new FileBackedOutputStreamFactory(10000000, "target")) + doReturn(new FileBackedOutputStreamFactory(10000000, tempDir.getRoot().toPath())) .when(mockRaftActorContext).getFileBackedOutputStreamFactory(); snapshotManager = new SnapshotManager(mockRaftActorContext); diff --git a/raft/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/SnapshotTrackerTest.java b/raft/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/SnapshotTrackerTest.java index 967251821d..a320037feb 100644 --- a/raft/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/SnapshotTrackerTest.java +++ b/raft/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/SnapshotTrackerTest.java @@ -14,6 +14,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import java.nio.file.Path; import java.util.Arrays; import java.util.HashMap; import java.util.OptionalInt; @@ -22,6 +23,7 @@ import org.apache.pekko.protobuf.ByteString; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.opendaylight.controller.cluster.raft.RaftActorContext; @@ -33,8 +35,11 @@ import org.opendaylight.raft.spi.FileBackedOutputStreamFactory; class SnapshotTrackerTest { private final HashMap data = new HashMap<>(); + @TempDir + private Path tempDir; @Mock private RaftActorContext mockContext; + private FileBackedOutputStream fbos; private ByteString byteString; private byte[] chunk1; @@ -52,7 +57,7 @@ class SnapshotTrackerTest { chunk2 = getNextChunk(byteString, 10, 10); chunk3 = getNextChunk(byteString, 20, byteString.size()); - fbos = spy(new FileBackedOutputStream(100000000, "target")); + fbos = spy(new FileBackedOutputStream(100000000, tempDir)); final var mockFactory = mock(FileBackedOutputStreamFactory.class); doReturn(fbos).when(mockFactory).newInstance(); doReturn(mockFactory).when(mockContext).getFileBackedOutputStreamFactory();