DomGarguilo commented on code in PR #4082:
URL: https://github.com/apache/accumulo/pull/4082#discussion_r1432947492


##########
core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java:
##########
@@ -27,103 +27,153 @@
 import org.apache.accumulo.core.data.Value;
 import 
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LogColumnFamily;
 import org.apache.hadoop.io.Text;
+import org.checkerframework.checker.nullness.qual.NonNull;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.net.HostAndPort;
 
-public class LogEntry {
+public final class LogEntry {
 
-  private final String filePath;
+  private final String path;
+  private final HostAndPort tserver;
+  private final UUID uniqueId;
 
-  public LogEntry(String filePath) {
-    validateFilePath(filePath);
-    this.filePath = filePath;
-  }
-
-  public String getFilePath() {
-    return this.filePath;
+  private LogEntry(String path, HostAndPort tserver, UUID uniqueId) {
+    this.path = path;
+    this.tserver = tserver;
+    this.uniqueId = uniqueId;
   }
 
   /**
-   * Validates the expected format of the file path. We expect the path to 
contain a tserver
-   * (host:port) followed by a UUID as the file name. For example,
-   * localhost:1234/927ba659-d109-4bce-b0a5-bcbbcb9942a2 is a valid file path.
+   * Creates a new LogEntry object after validating the expected format of the 
path. We expect the
+   * path to contain a tserver (host+port) followed by a UUID as the file name 
as the last two
+   * components.<br>
+   * For example, 
file:///some/dir/path/localhost+1234/927ba659-d109-4bce-b0a5-bcbbcb9942a2 is a
+   * valid path.
    *
-   * @param filePath path to validate
-   * @throws IllegalArgumentException if the filePath is invalid
+   * @param path path to validate
+   * @return an object representation of this log entry
+   * @throws IllegalArgumentException if the path is invalid
    */
-  private static void validateFilePath(String filePath) {
-    String[] parts = filePath.split("/");
+  public static LogEntry fromPath(String path) {
+    String[] parts = path.split("/");
 
     if (parts.length < 2) {
       throw new IllegalArgumentException(
-          "Invalid filePath format. The path should at least contain 
tserver/UUID.");
+          "Invalid path format. The path should end with tserver/UUID.");
     }
 
     String tserverPart = parts[parts.length - 2];
     String uuidPart = parts[parts.length - 1];
 
+    String badTServerMsg =
+        "Invalid tserver in path. Expected: host+port. Found '" + tserverPart 
+ "'";
+    if (tserverPart.contains(":") || !tserverPart.contains("+")) {
+      throw new IllegalArgumentException(badTServerMsg);
+    }
+    HostAndPort tserver;
     try {
-      HostAndPort.fromString(tserverPart);
+      tserver = HostAndPort.fromString(tserverPart.replace("+", ":"));

Review Comment:
   I guess this is expected behavior but it seems odd that the following code 
causes the highlighed exception to be thrown since the host and port are 
separated by a `:` rather than `+`:
   ```java
         HostAndPort server = HostAndPort.fromParts("server1", 8555); // 
server1:8555
   
         String walFilePath =
             java.nio.file.Path.of(server.toString(), 
UUID.randomUUID().toString()).toString();
         LogEntry wal = LogEntry.fromPath(walFilePath);
   ```
   Especially since internally, the tserver string is being converted to a 
`HostAndPost` object. Not suggesting that any changes need to be or can be 
made, just thought it was interesting. 



-- 
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]

Reply via email to