Skip to content

Commit

Permalink
Require explicit values
Browse files Browse the repository at this point in the history 
These map 1:1 with `CachedLayerDefinition`. The extra indirection of supporting default values isn't buying us much.
  • Loading branch information
@schneems
schneems committed Dec 19, 2024
1 parent aa76d60 commit 193ff02
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 50 deletions.
6 changes: 5 additions & 1 deletion buildpacks/ruby/src/layers/bundle_download_layer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ pub(crate) fn handle(
mut bullet: Print<SubBullet<Stdout>>,
metadata: &Metadata,
) -> libcnb::Result<(Print<SubBullet<Stdout>>, LayerEnv), RubyBuildpackError> {
let layer_ref = CacheBuddy::new().layer(layer_name!("bundler"), context, metadata)?;
let layer_ref = CacheBuddy {
build: true,
launch: true,
}
.layer(layer_name!("bundler"), context, metadata)?;
match &layer_ref.state {
LayerState::Restored { cause } => {
bullet = bullet.sub_bullet(cause);
Expand Down
6 changes: 5 additions & 1 deletion buildpacks/ruby/src/layers/bundle_install_layer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ pub(crate) fn handle(
metadata: &Metadata,
without: &BundleWithout,
) -> libcnb::Result<(Print<SubBullet<Stdout>>, LayerEnv), RubyBuildpackError> {
let layer_ref = CacheBuddy::new().layer(layer_name!("gems"), context, metadata)?;
let layer_ref = CacheBuddy {
build: true,
launch: true,
}
.layer(layer_name!("gems"), context, metadata)?;
let install_state = match &layer_ref.state {
LayerState::Restored { cause } => {
bullet = bullet.sub_bullet(cause);
Expand Down
33 changes: 23 additions & 10 deletions buildpacks/ruby/src/layers/ruby_install_layer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@ pub(crate) fn handle(
mut bullet: Print<SubBullet<Stdout>>,
metadata: &Metadata,
) -> libcnb::Result<(Print<SubBullet<Stdout>>, LayerEnv), RubyBuildpackError> {
let layer_ref = CacheBuddy::new().layer(layer_name!("ruby"), context, metadata)?;
let layer_ref = CacheBuddy {
build: true,
launch: true,
}
.layer(layer_name!("ruby"), context, metadata)?;
match &layer_ref.state {
LayerState::Restored { cause } => {
bullet = bullet.sub_bullet(cause);
Expand Down Expand Up @@ -388,12 +392,18 @@ version = "3.1.3"
let differences = old.diff(&old);
assert_eq!(differences, Vec::<String>::new());

CacheBuddy::new()
.layer(layer_name!("ruby"), &context, &old)
.unwrap();
let result = CacheBuddy::new()
.layer(layer_name!("ruby"), &context, &old)
.unwrap();
CacheBuddy {
build: true,
launch: true,
}
.layer(layer_name!("ruby"), &context, &old)
.unwrap();
let result = CacheBuddy {
build: true,
launch: true,
}
.layer(layer_name!("ruby"), &context, &old)
.unwrap();
let actual = result.state;
assert!(matches!(actual, LayerState::Restored { .. }));

Expand All @@ -404,9 +414,12 @@ version = "3.1.3"
let differences = now.diff(&old);
assert_eq!(differences.len(), 1);

let result = CacheBuddy::new()
.layer(layer_name!("ruby"), &context, &now)
.unwrap();
let result = CacheBuddy {
build: true,
launch: true,
}
.layer(layer_name!("ruby"), &context, &now)
.unwrap();
assert!(matches!(
result.state,
LayerState::Empty {
Expand Down
78 changes: 41 additions & 37 deletions commons/src/layer/cache_buddy.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,13 @@ use std::fmt::Debug;
/// the layer is deleted and the changes are returned.
///
#[doc = include_str!("./fixtures/cache_buddy_example.md")]
#[derive(Default, Debug, Clone, Eq, PartialEq)]
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct CacheBuddy {
pub build: Option<bool>,
pub launch: Option<bool>,
pub build: bool,
pub launch: bool,
}

impl CacheBuddy {
#[must_use]
pub fn new() -> Self {
Self::default()
}

/// Writes metadata to a layer and returns a layer reference with info about prior cache state
///
/// See the struct documentation for more information.
Expand All @@ -99,8 +94,8 @@ impl CacheBuddy {
let layer_ref = context.cached_layer(
layer_name,
CachedLayerDefinition {
build: self.build.unwrap_or(true),
launch: self.launch.unwrap_or(true),
build: self.build,
launch: self.launch,
invalid_metadata_action: &invalid_metadata_action,
restored_layer_action: &|old: &M, _| restored_layer_action(old, metadata),
},
Expand Down Expand Up @@ -271,15 +266,18 @@ mod tests {
);

// First write
let result = CacheBuddy::new()
.layer(
layer_name!("testing"),
&context,
&TestMetadata {
value: "hello".to_string(),
},
)
.unwrap();
let result = CacheBuddy {
build: true,
launch: true,
}
.layer(
layer_name!("testing"),
&context,
&TestMetadata {
value: "hello".to_string(),
},
)
.unwrap();
assert!(matches!(
result.state,
LayerState::Empty {
Expand All @@ -288,30 +286,36 @@ mod tests {
));

// Second write, preserve the contents
let result = CacheBuddy::new()
.layer(
layer_name!("testing"),
&context,
&TestMetadata {
value: "hello".to_string(),
},
)
.unwrap();
let result = CacheBuddy {
build: true,
launch: true,
}
.layer(
layer_name!("testing"),
&context,
&TestMetadata {
value: "hello".to_string(),
},
)
.unwrap();
let LayerState::Restored { cause } = &result.state else {
panic!("Expected restored layer")
};
assert_eq!(cause.as_ref(), "Using cache");

// Third write, change the data
let result = CacheBuddy::new()
.layer(
layer_name!("testing"),
&context,
&TestMetadata {
value: "world".to_string(),
},
)
.unwrap();
let result = CacheBuddy {
build: true,
launch: true,
}
.layer(
layer_name!("testing"),
&context,
&TestMetadata {
value: "world".to_string(),
},
)
.unwrap();

let LayerState::Empty {
cause: EmptyLayerCause::RestoredLayerAction { cause },
Expand Down
6 changes: 5 additions & 1 deletion commons/src/layer/fixtures/cache_buddy_example.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ use libcnb::data::layer_name;
# ) -> libcnb::Result<(), <FakeBuildpack as libcnb::Buildpack>::Error> {
# let metadata_owned = TestMetadata { value: "Hello".to_string() };
# let metadata = &metadata_owned;
let layer_ref = CacheBuddy::new().layer(layer_name!("ruby"), context, metadata)?;
let layer_ref = CacheBuddy {
build: true,
launch: true,
}
.layer(layer_name!("ruby"), context, metadata)?;
match &layer_ref.state {
// CacheDiff reported no difference, cache was kept
LayerState::Restored { cause } => {
Expand Down

0 comments on commit 193ff02

Please sign in to comment.