@@ -318,9 +318,17 @@ impl SelectionMap {
318318 }
319319 }
320320
321+ /// Insert a selection into the map. If a selection with an equal key is already present, it
322+ /// is overwritten in place (keeping its original position); otherwise the new selection is
323+ /// appended.
321324pub ( crate ) fn insert ( & mut self , value : Selection ) {
322- let hash = self . hash_key ( value. key ( ) ) ;
323- self . raw_insert ( hash, value) ;
325+ let key = value. key ( ) ;
326+ let hash = self . hash_key ( key) ;
327+ if let Some ( bucket) = self . table . find_mut ( hash, key_eq ( & self . selections , key) ) {
328+ self . selections [ bucket. index ] = value;
329+ } else {
330+ self . raw_insert ( hash, value) ;
331+ }
324332 }
325333
326334 /// Remove a selection from the map. Returns the selection and its numeric index.
@@ -619,3 +627,226 @@ impl<'a> VacantEntry<'a> {
619627 Ok ( SelectionValue :: new ( self . map . raw_insert ( self . hash , value) ) )
620628 }
621629}
630+
631+ #[ cfg( test) ]
632+ mod tests {
633+ use std:: collections:: HashSet ;
634+
635+ use proptest:: prelude:: * ;
636+
637+ use super :: * ;
638+ use crate :: operation:: tests:: parse_operation;
639+ use crate :: operation:: tests:: parse_schema;
640+
641+ const SCHEMA : & str = r#"
642+ type Query {
643+ a: T
644+ b: T
645+ c: T
646+ }
647+
648+ type T {
649+ x: Int
650+ y: Int
651+ }
652+ "# ;
653+
654+ fn selection_from_source (
655+ schema : & crate :: schema:: ValidFederationSchema ,
656+ source : & str ,
657+ ) -> Selection {
658+ let query = format ! ( "{{ {source} }}" ) ;
659+ let operation = parse_operation ( schema, & query) ;
660+ operation
661+ . selection_set
662+ . selections
663+ . values ( )
664+ . next ( )
665+ . expect ( "selection source must produce exactly one top-level selection" )
666+ . clone ( )
667+ }
668+
669+ /// Minimized regression for the bug found by `selection_map_matches_last_write_wins_model`:
670+ /// inserting a selection under a key that's already present used to append a second entry
671+ /// instead of overwriting the first, in violation of the documented last-write-wins contract.
672+ #[ test]
673+ fn insert_overwrites_equal_key_without_changing_position ( ) {
674+ let schema = parse_schema ( SCHEMA ) ;
675+ let mut map = SelectionMap :: new ( ) ;
676+
677+ map. insert ( selection_from_source ( & schema, "a { x }" ) ) ;
678+ map. insert ( selection_from_source ( & schema, "b { x }" ) ) ;
679+ map. insert ( selection_from_source ( & schema, "b { y }" ) ) ;
680+
681+ assert_eq ! (
682+ map. len( ) ,
683+ 2 ,
684+ "the second `b` insertion should overwrite the first"
685+ ) ;
686+ let values: Vec < & Selection > = map. values ( ) . collect ( ) ;
687+ assert_eq ! ( values[ 0 ] , & selection_from_source( & schema, "a { x }" ) ) ;
688+ assert_eq ! (
689+ values[ 1 ] ,
690+ & selection_from_source( & schema, "b { y }" ) ,
691+ "the surviving `b` entry should hold the last-written value, in its original position"
692+ ) ;
693+ }
694+
695+ /// A slow, obviously-correct model of `SelectionMap`'s documented last-write-wins overwrite
696+ /// semantics: a key that already exists keeps its position but gets the new value.
697+ fn model_insert ( model : & mut Vec < ( OwnedSelectionKey , Selection ) > , value : Selection ) {
698+ let key = value. key ( ) . to_owned_key ( ) ;
699+ if let Some ( existing) = model. iter_mut ( ) . find ( |( k, _) | * k == key) {
700+ existing. 1 = value;
701+ } else {
702+ model. push ( ( key, value) ) ;
703+ }
704+ }
705+
706+ fn response_name ( ) -> impl Strategy < Value = & ' static str > {
707+ prop_oneof ! [ Just ( "a" ) , Just ( "b" ) , Just ( "c" ) ]
708+ }
709+
710+ fn key_name ( ) -> impl Strategy < Value = & ' static str > {
711+ prop_oneof ! [ Just ( "a" ) , Just ( "b" ) , Just ( "c" ) , Just ( "z" ) ]
712+ }
713+
714+ fn sub_selection ( ) -> impl Strategy < Value = & ' static str > {
715+ prop_oneof ! [ Just ( "x" ) , Just ( "y" ) , Just ( "x y" ) ]
716+ }
717+
718+ fn selection_source ( ) -> impl Strategy < Value = String > {
719+ ( response_name ( ) , sub_selection ( ) ) . prop_map ( |( name, sub) | format ! ( "{name} {{ {sub} }}" ) )
720+ }
721+
722+ #[ derive( Debug , Clone ) ]
723+ enum Command {
724+ Insert ( String ) ,
725+ Get ( String ) ,
726+ Remove ( String ) ,
727+ Retain ( Vec < String > ) ,
728+ Extend ( Vec < String > ) ,
729+ }
730+
731+ fn command ( ) -> impl Strategy < Value = Command > {
732+ prop_oneof ! [
733+ 3 => selection_source( ) . prop_map( Command :: Insert ) ,
734+ 2 => key_name( ) . prop_map( |n| Command :: Get ( n. to_string( ) ) ) ,
735+ 2 => key_name( ) . prop_map( |n| Command :: Remove ( n. to_string( ) ) ) ,
736+ 1 => prop:: collection:: vec( key_name( ) , 0 ..3 )
737+ . prop_map( |names| Command :: Retain ( names. into_iter( ) . map( String :: from) . collect( ) ) ) ,
738+ 2 => prop:: collection:: vec( selection_source( ) , 0 ..3 ) . prop_map( Command :: Extend ) ,
739+ ]
740+ }
741+
742+ /// Compares production state against the model: length, iteration order and content,
743+ /// structural (order-independent) equality against a freshly reconstructed map, and
744+ /// lookups for every name in the small generated key universe.
745+ fn assert_map_matches_model ( map : & SelectionMap , model : & [ ( OwnedSelectionKey , Selection ) ] ) {
746+ let actual: Vec < & Selection > = map. values ( ) . collect ( ) ;
747+ let expected: Vec < & Selection > = model. iter ( ) . map ( |( _, value) | value) . collect ( ) ;
748+ assert_eq ! ( actual, expected, "iteration order/content mismatch" ) ;
749+ assert_eq ! ( map. len( ) , model. len( ) ) ;
750+ assert_eq ! ( map. is_empty( ) , model. is_empty( ) ) ;
751+
752+ let rebuilt: SelectionMap = model. iter ( ) . map ( |( _, value) | value. clone ( ) ) . collect ( ) ;
753+ assert_eq ! (
754+ map, & rebuilt,
755+ "map should equal a freshly reconstructed map"
756+ ) ;
757+
758+ for name in [ "a" , "b" , "c" , "z" ] {
759+ let name = Name :: new ( name) . unwrap ( ) ;
760+ let key = SelectionKey :: field_name ( & name) ;
761+ let owned_key = key. to_owned_key ( ) ;
762+ let expected = model
763+ . iter ( )
764+ . find ( |( k, _) | * k == owned_key)
765+ . map ( |( _, value) | value) ;
766+ assert_eq ! ( map. get( key) , expected, "lookup mismatch for {name}" ) ;
767+ assert_eq ! ( map. contains_key( key) , expected. is_some( ) ) ;
768+ }
769+ }
770+
771+ proptest ! {
772+ #![ proptest_config( ProptestConfig :: with_cases( 256 ) ) ]
773+
774+ #[ test]
775+ fn selection_map_matches_last_write_wins_model( commands in prop:: collection:: vec( command( ) , 0 ..40 ) ) {
776+ let schema = parse_schema( SCHEMA ) ;
777+ let mut map = SelectionMap :: new( ) ;
778+ let mut model: Vec <( OwnedSelectionKey , Selection ) > = Vec :: new( ) ;
779+
780+ for command in commands {
781+ match command {
782+ Command :: Insert ( source) => {
783+ let selection = selection_from_source( & schema, & source) ;
784+ map. insert( selection. clone( ) ) ;
785+ model_insert( & mut model, selection) ;
786+ }
787+ Command :: Get ( name) => {
788+ let name = Name :: new( & name) . unwrap( ) ;
789+ let key = SelectionKey :: field_name( & name) ;
790+ let owned_key = key. to_owned_key( ) ;
791+ let expected = model
792+ . iter( )
793+ . find( |( k, _) | * k == owned_key)
794+ . map( |( _, value) | value. clone( ) ) ;
795+ prop_assert_eq!( map. get( key) . cloned( ) , expected) ;
796+ }
797+ Command :: Remove ( name) => {
798+ let name = Name :: new( & name) . unwrap( ) ;
799+ let key = SelectionKey :: field_name( & name) ;
800+ let owned_key = key. to_owned_key( ) ;
801+ let model_index = model. iter( ) . position( |( k, _) | * k == owned_key) ;
802+ let removed = map. remove( key) ;
803+ match ( removed, model_index) {
804+ ( Some ( ( index, selection) ) , Some ( model_index) ) => {
805+ prop_assert_eq!( index, model_index) ;
806+ let ( _, model_selection) = model. remove( model_index) ;
807+ prop_assert_eq!( selection, model_selection) ;
808+ }
809+ ( None , None ) => { }
810+ ( actual, expected_index) => {
811+ prop_assert!(
812+ false ,
813+ "remove presence mismatch: actual={:?} expected_index={:?}" ,
814+ actual,
815+ expected_index
816+ ) ;
817+ }
818+ }
819+ }
820+ Command :: Retain ( names) => {
821+ let keep: HashSet <String > = names. into_iter( ) . collect( ) ;
822+ map. retain( |key, _| match key {
823+ SelectionKey :: Field { response_name, .. } => {
824+ keep. contains( response_name. as_str( ) )
825+ }
826+ _ => false ,
827+ } ) ;
828+ model. retain( |( key, _) | match key {
829+ OwnedSelectionKey :: Field { response_name, .. } => {
830+ keep. contains( response_name. as_str( ) )
831+ }
832+ _ => false ,
833+ } ) ;
834+ }
835+ Command :: Extend ( sources) => {
836+ let selections: Vec <Selection > = sources
837+ . iter( )
838+ . map( |source| selection_from_source( & schema, source) )
839+ . collect( ) ;
840+ let other: SelectionMap = selections. iter( ) . cloned( ) . collect( ) ;
841+ map. extend( other) ;
842+ for selection in selections {
843+ model_insert( & mut model, selection) ;
844+ }
845+ }
846+ }
847+
848+ assert_map_matches_model( & map, & model) ;
849+ }
850+ }
851+ }
852+ }
0 commit comments