Add new Cargo fetch tests and remove old mocked pseudo-test (#1776)

<!-- start git-machete generated -->

## Tree of downstream PRs as of 2024-11-12

* **PR #1776 (THIS ONE)**:
  `main` ← `wb/froeht/new-fetch-test`

    * PR #1774:
`wb/froeht/new-fetch-test` ← `wb/froeht/protoconv-layout-positioning`

      * PR #1782:
`wb/froeht/protoconv-layout-positioning` ← `wb/froeht/1731-meter-data`

<!-- end git-machete generated -->

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.

.github/workflows/main.yml

@@ -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

crates/dc_jni/src/jni.rs

@@ -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);
@@ -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);
             }

crates/figma_import/Cargo.toml

@@ -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]
@@ -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

crates/figma_import/src/document.rs

@@ -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},
@@ -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();
@@ -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)]

crates/figma_import/src/fetch.rs

@@ -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>>,
 
@@ -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>),
@@ -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

crates/figma_import/src/figma_schema.rs

@@ -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> {
@@ -1243,6 +1244,9 @@ impl VariableAliasOrList {
                     return Some(alias.id.clone());
                 }
             }
+            VariableAliasOrList::Map(_) => {
+                panic!("We don't support Maps of Variables!");
+            }
         }
         None
     }

crates/figma_import/src/lib.rs

@@ -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")]