[BAEL-3601] - Fix code review comments.

This commit is contained in:
Philippe Soares 2020-02-09 00:49:26 -05:00
parent e94f0ad2f1
commit 696e4943df
14 changed files with 156 additions and 520 deletions

View File

@ -8,5 +8,4 @@ This module contains articles about core Java non-blocking input and output (IO)
- [Create a Symbolic Link with Java](https://www.baeldung.com/java-symlink)
- [Introduction to the Java NIO Selector](https://www.baeldung.com/java-nio-selector)
- [Using Java MappedByteBuffer](https://www.baeldung.com/java-mapped-byte-buffer)
- [How to Lock a File in Java](https://www.baeldung.com/how-to-lock-a-file-in-java)
- [[<-- Prev]](/core-java-modules/core-java-nio)
- [[<-- Prev]](/core-java-modules/core-java-nio)

View File

@ -1,7 +1,6 @@
package com.baeldung.lock;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.RandomAccessFile;
@ -10,230 +9,131 @@ import java.nio.channels.FileChannel;
import java.nio.channels.FileLock;
import java.nio.channels.NonReadableChannelException;
import java.nio.channels.NonWritableChannelException;
import java.nio.channels.OverlappingFileLockException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardOpenOption;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Stream;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.base.Charsets;
public class FileLocks {
private static Logger log = LoggerFactory.getLogger(FileLocks.class);
// Write locks
/**
* Trying to get an exclusive lock on a read-only FileChannel won't work.
*/
static void getExclusiveLockFromInputStream() throws IOException, NonWritableChannelException {
Path path = Files.createTempFile("foo", "txt");
try (FileInputStream fis = new FileInputStream(path.toFile()); FileLock lock = fis.getChannel().lock()) {
log.debug("This won't happen");
} catch (NonWritableChannelException e) {
log.error(
"The channel obtained through a FileInputStream isn't writable. "
+ "You can't obtain an exclusive lock on it!");
throw e;
}
}
private static final Logger LOG = LoggerFactory.getLogger(FileLocks.class);
/**
* Getting an exclusive lock from a RandomAccessFile works if the file is in write mode.
* @param from beginning of the locked region
* @param size how many bytes to lock
* @return
* @throws IOException
*/
static FileLock getExclusiveLockFromRandomAccessFile(long from, long size) throws IOException {
Path path = Files.createTempFile("foo", "txt");
try (RandomAccessFile file = new RandomAccessFile(path.toFile(), "rw");
FileLock lock = file.getChannel().lock(from, size, false)) {
if (lock.isValid()) {
log.debug("This is a valid exclusive lock");
return lock;
}
return null;
} catch (Exception e) {
System.out.println(e.getMessage());
}
return null;
}
/**
* Getting a write lock on a file region
*/
static FileLock getExclusiveLockFromFileChannelOpen(long from, long size) throws IOException {
Path path = Files.createTempFile("foo", "txt");
try (FileChannel channel = FileChannel.open(path, StandardOpenOption.APPEND);
FileLock lock = channel.lock(from, size, false)) {
String text = "Hello, world.";
ByteBuffer buffer = ByteBuffer.allocate(text.length() + System.lineSeparator().length());
buffer.put((text + System.lineSeparator()).getBytes(Charsets.UTF_8));
buffer.flip();
while (buffer.hasRemaining()) {
channel.write(buffer, channel.size());
}
log.debug("This was written to the file");
Files.lines(path).forEach(System.out::println);
return lock;
}
}
// Write locks
// Read locks
/**
* Trying to get a shared lock on a write-only FileChannel won't work.
*/
static void getReadLockFromOutputStream(long from, long size) throws IOException {
Path path = Files.createTempFile("foo", "txt");
try (FileOutputStream fis = new FileOutputStream(path.toFile());
FileLock lock = fis.getChannel().lock(0, Long.MAX_VALUE, true)) {
log.debug("This won't happen");
} catch (NonReadableChannelException e) {
log.error(
"The channel obtained through a FileOutputStream isn't readable. "
+ "You can't obtain an shared lock on it!");
throw e;
}
}
/**
* Locking a file for reading doesn't require a writable FileChannel.
*
* @param from beginning of the locked region
* @param size how many bytes to lock
* @return
* @throws IOException
*/
static FileLock getReadLockFromInputStream(long from, long size) throws IOException {
Path path = Files.createTempFile("foo", "txt");
try (FileInputStream fis = new FileInputStream(path.toFile());
FileLock lock = fis.getChannel().lock(from, size, true)) {
if (lock.isValid()) {
log.debug("This is a valid shared lock");
return lock;
}
return null;
}
}
/**
* Getting an exclusive lock from a RandomAccessFile works if the file is in read mode.
* @param from beginning of the locked region
* @param size how many bytes to lock
* @return
* @throws IOException
*/
static FileLock getReadLockFromRandomAccessFile(long from, long size) throws IOException {
Path path = Files.createTempFile("foo", "txt");
try (RandomAccessFile file = new RandomAccessFile(path.toFile(), "r"); // could also be "rw", but "r" is sufficient for reading
FileLock lock = file.getChannel().lock(from, size, true)) {
if (lock.isValid()) {
log.debug("This is a valid shared lock");
return lock;
}
return null;
} catch (Exception e) {
log.error(e.getMessage());
}
return null;
}
/**
* Trying to get an exclusive lock on a read-only FileChannel won't work.
*/
static void getExclusiveLockFromInputStream() throws IOException {
Path path = Files.createTempFile("foo", "txt");
try (FileInputStream fis = new FileInputStream(path.toFile()); FileLock lock = fis.getChannel().lock()) {
LOG.debug("This won't happen");
} catch (NonWritableChannelException e) {
LOG.error("The channel obtained through a FileInputStream isn't writable. You can't obtain an exclusive lock on it!");
throw e;
}
}
/**
* Gets an exclusive lock from a RandomAccessFile. Works because the file is writable.
* @param from beginning of the locked region
* @param size how many bytes to lock
* @return A lock object representing the newly-acquired lock
* @throws IOException if there is a problem creating the temporary file
*/
static FileLock getExclusiveLockFromRandomAccessFile(long from, long size) throws IOException {
Path path = Files.createTempFile("foo", "txt");
try (RandomAccessFile file = new RandomAccessFile(path.toFile(), "rw"); FileLock lock = file.getChannel().lock(from, size, false)) {
if (lock.isValid()) {
LOG.debug("This is a valid exclusive lock");
return lock;
}
return null;
} catch (Exception e) {
LOG.error(e.getMessage());
}
return null;
}
static class Writer implements Runnable {
/**
* Acquires an exclusive lock on a file region.
* @param from beginning of the locked region
* @param size how many bytes to lock
* @return A lock object representing the newly-acquired lock
* @throws IOException if there is a problem creating the temporary file
*/
static FileLock getExclusiveLockFromFileChannelOpen(long from, long size) throws IOException {
Path path = Files.createTempFile("foo", "txt");
try (FileChannel channel = FileChannel.open(path, StandardOpenOption.APPEND); FileLock lock = channel.lock(from, size, false)) {
String text = "Hello, world.";
ByteBuffer buffer = ByteBuffer.allocate(text.length() + System.lineSeparator().length());
buffer.put((text + System.lineSeparator()).getBytes(StandardCharsets.UTF_8));
buffer.flip();
while (buffer.hasRemaining()) {
channel.write(buffer, channel.size());
}
LOG.debug("This was written to the file");
Files.lines(path).forEach(LOG::debug);
return lock;
}
}
private Path path;
// Read locks
private volatile RandomAccessFile file;
/**
* Trying to get a shared lock on a write-only FileChannel won't work.
*/
static void getReadLockFromOutputStream() throws IOException {
Path path = Files.createTempFile("foo", "txt");
try (FileOutputStream fis = new FileOutputStream(path.toFile()); FileLock lock = fis.getChannel().lock(0, Long.MAX_VALUE, true)) {
LOG.debug("This won't happen");
} catch (NonReadableChannelException e) {
LOG.error("The channel obtained through a FileOutputStream isn't readable. " + "You can't obtain an shared lock on it!");
throw e;
}
}
private String text;
/**
* Gets a lock from an <tt>InputStream</tt>.
* @param from beginning of the locked region
* @param size how many bytes to lock
* @return A lock object representing the newly-acquired lock
* @throws IOException if there is a problem creating the temporary file
*/
static FileLock getReadLockFromInputStream(long from, long size) throws IOException {
Path path = Files.createTempFile("foo", "txt");
try (FileInputStream fis = new FileInputStream(path.toFile()); FileLock lock = fis.getChannel().lock(from, size, true)) {
if (lock.isValid()) {
LOG.debug("This is a valid shared lock");
return lock;
}
return null;
}
}
private volatile CountDownLatch countDownLatch;
/**
* Gets an exclusive lock from a RandomAccessFile. Works because the file is readable.
* @param from beginning of the locked region
* @param size how many bytes to lock
* @return A lock object representing the newly-acquired lock
* @throws IOException if there is a problem creating the temporary file
*/
static FileLock getReadLockFromRandomAccessFile(long from, long size) throws IOException {
Path path = Files.createTempFile("foo", "txt");
try (RandomAccessFile file = new RandomAccessFile(path.toFile(), "r"); // could also be "rw", but "r" is sufficient for reading
FileLock lock = file.getChannel().lock(from, size, true)) {
if (lock.isValid()) {
LOG.debug("This is a valid shared lock");
return lock;
}
} catch (Exception e) {
LOG.error(e.getMessage());
}
return null;
}
/**
*
* @param path The path to the file we will write into
* @param text The text to write
* @param countDownLatch A counter for thread synchronization
*
*/
public Writer(Path path, String text, CountDownLatch countDownLatch) {
this.path = path;
this.text = text;
this.countDownLatch = countDownLatch;
}
@Override
public void run() {
try {
lockAndWrite();
} catch (InterruptedException | FileNotFoundException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
countDownLatch.countDown();
}
private void lockAndWrite() throws InterruptedException, FileNotFoundException {
ByteBuffer buffer = null;
if (file == null) {
file = new RandomAccessFile(path.toFile(), "rw");
}
try (FileChannel channel = file.getChannel()) {
try (FileLock lock = channel.tryLock(channel.size(),
channel.size() + text.length() + System.lineSeparator().length(), true)) {
if (lock != null) {
String text = "Hello, world.";
buffer = ByteBuffer.allocate(text.length() + System.lineSeparator().length());
buffer.put((text + System.lineSeparator()).getBytes(Charsets.UTF_8));
buffer.flip();
while (buffer.hasRemaining()) {
channel.write(buffer, channel.size());
}
}
} catch (OverlappingFileLockException e) {
// Failed to lock. Try again later.
Thread.sleep(50);
lockAndWrite();
}
} catch (IOException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
}
}
public static void main(String[] args) throws InterruptedException, IOException {
Path path = Paths.get("/tmp/foo");
Files.deleteIfExists(path);
Files.createFile(path);
int concurrentWriters = 5;
CountDownLatch countDownLatch = new CountDownLatch(concurrentWriters);
// Launch 10 writers in parallel
final AtomicInteger count = new AtomicInteger(0);
Stream.generate(() -> new Thread(new Writer(path, "foo " + count.incrementAndGet(), countDownLatch)))
.limit(concurrentWriters).forEach(Thread::start);
countDownLatch.await();
AtomicInteger lineCount = new AtomicInteger(0);
Files.lines(path).forEach((line) -> {
lineCount.incrementAndGet();
System.out.println(line);
});
log.info("Total lines written = " + lineCount.get());
}
}

View File

@ -1,87 +0,0 @@
package com.baeldung.lock;
import static org.junit.Assert.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.IOException;
import java.nio.channels.FileLock;
import java.nio.channels.NonReadableChannelException;
import java.nio.channels.NonWritableChannelException;
import org.junit.jupiter.api.Test;
class FileLocksTest {
// Exclusive locks
/**
* Getting an exclusive (write) lock on the entire file.
* Fails when getting the lock from a FileChannel obtained through a FileInputStream.
*/
@Test
void givenAnInputStream_whenGetWriteLock_throwNonWritableChannelException() {
assertThrows(NonWritableChannelException.class,
() -> FileLocks.getExclusiveLockFromInputStream());
}
/**
* Getting an exclusive lock from a RandomAccessFile
* @throws IOException
*/
@Test
void givenARandomAccessFileWithReadWriteAccess_whenGetWriteLock_lock() throws IOException {
FileLock lock = FileLocks.getExclusiveLockFromRandomAccessFile(0, 10);
assertNotNull(lock);
assertFalse(lock.isShared());
}
/**
* Getting an exclusive lock from FileChannel::open
* @throws IOException
*/
@Test
void givenAPath_whenGetExclusiveLock_lock() throws IOException {
FileLock lock = FileLocks.getExclusiveLockFromFileChannelOpen(0, 10);
assertNotNull(lock);
assertFalse(lock.isShared());
}
// Shared locks
/**
* Getting a shared (read) lock on the entire file.
* Fails when getting the lock from a FileChannel obtained through a FileOutputStream.
*/
@Test
void givenAFileOutputStream_whenGetSharedLock_throwNonReadableChannelException() {
assertThrows(NonReadableChannelException.class,
() -> FileLocks.getReadLockFromOutputStream(0, 10));
}
/**
* Getting a shared (read) lock works fine when getting the lock from a FileChannel obtained through a FileInputStream.
* @throws IOException
*/
@Test
void givenAnInputStream_whenGetSharedLock_lock() throws IOException {
FileLock lock = FileLocks.getReadLockFromInputStream(0, 10);
assertNotNull(lock);
assertTrue(lock.isShared());
}
/**
* Getting a shared lock from a RandomAccessFile
* @throws IOException
*/
@Test
void givenARandomAccessFile_whenGetSharedLock_lock() throws IOException {
FileLock lock = FileLocks.getReadLockFromRandomAccessFile(0, 10);
assertNotNull(lock);
assertTrue(lock.isShared());
}
}

View File

@ -0,0 +1,48 @@
package com.baeldung.lock;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.IOException;
import java.nio.channels.FileLock;
import java.nio.channels.NonReadableChannelException;
import java.nio.channels.NonWritableChannelException;
import org.junit.jupiter.api.Test;
class FileLocksUnitTest {
@Test void givenAnInputStream_whenGetWriteLock_throwNonWritableChannelException() {
assertThrows(NonWritableChannelException.class, () -> FileLocks.getExclusiveLockFromInputStream());
}
@Test void givenARandomAccessFileWithReadWriteAccess_whenGetWriteLock_lock() throws IOException {
FileLock lock = FileLocks.getExclusiveLockFromRandomAccessFile(0, 10);
assertNotNull(lock);
assertFalse(lock.isShared());
}
@Test void givenAPath_whenGetExclusiveLock_lock() throws IOException {
FileLock lock = FileLocks.getExclusiveLockFromFileChannelOpen(0, 10);
assertNotNull(lock);
assertFalse(lock.isShared());
}
@Test void givenAFileOutputStream_whenGetSharedLock_throwNonReadableChannelException() {
assertThrows(NonReadableChannelException.class, FileLocks::getReadLockFromOutputStream);
}
@Test void givenAnInputStream_whenGetSharedLock_lock() throws IOException {
FileLock lock = FileLocks.getReadLockFromInputStream(0, 10);
assertNotNull(lock);
assertTrue(lock.isShared());
}
@Test void givenARandomAccessFile_whenGetSharedLock_lock() throws IOException {
FileLock lock = FileLocks.getReadLockFromRandomAccessFile(0, 10);
assertNotNull(lock);
assertTrue(lock.isShared());
}
}

View File

@ -1,4 +1,3 @@
### Relevant Articles:
- [Service Locator Pattern](https://www.baeldung.com/java-service-locator-pattern)
- [The DAO Pattern in Java](https://www.baeldung.com/java-dao-pattern)
- [A Practical Example of Hexagonal Architecture in Java](https://www.baeldung.com/java-hexagonal-pattern)

View File

@ -1,45 +0,0 @@
package com.baeldung.hexagonal;
import com.baeldung.hexagonal.adapters.DisplayInConsoleAdapter;
import com.baeldung.hexagonal.adapters.InMemoryMessageStore;
import com.baeldung.hexagonal.application.ChatManager;
import com.baeldung.hexagonal.domain.ChatUser;
import com.baeldung.hexagonal.domain.IDisplayMessages;
import java.util.ArrayDeque;
import java.util.Scanner;
public class ChatApplication {
public static void main(String[] args) {
Scanner console = new Scanner(System.in);
System.out.print("Enter username of user 1: ");
ChatUser user1 = new ChatUser(console.nextLine());
System.out.printf("Enter username of user 2: ");
ChatUser user2 = new ChatUser(console.nextLine());
System.out.println("Chat will end when any user uses the word bye in a message.");
InMemoryMessageStore messageStore = new InMemoryMessageStore(new ArrayDeque<>(10));
IDisplayMessages messageDisplayer = new DisplayInConsoleAdapter(messageStore);
ChatManager chatManager = new ChatManager(messageStore, messageDisplayer);
while (true) {
System.out.printf("From %s to %s : ", user1, user2);
String message = console.nextLine();
if (message.toLowerCase().contains("bye")) {
chatManager.sendMessage(user1, user2, message);
System.out.println("Chat recap:");
messageDisplayer.displayMessages();
System.exit(0);
}
chatManager.sendMessage(user1, user2, message);
ChatUser temp = user1;
user1 = user2;
user2 = temp;
}
}
}

View File

@ -1,36 +0,0 @@
package com.baeldung.hexagonal.adapters;
import com.baeldung.hexagonal.domain.ChatMessage;
import com.baeldung.hexagonal.domain.IDisplayMessages;
import com.baeldung.hexagonal.domain.IStoreMessages;
/**
* A Utility adapter to display the chat messages in the console
*/
public class DisplayInConsoleAdapter implements IDisplayMessages {
IStoreMessages messageStore;
public DisplayInConsoleAdapter(IStoreMessages messageStore) {
this.messageStore = messageStore;
}
public static void clearScreen() {
System.out.print("\033[H\033[2J");
System.out.flush();
}
@Override
public void displayMessages() {
clearScreen();
for (ChatMessage message: messageStore.getMessages(10)) {
System.out.printf(
"%tF %tT [%s to %s]: %s %n",
message.getTimeSent(),
message.getTimeSent(),
message.getFrom(),
message.getTo(),
message.getContents());
}
}
}

View File

@ -1,37 +0,0 @@
package com.baeldung.hexagonal.adapters;
import com.baeldung.hexagonal.domain.ChatMessage;
import com.baeldung.hexagonal.domain.IStoreMessages;
import java.util.Collection;
import java.util.Comparator;
import java.util.Queue;
import java.util.stream.Collectors;
/**
* We're storing the messages in memory. We could later opt to switch to an implementation that stores messages in a
* database.
*/
public class InMemoryMessageStore implements IStoreMessages {
private Queue<ChatMessage> messages;
public InMemoryMessageStore(Queue<ChatMessage> messages) {
this.messages = messages;
}
@Override
public void storeMessage(ChatMessage message) {
this.messages.add(message);
}
@Override
public Collection<ChatMessage> getMessages(long maxNbMessages) {// @formatter:off
return messages.stream()
.sorted((m1, m2) -> m2.getTimeSent().compareTo(m1.getTimeSent()))
.limit(maxNbMessages)
.sorted(Comparator.comparing(ChatMessage::getTimeSent))
.collect(Collectors.toList());
// @formatter:on
}
}

View File

@ -1,27 +0,0 @@
package com.baeldung.hexagonal.application;
import com.baeldung.hexagonal.domain.*;
import java.time.LocalDateTime;
public class ChatManager implements ISendMessage {
// The domain doesn't need to know where messages will be stored
private IStoreMessages messageStore;
// The domain doesn't need to know how messages will be displayed
private IDisplayMessages messageDisplayer;
public ChatManager(IStoreMessages messageStore, IDisplayMessages displayMessages) {
this.messageStore = messageStore;
this.messageDisplayer = displayMessages;
}
@Override
public void sendMessage(ChatUser from, ChatUser to, String message) {
ChatMessage chatMessage = new ChatMessage(LocalDateTime.now(), from, to, message);
this.messageStore.storeMessage(chatMessage);
this.messageDisplayer.displayMessages();
}
}

View File

@ -1,34 +0,0 @@
package com.baeldung.hexagonal.domain;
import java.time.LocalDateTime;
public class ChatMessage {
private LocalDateTime timeSent;
private ChatUser from;
private ChatUser to;
private String contents;
public LocalDateTime getTimeSent() {
return timeSent;
}
public ChatUser getFrom() {
return from;
}
public ChatUser getTo() {
return to;
}
public String getContents() {
return contents;
}
public ChatMessage(LocalDateTime timeSent, ChatUser from, ChatUser to, String contents) {
this.timeSent = timeSent;
this.from = from;
this.to = to;
this.contents = contents;
}
}

View File

@ -1,14 +0,0 @@
package com.baeldung.hexagonal.domain;
public class ChatUser {
private String name;
public ChatUser(String name) {
this.name = name;
}
@Override
public String toString() {
return name;
}
}

View File

@ -1,8 +0,0 @@
package com.baeldung.hexagonal.domain;
/**
* An external system displays the messages sent by users.
*/
public interface IDisplayMessages {
void displayMessages();
}

View File

@ -1,8 +0,0 @@
package com.baeldung.hexagonal.domain;
/**
* A user sends a message (application's use case)
*/
public interface ISendMessage {
void sendMessage(ChatUser from, ChatUser to, String message);
}

View File

@ -1,14 +0,0 @@
package com.baeldung.hexagonal.domain;
import java.util.Collection;
/**
* An external system (infrastructure) stores a message
*/
public interface IStoreMessages {
void storeMessage(ChatMessage message);
Collection<ChatMessage> getMessages(long maxNbMessages);
}