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.

Sign up for GitHub

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

Jump to bottom

Extend 'addFixedExposedPort' method with InternetProtocol argument #586

Merged
merged 3 commits into from
Feb 21, 2018
Merged

Extend 'addFixedExposedPort' method with InternetProtocol argument #586

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c41bbe5
Extend 'addFixedExposedPort' method with InternetProtocol argument
agafox Feb 17, 2018
e1165a3
'InternetProtocol' enum introduced to avoid docker-java in public API
agafox Feb 17, 2018
7934570
'InternetProtocol' enum values in upper case in accordance with code …
agafox Feb 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions core/src/main/java/org/testcontainers/containers/FixedHostPortGenericContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.testcontainers.containers;

import com.github.dockerjava.api.model.InternetProtocol;
import org.jetbrains.annotations.NotNull;

/**
Expand All @@ -17,14 +18,26 @@ public FixedHostPortGenericContainer(@NotNull String dockerImageName) {
}

/**
* Bind a fixed port on the docker host to a container port
* Bind a fixed TCP port on the docker host to a container port
* @param hostPort a port on the docker host, which must be available
* @param containerPort a port in the container
* @return this container
*/
public SELF withFixedExposedPort(int hostPort, int containerPort) {

super.addFixedExposedPort(hostPort, containerPort);
return withFixedExposedPort(hostPort, containerPort, InternetProtocol.DEFAULT);
}

/**
* Bind a fixed port on the docker host to a container port
* @param hostPort a port on the docker host, which must be available
* @param containerPort a port in the container
* @param protocol an internet protocol (tcp or udp)
* @return this container
*/
public SELF withFixedExposedPort(int hostPort, int containerPort, InternetProtocol protocol) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather not leak docker-java implementation details into the public API. Instead I think we should use our own Enums here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point @kiview
Fixed.


super.addFixedExposedPort(hostPort, containerPort, protocol);

return self();
}
Expand Down
19 changes: 17 additions & 2 deletions core/src/main/java/org/testcontainers/containers/GenericContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ public SELF withExposedPorts(Integer... ports) {
}

/**
* Add a container port that should be bound to a fixed port on the docker host.
* Add a TCP container port that should be bound to a fixed port on the docker host.
* <p>
* Note that this method is protected scope to discourage use, as clashes or instability are more likely when
* using fixed port mappings. If you need to use this method from a test, please use {@link FixedHostPortGenericContainer}
Expand All @@ -654,7 +654,22 @@ public SELF withExposedPorts(Integer... ports) {
* @param containerPort
*/
protected void addFixedExposedPort(int hostPort, int containerPort) {
portBindings.add(String.format("%d:%d", hostPort, containerPort));
addFixedExposedPort(hostPort, containerPort, InternetProtocol.DEFAULT);
}

/**
* Add a container port that should be bound to a fixed port on the docker host.
* <p>
* Note that this method is protected scope to discourage use, as clashes or instability are more likely when
* using fixed port mappings. If you need to use this method from a test, please use {@link FixedHostPortGenericContainer}
* instead of GenericContainer.
*
* @param hostPort
* @param containerPort
* @param protocol
*/
protected void addFixedExposedPort(int hostPort, int containerPort, InternetProtocol protocol) {
portBindings.add(String.format("%d:%d/%s", hostPort, containerPort, protocol));
}

/**
Expand Down