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

refactor(image): avoid unnecessary owned structs and boxing #651

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

DDtKey
Copy link
Collaborator

@DDtKey DDtKey commented Jun 11, 2024

It's quite big incompatible change, but in general it doesn't require a lot of work to switch to new API.
And it will be mostly transparent for users of ready-to-use modules

  • get rid of Box<dyn Iterator<..>, use RPIT instead
  • use Into<Cow<'_, str> instead of &String
  • use &str instead of String

In fact, we don't need owned structs from image methods, and even if we need - it's better to cast implicitly under the hood, avoiding extra cloning in every image implementation.

Also, a new API relaxes requirements of returned type and makes some use-cases simpler.

- get rid of `Box<dyn Iterator<..>`, use RPIT instead
- use `Into<Cow<'_, str>` instead of `&String`
- use `&str` instead of `String`

In fact, we don't need owned structs from image methods, and even if we need - it's better to cast implicitly under the hood, avoiding extra cloning in every image implementation.

Also, a new API relaxes requirements of returned type and makes some use-cases simpler.
Copy link

netlify bot commented Jun 11, 2024

Deploy Preview for testcontainers-rust ready!

Name Link
🔨 Latest commit da614e5
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-rust/deploys/66689b308899d90008186c3f
😎 Deploy Preview https://deploy-preview-651--testcontainers-rust.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@DDtKey DDtKey merged commit 4b3d2d8 into main Jun 11, 2024
13 checks passed
@DDtKey DDtKey deleted the refactor/drop-box-dyn branch June 11, 2024 18:51
@DDtKey DDtKey added this to the 0.18.0 milestone Jun 12, 2024
This was referenced Jun 15, 2024
DDtKey pushed a commit that referenced this pull request Jun 15, 2024
## 🤖 New release
* `testcontainers`: 0.17.0 -> 0.18.0

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [0.18.0] - 2024-06-15

### Details
#### Bug Fixes
- [❗] Make `DOCKER_CONFIG` usage consistent with Docker CLI
([#654](#654))

#### Features
- [❗] Support UDP and SCTP port mappings
([#655](#655))
- Impl `From<u16>` for `ContainerPort` with TCP default
([#658](#658))
- Support HTTP wait strategy
([#659](#659))
- Allow passing `u16` to `Ports`

#### Miscellaneous Tasks
- Use nightly `rustfmt`
([#657](#657))

#### Refactor
- [❗] Get rid of associated type `ImageArgs` and rename to `cmd`
([#649](#649))
- Avoid unnecessary owned structs and boxing
([#651](#651))
- [❗] Add `ImageExt` trait to avoid explicit conversion to
`RunnableImage`
([#652](#652))
- [❗] Rename `RunnableImage` to `ContainerRequest`
([#653](#653))
- [❗] Exposed and mapped ports api
([#656](#656))
- Preliminary refactoring of `wait` strategies
([#661](#661))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

## Migration Guide

### Overview
Most of the breaking changes introduced in this version primarily impact
developers who implement their own images. For general usage, the API
remains mostly the same, with some improvements and enhancements for
better performance and flexibility.

### 1. Renaming of `RunnableImage`
- **Old**: `RunnableImage`
- **New**: `ContainerRequest`
- **Update**: The explicit conversion from `Image` to `ContainerRequest`
(formerly `RunnableImage`) is no longer necessary. Instead, you can now
directly import `testcontainers::ImageExt` and override image parameters
as needed.

### 2. Changes to `Image` Methods
- **Method**: `Image::tag` and `Image::name`
  - **Old Return Type**: `String`
  - **New Return Type**: `&str`
- **Update**: Update any code that relies on these methods to handle the
new return type `&str`. This change helps improve performance by
avoiding unnecessary allocations.

### 3. Changes to `Image::exposed_ports`
- **Method**: `Image::exposed_ports`
- **Old Return Type**: Implementation-specific or previously different.
  - **New Return Type**: `&[ContainerPort]`
- **Update**: The method now returns a slice of `ContainerPort`, which
supports exposing ports with protocols `TCP`, `UDP`, and `SCTP`. Update
your code to handle the slice accordingly.

### 4. Removal of Associated Type `Args` in `Image`
- **Old**: `Image` had an associated type `Args`.
- **New**: The associated type `Args` is removed.
- **Update**: Command arguments are now part of `Image::cmd`, which
returns `impl IntoIterator<Item = impl Into<String>>`. This change
allows more flexibility in specifying command arguments. Ensure your
code is updated to work with the new method signature.

### 5. Simplification of Trait Implementation in `Image`
- **Old**: `Image` required `Box<dyn ..>` for certain traits.
- **New**: Utilizes Return Position `impl` Trait in Trait (RPITIT).
- **Update**: Instead of requiring `Box<dyn ..>`, `Image` now uses
RPITIT for trait returns. This change simplifies the code and improves
readability and performance. Familiarize yourself with
[RPITIT](https://rustc-dev-guide.rust-lang.org/return-position-impl-trait-in-trait.html)
to understand its benefits and applications in your implementation.

### 6. Changes to `RunnableImage::with_mapped_port`
- **Old**: `RunnableImage::with_mapped_port`
- **New**: Accessible through `ImageExt::with_mapped_port`
- **Update**: This method now accepts two parameters instead of a tuple.
Adjust your method calls to pass the parameters separately.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant