-
Notifications
You must be signed in to change notification settings - Fork 2.6k
refactor: Centralise deeplink encode and decode into server #3489
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
Changes from all commits
3451eb3
b55ac93
06baafc
cdc6eed
8e756f2
fbaab29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ use std::sync::Arc; | |
| use axum::{extract::State, http::StatusCode, routing::post, Json, Router}; | ||
| use goose::message::Message; | ||
| use goose::recipe::Recipe; | ||
| use goose::recipe_deeplink; | ||
| use serde::{Deserialize, Serialize}; | ||
|
|
||
| use crate::state::AppState; | ||
|
|
@@ -34,6 +35,26 @@ pub struct CreateRecipeResponse { | |
| error: Option<String>, | ||
| } | ||
|
|
||
| #[derive(Debug, Deserialize)] | ||
| pub struct EncodeRecipeRequest { | ||
| recipe: Recipe, | ||
| } | ||
|
|
||
| #[derive(Debug, Serialize)] | ||
| pub struct EncodeRecipeResponse { | ||
| deeplink: String, | ||
| } | ||
|
|
||
| #[derive(Debug, Deserialize)] | ||
| pub struct DecodeRecipeRequest { | ||
| deeplink: String, | ||
| } | ||
|
|
||
| #[derive(Debug, Serialize)] | ||
| pub struct DecodeRecipeResponse { | ||
| recipe: Recipe, | ||
| } | ||
|
|
||
| /// Create a Recipe configuration from the current state of an agent | ||
| async fn create_recipe( | ||
| State(state): State<Arc<AppState>>, | ||
|
|
@@ -84,8 +105,70 @@ async fn create_recipe( | |
| } | ||
| } | ||
|
|
||
| async fn encode_recipe( | ||
| Json(request): Json<EncodeRecipeRequest>, | ||
| ) -> Result<Json<EncodeRecipeResponse>, StatusCode> { | ||
| match recipe_deeplink::encode(&request.recipe) { | ||
| Ok(encoded) => Ok(Json(EncodeRecipeResponse { deeplink: encoded })), | ||
| Err(err) => { | ||
| tracing::error!("Failed to encode recipe: {}", err); | ||
| Err(StatusCode::BAD_REQUEST) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| async fn decode_recipe( | ||
| Json(request): Json<DecodeRecipeRequest>, | ||
| ) -> Result<Json<DecodeRecipeResponse>, StatusCode> { | ||
| match recipe_deeplink::decode(&request.deeplink) { | ||
| Ok(recipe) => Ok(Json(DecodeRecipeResponse { recipe })), | ||
| Err(err) => { | ||
| tracing::error!("Failed to decode deeplink: {}", err); | ||
| Err(StatusCode::BAD_REQUEST) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pub fn routes(state: Arc<AppState>) -> Router { | ||
| Router::new() | ||
| .route("/recipe/create", post(create_recipe)) | ||
| .route("/recipes/encode", post(encode_recipe)) | ||
| .route("/recipes/decode", post(decode_recipe)) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should move /recipe to /recipes in a future pr
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree |
||
| .with_state(state) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use goose::recipe::Recipe; | ||
|
|
||
| #[tokio::test] | ||
| async fn test_decode_and_encode_recipe() { | ||
| let original_recipe = Recipe::builder() | ||
| .title("Test Recipe") | ||
| .description("A test recipe") | ||
| .instructions("Test instructions") | ||
| .build() | ||
| .unwrap(); | ||
| let encoded = recipe_deeplink::encode(&original_recipe).unwrap(); | ||
|
|
||
| let request = DecodeRecipeRequest { | ||
| deeplink: encoded.clone(), | ||
| }; | ||
| let response = decode_recipe(Json(request)).await; | ||
|
|
||
| assert!(response.is_ok()); | ||
| let decoded = response.unwrap().0.recipe; | ||
| assert_eq!(decoded.title, original_recipe.title); | ||
| assert_eq!(decoded.description, original_recipe.description); | ||
| assert_eq!(decoded.instructions, original_recipe.instructions); | ||
|
|
||
| let encode_request = EncodeRecipeRequest { recipe: decoded }; | ||
| let encode_response = encode_recipe(Json(encode_request)).await; | ||
|
|
||
| assert!(encode_response.is_ok()); | ||
| let encoded_again = encode_response.unwrap().0.deeplink; | ||
| assert!(!encoded_again.is_empty()); | ||
| assert_eq!(encoded, encoded_again); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| use anyhow::Result; | ||
| use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _}; | ||
| use thiserror::Error; | ||
|
|
||
| use crate::recipe::Recipe; | ||
|
|
||
| #[derive(Error, Debug)] | ||
| pub enum DecodeError { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For future, it would be nice to distinguish the errors. #[derive(Error, Debug)]
pub enum DecodeError {
#[error("Invalid base64 encoding")]
InvalidBase64,
#[error("Invalid JSON format: {0}")]
InvalidJson(String),
#[error("Invalid recipe format: missing required fields")]
InvalidRecipe,
#[error("Unsupported deeplink format")]
UnsupportedFormat,
} |
||
| #[error("All decoding methods failed")] | ||
| AllMethodsFailed, | ||
| } | ||
|
|
||
| pub fn encode(recipe: &Recipe) -> Result<String, serde_json::Error> { | ||
| let recipe_json = serde_json::to_string(recipe)?; | ||
| let encoded = URL_SAFE_NO_PAD.encode(recipe_json.as_bytes()); | ||
| Ok(encoded) | ||
| } | ||
|
|
||
| pub fn decode(link: &str) -> Result<Recipe, DecodeError> { | ||
| // Handle the current format: URL-safe Base64 without padding. | ||
| if let Ok(decoded_bytes) = URL_SAFE_NO_PAD.decode(link) { | ||
| if let Ok(recipe_json) = String::from_utf8(decoded_bytes) { | ||
| if let Ok(recipe) = serde_json::from_str::<Recipe>(&recipe_json) { | ||
| return Ok(recipe); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Handle legacy formats of 'standard base64 encoded' and standard base64 encoded that was then url encoded. | ||
| if let Ok(url_decoded) = urlencoding::decode(link) { | ||
| if let Ok(decoded_bytes) = | ||
| base64::engine::general_purpose::STANDARD.decode(url_decoded.as_bytes()) | ||
| { | ||
| if let Ok(recipe_json) = String::from_utf8(decoded_bytes) { | ||
| if let Ok(recipe) = serde_json::from_str::<Recipe>(&recipe_json) { | ||
| return Ok(recipe); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Err(DecodeError::AllMethodsFailed) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use crate::recipe::Recipe; | ||
|
|
||
| fn create_test_recipe() -> Recipe { | ||
| Recipe::builder() | ||
| .title("Test Recipe") | ||
| .description("A test recipe for deeplink encoding/decoding") | ||
| .instructions("Act as a helpful assistant") | ||
| .build() | ||
| .expect("Failed to build test recipe") | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_encode_decode_round_trip() { | ||
| let original_recipe = create_test_recipe(); | ||
|
|
||
| let encoded = encode(&original_recipe).expect("Failed to encode recipe"); | ||
| assert!(!encoded.is_empty()); | ||
|
|
||
| let decoded_recipe = decode(&encoded).expect("Failed to decode recipe"); | ||
|
|
||
| assert_eq!(original_recipe.title, decoded_recipe.title); | ||
| assert_eq!(original_recipe.description, decoded_recipe.description); | ||
| assert_eq!(original_recipe.instructions, decoded_recipe.instructions); | ||
| assert_eq!(original_recipe.version, decoded_recipe.version); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_decode_legacy_standard_base64() { | ||
| let recipe = create_test_recipe(); | ||
| let recipe_json = serde_json::to_string(&recipe).unwrap(); | ||
| let legacy_encoded = | ||
| base64::engine::general_purpose::STANDARD.encode(recipe_json.as_bytes()); | ||
|
|
||
| let decoded_recipe = decode(&legacy_encoded).expect("Failed to decode legacy format"); | ||
| assert_eq!(recipe.title, decoded_recipe.title); | ||
| assert_eq!(recipe.description, decoded_recipe.description); | ||
| assert_eq!(recipe.instructions, decoded_recipe.instructions); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_decode_legacy_url_encoded_base64() { | ||
| let recipe = create_test_recipe(); | ||
| let recipe_json = serde_json::to_string(&recipe).unwrap(); | ||
| let base64_encoded = | ||
| base64::engine::general_purpose::STANDARD.encode(recipe_json.as_bytes()); | ||
| let url_encoded = urlencoding::encode(&base64_encoded); | ||
|
|
||
| let decoded_recipe = | ||
| decode(&url_encoded).expect("Failed to decode URL-encoded legacy format"); | ||
| assert_eq!(recipe.title, decoded_recipe.title); | ||
| assert_eq!(recipe.description, decoded_recipe.description); | ||
| assert_eq!(recipe.instructions, decoded_recipe.instructions); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_decode_invalid_input() { | ||
| let result = decode("invalid_base64!"); | ||
| assert!(result.is_err()); | ||
| assert!(matches!(result.unwrap_err(), DecodeError::AllMethodsFailed)); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this entire recipe thing is not part of our open-api integration somehow. this should be integrated with openapi.rs and then we can autogenerate the ts code. can you check how much work it is to do this the right way? if too much, we can do in a follow up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with the open-api pieces. I can rectifying that in a future pr.