Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept image names with library/ prefix as a valid substitute #6174

Merged
merged 11 commits into from
Dec 14, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ public final class DockerImageName {

private static final Pattern REPO_NAME = Pattern.compile(REPO_NAME_PART + "(/" + REPO_NAME_PART + ")*");

private static final String LIBRARY_PREFIX = "library/";

private final String rawName;

@With
Expand Down Expand Up @@ -231,8 +233,15 @@ public DockerImageName asCompatibleSubstituteFor(DockerImageName otherImageName)
* @return whether this image has declared compatibility.
*/
public boolean isCompatibleWith(DockerImageName other) {
// is this image already the same or equivalent?
if (other.equals(this)) {
// Make sure we always compare against a version of the image name containing the LIBRARY_PREFIX
String finalImageName;
if (this.repository.startsWith(LIBRARY_PREFIX)) {
eddumelendez marked this conversation as resolved.
Show resolved Hide resolved
finalImageName = this.repository;
} else {
finalImageName = LIBRARY_PREFIX + this.repository;
}
DockerImageName imageWithLibraryPrefix = DockerImageName.parse(finalImageName);
if (other.equals(this) || imageWithLibraryPrefix.equals(this)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Minor) More cleaner

if (Objects.equals(other, this) || Objects.equals(imageWithLibraryPrefix, this)) {
   ....
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this changes the degree of "cleanness", but a question of taste (beside the null-safety of Objects.equals() in general). Either style should be fine in this case.

return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ public void testAssertMethodAcceptsCompatible() {
subject.assertCompatibleWith(DockerImageName.parse("bar"));
}

@Test
public void testAssertMethodAcceptsCompatibleLibraryPrefix() {
eddumelendez marked this conversation as resolved.
Show resolved Hide resolved
DockerImageName subject = DockerImageName.parse("library/foo");
subject.assertCompatibleWith(DockerImageName.parse("foo"));
}

@Test
public void testAssertMethodRejectsIncompatible() {
DockerImageName subject = DockerImageName.parse("foo");
Expand Down