Fix for trailing / #166
Fix for trailing / #166
Conversation
| */ | ||
| public class LocationNormalizer { | ||
|
|
||
| public String normalize(String location) { |
There was a problem hiding this comment.
I think we can expand on this in follow up PR to normalize scheme s3a/s3n/s3 to s3
| } | ||
| String normalizedOldLocation = locationNormalizer.normalize(oldLocation); | ||
| String normalizedLocation = locationNormalizer.normalize(location); | ||
| return normalizedOldLocation.equals(normalizedLocation); |
There was a problem hiding this comment.
We should expand in follow up PR to check that location doesn't start with oldLocation and vica versa.
old: s3://foo/bar
new: s3:/foo
Should be considered same.
There was a problem hiding this comment.
I can't say right now if it's a good idea.
There was a problem hiding this comment.
that's why I'm holding off, need to think about this.
| } | ||
| String normalizedOldLocation = locationNormalizer.normalize(oldLocation); | ||
| String normalizedLocation = locationNormalizer.normalize(location); | ||
| return normalizedOldLocation.equals(normalizedLocation); |
There was a problem hiding this comment.
I can't say right now if it's a good idea.
| void normalizeTrailingSlash() { | ||
| assertThat(normalizer.normalize("s3://bucket/prefix/")).isEqualTo("s3://bucket/prefix"); | ||
| assertThat(normalizer.normalize("s3://bucket/prefix///")).isEqualTo("s3://bucket/prefix"); | ||
| assertThat(normalizer.normalize("s3://bucket/prefix/../")).isEqualTo("s3://bucket/prefix/.."); |
There was a problem hiding this comment.
This looks wrong in a traditional file system.
s3://bucket/prefix/../ is equal to s3://bucket.
There was a problem hiding this comment.
yeah I'll remove the example
There was a problem hiding this comment.
Even if you remove the example, but that behaviour is still taking place, the algorithm is wrong.
There was a problem hiding this comment.
I agree but the "algorithm" just does / removal. I'm not even sure you can have relative paths in hfds (s3 for sure not).
There are probably more usecase where you can specify a path in multiple ways pointing to the same thing.
My example was made up I'm not sure if it's relevant (at this point).
There was a problem hiding this comment.
For future discussion: https://issues.apache.org/jira/browse/HDFS-836
There was a problem hiding this comment.
the licence header is different because this class was copied from https://github.com/ThreeTen/threeten-extra/blob/main/src/main/java/org/threeten/extra/PeriodDuration.java
There was a problem hiding this comment.
ah ok these shouldn't be changed at all I thought it was just copyright, we should exlude them I'll have a look
No description provided.