StrafesNET/maps-service
5
Fork 4
Code Issues 17 Pull Requests 1 Packages Projects Releases Wiki Activity

Check that mapfixes do not change DisplayName, Creator, GameID #324

Open
Quaternions wants to merge 3 commits from mapfix-checks into 50-chars
pull from: mapfix-checks
merge into: StrafesNET:50-chars
Conversation 0 Commits 3 Files Changed 10 +257 -93
10 changed files with 257 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
Cargo.lock generated
View File

@@ -1044,6 +1044,7 @@ dependencies = [
"rust-grpc",
"serde",
"serde_json",
"serde_repr",
"siphasher",
"tokio",
"tonic",

9
pkg/model/nats.go
View File

@@ -31,9 +31,12 @@ type CheckSubmissionRequest struct{
}
type CheckMapfixRequest struct{
MapfixID int64
ModelID uint64
SkipChecks bool
MapfixID int64
ModelID uint64
SkipChecks bool
DisplayName string
Creator string
GameID uint32
}
type ValidateSubmissionRequest struct {

12
pkg/service/nats_mapfix.go
View File

@@ -33,14 +33,20 @@ func (svc *Service) NatsCreateMapfix(
}
func (svc *Service) NatsCheckMapfix(
MapfixID int64,
ModelID uint64,
SkipChecks bool,
MapfixID int64,
ModelID uint64,
SkipChecks bool,
DisplayName string,
Creator string,
GameID uint32,
) error {
validate_request := model.CheckMapfixRequest{
MapfixID: MapfixID,
ModelID: ModelID,
SkipChecks: SkipChecks,
DisplayName: DisplayName,
Creator: Creator,
GameID: GameID,
}
j, err := json.Marshal(validate_request)

30
pkg/web_api/mapfixes.go
View File

@@ -538,13 +538,13 @@ func (svc *Service) ActionMapfixTriggerSubmit(ctx context.Context, params api.Ac
return ErrUserInfo
}
// read mapfix (this could be done with a transaction WHERE clause)
mapfix, err := svc.inner.GetMapfix(ctx, params.MapfixID)
userId, err := userInfo.GetUserID()
if err != nil {
return err
}
userId, err := userInfo.GetUserID()
// read mapfix (this could be done with a transaction WHERE clause)
mapfix, err := svc.inner.GetMapfix(ctx, params.MapfixID)
if err != nil {
return err
}
@@ -555,6 +555,12 @@ func (svc *Service) ActionMapfixTriggerSubmit(ctx context.Context, params api.Ac
return ErrPermissionDeniedNotSubmitter
}
// read map to get current DisplayName and such
target_map, err := svc.inner.GetMap(ctx, int64(mapfix.TargetAssetID))
if err != nil {
return err
}
// transaction
target_status := model.MapfixStatusSubmitting
update := service.NewMapfixUpdate()
@@ -569,6 +575,9 @@ func (svc *Service) ActionMapfixTriggerSubmit(ctx context.Context, params api.Ac
mapfix.ID,
mapfix.AssetID,
false,
target_map.DisplayName,
target_map.Creator,
target_map.GameID,
)
if err != nil {
return err
@@ -600,13 +609,13 @@ func (svc *Service) ActionMapfixTriggerSubmitUnchecked(ctx context.Context, para
return ErrUserInfo
}
// read mapfix (this could be done with a transaction WHERE clause)
mapfix, err := svc.inner.GetMapfix(ctx, params.MapfixID)
userId, err := userInfo.GetUserID()
if err != nil {
return err
}
userId, err := userInfo.GetUserID()
// read mapfix (this could be done with a transaction WHERE clause)
mapfix, err := svc.inner.GetMapfix(ctx, params.MapfixID)
if err != nil {
return err
}
@@ -626,6 +635,12 @@ func (svc *Service) ActionMapfixTriggerSubmitUnchecked(ctx context.Context, para
return ErrPermissionDeniedNeedRoleMapfixReview
}
// read map to get current DisplayName and such
target_map, err := svc.inner.GetMap(ctx, int64(mapfix.TargetAssetID))
if err != nil {
return err
}
// transaction
target_status := model.MapfixStatusSubmitting
update := service.NewMapfixUpdate()
@@ -640,6 +655,9 @@ func (svc *Service) ActionMapfixTriggerSubmitUnchecked(ctx context.Context, para
mapfix.ID,
mapfix.AssetID,
true,
target_map.DisplayName,
target_map.Creator,
target_map.GameID,
)
if err != nil {
return err

1
validation/Cargo.toml
View File

@@ -14,6 +14,7 @@ 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_repr = "0.1.19"
siphasher = "1.0.1"
tokio = { version = "1.41.1", features = ["macros", "rt-multi-thread", "signal"] }
heck = "0.5.0"

277
validation/src/check.rs
View File

@@ -33,29 +33,6 @@ macro_rules! lazy_regex{
}};
}
#[expect(nonstandard_style)]
pub struct CheckRequest{
ModelID:u64,
SkipChecks:bool,
}
impl From<crate::nats_types::CheckMapfixRequest> for CheckRequest{
fn from(value:crate::nats_types::CheckMapfixRequest)->Self{
Self{
ModelID:value.ModelID,
SkipChecks:value.SkipChecks,
}
}
}
impl From<crate::nats_types::CheckSubmissionRequest> for CheckRequest{
fn from(value:crate::nats_types::CheckSubmissionRequest)->Self{
Self{
ModelID:value.ModelID,
SkipChecks:value.SkipChecks,
}
}
}
#[derive(Clone,Copy,Debug,Hash,Eq,PartialEq,Ord,PartialOrd)]
struct ModeID(u64);
impl ModeID{
@@ -323,14 +300,14 @@ 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 StringEquality<'a,Str>{
observed:&'a str,
expected:Str,
// check if an observed value matches an expected value
pub struct EqualityCheck<Obs,Exp>{
observed:Obs,
expected:Exp,
}
impl<'a,Str> StringEquality<'a,Str>
impl<Obs,Exp> EqualityCheck<Obs,Exp>
where
&'a str:PartialEq<Str>,
Obs:PartialEq<Exp>,
{
/// Compute the StringCheck, passing through the provided value on success.
fn check<T>(self,value:T)->Result<T,Self>{
@@ -341,7 +318,7 @@ impl<'a,Str> StringEquality<'a,Str>
}
}
}
impl<Str:std::fmt::Display> std::fmt::Display for StringEquality<'_,Str>{
impl<Obs:std::fmt::Display,Exp:std::fmt::Display> std::fmt::Display for EqualityCheck<Obs,Exp>{
fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
write!(f,"expected: {}, observed: {}",self.expected,self.observed)
}
@@ -464,68 +441,163 @@ struct Exists;
struct Absent;
enum DisplayNameError<'a>{
TitleCase(StringEquality<'a,String>),
TitleCase(EqualityCheck<&'a str,String>),
CannotChange(EqualityCheck<&'a str,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{
enum CreatorError<'a>{
CannotChange(EqualityCheck<&'a str,String>),
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)?;
enum GameIDError{
CannotChange(EqualityCheck<GameID,GameID>),
Parse(ParseGameIDError),
}
// Creator cannot exceed 50 characters
if 50<creator.len(){
return Err(CreatorError::TooLong(creator.len()));
pub struct CheckedMapInfo<'a>{
display_name:Result<&'a str,DisplayNameError<'a>>,
creator:Result<&'a str,CreatorError<'a>>,
game_id:Result<GameID,GameIDError>,
}
pub struct NoMapInfo;
impl CheckModelInfo for NoMapInfo{
fn check<'a>(self,map_info:MapInfo<'a>)->CheckedMapInfo<'a>{
fn check_display_name(display_name:Result<&str,StringValueError>)->Result<&str,DisplayNameError<'_>>{
// 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=EqualityCheck{
observed:display_name,
expected:display_name.to_title_case(),
}.check(display_name).map_err(DisplayNameError::TitleCase)?;
Ok(display_name)
}
fn check_creator(creator:Result<&str,StringValueError>)->Result<&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)
}
fn check_game_id(game_id:Result<GameID,ParseGameIDError>)->Result<GameID,GameIDError>{
// Creator StringValue can be missing or whatever
let game_id=game_id.map_err(GameIDError::Parse)?;
Ok(game_id)
}
// Check display name is not empty and has title case
let display_name=check_display_name(map_info.display_name);
// Check Creator is not empty
let creator=check_creator(map_info.creator);
// Check GameID (model name was prefixed with bhop_ surf_ etc)
let game_id=check_game_id(map_info.game_id);
CheckedMapInfo{
display_name,
creator,
game_id,
}
}
}
impl CheckModelInfo for MapInfoOwned{
fn check<'a>(self,map_info:MapInfo<'a>)->CheckedMapInfo<'a>{
fn check_display_name(display_name:Result<&str,StringValueError>,target_display_name:String)->Result<&str,DisplayNameError<'_>>{
// DisplayName StringValue can be missing or whatever
let display_name=display_name.map_err(DisplayNameError::StringValue)?;
Ok(creator)
// Mapfix cannot change display name
let display_name=EqualityCheck{
observed:display_name,
expected:target_display_name,
}.check(display_name).map_err(DisplayNameError::CannotChange)?;
Ok(display_name)
}
fn check_creator(creator:Result<&str,StringValueError>,target_creator:String)->Result<&str,CreatorError<'_>>{
// Creator StringValue can be missing or whatever
let creator=creator.map_err(CreatorError::StringValue)?;
// Mapfix cannot change creator
let creator=EqualityCheck{
observed:creator,
expected:target_creator,
}.check(creator).map_err(CreatorError::CannotChange)?;
Ok(creator)
}
fn check_game_id(game_id:Result<GameID,ParseGameIDError>,target_game_id:GameID)->Result<GameID,GameIDError>{
// Creator StringValue can be missing or whatever
let game_id=game_id.map_err(GameIDError::Parse)?;
// Mapfix cannot change game_id
let game_id=EqualityCheck{
observed:game_id,
expected:target_game_id,
}.check(game_id).map_err(GameIDError::CannotChange)?;
Ok(game_id)
}
// Check display name is not empty and has title case
let display_name=check_display_name(map_info.display_name,self.display_name);
// Check Creator is not empty
let creator=check_creator(map_info.creator,self.creator);
// Check GameID (model name was prefixed with bhop_ surf_ etc)
let game_id=check_game_id(map_info.game_id,self.game_id);
CheckedMapInfo{
display_name,
creator,
game_id,
}
}
}
/// The result of every map check.
struct MapCheck<'a>{
// === METADATA CHECKS ===
// The root must be of class Model
model_class:Result<(),StringEquality<'a,&'static str>>,
model_class:Result<(),EqualityCheck<&'a str,&'static str>>,
// Model's name must be in snake case
model_name:Result<(),StringEquality<'a,String>>,
model_name:Result<(),EqualityCheck<&'a str,String>>,
// Map must have a StringValue named DisplayName.
// Value must not be empty, must be in title case.
display_name:Result<&'a str,DisplayNameError<'a>>,
// Map must have a StringValue named Creator.
// Value must not be empty.
creator:Result<&'a str,CreatorError>,
creator:Result<&'a str,CreatorError<'a>>,
// 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>,
game_id:Result<GameID,GameIDError>,
// === MODE CHECKS ===
// MapStart must exist
@@ -555,27 +627,24 @@ struct MapCheck<'a>{
}
impl<'a> ModelInfo<'a>{
fn check(self)->MapCheck<'a>{
fn check<I:CheckModelInfo>(self,model_info:I)->MapCheck<'a>{
// Check class is exactly "Model"
let model_class=StringEquality{
let model_class=EqualityCheck{
observed:self.model_class,
expected:"Model",
}.check(());
// Check model name is snake case
let model_name=StringEquality{
let model_name=EqualityCheck{
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=check_display_name(self.map_info.display_name);
// Check Creator is not 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;
let CheckedMapInfo{
display_name,
creator,
game_id,
}=model_info.check(self.map_info);
// MapStart must exist
let mapstart=if self.counts.mode_start_counts.contains_key(&ModeID::MAIN){
@@ -788,19 +857,22 @@ impl MapCheck<'_>{
let display_name=match &self.display_name{
Ok(_)=>passed!("DisplayName"),
Err(DisplayNameError::TitleCase(context))=>summary_format!("DisplayName","DisplayName must have Title Case: {context}"),
Err(DisplayNameError::CannotChange(context))=>summary_format!("DisplayName","DisplayName cannot be changed: {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(_)=>passed!("Creator"),
Err(CreatorError::CannotChange(context))=>summary_format!("Creator","Creator cannot be changed: {context}"),
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"),
Err(ParseGameIDError)=>summary!("GameID","Model name must be prefixed with bhop_ surf_ or flytrials_".to_owned()),
Err(GameIDError::CannotChange(context))=>summary_format!("GameID","GameID cannot be changed: {context}"),
Err(GameIDError::Parse(ParseGameIDError))=>summary!("GameID","Model name must be prefixed with bhop_ surf_ or flytrials_".to_owned()),
};
let mapstart=match &self.mapstart{
Ok(Exists)=>passed!("MapStart"),
@@ -961,8 +1033,55 @@ pub struct CheckListAndVersion{
pub version:u64,
}
pub trait CheckModelInfo{
fn check<'a>(self,map_info:MapInfo<'a>)->CheckedMapInfo<'a>;
}
pub trait CheckSpecialization{
type ModelInfo:CheckModelInfo;
fn info(self)->(CheckRequest,Self::ModelInfo);
}
#[expect(nonstandard_style)]
pub struct CheckRequest{
ModelID:u64,
SkipChecks:bool,
}
impl CheckSpecialization for crate::nats_types::CheckMapfixRequest{
type ModelInfo=MapInfoOwned;
fn info(self)->(CheckRequest,Self::ModelInfo) {
(
CheckRequest{
ModelID:self.ModelID,
SkipChecks:self.SkipChecks,
},
MapInfoOwned{
display_name:self.DisplayName,
creator:self.Creator,
game_id:self.GameID,
}
)
}
}
impl CheckSpecialization for crate::nats_types::CheckSubmissionRequest{
type ModelInfo=NoMapInfo;
fn info(self)->(CheckRequest,Self::ModelInfo) {
(
CheckRequest{
ModelID:self.ModelID,
SkipChecks:self.SkipChecks,
},
NoMapInfo
)
}
}
impl crate::message_handler::MessageHandler{
pub async fn check_inner(&self,check_info:CheckRequest)->Result<CheckListAndVersion,Error>{
pub async fn check_inner<R:CheckSpecialization>(&self,check_info:R)->Result<CheckListAndVersion,Error>{
let (check_info,target_model_info)=check_info.info();
// discover asset creator and latest version
let info=self.cloud_context.get_asset_info(
rbx_asset::cloud::GetAssetLatestRequest{asset_id:check_info.ModelID}
@@ -1002,7 +1121,7 @@ impl crate::message_handler::MessageHandler{
let model_info=get_model_info(&dom,model_instance);
// convert the model information into a structured report
let map_check=model_info.check();
let map_check=model_info.check(target_model_info);
// check the report, generate an error message if it fails the check
let status=match map_check.result(){

2
validation/src/check_mapfix.rs
View File

@@ -17,7 +17,7 @@ impl std::error::Error for Error{}
impl crate::message_handler::MessageHandler{
pub async fn check_mapfix(&self,check_info:CheckMapfixRequest)->Result<(),Error>{
let mapfix_id=check_info.MapfixID;
let check_result=self.check_inner(check_info.into()).await;
let check_result=self.check_inner(check_info).await;
// update the mapfix depending on the result
match check_result{

2
validation/src/check_submission.rs
View File

@@ -17,7 +17,7 @@ impl std::error::Error for Error{}
impl crate::message_handler::MessageHandler{
pub async fn check_submission(&self,check_info:CheckSubmissionRequest)->Result<(),Error>{
let submission_id=check_info.SubmissionID;
let check_result=self.check_inner(check_info.into()).await;
let check_result=self.check_inner(check_info).await;
// update the submission depending on the result
match check_result{

4
validation/src/nats_types.rs
View File

@@ -41,6 +41,10 @@ pub struct CheckMapfixRequest{
pub MapfixID:u64,
pub ModelID:u64,
pub SkipChecks:bool,
// target map info
pub DisplayName:String,
pub Creator:String,
pub GameID:crate::rbx_util::GameID,
}
#[expect(nonstandard_style)]

12
validation/src/rbx_util.rs
View File

@@ -31,6 +31,9 @@ fn find_first_child_name_and_class<'a>(dom:&'a rbx_dom_weak::WeakDom,instance:&r
instance.children().iter().filter_map(|&r|dom.get_by_ref(r)).find(|inst|inst.name==name&&inst.class==class)
}
#[derive(serde_repr::Deserialize_repr)]
#[repr(u32)]
#[derive(Clone,Copy,PartialEq)]
pub enum GameID{
Bhop=1,
Surf=2,
@@ -66,6 +69,15 @@ impl TryFrom<u32> for GameID{
}
}
}
impl std::fmt::Display for GameID{
fn fmt(&self,f:&mut std::fmt::Formatter<'_>)->std::fmt::Result{
match self{
GameID::Bhop=>write!(f,"Bhop"),
GameID::Surf=>write!(f,"Surf"),
GameID::FlyTrials=>write!(f,"FlyTrials"),
}
}
}
pub struct MapInfo<'a>{
pub display_name:Result<&'a str,StringValueError>,