validation: skip upload if model validates as-is

validation/src/validator.rs

@@ -32,7 +32,7 @@ fn hash_source(source:&str)->String{
 
 #[allow(dead_code)]
 #[derive(Debug)]
-pub enum ValidateError{
+pub enum Error{
 	ScriptFlaggedIllegalKeyword(String),
 	ScriptBlocked(Option<submissions_api::types::ScriptID>),
 	ScriptNotYetReviewed(Option<submissions_api::types::ScriptID>),
@@ -51,12 +51,12 @@ pub enum ValidateError{
 	AssetUpload(rbx_asset::cookie::UploadError),
 	AssetCreate(rbx_asset::cookie::CreateError),
 }
-impl std::fmt::Display for ValidateError{
+impl std::fmt::Display for Error{
 	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
 		write!(f,"{self:?}")
 	}
 }
-impl std::error::Error for ValidateError{}
+impl std::error::Error for Error{}
 
 #[allow(nonstandard_style)]
 pub struct ValidateRequest{
@@ -88,15 +88,15 @@ impl From<crate::nats_types::ValidateSubmissionRequest> for ValidateRequest{
 }
 
 impl crate::message_handler::MessageHandler{
-	pub async fn validate_inner(&self,validate_info:ValidateRequest)->Result<(),ValidateError>{
+	pub async fn validate_inner(&self,validate_info:ValidateRequest)->Result<(),Error>{
 		// download map
 		let data=self.cookie_context.get_asset(rbx_asset::cookie::GetAssetRequest{
 			asset_id:validate_info.ModelID,
 			version:Some(validate_info.ModelVersion),
-		}).await.map_err(ValidateError::ModelFileDownload)?;
+		}).await.map_err(Error::ModelFileDownload)?;
 
 		// decode dom (slow!)
-		let mut dom=read_dom(&mut std::io::Cursor::new(data)).map_err(ValidateError::ModelFileDecode)?;
+		let mut dom=read_dom(&mut std::io::Cursor::new(data)).map_err(Error::ModelFileDecode)?;
 
 		/* VALIDATE MAP */
 
@@ -111,7 +111,7 @@ impl crate::message_handler::MessageHandler{
 						// immediately abort
 						// grab path to offending script
 						let path=get_partial_path(&dom,script);
-						return Err(ValidateError::ScriptFlaggedIllegalKeyword(path));
+						return Err(Error::ScriptFlaggedIllegalKeyword(path));
 					}
 					// associate a name and policy with the source code
 					// policy will be fetched from the database to replace the default policy
@@ -132,7 +132,7 @@ impl crate::message_handler::MessageHandler{
 			// fetch the script policy
 			let script_policy=self.api.get_script_policy_from_hash(submissions_api::types::HashRequest{
 				hash:hash.as_str(),
-			}).await.map_err(ValidateError::ApiGetScriptPolicyFromHash)?;
+			}).await.map_err(Error::ApiGetScriptPolicyFromHash)?;
 
 			// write the policy to the script_map, fetching the replacement code if necessary
 			if let Some(script_policy)=script_policy{
@@ -144,7 +144,7 @@ impl crate::message_handler::MessageHandler{
 					submissions_api::types::Policy::Replace=>{
 						let script=self.api.get_script(submissions_api::types::GetScriptRequest{
 							ScriptID:script_policy.ToScriptID,
-						}).await.map_err(ValidateError::ApiGetScript)?;
+						}).await.map_err(Error::ApiGetScript)?;
 						Policy::Replace(script.Source)
 					},
 				};
@@ -160,14 +160,14 @@ impl crate::message_handler::MessageHandler{
 					Source:source.as_str(),
 					ResourceType:resource_type,
 					ResourceID:Some(resource_id),
-				}).await.map_err(ValidateError::ApiCreateScript)?;
+				}).await.map_err(Error::ApiCreateScript)?;
 
 				// create a None policy (pending review by yours truly)
 				self.api.create_script_policy(submissions_api::types::CreateScriptPolicyRequest{
 					ToScriptID:script.ScriptID,
 					FromScriptID:script.ScriptID,
 					Policy:submissions_api::types::Policy::None,
-				}).await.map_err(ValidateError::ApiCreateScriptPolicy)?;
+				}).await.map_err(Error::ApiCreateScriptPolicy)?;
 			}
 
 			Ok(())
@@ -175,7 +175,7 @@ impl crate::message_handler::MessageHandler{
 		.await?;
 
 		// make the replacements
-		let mut modified=true;
+		let mut modified=false;
 		for &script_ref in &script_refs{
 			if let Some(script)=dom.get_by_ref_mut(script_ref){
 				if let Some(rbx_dom_weak::types::Variant::String(source))=script.properties.get_mut("Source"){
@@ -184,8 +184,8 @@ impl crate::message_handler::MessageHandler{
 							let hash=hash_source(source.as_str());
 							let script=self.api.get_script_from_hash(submissions_api::types::HashRequest{
 								hash:hash.as_str(),
-							}).await.map_err(ValidateError::ApiGetScriptFromHash)?;
-							return Err(ValidateError::ScriptBlocked(script.map(|s|s.ID)));
+							}).await.map_err(Error::ApiGetScriptFromHash)?;
+							return Err(Error::ScriptBlocked(script.map(|s|s.ID)));
 						},
 						None
 						|Some(Policy::None)
@@ -193,8 +193,8 @@ impl crate::message_handler::MessageHandler{
 							let hash=hash_source(source.as_str());
 							let script=self.api.get_script_from_hash(submissions_api::types::HashRequest{
 								hash:hash.as_str(),
-							}).await.map_err(ValidateError::ApiGetScriptFromHash)?;
-							return Err(ValidateError::ScriptNotYetReviewed(script.map(|s|s.ID)));
+							}).await.map_err(Error::ApiGetScriptFromHash)?;
+							return Err(Error::ScriptNotYetReviewed(script.map(|s|s.ID)));
 						},
 						Some(Policy::Allowed)=>(),
 						Some(Policy::Delete)=>{
@@ -211,19 +211,17 @@ impl crate::message_handler::MessageHandler{
 			}
 		}
 
-		println!("[Validator] Forcing model upload! modified=true");
-
 		// if the model was validated, the submission must be changed to use the modified model
-		if modified{
+		let (validated_model_id,validated_model_version)=if modified{
 			// serialize model (slow!)
 			let mut data=Vec::new();
 			let &[map_ref]=dom.root().children()else{
-				return Err(ValidateError::ModelFileRootMustHaveOneChild);
+				return Err(Error::ModelFileRootMustHaveOneChild);
 			};
-			rbx_binary::to_writer(&mut data,&dom,&[map_ref]).map_err(ValidateError::ModelFileEncode)?;
+			rbx_binary::to_writer(&mut data,&dom,&[map_ref]).map_err(Error::ModelFileEncode)?;
 
 			// upload a model lol
-			let model_id=if let Some(model_id)=validate_info.ValidatedModelID{
+			if let Some(model_id)=validate_info.ValidatedModelID{
 				// upload to existing id
 				let response=self.cookie_context.upload(rbx_asset::cookie::UploadRequest{
 					assetid:model_id,
@@ -232,13 +230,14 @@ impl crate::message_handler::MessageHandler{
 					ispublic:None,
 					allowComments:None,
 					groupId:None,
-				},data).await.map_err(ValidateError::AssetUpload)?;
+				},data).await.map_err(Error::AssetUpload)?;
 
-				response.AssetId
+				// TODO: figure out model version
+				(response.AssetId,1)
 			}else{
-				// grab the map instance from the map re
+				// grab the map instance from the map ref
 				let Some(map_instance)=dom.get_by_ref(map_ref)else{
-					return Err(ValidateError::ModelFileChildRefIsNil);
+					return Err(Error::ModelFileChildRefIsNil);
 				};
 				// create new model
 				let response=self.cookie_context.create(rbx_asset::cookie::CreateRequest{
@@ -247,29 +246,31 @@ impl crate::message_handler::MessageHandler{
 					ispublic:true,
 					allowComments:true,
 					groupId:None,
-				},data).await.map_err(ValidateError::AssetCreate)?;
+				},data).await.map_err(Error::AssetCreate)?;
 
-				response.AssetId
-			};
-
-			match validate_info.ResourceID{
-				ResourceID::Mapfix(mapfix_id)=>{
-					// update the mapfix to use the validated model
-					self.api.update_mapfix_validated_model(submissions_api::types::UpdateMapfixModelRequest{
-						MapfixID:mapfix_id,
-						ModelID:model_id,
-						ModelVersion:1, //TODO
-					}).await.map_err(ValidateError::ApiUpdateMapfixModel)?;
-				},
-				ResourceID::Submission(submission_id)=>{
-					// update the submission to use the validated model
-					self.api.update_submission_validated_model(submissions_api::types::UpdateSubmissionModelRequest{
-						SubmissionID:submission_id,
-						ModelID:model_id,
-						ModelVersion:1, //TODO
-					}).await.map_err(ValidateError::ApiUpdateSubmissionModel)?;
-				},
+				(response.AssetId,1)
 			}
+		}else{
+			(validate_info.ModelID,validate_info.ModelVersion)
+		};
+
+		match validate_info.ResourceID{
+			ResourceID::Mapfix(mapfix_id)=>{
+				// update the mapfix to use the validated model
+				self.api.update_mapfix_validated_model(submissions_api::types::UpdateMapfixModelRequest{
+					MapfixID:mapfix_id,
+					ModelID:validated_model_id,
+					ModelVersion:validated_model_version,
+				}).await.map_err(Error::ApiUpdateMapfixModel)?;
+			},
+			ResourceID::Submission(submission_id)=>{
+				// update the submission to use the validated model
+				self.api.update_submission_validated_model(submissions_api::types::UpdateSubmissionModelRequest{
+					SubmissionID:submission_id,
+					ModelID:validated_model_id,
+					ModelVersion:validated_model_version,
+				}).await.map_err(Error::ApiUpdateSubmissionModel)?;
+			},
 		}
 
 		Ok(())