Skip to content

Commit

Permalink
Add new Cargo fetch tests and remove old mocked pseudo-test
Browse files Browse the repository at this point in the history
This adds a new Cargo fetch test to check that a doc can be fetched and serialized. Because we don't want GitHub to be trying to fetch files, it's locked behind a new feature, "test_fetches".

This also removes the figma_v1_mock file and feature. This was a pseudo test that wasn't being executed by anything. It worked by using a feature flag to mock the http fetch, which meant that if the feature flag was included that no http queries would work, including the new fetch test.
  • Loading branch information
timothyfroehlich committed Nov 12, 2024
1 parent 417680d commit 1e2faae
Show file tree
Hide file tree
Showing 12 changed files with 188 additions and 97 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ jobs:
- name: Build all
run: cargo build --all-targets --all-features
- name: Test all
run: cargo test --all-targets --all-features
run: cargo test --all-targets --features=reflection,fetch,dcf_info
############ Figma resources
figma-resources:
runs-on: ubuntu-latest
Expand Down
20 changes: 20 additions & 0 deletions .idea/runConfigurations/Fetch_HelloWorld.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 19 additions & 0 deletions .idea/runConfigurations/Figma_Fetch_Tests.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

55 changes: 6 additions & 49 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 12 additions & 2 deletions crates/dc_jni/src/jni.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use jni::sys::{jint, JNI_VERSION_1_6};
use jni::{JNIEnv, JavaVM};

use lazy_static::lazy_static;
use log::{error, LevelFilter};
use log::{error, info, LevelFilter};

lazy_static! {
static ref JAVA_VM: Mutex<Option<Arc<JavaVM>>> = Mutex::new(None);
Expand Down Expand Up @@ -126,9 +126,19 @@ fn jni_fetch_doc_impl(
let request: ConvertRequest = serde_json::from_str(&request_json)?;

let convert_result: figma_import::ConvertResponse =
match fetch_doc(&doc_id, &version_id, request, proxy_config).map_err(Error::from) {
match fetch_doc(&doc_id, &version_id, &request, proxy_config).map_err(Error::from) {
Ok(it) => it,
Err(err) => {
let queries_string = request
.queries
.iter()
.map(|q| format!("--nodes=\"{}\" ", q))
.collect::<Vec<String>>()
.join(" ");

info!("Failed to fetch {}, Try fetching locally", doc_id);
info!("fetch --doc-id={} --version-id={} {} ", doc_id, version_id, queries_string);

map_err_to_exception(env, &err, doc_id).expect("Failed to throw exception");
return Err(err);
}
Expand Down
3 changes: 1 addition & 2 deletions crates/figma_import/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ rust-version.workspace = true
[features]
default = []
reflection = ["serde-reflection", "serde-generate", "clap"]
http_mock = ["phf"]
fetch = ["clap"]
dcf_info = ["clap"]
fetch_layout = ["clap"]
test_fetches = ["fetch"]


[dependencies]
Expand All @@ -31,7 +31,6 @@ svgtypes.workspace = true
unicode-segmentation.workspace = true
image.workspace = true
euclid.workspace = true
phf = { workspace = true, optional = true }

# layout dependencies
taffy.workspace = true
Expand Down
6 changes: 0 additions & 6 deletions crates/figma_import/src/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use dc_bundle::definition::element::{
variable_map::NameIdMap, Collection, Mode, Variable, VariableMap,
};
use dc_bundle::legacy_definition::element::node::NodeQuery;
#[cfg(not(feature = "http_mock"))]
use std::time::Duration;
use std::{
collections::{HashMap, HashSet},
Expand All @@ -40,13 +39,11 @@ use dc_bundle::legacy_definition::EncodedImageMap;
use dc_bundle::legacy_figma_live_update::FigmaDocInfo;
use log::error;

#[cfg(not(feature = "http_mock"))]
const FIGMA_TOKEN_HEADER: &str = "X-Figma-Token";
const BASE_FILE_URL: &str = "https://api.figma.com/v1/files/";
const BASE_COMPONENT_URL: &str = "https://api.figma.com/v1/components/";
const BASE_PROJECT_URL: &str = "https://api.figma.com/v1/projects/";

#[cfg(not(feature = "http_mock"))]
fn http_fetch(api_key: &str, url: String, proxy_config: &ProxyConfig) -> Result<String, Error> {
let mut agent_builder = ureq::AgentBuilder::new();
let mut buffer = Vec::new();
Expand All @@ -69,9 +66,6 @@ fn http_fetch(api_key: &str, url: String, proxy_config: &ProxyConfig) -> Result<
Ok(body)
}

#[cfg(feature = "http_mock")]
use crate::figma_v1_document_mocks::http_fetch;

/// Document update requests return this value to indicate if an update was
/// made or not.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
Expand Down
21 changes: 17 additions & 4 deletions crates/figma_import/src/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub enum ProxyConfig {
pub struct ConvertRequest<'r> {
figma_api_key: &'r str,
// Node names
queries: Vec<&'r str>,
pub queries: Vec<&'r str>,
// Ignored images
ignored_images: Vec<IgnoredImage<'r>>,

Expand All @@ -65,6 +65,19 @@ pub struct ConvertRequest<'r> {
image_session: Option<ImageContextSession>,
}

impl<'r> ConvertRequest<'r> {
pub fn new(figma_api_key: &'r str, queries: Vec<&'r str>, version: Option<String>) -> Self {
Self {
figma_api_key,
queries,
ignored_images: Vec::new(),
last_modified: None,
version,
image_session: None,
}
}
}

#[derive(Serialize, Deserialize)]
pub enum ConvertResponse {
Document(Vec<u8>),
Expand All @@ -74,16 +87,16 @@ pub enum ConvertResponse {
pub fn fetch_doc(
id: &str,
requested_version_id: &str,
rq: ConvertRequest,
rq: &ConvertRequest,
proxy_config: &ProxyConfig,
) -> Result<ConvertResponse, crate::Error> {
if let Some(mut doc) = Document::new_if_changed(
rq.figma_api_key,
id.into(),
requested_version_id.into(),
proxy_config,
rq.last_modified.unwrap_or(String::new()),
rq.version.unwrap_or(String::new()),
rq.last_modified.clone().unwrap_or(String::new()),
rq.version.clone().unwrap_or(String::new()),
rq.image_session.clone(),
)? {
// The document has changed since the version the client has, so we should fetch
Expand Down
4 changes: 4 additions & 0 deletions crates/figma_import/src/figma_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1230,6 +1230,7 @@ pub struct VariableAlias {
pub enum VariableAliasOrList {
Alias(VariableAlias),
List(Vec<VariableAlias>),
Map(HashMap<String, VariableAlias>),
}
impl VariableAliasOrList {
fn get_name(&self) -> Option<String> {
Expand All @@ -1243,6 +1244,9 @@ impl VariableAliasOrList {
return Some(alias.id.clone());
}
}
VariableAliasOrList::Map(_) => {
panic!("We don't support Maps of Variables!");
}
}
None
}
Expand Down
2 changes: 0 additions & 2 deletions crates/figma_import/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ pub use dc_bundle::legacy_definition::element::node::NodeQuery;
pub use dc_bundle::legacy_definition::view::view::View;
pub use dc_bundle::legacy_definition::view::view::ViewData;

#[cfg(feature = "http_mock")]
mod figma_v1_document_mocks;
/// Functionality related to reflection for deserializing our bincode archives in other
/// languages
#[cfg(feature = "reflection")]
Expand Down
Loading

0 comments on commit 1e2faae

Please sign in to comment.