Factor out property filtering code to simplify web server

src/snapshot_middleware/lua.rs

@@ -72,8 +72,8 @@ pub fn snapshot_lua_init(
         anyhow::bail!(
             "init.lua, init.server.lua, and init.client.lua can \
              only be used if the instance produced by the containing \
-             directory would be a Folder.\n\n\
+             directory would be a Folder.\n\
-
+             \n\
              The directory {} turned into an instance of class {}.",
             folder_path.display(),
             dir_snapshot.class_name

src/web/api.rs

@@ -11,9 +11,9 @@ use crate::{
     snapshot::{InstanceWithMeta, PatchSet, PatchUpdate},
     web::{
         interface::{
-            ErrorResponse, Instance, InstanceMetadata as WebInstanceMetadata, InstanceUpdate,
+            ErrorResponse, Instance, OpenResponse, ReadResponse, ServerInfoResponse,
-            OpenResponse, ReadResponse, ServerInfoResponse, SubscribeMessage, SubscribeResponse,
+            SubscribeMessage, SubscribeResponse, WriteRequest, WriteResponse, PROTOCOL_VERSION,
-            WriteRequest, WriteResponse, PROTOCOL_VERSION, SERVER_VERSION,
+            SERVER_VERSION,
         },
         util::{json, json_ok},
     },
@@ -99,44 +99,7 @@ impl ApiService {
 
                 let api_messages = messages
                     .into_iter()
-                    .map(|message| {
+                    .map(|patch| SubscribeMessage::from_patch_update(&tree, patch))
-                        let removed = message.removed;
-
-                        let mut added = HashMap::new();
-                        for id in message.added {
-                            let instance = tree.get_instance(id).unwrap();
-                            added.insert(id, Instance::from_rojo_instance(instance));
-
-                            for instance in tree.descendants(id) {
-                                added.insert(instance.id(), Instance::from_rojo_instance(instance));
-                            }
-                        }
-
-                        let updated = message
-                            .updated
-                            .into_iter()
-                            .map(|update| {
-                                let changed_metadata = update
-                                    .changed_metadata
-                                    .as_ref()
-                                    .map(WebInstanceMetadata::from_rojo_metadata);
-
-                                InstanceUpdate {
-                                    id: update.id,
-                                    changed_name: update.changed_name,
-                                    changed_class_name: update.changed_class_name,
-                                    changed_properties: update.changed_properties,
-                                    changed_metadata,
-                                }
-                            })
-                            .collect();
-
-                        SubscribeMessage {
-                            removed,
-                            added,
-                            updated,
-                        }
-                    })
                     .collect();
 
                 json_ok(SubscribeResponse {

src/web/interface.rs

@@ -7,12 +7,14 @@ use std::{
     collections::{HashMap, HashSet},
 };
 
-use rbx_dom_weak::types::{Ref, Variant};
+use rbx_dom_weak::types::{Ref, Variant, VariantType};
 use serde::{Deserialize, Serialize};
 
 use crate::{
     session_id::SessionId,
-    snapshot::{InstanceMetadata as RojoInstanceMetadata, InstanceWithMeta},
+    snapshot::{
+        AppliedPatchSet, InstanceMetadata as RojoInstanceMetadata, InstanceWithMeta, RojoTree,
+    },
 };
 
 /// Server version to report over the API, not exposed outside this crate.
@@ -30,6 +32,53 @@ pub struct SubscribeMessage<'a> {
     pub updated: Vec<InstanceUpdate>,
 }
 
+impl<'a> SubscribeMessage<'a> {
+    pub(crate) fn from_patch_update(tree: &'a RojoTree, patch: AppliedPatchSet) -> Self {
+        let removed = patch.removed;
+
+        let mut added = HashMap::new();
+        for id in patch.added {
+            let instance = tree.get_instance(id).unwrap();
+            added.insert(id, Instance::from_rojo_instance(instance));
+
+            for instance in tree.descendants(id) {
+                added.insert(instance.id(), Instance::from_rojo_instance(instance));
+            }
+        }
+
+        let updated = patch
+            .updated
+            .into_iter()
+            .map(|update| {
+                let changed_metadata = update
+                    .changed_metadata
+                    .as_ref()
+                    .map(InstanceMetadata::from_rojo_metadata);
+
+                let changed_properties = update
+                    .changed_properties
+                    .into_iter()
+                    .filter(|(_key, value)| property_filter(value.as_ref()))
+                    .collect();
+
+                InstanceUpdate {
+                    id: update.id,
+                    changed_name: update.changed_name,
+                    changed_class_name: update.changed_class_name,
+                    changed_properties,
+                    changed_metadata,
+                }
+            })
+            .collect();
+
+        Self {
+            removed,
+            added,
+            updated,
+        }
+    }
+}
+
 #[derive(Debug, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct InstanceUpdate {
@@ -75,14 +124,8 @@ impl<'a> Instance<'a> {
         let properties = source
             .properties()
             .iter()
-            .filter_map(|(key, value)| {
+            .filter(|(_key, value)| property_filter(Some(value)))
-                // SharedString values can't be serialized via Serde
+            .map(|(key, value)| (key.clone(), Cow::Borrowed(value)))
-                if matches!(value, Variant::SharedString(_)) {
-                    return None;
-                }
-
-                Some((key.clone(), Cow::Borrowed(value)))
-            })
             .collect();
 
         Instance {
@@ -97,6 +140,18 @@ impl<'a> Instance<'a> {
     }
 }
 
+fn property_filter(value: Option<&Variant>) -> bool {
+    let ty = value.map(|value| value.ty());
+
+    // Lua can't do anything with SharedString values. They also can't be
+    // serialized directly by Serde!
+    if ty == Some(VariantType::SharedString) {
+        return false;
+    }
+
+    return true;
+}
+
 /// Response body from /api/rojo
 #[derive(Debug, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]