Added prior state to app.update event

[shashidharatd]

Solution to the story :- 'As a CF api consumer I expect that /v2/events returns prior state as a part of its metadata for all app.update events'
[#74624460]

After this commit, /v2/events will contain prior state as part of metadata for all app.update event.
For eg...

{
         "metadata": {
            "guid": "efcf3dab-4314-406c-b796-98973cff1abf",
            "url": "/v2/events/efcf3dab-4314-406c-b796-98973cff1abf",
            "created_at": "2015-01-28T12:13:17+00:00",
            "updated_at": null
         },
         "entity": {
            "type": "audit.app.update",
            "actor": "c85f70bb-0e7f-44f1-ae2b-63429309c89c",
            "actor_type": "user",
            "actor_name": "admin",
            "actee": "fbeb946e-c2fd-49c3-abf7-99918c1572a2",
            "actee_type": "app",
            "actee_name": "Dev6",
            "timestamp": "2015-01-28T12:13:17+00:00",
            "metadata": {
               "request": {
                  "state": "STARTED"
               },
               "previous_state": {
                  "state": "STOPPED"
               }
            },
            "space_guid": "57921333-7f00-4151-8dc3-fc654e8e332f",
            "organization_guid": "e28b54ce-acff-4dac-afe7-ad59fd446f53"
         }
      }

[cfdreddbot]

Hey shashidharatd!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you've already signed the CLA.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/87921292.

[jfmyers9]

Hi @shashidharatd,

We noticed that you used the define_singleton_method in the before_update of the apps_controller. While this does work we find that it is somewhat confusing and error prone.

We have enabled a Sequel plugin that keeps track of previous changes on the model from the previous update. Could you possibly use app.previous_changes in the app_event_repository in order to keep track of whether or not the applications state has changed? This would remove the need for a before_update in the apps_controller. Also for this test case here, could you add state change and update the corresponding expectation?

Thanks,

@jfmyers9 && @sergueif, CF Runtime Team

[shashidharatd]

Hi @jfmyers9 && @sergueif

We already tested the idea of using dirty plugin, but there is a problem in one scenario.
When an app is pushed for the first time there is a check to see if app is already staged.
If not staged, the app table will be updated with staging task id. So when app.previous_changes is called after this, the app state will not be reflected.

Instead of define_singleton_method we can also use instance_variable_set for dynamically adding instance variable to app object and store the app state.
We had tested with this and seems to be working. If it is Ok, we will do the change and commit.

Thanks
@AmitRoushan && @shashidharatd

[zrob]

Hi @shashidharatd & @AmitRoushan

We are uncomfortable with an approach that uses meta-programming to dynamically update the app object. We would prefer to see an approach that is able to gather the information about the changeset and pass it into the event repository.

Additionally, we are now using this repository in multiple places, so the way we want it to work may be in a bit of flux at this time: https://github.com/cloudfoundry/cloud_controller_ng/blob/master/app/handlers/processes_handler.rb#L151

Thanks,
@zrob & @zaksoup

[shashidharatd]

Hi @zrob & @zaksoup

As per comment, now we are not using meta-programming. We have added a new method prior_state in App class which will decide and return prior state of app by analyzing current state of app and changes returned by dirty plugin. Also taken care of updating prior state for app event in V3.

Please review and let us know if any things missed or any concerns

Thanks
@AmitRoushan && @shashidharatd

[crhino]

Hi @shashidharatd and @AmitRoushan,

We think the changes look good. We would like to see some unit tests around the prior_state method in the app spec, since it is a public method on that model. This would also help us understand the reason behind setting prior_state to STOPPED when the staging_task_id has changed.

Thanks,

@crhino && @jfmyers9, CF Runtime Team

[shashidharatd]

Hi,
We have added few test cases for prior_state function.

Test cases are covering following scenarios:

1. when 'STOPPED' app is 'STARTED'.
2. when app is 'STARTED' which is not staged and not started.
3. when staging is pending and app is re-staged.
4. prior state for different state changes.

Also we have added the comments in test cases for scenarios.

Please let us know if this is sufficient.

Thanks,
@AmitRoushan && @shashidharatd

[SocalNick]

Closing due to inactivity. This is a good idea, however, there are so many other changes that happen to an app besides START and STOP.