"getValue() method is redundant, use fromMetaWalEntry instead"

[rsingh433]

getValue() method is redundant, the same value can be obtained using the fromMetaWalEntry method.

[keith-turner]

For the test changes, thinking those should not change. If those test are failing let me know and I would be happy to take a look and see what is happening.

[ctubbsii]

At a glance, these changes look good to me, but I'm not sure I understand why the test is failing. Or rather, I'm not sure how the test was passing before.

One thing I saw was that the test can be simplified by using a new SimpleImmutableEntry(key, entry.getValue()); rather than creating an anonymous subclass of Entry on lines 60-75.

Another issue is that the LogEntry.validateFilePath method (where this is failing) seems to have an out-of-date concept of the format of the file path. I don't think it's correct.

Given the out of date format that is being validated in validateFilePath method, I see why it's failing (because the test's file name is "foo" instead of a UUID), but I don't understand why it wasn't already failing before.

This will need to be tracked down and understood, I think, in order to make progress here.

[ctubbsii]

@keith-turner helped me understand the test failures (or rather, why the current test is passing in the main branch when it probably shouldn't), and I opened up #4035 to help with that. Once that is merged, I'll help resolve the conflicts with this PR, so we can merge this in after that.

[ctubbsii]

These changes look good, as-is, and can be merged now to drop storing the "filePath" in the Value and reading it from the column qualifier instead. However, there's some follow-on actions that you can add to this PR, or can create a separate PR for, at your discretion. Here's some things I saw while looking at this code, where there's room for improvement:

1. Pull in the mutation modification work into the LogEntry class. So, instead of calling code calling in to LogEntry to get the column qualifier, value, etc. to update a mutation, you just pass a mutation into the LogEntry class like: logEntry.addToMutation(mutation);, and the mutation updating is done internal to that method. That way, the column qualifier, value, etc. doesn't need to be used outside the class (except maybe for unit testing) and they can be made private, or even simplified or removed.
2. Get rid of switchFile method, since that's outdated, and simplify how VolumeUtil is using it
3. Rename "filePath", "filename", and "getFilePath()", etc. to something that reflects what is actually stored there. From what I can tell, what's stored there is a "-/host:port/UUID" (with "-/" ignored). So, maybe a better name is "logReference", since the host and UUID together could be called that. Or something like "serverAndUUID", but that's not really very friendly. If there's a better name, I'm open to that as well.
4. Make the validate method more rigorous and expect exactly 3 items ("-/host:port/UUID" -> {"-", "host:port", "UUID"}. Right now validates either 2 or 3... and it's not clear why. Probably so it could validate the old value, which did not contain the "-/" portion, as well as the column qualifier, which did. But it doesn't need to validate the 2-item version, since that shouldn't exist
5. Add upgrade code to drop the "-/" portion entirely, and further simplify all the code that tries to handle that, so it's only "host:port/UUID" instead in the column qualifier.

Some of these are certainly more complicated than others, and would require a separate PR. But some of these are small enough to include with this.

[rsingh433]

I removed the switchFile method and updated the switchVolumes method to directly create a new LogEntry instance with the switched file path. Now, the LogEntry class itself is responsible for creating instances based on the file path. Also, I added the functionality to add LogEntry information to a mutation by calling logEntry.addToMutation(mutation).