Cargo.toml

@@ -5,3 +5,12 @@ members = [
 	"submissions-api-rs",
 ]
 resolver = "2"
+
+[workspace.dependencies]
+async-nats = "0.46.0"
+rbx_asset = { version = "0.5.0", features = ["gzip", "rustls-tls"], default-features = false, registry = "strafesnet" }
+rbx_binary = "2.0.1"
+rbx_dom_weak = "4.1.0"
+serde = { version = "1.0.215", features = ["derive"] }
+serde_json = "1.0.133"
+tokio = { version = "1.41.1", features = ["macros", "rt-multi-thread", "signal"] }

combobulator/Cargo.toml

@@ -4,18 +4,18 @@ version = "0.1.0"
 edition = "2024"
 
 [dependencies]
-async-nats = "0.45.0"
+async-nats.workspace = true
 aws-config = { version = "1", features = ["behavior-version-latest"] }
 aws-sdk-s3 = "1"
 map-tool = { version = "3.0.0", registry = "strafesnet", features = ["roblox"], default-features = false }
-rbx_asset = { version = "0.5.0", features = ["gzip", "rustls-tls"], default-features = false, registry = "strafesnet" }
-rbx_binary = "2.0.1"
-rbx_dom_weak = "4.1.0"
+rbx_asset.workspace = true
+rbx_binary.workspace = true
+rbx_dom_weak.workspace = true
 rbxassetid = { version = "0.1.0", registry = "strafesnet" }
-serde = { version = "1.0.215", features = ["derive"] }
-serde_json = "1.0.133"
+serde.workspace = true
+serde_json.workspace = true
 strafesnet_deferred_loader = { version = "0.6.0", registry = "strafesnet" }
-strafesnet_rbx_loader = { version = "0.9.0", registry = "strafesnet" }
+strafesnet_rbx_loader = { version = "0.10.0", registry = "strafesnet" }
 strafesnet_snf = { version = "0.3.2", registry = "strafesnet" }
-tokio = { version = "1.41.1", features = ["macros", "rt-multi-thread", "signal"] }
+tokio.workspace = true
 tokio-stream = "0.1"

combobulator/src/loader.rs

@@ -3,7 +3,6 @@ use std::collections::HashMap;
 use rbxassetid::{RobloxAssetId,RobloxAssetIdParseErr};
 use strafesnet_deferred_loader::{loader::Loader,texture::Texture};
 
-use strafesnet_rbx_loader::data::RobloxMeshBytes;
 use strafesnet_rbx_loader::mesh::{MeshIndex,MeshType,MeshWithSize};
 
 // disallow non-static lifetimes
@@ -102,10 +101,8 @@ impl MeshLoader{
 			unions:HashMap::new(),
 		}
 	}
-	pub fn insert_mesh(&mut self,asset_id:RobloxAssetId,mesh:Vec<u8>)->Result<(),strafesnet_rbx_loader::mesh::Error>{
-		let mesh=strafesnet_rbx_loader::mesh::convert(RobloxMeshBytes::new(mesh))?;
+	pub fn insert_mesh(&mut self,asset_id:RobloxAssetId,mesh:MeshWithSize){
 		self.meshes.insert(asset_id,mesh);
-		Ok(())
 	}
 	pub fn insert_union(&mut self,asset_id:RobloxAssetId,union:rbx_dom_weak::WeakDom){
 		self.unions.insert(asset_id,union);

combobulator/src/process.rs

@@ -162,17 +162,24 @@ impl Processor{
 			let asset_id=id.0;
 			let mesh_key=S3Cache::mesh_key(asset_id);
 
-			let data=if let Some(data)=self.s3.get(&mesh_key).await.map_err(Error::S3Get)?{
-				data
+			let mesh_result=if let Some(data)=self.s3.get(&mesh_key).await.map_err(Error::S3Get)?{
+				strafesnet_rbx_loader::mesh::convert(&data)
 			}else{
 				println!("[combobulator] Downloading mesh {asset_id}");
 				let Some(data)=self.download_asset(asset_id).await? else{continue};
+
+				// decode while we have ownership
+				let mesh_result=strafesnet_rbx_loader::mesh::convert(&data);
+
 				self.s3.put(&mesh_key,data.clone()).await.map_err(Error::S3Put)?;
-				data
+				mesh_result
 			};
 			println!("[combobulator] Mesh {asset_id} processed");
 
-			mesh_loader.insert_mesh(id,data).map_err(Error::Mesh)?;
+			// handle error after caching data
+			let mesh=mesh_result.map_err(Error::Mesh)?;
+
+			mesh_loader.insert_mesh(id,mesh);
 		}
 
 		// process unions

pkg/web_api/mapfixes.go

@@ -2,7 +2,6 @@ package web_api
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"io"
 	"time"
@@ -35,10 +34,10 @@ var(
 )
 
 var (
-	ErrCreationPhaseMapfixesLimit = errors.New("Active mapfixes limited to 20")
-	ErrActiveMapfixSameTargetAssetID = errors.New("There is an active mapfix for this map already")
+	ErrCreationPhaseMapfixesLimit = fmt.Errorf("%w: Active mapfixes limited to 20", ErrPermissionDenied)
+	ErrActiveMapfixSameTargetAssetID = fmt.Errorf("%w: There is an active mapfix for this map already", ErrPermissionDenied)
 	ErrAcceptOwnMapfix = fmt.Errorf("%w: You cannot accept your own mapfix as the submitter", ErrPermissionDenied)
-	ErrCreateMapfixRateLimit = errors.New("You must not create more than 5 mapfixes every 10 minutes")
+	ErrCreateMapfixRateLimit = fmt.Errorf("%w: You must not create more than 5 mapfixes every 10 minutes", ErrTooManyRequests)
 )
 
 // POST /mapfixes

pkg/web_api/security.go

@@ -2,7 +2,7 @@ package web_api
 
 import (
 	"context"
-	"errors"
+	"fmt"
 
 	"git.itzana.me/strafesnet/go-grpc/auth"
 	"git.itzana.me/strafesnet/maps-service/pkg/api"
@@ -11,9 +11,9 @@ import (
 
 var (
 	// ErrMissingSessionID there is no session id
-	ErrMissingSessionID = errors.New("SessionID missing")
+	ErrMissingSessionID = fmt.Errorf("%w: SessionID missing", ErrUserInfo)
 	// ErrInvalidSession caller does not have a valid session
-	ErrInvalidSession = errors.New("Session invalid")
+	ErrInvalidSession = fmt.Errorf("%w: Session invalid", ErrUserInfo)
 )
 
 type UserInfoHandle struct {

pkg/web_api/service.go

@@ -12,6 +12,8 @@ import (
 )
 
 var (
+	ErrBadRequest = errors.New("Bad request")
+	ErrTooManyRequests = errors.New("Too many requests")
 	// ErrPermissionDenied caller does not have the required role
 	ErrPermissionDenied = errors.New("Permission denied")
 	// ErrUserInfo user info is missing for some reason
@@ -26,7 +28,7 @@ var (
 	ErrPermissionDeniedNeedRoleMapDownload = fmt.Errorf("%w: Need Role MapDownload", ErrPermissionDenied)
 	ErrPermissionDeniedNeedRoleScriptWrite = fmt.Errorf("%w: Need Role ScriptWrite", ErrPermissionDenied)
 	ErrPermissionDeniedNeedRoleMaptest = fmt.Errorf("%w: Need Role Maptest", ErrPermissionDenied)
-	ErrNegativeID = errors.New("A negative ID was provided")
+	ErrNegativeID = fmt.Errorf("%w: A negative ID was provided", ErrBadRequest)
 )
 
 type Service struct {
@@ -49,14 +51,20 @@ func NewService(
 // Used for common default response.
 func (svc *Service) NewError(ctx context.Context, err error) *api.ErrorStatusCode {
 	status := 500
-	if errors.Is(err, datastore.ErrNotExist) {
-		status = 404
+	if errors.Is(err, ErrBadRequest) {
+		status = 400
+	}
+	if errors.Is(err, ErrUserInfo) {
+		status = 401
 	}
 	if errors.Is(err, ErrPermissionDenied) {
 		status = 403
 	}
-	if errors.Is(err, ErrUserInfo) {
-		status = 401
+	if errors.Is(err, datastore.ErrNotExist) {
+		status = 404
+	}
+	if errors.Is(err, ErrTooManyRequests) {
+		status = 429
 	}
 	return &api.ErrorStatusCode{
 		StatusCode: status,

pkg/web_api/submissions.go

@@ -2,7 +2,6 @@ package web_api
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"io"
 	"time"
@@ -26,12 +25,13 @@ var(
 )
 
 var (
-	ErrCreationPhaseSubmissionsLimit = errors.New("Active submissions limited to 20")
-	ErrUploadedAssetIDAlreadyExists = errors.New("The submission UploadedAssetID is already set")
-	ErrReleaseInvalidStatus = errors.New("Only submissions with Uploaded status can be released")
-	ErrReleaseNoUploadedAssetID = errors.New("Only submissions with a UploadedAssetID can be released")
+	ErrCreationPhaseSubmissionsLimit = fmt.Errorf("%w: Active submissions limited to 20", ErrPermissionDenied)
+	ErrUploadedAssetIDAlreadyExists = fmt.Errorf("%w: The submission UploadedAssetID is already set", ErrPermissionDenied)
+	ErrReleaseInvalidStatus = fmt.Errorf("%w: Only submissions with Uploaded status can be released", ErrPermissionDenied)
+	ErrReleaseNoUploadedAssetID = fmt.Errorf("%w: Only submissions with a UploadedAssetID can be released", ErrPermissionDenied)
 	ErrAcceptOwnSubmission = fmt.Errorf("%w: You cannot accept your own submission as the submitter", ErrPermissionDenied)
-	ErrCreateSubmissionRateLimit = errors.New("You must not create more than 5 submissions every 10 minutes")
+	ErrCreateSubmissionRateLimit = fmt.Errorf("%w: You must not create more than 5 submissions every 10 minutes", ErrTooManyRequests)
+	ErrDisplayNameNotUnique = fmt.Errorf("%w: Cannot submit: A map exists with the same DisplayName", ErrPermissionDenied)
 )
 
 // POST /submissions
@@ -552,6 +552,24 @@ func (svc *Service) ActionSubmissionTriggerSubmit(ctx context.Context, params ap
 		}
 	}
 
+	// check for maps with the exact same name
+	filter := service.NewMapFilter()
+	filter.SetDisplayName(submission.DisplayName)
+	maps_list, err := svc.inner.ListMaps(
+		ctx,
+		filter,
+		model.Page{
+			Number: 1,
+			Size:   1,
+		},
+	)
+	if err != nil {
+		return err
+	}
+	if len(maps_list) != 0 {
+		return ErrDisplayNameNotUnique
+	}
+
 	// transaction
 	target_status := model.SubmissionStatusSubmitting
 	update := service.NewSubmissionUpdate()

submissions-api-rs/Cargo.toml

@@ -17,7 +17,7 @@ reqwest = { version = "0", features = [
 	# default features
 	"charset", "http2", "system-proxy"
 ], default-features = false }
-serde = { version = "1", features = ["derive"] }
-serde_json = "1"
+serde.workspace = true
+serde_json.workspace = true
 serde_repr = "0.1.19"
 url = "2"

validation/Cargo.toml

@@ -4,18 +4,18 @@ version = "0.1.1"
 edition = "2024"
 
 [dependencies]
-async-nats = "0.45.0"
+async-nats.workspace = true
 futures = "0.3.31"
-rbx_asset = { version = "0.5.0", features = ["gzip", "rustls-tls"], default-features = false, registry = "strafesnet" }
-rbx_binary = "2.0.0"
-rbx_dom_weak = "4.0.0"
+rbx_asset.workspace = true
+rbx_binary.workspace = true
+rbx_dom_weak.workspace = true
 rbx_reflection_database = "2.0.1"
 rbx_xml = "2.0.0"
 regex = { version = "1.11.3", default-features = false }
-serde = { version = "1.0.215", features = ["derive"] }
-serde_json = "1.0.133"
+serde.workspace = true
+serde_json.workspace = true
 siphasher = "1.0.1"
-tokio = { version = "1.41.1", features = ["macros", "rt-multi-thread", "signal"] }
+tokio.workspace = true
 heck = "0.5.0"
 rust-grpc = { version = "1.6.1", registry = "strafesnet" }
 tonic = "0.14.1"

validation/src/check.rs

@@ -324,25 +324,24 @@ pub fn get_model_info<'a>(dom:&'a rbx_dom_weak::WeakDom,model_instance:&'a rbx_d
 }
 
 // check if an observed string matches an expected string
-pub struct StringCheck<'a,T,Str>(Result<T,StringCheckContext<'a,Str>>);
-pub struct StringCheckContext<'a,Str>{
+pub struct StringEquality<'a,Str>{
 	observed:&'a str,
 	expected:Str,
 }
-impl<'a,Str> StringCheckContext<'a,Str>
+impl<'a,Str> StringEquality<'a,Str>
 	where
 		&'a str:PartialEq<Str>,
 {
 	/// Compute the StringCheck, passing through the provided value on success.
-	fn check<T>(self,value:T)->StringCheck<'a,T,Str>{
+	fn check<T>(self,value:T)->Result<T,Self>{
 		if self.observed==self.expected{
-			StringCheck(Ok(value))
+			Ok(value)
 		}else{
-			StringCheck(Err(self))
+			Err(self)
 		}
 	}
 }
-impl<Str:std::fmt::Display> std::fmt::Display for StringCheckContext<'_,Str>{
+impl<Str:std::fmt::Display> std::fmt::Display for StringEquality<'_,Str>{
 	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
 		write!(f,"expected: {}, observed: {}",self.expected,self.observed)
 	}
@@ -464,19 +463,66 @@ impl TryFrom<MapInfo<'_>> for MapInfoOwned{
 struct Exists;
 struct Absent;
 
+enum DisplayNameError<'a>{
+	TitleCase(StringEquality<'a,String>),
+	Empty(StringEmpty),
+	TooLong(usize),
+	StringValue(StringValueError),
+}
+fn check_display_name<'a>(display_name:Result<&'a str,StringValueError>)->Result<&'a str,DisplayNameError<'a>>{
+	// DisplayName StringValue can be missing or whatever
+	let display_name=display_name.map_err(DisplayNameError::StringValue)?;
+
+	// DisplayName cannot be ""
+	let display_name=check_empty(display_name).map_err(DisplayNameError::Empty)?;
+
+	// DisplayName cannot exceed 50 characters
+	if 50<display_name.len(){
+		return Err(DisplayNameError::TooLong(display_name.len()));
+	}
+
+	// Check title case
+	let display_name=StringEquality{
+		observed:display_name,
+		expected:display_name.to_title_case(),
+	}.check(display_name).map_err(DisplayNameError::TitleCase)?;
+
+	Ok(display_name)
+}
+
+enum CreatorError{
+	Empty(StringEmpty),
+	TooLong(usize),
+	StringValue(StringValueError),
+}
+fn check_creator<'a>(creator:Result<&'a str,StringValueError>)->Result<&'a str,CreatorError>{
+	// Creator StringValue can be missing or whatever
+	let creator=creator.map_err(CreatorError::StringValue)?;
+
+	// Creator cannot be ""
+	let creator=check_empty(creator).map_err(CreatorError::Empty)?;
+
+	// Creator cannot exceed 50 characters
+	if 50<creator.len(){
+		return Err(CreatorError::TooLong(creator.len()));
+	}
+
+	Ok(creator)
+}
+
 /// The result of every map check.
 struct MapCheck<'a>{
 	// === METADATA CHECKS ===
 	// The root must be of class Model
-	model_class:StringCheck<'a,(),&'static str>,
+	model_class:Result<(),StringEquality<'a,&'static str>>,
 	// Model's name must be in snake case
-	model_name:StringCheck<'a,(),String>,
+	model_name:Result<(),StringEquality<'a,String>>,
 	// Map must have a StringValue named DisplayName.
 	// Value must not be empty, must be in title case.
-	display_name:Result<Result<StringCheck<'a,&'a str,String>,StringEmpty>,StringValueError>,
+	display_name:Result<&'a str,DisplayNameError<'a>>,
 	// Map must have a StringValue named Creator.
 	// Value must not be empty.
-	creator:Result<Result<&'a str,StringEmpty>,StringValueError>,
+	creator:Result<&'a str,CreatorError>,
 	// The prefix of the model's name must match the game it was submitted for.
 	// bhop_ for bhop, and surf_ for surf
 	game_id:Result<GameID,ParseGameIDError>,
@@ -511,27 +557,22 @@ struct MapCheck<'a>{
 impl<'a> ModelInfo<'a>{
 	fn check(self)->MapCheck<'a>{
 		// Check class is exactly "Model"
-		let model_class=StringCheckContext{
+		let model_class=StringEquality{
 			observed:self.model_class,
 			expected:"Model",
 		}.check(());
 
 		// Check model name is snake case
-		let model_name=StringCheckContext{
+		let model_name=StringEquality{
 			observed:self.model_name,
 			expected:self.model_name.to_snake_case(),
 		}.check(());
 
 		// Check display name is not empty and has title case
-		let display_name=self.map_info.display_name.map(|display_name|{
-			check_empty(display_name).map(|display_name|StringCheckContext{
-				observed:display_name,
-				expected:display_name.to_title_case(),
-			}.check(display_name))
-		});
+		let display_name=check_display_name(self.map_info.display_name);
 
 		// Check Creator is not empty
-		let creator=self.map_info.creator.map(check_empty);
+		let creator=check_creator(self.map_info.creator);
 
 		// Check GameID (model name was prefixed with bhop_ surf_ etc)
 		let game_id=self.map_info.game_id;
@@ -630,10 +671,10 @@ impl MapCheck<'_>{
 	fn result(self)->Result<MapInfoOwned,Result<MapCheckList,serde_json::Error>>{
 		match self{
 			MapCheck{
-				model_class:StringCheck(Ok(())),
-				model_name:StringCheck(Ok(())),
-				display_name:Ok(Ok(StringCheck(Ok(display_name)))),
-				creator:Ok(Ok(creator)),
+				model_class:Ok(()),
+				model_name:Ok(()),
+				display_name:Ok(display_name),
+				creator:Ok(creator),
 				game_id:Ok(game_id),
 				mapstart:Ok(Exists),
 				mode_start_counts:DuplicateCheck(Ok(())),
@@ -737,27 +778,25 @@ macro_rules! summary_format{
 impl MapCheck<'_>{
 	fn itemize(&self)->Result<MapCheckList,serde_json::Error>{
 		let model_class=match &self.model_class{
-			StringCheck(Ok(()))=>passed!("ModelClass"),
-			StringCheck(Err(context))=>summary_format!("ModelClass","Invalid model class: {context}"),
+			Ok(())=>passed!("ModelClass"),
+			Err(context)=>summary_format!("ModelClass","Invalid model class: {context}"),
 		};
 		let model_name=match &self.model_name{
-			StringCheck(Ok(()))=>passed!("ModelName"),
-			StringCheck(Err(context))=>summary_format!("ModelName","Model name must have snake_case: {context}"),
+			Ok(())=>passed!("ModelName"),
+			Err(context)=>summary_format!("ModelName","Model name must have snake_case: {context}"),
 		};
 		let display_name=match &self.display_name{
-			Ok(Ok(StringCheck(Ok(_))))=>passed!("DisplayName"),
-			Ok(Ok(StringCheck(Err(context))))=>summary_format!("DisplayName","DisplayName must have Title Case: {context}"),
-			Ok(Err(context))=>summary_format!("DisplayName","Invalid DisplayName: {context}"),
-			Err(StringValueError::ObjectNotFound)=>summary!("DisplayName","Missing DisplayName StringValue".to_owned()),
-			Err(StringValueError::ValueNotSet)=>summary!("DisplayName","DisplayName Value not set".to_owned()),
-			Err(StringValueError::NonStringValue)=>summary!("DisplayName","DisplayName Value is not a String".to_owned()),
+			Ok(_)=>passed!("DisplayName"),
+			Err(DisplayNameError::TitleCase(context))=>summary_format!("DisplayName","DisplayName must have Title Case: {context}"),
+			Err(DisplayNameError::Empty(context))=>summary_format!("DisplayName","Invalid DisplayName: {context}"),
+			Err(DisplayNameError::TooLong(context))=>summary_format!("DisplayName","DisplayName is too long: {context} characters (50 characters max)"),
+			Err(DisplayNameError::StringValue(context))=>summary_format!("DisplayName","DisplayName StringValue: {context}"),
 		};
 		let creator=match &self.creator{
-			Ok(Ok(_))=>passed!("Creator"),
-			Ok(Err(context))=>summary_format!("Creator","Invalid Creator: {context}"),
-			Err(StringValueError::ObjectNotFound)=>summary!("Creator","Missing Creator StringValue".to_owned()),
-			Err(StringValueError::ValueNotSet)=>summary!("Creator","Creator Value not set".to_owned()),
-			Err(StringValueError::NonStringValue)=>summary!("Creator","Creator Value is not a String".to_owned()),
+			Ok(_)=>passed!("Creator"),
+			Err(CreatorError::Empty(context))=>summary_format!("Creator","Invalid Creator: {context}"),
+			Err(CreatorError::TooLong(context))=>summary_format!("Creator","Creator is too long: {context} characters (50 characters max)"),
+			Err(CreatorError::StringValue(context))=>summary_format!("Creator","Creator StringValue: {context}"),
 		};
 		let game_id=match &self.game_id{
 			Ok(_)=>passed!("GameID"),

validation/src/rbx_util.rs

@@ -79,6 +79,15 @@ pub enum StringValueError{
 	ValueNotSet,
 	NonStringValue,
 }
+impl std::fmt::Display for StringValueError{
+	fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
+		match self{
+			StringValueError::ObjectNotFound=>write!(f,"Missing StringValue"),
+			StringValueError::ValueNotSet=>write!(f,"Value not set"),
+			StringValueError::NonStringValue=>write!(f,"Value is not a String"),
+		}
+	}
+}
 
 fn string_value(instance:Option<&rbx_dom_weak::Instance>)->Result<&str,StringValueError>{
 	let instance=instance.ok_or(StringValueError::ObjectNotFound)?;