Schema Upgrade Issue (Postgres)

[montymxb]

Issue Description

When upgrading parse-server it's possible to have a situation where, if the default Schemas have been changed in parse-server, and the database is postgres, the underlying database is not updated.

Steps to reproduce

Reproducing with a change in expiration_interval for _PushStatus.

1. Start parse-server 2.6.4 using a fresh db in postgres 9.5. A garbage push configuration will do just fine for testing.

push: {
    android: {
      senderId: "blank-sender-id",
      apiKey: "not-a-real-api-key"
    }
}

2. Attempt to send a push using your preferred method. Can use cloud code to do this as well.

Parse.Push.send({
  where: {
    deviceType: 'ios'
  },
  data: {
    badge: 'Increment',
  	alert: 'Hello world!'
  }
});

3. Push should complete, logs should indicate nothing afoot.
4. Stop parse-server and upgrade to 2.6.5.
5. Restart parse-server, connecting to the still running postgres instance.
6. Attempt to send a push again as stated in step 2.
7. Observe the following in the log entries (response to push may hang as well)

..."message":"_PushStatus undefined: error while sending push","stack":"error: column "expiration_interval" of relation "_PushStatus" does not exist...

Expected Results

Following observed behavior when using mongodb the underlying schema should be updated when new default fields are available but not present.

Actual Outcome

The underlying schema is not updated when new default fields are available for parse-server's reserved classes.

Environment Setup

• Server

  • parse-server version: 2.6.4 & 2.6.5
  • Operating System: OSX
  • localhost

• Database

  • MongoDB version: 3.4.4
  • Postgres version: 9.5
  • localhost

Logs/Trace

Annotated verbose logs entries below. Tests were performed from the php sdk, version 1.3.0.

2.6.4/ initial push request
{"method":"POST","url":"/parse/push","headers":{"host":"localhost:1337","accept":"/","x-parse-application-id":"app-id-here","x-parse-client-version":"php1.3.0","x-parse-master-key":"master-key-here","content-type":"application/json","content-length":"51"},"body":{"channels":[""],"data":{"alert":"sample message"}},"level":"verbose","message":"REQUEST for [POST] /parse/push: {\n "channels": [\n ""\n ],\n "data": {\n "alert": "sample message"\n }\n}","timestamp":"2017-11-11T04:59:08.955Z"}

2.6.4/ response to push request
{"result":{"headers":{"X-Parse-Push-Status-Id":"W7w4WdXQix"},"response":{"result":true}},"level":"verbose","message":"RESPONSE from [POST] /parse/push: {\n "headers": {\n "X-Parse-Push-Status-Id": "W7w4WdXQix"\n },\n "response": {\n "result": true\n }\n}","timestamp":"2017-11-11T04:59:08.973Z"}

upgraded to 2.6.5/ initial push request
{"method":"POST","url":"/parse/push","headers":{"host":"localhost:1337","accept":"/","x-parse-application-id":"app-id-here","x-parse-client-version":"php1.3.0","x-parse-master-key":"master-key-here","content-type":"application/json","content-length":"51"},"body":{"channels":[""],"data":{"alert":"sample message"}},"level":"verbose","message":"REQUEST for [POST] /parse/push: {\n "channels": [\n ""\n ],\n "data": {\n "alert": "sample message"\n }\n}","timestamp":"2017-11-11T05:13:23.205Z"}

upgraded to 2.6.5/ response to push request
{"message":"_PushStatus undefined: error while sending push","stack":"error: column "expiration_interval" of relation "_PushStatus" does not exist\n at Connection.parseE (/Users/Bfriedman/Documents/parse-server-test/node_modules/pg/lib/connection.js:546:11)\n at Connection.parseMessage (/Users/Bfriedman/Documents/parse-server-test/node_modules/pg/lib/connection.js:371:19)\n at Socket. (/Users/Bfriedman/Documents/parse-server-test/node_modules/pg/lib/connection.js:114:22)\n at emitOne (events.js:96:13)\n at Socket.emit (events.js:188:7)\n at readableAddChunk (_stream_readable.js:176:18)\n at Socket.Readable.push (_stream_readable.js:134:10)\n at TCP.onread (net.js:547:20)","name":"error","length":135,"severity":"ERROR","code":"42703","position":"83","file":"parse_target.c","line":"954","routine":"checkInsertTargets","level":"error","timestamp":"2017-11-11T05:13:23.225Z"}

[paulovitin]

@montymxb I checked the PostgresStorageAdapter and I dont saw any routine to check this updates. Probably needs to create something to make a differ and send to addFieldIfNotExists to create the fields. Im correct?

[montymxb]

@paulovitin Yeah, that's roughly what we would need. In the mongodb storage adapter I don't believe there's such a function either, as it seems to auto-create on it's own internally. We just need to make that behavior explicit when working within Postgres and we should be good.

[paulovitin]

@montymxb I believe I can solve this until weekend, can I? If not I tell you. Ok?

[paulovitin]

@montymxb just one question. This process just will be apply in the volatile classes, correct?

[montymxb]

That sounds good to me! As for how this will apply let's have it work on the built in classes for parse. Just to restate the issue originally was that the underlying schema was changed and code was updated to reflect that, but the underlying DB did not change and broke that expectation. So we would want a mechanism for that on startup, when the db is setup/checked.

There actually is a check in place for determining if all the expected classes are present and sets them up if they're not there. However if the schema has changed for any default classes (volatiles included) this check doesn't quite catch that. I think a good solution would be a more thorough check in that region during setup. I'll flip back through the src and find that spot again so you can see what I'm talking about.

[paulovitin]

@montymxb take a look at this

https://github.com/paulovitin/parse-server/blob/fix/4338-pg-schema-upgrade/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js#L691-L720

and I execute in performIntialization method here:

https://github.com/paulovitin/parse-server/blob/fix/4338-pg-schema-upgrade/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js#L1560

I tested and works with the VolatileClassesSchemas.

Now needs write some tests to send a PR.

[montymxb]

Yep that looks like a good start. I would not go so far as to delete schema columns though, as that would create the unintended side-effect of clearing any additional user defined fields on any classes. Even if they're not intended to be modified we wouldn't want to just delete a whole field of values on upgrade.

A good test case could be written around directly calling schemaUpgrade with varying params, and verifying that the intended default field(s) are added.

[montymxb]

Once this gets in a PR I'll still want to verify this actually does resolve the specific upgrade issue that brought this up in the first place.

[paulovitin]

@montymxb Sure. Let me know when you've tested.

[dplewis]

Closing via #4375