Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions engine/crates/engine-config-builder/src/from_sdl_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub fn build_with_sdl_config(config: &FederatedGraphConfig, graph: FederatedGrap
rate_limit: context.rate_limit,
timeout: config.timeout,
entity_caching: match config.entity_caching {
EntityCachingConfig::Enabled { ttl } => EntityCaching::Enabled { ttl },
EntityCachingConfig::Enabled { ttl, .. } => EntityCaching::Enabled { ttl },
_ => EntityCaching::Disabled,
},
})
Expand Down Expand Up @@ -211,7 +211,7 @@ impl<'a> BuildContext<'a> {
retry,
entity_caching: entity_caching.as_ref().map(|config| match config {
EntityCachingConfig::Disabled => EntityCaching::Disabled,
EntityCachingConfig::Enabled { ttl } => EntityCaching::Enabled { ttl: *ttl },
EntityCachingConfig::Enabled { ttl, .. } => EntityCaching::Enabled { ttl: *ttl },
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggestion
Ensure that the handling of additional fields in the pattern matching is consistent across all usages to avoid potential bugs.

}),
},
);
Expand Down
1 change: 1 addition & 0 deletions engine/crates/engine/response/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ mod streaming;
pub struct GraphqlOperationAnalyticsAttributes {
pub name: Option<String>,
pub r#type: common_types::OperationType,
#[serde(default)]
pub used_fields: String,
}

Expand Down
69 changes: 58 additions & 11 deletions engine/crates/parser-sdl/src/federation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,20 +61,62 @@ pub enum EntityCachingConfig {
Disabled,
Enabled {
ttl: Option<Duration>,
storage: EntityCacheStorage,
},
}

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Default)]
pub enum EntityCacheStorage {
#[default]
Memory,
Redis(RedisConfig),
}

impl From<gateway_config::EntityCachingConfig> for EntityCachingConfig {
fn from(config: gateway_config::EntityCachingConfig) -> Self {
match (config.enabled, config.ttl) {
(Some(false), _) => EntityCachingConfig::Disabled,
(Some(true), ttl) => EntityCachingConfig::Enabled { ttl },
(_, Some(ttl)) => EntityCachingConfig::Enabled { ttl: Some(ttl) },
(Some(true), ttl) => EntityCachingConfig::Enabled {
ttl,
storage: entity_cache_storage(config.storage, config.redis),
},
(_, Some(ttl)) => EntityCachingConfig::Enabled {
ttl: Some(ttl),
storage: entity_cache_storage(config.storage, config.redis),
},
_ => EntityCachingConfig::Disabled,
}
}
}

fn entity_cache_storage(
storage: gateway_config::EntityCachingStorage,
redis: Option<gateway_config::EntityCachingRedisConfig>,
) -> EntityCacheStorage {
match storage {
gateway_config::EntityCachingStorage::Memory => EntityCacheStorage::Memory,
gateway_config::EntityCachingStorage::Redis => EntityCacheStorage::Redis(redis.unwrap_or_default().into()),
}
}

impl From<gateway_config::EntityCachingRedisConfig> for RedisConfig {
fn from(value: gateway_config::EntityCachingRedisConfig) -> Self {
let gateway_config::EntityCachingRedisConfig { url, key_prefix, tls } = value;
RedisConfig {
url,
key_prefix,
tls: tls.map(Into::into),
}
}
}

impl From<gateway_config::EntityCachingRedisTlsConfig> for RedisTlsConfig {
fn from(value: gateway_config::EntityCachingRedisTlsConfig) -> Self {
let gateway_config::EntityCachingRedisTlsConfig { cert, key, ca } = value;
RedisTlsConfig { cert, key, ca }
}
}

impl From<(String, ConnectorHeaderValue)> for SubgraphHeaderRule {
fn from((name, value): (String, ConnectorHeaderValue)) -> Self {
match value {
Expand All @@ -100,7 +142,7 @@ pub struct GraphRateLimit {
pub struct RateLimitConfig {
pub global: Option<GraphRateLimit>,
pub storage: RateLimitStorage,
pub redis: RateLimitRedisConfig,
pub redis: RedisConfig,
}

impl From<gateway_config::RateLimitConfig> for RateLimitConfig {
Expand Down Expand Up @@ -131,7 +173,7 @@ impl From<gateway_config::RateLimitStorage> for RateLimitStorage {
}
}

impl From<gateway_config::RateLimitRedisConfig> for RateLimitRedisConfig {
impl From<gateway_config::RateLimitRedisConfig> for RedisConfig {
fn from(value: gateway_config::RateLimitRedisConfig) -> Self {
Self {
url: value.url,
Expand All @@ -141,7 +183,7 @@ impl From<gateway_config::RateLimitRedisConfig> for RateLimitRedisConfig {
}
}

impl From<gateway_config::RateLimitRedisTlsConfig> for RateLimitRedisTlsConfig {
impl From<gateway_config::RateLimitRedisTlsConfig> for RedisTlsConfig {
fn from(value: gateway_config::RateLimitRedisTlsConfig) -> Self {
Self {
cert: value.cert,
Expand All @@ -158,14 +200,14 @@ pub enum RateLimitStorage {
}

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub struct RateLimitRedisConfig {
pub struct RedisConfig {
pub url: url::Url,
pub key_prefix: String,
pub tls: Option<RateLimitRedisTlsConfig>,
pub tls: Option<RedisTlsConfig>,
}

#[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord)]
pub struct RateLimitRedisTlsConfig {
pub struct RedisTlsConfig {
pub cert: Option<PathBuf>,
pub key: Option<PathBuf>,
pub ca: Option<PathBuf>,
Expand Down Expand Up @@ -201,7 +243,8 @@ mod tests {
assert_eq!(
EntityCachingConfig::from(config.entity_caching),
EntityCachingConfig::Enabled {
ttl: Some(Duration::from_secs(60))
ttl: Some(Duration::from_secs(60)),
storage: Default::default(),
}
)
}
Expand All @@ -218,7 +261,8 @@ mod tests {
assert_eq!(
EntityCachingConfig::from(config.subgraphs.remove("products").unwrap().entity_caching.unwrap()),
EntityCachingConfig::Enabled {
ttl: Some(Duration::from_secs(60))
ttl: Some(Duration::from_secs(60)),
storage: Default::default()
}
)
}
Expand All @@ -234,7 +278,10 @@ mod tests {

assert_eq!(
EntityCachingConfig::from(config.subgraphs.remove("products").unwrap().entity_caching.unwrap()),
EntityCachingConfig::Enabled { ttl: None }
EntityCachingConfig::Enabled {
ttl: None,
storage: Default::default()
}
)
}

Expand Down
24 changes: 12 additions & 12 deletions engine/crates/parser-sdl/src/rules/subgraph_directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,18 +190,18 @@ impl Visitor<'_> for SubgraphDirectiveVisitor {
subgraph.development_url = Some(url.to_string())
}

if let Some(enabled) = directive.entity_caching_enabled {
if enabled {
subgraph.entity_caching = Some(EntityCachingConfig::Enabled { ttl: None });
} else {
subgraph.entity_caching = Some(EntityCachingConfig::Disabled);
}
}

if let Some(ttl) = directive.entity_caching_ttl {
// If there's a ttl we always enable
subgraph.entity_caching = Some(EntityCachingConfig::Enabled { ttl: Some(ttl) });
}
subgraph.entity_caching = match (directive.entity_caching_enabled, directive.entity_caching_ttl) {
(Some(false), _) => Some(EntityCachingConfig::Disabled),
(Some(true), ttl) => Some(EntityCachingConfig::Enabled {
ttl,
storage: Default::default(),
}),
(_, Some(ttl)) => Some(EntityCachingConfig::Enabled {
ttl: Some(ttl),
storage: Default::default(),
}),
_ => None,
};

subgraph.header_rules.extend(
directive
Expand Down
1 change: 1 addition & 0 deletions engine/crates/runtime-local/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ mod kv;
mod log;
mod pg;
pub mod rate_limiting;
#[cfg(feature = "redis")]
pub mod redis;
mod ufd_invoker;

Expand Down
54 changes: 53 additions & 1 deletion gateway/crates/config/src/entity_caching.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,62 @@
use std::time::Duration;
use std::{path::PathBuf, time::Duration};

#[derive(Debug, Default, serde::Deserialize, Clone, PartialEq)]
pub struct EntityCachingConfig {
pub enabled: Option<bool>,

#[serde(default)]
pub storage: EntityCachingStorage,

#[serde(default)]
pub redis: Option<EntityCachingRedisConfig>,

/// The ttl to store cache entries with. Defaults to 60s
#[serde(deserialize_with = "duration_str::deserialize_option_duration", default)]
pub ttl: Option<Duration>,
}

#[derive(Debug, Clone, Default, PartialEq, serde::Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum EntityCachingStorage {
#[default]
Memory,
Redis,
}

#[derive(Debug, Clone, PartialEq, serde::Deserialize)]
#[serde(deny_unknown_fields)]
pub struct EntityCachingRedisConfig {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggestion
Consider adding a method to EntityCachingRedisConfig to validate the configuration, ensuring that the url is correctly formatted and the key_prefix is not empty, which can prevent runtime errors.

#[serde(default = "EntityCachingRedisConfig::default_url")]
pub url: url::Url,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggestion
Consider adding validation logic in the EntityCachingRedisConfig struct to ensure that the url is a valid Redis URL when deserializing, which can prevent runtime errors.

#[serde(default = "EntityCachingRedisConfig::default_key_prefix")]
pub key_prefix: String,
pub tls: Option<EntityCachingRedisTlsConfig>,
}

impl Default for EntityCachingRedisConfig {
fn default() -> Self {
Self {
url: Self::default_url(),
key_prefix: Self::default_key_prefix(),
tls: None,
}
}
}

impl EntityCachingRedisConfig {
fn default_url() -> url::Url {
url::Url::parse("redis://localhost:6379").expect("must be correct")
}

fn default_key_prefix() -> String {
String::from("grafbase-cache")
}
}

#[derive(Debug, Clone, PartialEq, Default, serde::Deserialize)]
#[serde(deny_unknown_fields)]
pub struct EntityCachingRedisTlsConfig {
pub cert: Option<PathBuf>,
pub key: Option<PathBuf>,
pub ca: Option<PathBuf>,
}