OWASP / Maturity-Models

Node application to help managing Maturity Models like the ones created by BSIMM and OpenSAMM
Apache License 2.0
187 stars 51 forks source link

Race condition on multiple calls to Team_Data load #213

Closed DinisCruz closed 7 years ago

DinisCruz commented 7 years ago

After refactoring for #206 there is an issue when multiple calls are made to load_Data: (callback)=>

image

This looks like a good solution (and similar problem) http://stackoverflow.com/questions/32958046/preventing-multiple-http-requests-in-angular-is-there-a-better-way

DinisCruz commented 7 years ago

Here is the test that confirms the bug

  fit '#213 - Race condition on multiple calls to Team_Data load', ->

    inject ($injector, $routeParams, $httpBackend)->
      team_Data            = $injector.get('team_Data')         # get service
      $routeParams.project = 'bsimm'                            # set project 
      $routeParams.team    = 'team-A'                           # and team that have data
      sequence = 0                                              # capture execution order
      team_Data.load_Data ()->                                  # 1st call to load_Data (queues request)
        (++sequence).assert_Is 3                                # confirms this is called after the 2nd call to load_data
        team_Data.data.metadata.team.assert_Is 'Team A'         # confirm data was retrieved 
      team_Data.load_Data (data)->                              # 2nd call to load_Data
        (++sequence).assert_Is 1                                # confirm this is call first (HERE IS THE BUG)
        (team_Data.data is null).assert_Is_True()               # confirm we get no data
        data.assert_Is no: 'data'                               # note: this will run before the flush below
      (++sequence).assert_Is 2          
      $httpBackend.flush()                                      # flush request and trigger callback to first load_Data call                      
      $httpBackend.verifyNoOutstandingExpectation()
      $httpBackend.verifyNoOutstandingRequest()

here is the load_Data function

  load_Data: (callback)=>
    if not (@.$routeParams.project and @.$routeParams.team)                     # check that project and team are set
      return callback()
    if (@.$routeParams.project is @.project and @.$routeParams.team is @.team)  # check if either the project or team have changed
      if not @.data
        console.log 'DATA NOT READY'
        return callback {no:'data'}

    @.project = @.$routeParams.project
    @.team    = @.$routeParams.team

    @.API.project_Schema @.project, (schema)=>
      @.API.data_Score @.project, @.team, (scores)=>
        @.API.team_Get @.project, @.team, (data)=>
          @.scores = scores
          @.schema = schema
          @.data   = data
          callback()

image

DinisCruz commented 7 years ago

confirming behaviour of Javascript promise (as seen in action in http://stackoverflow.com/a/33312049/262379 )

  fit '#213 - Race condition on multiple calls to Team_Data load (using API._GET)', ->
    inject ($injector, $httpBackend)->
      api = $injector.get('API')
      url = '/api/v1/project/list'
      sequence = 0
      promise = api._GET url, (data)->
        (++sequence).assert_Is 2                                # only called after $httpBackend.flush()
        data.assert_Is ['bsimm', 'samm']
      promise.then (data)->
        (++sequence).assert_Is 3                                # called after original promise callback is invoked
        data.assert_Is ['bsimm', 'samm']
      promise.then (data)->
        (++sequence).assert_Is 4                                # confirms multiple calls to 'then' are honored
        data.assert_Is ['bsimm', 'samm']
      (++sequence).assert_Is 1
      $httpBackend.flush()                                      # flush request and trigger callback to first load_Data call
      (++sequence).assert_Is 5                                  # confirms that all 'then' have been called
DinisCruz commented 7 years ago

fixed in latest version