Change Layer interface from &self to &mut self

The primary driver for wanting to mutate a layer is a desire for stateful logging. I hit this in the consuming state-machine interface, and previously, Josh hit it in a logging interface designed to track indentation level. Upside: - Developers are now allowed to do whatever they want within a layer - It's more technically correct to require a single reference for these functions. With the prior interface, it would be theoretically possible to execute the same Layer in parallel (though unlikely) - We can now change things between function calls. Downside: - Mutable self means there's no guarantee something didn't change between function calls.
heroku · Jan 27, 2024 · bbb6d9c · bbb6d9c
1 parent 2209e8f
commit bbb6d9c
Show file tree

Hide file tree

Showing 9 changed files with 33 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- `libcnb`:
+  - Change Layer interface from `&self` to `&mut self`. ([#669](https://github.com/heroku/libcnb.rs/pull/669))
+
 
 ## [0.17.0] - 2023-12-06
 

diff --git a/examples/execd/src/layer.rs b/examples/execd/src/layer.rs
@@ -21,7 +21,7 @@ impl Layer for ExecDLayer {
     }
 
     fn create(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_path: &Path,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {

diff --git a/libcnb/src/build.rs b/libcnb/src/build.rs
@@ -85,7 +85,7 @@ impl<B: Buildpack + ?Sized> BuildContext<B> {
     /// #    }
     /// #
     ///     fn create(
-    ///         &self,
+    ///         &mut self,
     ///         context: &BuildContext<Self::Buildpack>,
     ///         layer_path: &Path,
     ///     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {

diff --git a/libcnb/src/layer/handling.rs b/libcnb/src/layer/handling.rs
@@ -21,10 +21,10 @@ use std::path::{Path, PathBuf};
 pub(crate) fn handle_layer<B: Buildpack + ?Sized, L: Layer<Buildpack = B>>(
     context: &BuildContext<B>,
     layer_name: LayerName,
-    layer: L,
+    mut layer: L,
 ) -> Result<LayerData<L::Metadata>, HandleLayerErrorOrBuildpackError<B::Error>> {
     match read_layer(&context.layers_dir, &layer_name) {
-        Ok(None) => handle_create_layer(context, &layer_name, &layer),
+        Ok(None) => handle_create_layer(context, &layer_name, &mut layer),
         Ok(Some(layer_data)) => {
             let existing_layer_strategy = layer
                 .existing_layer_strategy(context, &layer_data)
@@ -33,9 +33,11 @@ pub(crate) fn handle_layer<B: Buildpack + ?Sized, L: Layer<Buildpack = B>>(
             match existing_layer_strategy {
                 ExistingLayerStrategy::Recreate => {
                     delete_layer(&context.layers_dir, &layer_name)?;
-                    handle_create_layer(context, &layer_name, &layer)
+                    handle_create_layer(context, &layer_name, &mut layer)
+                }
+                ExistingLayerStrategy::Update => {
+                    handle_update_layer(context, &layer_data, &mut layer)
                 }
-                ExistingLayerStrategy::Update => handle_update_layer(context, &layer_data, &layer),
                 ExistingLayerStrategy::Keep => {
                     // We need to rewrite the metadata even if we just want to keep the layer around
                     // since cached layers are restored without their types, causing the layer to be
@@ -106,7 +108,7 @@ pub(crate) fn handle_layer<B: Buildpack + ?Sized, L: Layer<Buildpack = B>>(
 fn handle_create_layer<B: Buildpack + ?Sized, L: Layer<Buildpack = B>>(
     context: &BuildContext<B>,
     layer_name: &LayerName,
-    layer: &L,
+    layer: &mut L,
 ) -> Result<LayerData<L::Metadata>, HandleLayerErrorOrBuildpackError<B::Error>> {
     let layer_dir = context.layers_dir.join(layer_name.as_str());
 
@@ -138,7 +140,7 @@ fn handle_create_layer<B: Buildpack + ?Sized, L: Layer<Buildpack = B>>(
 fn handle_update_layer<B: Buildpack + ?Sized, L: Layer<Buildpack = B>>(
     context: &BuildContext<B>,
     layer_data: &LayerData<L::Metadata>,
-    layer: &L,
+    layer: &mut L,
 ) -> Result<LayerData<L::Metadata>, HandleLayerErrorOrBuildpackError<B::Error>> {
     let layer_result = layer
         .update(context, layer_data)

diff --git a/libcnb/src/layer/public_interface.rs b/libcnb/src/layer/public_interface.rs
@@ -45,7 +45,7 @@ pub trait Layer {
     /// # Implementation Requirements
     /// Implementations **MUST NOT** write to any other location than `layer_path`.
     fn create(
-        &self,
+        &mut self,
         context: &BuildContext<Self::Buildpack>,
         layer_path: &Path,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error>;
@@ -74,7 +74,7 @@ pub trait Layer {
     /// # Implementation Requirements
     /// Implementations **MUST NOT** modify the file-system.
     fn existing_layer_strategy(
-        &self,
+        &mut self,
         context: &BuildContext<Self::Buildpack>,
         layer_data: &LayerData<Self::Metadata>,
     ) -> Result<ExistingLayerStrategy, <Self::Buildpack as Buildpack>::Error> {
@@ -100,7 +100,7 @@ pub trait Layer {
     /// # Implementation Requirements
     /// Implementations **MUST NOT** write to any other location than `layer_path`.
     fn update(
-        &self,
+        &mut self,
         context: &BuildContext<Self::Buildpack>,
         layer_data: &LayerData<Self::Metadata>,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {

diff --git a/libcnb/src/layer/tests.rs b/libcnb/src/layer/tests.rs
@@ -61,7 +61,7 @@ impl Layer for TestLayer {
     }
 
     fn create(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         layer_path: &Path,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -79,15 +79,15 @@ impl Layer for TestLayer {
     }
 
     fn existing_layer_strategy(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_data: &LayerData<Self::Metadata>,
     ) -> Result<ExistingLayerStrategy, <Self::Buildpack as Buildpack>::Error> {
         Ok(self.existing_layer_strategy)
     }
 
     fn update(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         layer_data: &LayerData<Self::Metadata>,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -749,7 +749,7 @@ fn default_layer_method_implementations() {
         }
 
         fn create(
-            &self,
+            &mut self,
             _context: &BuildContext<Self::Buildpack>,
             _layer_path: &Path,
         ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -770,7 +770,7 @@ fn default_layer_method_implementations() {
     let temp_dir = tempdir().unwrap();
     let context = build_context(&temp_dir);
     let layer_name = random_layer_name();
-    let simple_layer = SimpleLayer;
+    let mut simple_layer = SimpleLayer;
 
     let simple_layer_metadata = SimpleLayerMetadata {
         field_one: String::from("value one"),
@@ -838,7 +838,7 @@ fn layer_env_read_write() {
         }
 
         fn create(
-            &self,
+            &mut self,
             _context: &BuildContext<Self::Buildpack>,
             _layer_path: &Path,
         ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -848,7 +848,7 @@ fn layer_env_read_write() {
         }
 
         fn existing_layer_strategy(
-            &self,
+            &mut self,
             _context: &BuildContext<Self::Buildpack>,
             layer_data: &LayerData<Self::Metadata>,
         ) -> Result<ExistingLayerStrategy, <Self::Buildpack as Buildpack>::Error> {
@@ -858,7 +858,7 @@ fn layer_env_read_write() {
         }
 
         fn update(
-            &self,
+            &mut self,
             _context: &BuildContext<Self::Buildpack>,
             layer_data: &LayerData<Self::Metadata>,
         ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {

diff --git a/test-buildpacks/readonly-layer-files/src/layer.rs b/test-buildpacks/readonly-layer-files/src/layer.rs
@@ -24,7 +24,7 @@ impl Layer for TestLayer {
     }
 
     fn create(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         layer_path: &Path,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -42,7 +42,7 @@ impl Layer for TestLayer {
     }
 
     fn existing_layer_strategy(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_data: &LayerData<Self::Metadata>,
     ) -> Result<ExistingLayerStrategy, <Self::Buildpack as Buildpack>::Error> {

diff --git a/test-buildpacks/sbom/src/test_layer.rs b/test-buildpacks/sbom/src/test_layer.rs
@@ -22,7 +22,7 @@ impl Layer for TestLayer {
     }
 
     fn create(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_path: &Path,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -43,15 +43,15 @@ impl Layer for TestLayer {
     }
 
     fn existing_layer_strategy(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_data: &LayerData<Self::Metadata>,
     ) -> Result<ExistingLayerStrategy, <Self::Buildpack as Buildpack>::Error> {
         Ok(ExistingLayerStrategy::Update)
     }
 
     fn update(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_data: &LayerData<Self::Metadata>,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {

diff --git a/test-buildpacks/sbom/src/test_layer_2.rs b/test-buildpacks/sbom/src/test_layer_2.rs
@@ -22,7 +22,7 @@ impl Layer for TestLayer2 {
     }
 
     fn create(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_path: &Path,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -43,7 +43,7 @@ impl Layer for TestLayer2 {
     }
 
     fn existing_layer_strategy(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_data: &LayerData<Self::Metadata>,
     ) -> Result<ExistingLayerStrategy, <Self::Buildpack as Buildpack>::Error> {