parse-community / parse-server

Parse Server for Node.js / Express
https://parseplatform.org
Apache License 2.0
20.92k stars 4.78k forks source link

afterLiveQueryEvent hook returns empty date object #7082

Closed NuclleaR closed 3 years ago

NuclleaR commented 3 years ago

New Issue Checklist

Issue Description

Date object is empty in afterLiveQueryEvent hook.

Steps to reproduce

Created hook

Parse.Cloud.afterLiveQueryEvent('Spendings', async function(request) {
  const category = await getCategory(request.object.get('category'));
  const store = await getStore(request.object.get('store'));

  console.log('===> request.object', JSON.stringify(request.object));

  request.object.set('category', category);
  request.object.set('store', store);
});

Actual Outcome

log that I received. date is empty ===> request.object {"date":{},"amount":444,"createdAt":"2020-12-18T13:21:49.770Z","category":{"type":"Pointer","className":"Categories","objectId":"SREwh7FEwF"},"parent":{"type":"Pointer","className":"ParentCategory","objectId":"yomXRG8Hcj"},"updatedAt":"2020-12-18T13:21:49.770Z","objectId":"3MdGVC5NuY"}

Expected Outcome

expected date object

Failing Test Case / Pull Request

Environment

Server

Database

Client

Logs

info: beforeSave triggered for Spendings for user undefined:
  Input: {"amount":444,"date":{"__type":"Date","iso":"2020-12-18T13:21:40.088Z"},"category":{"__type":"Pointer","className":"Categories","objectId":"SREwh7FEwF"},"parent":{"__type":"Pointer","className":"ParentCategory","objectId":"yomXRG8Hcj"}}
  Result: {"object":{"date":{"__type":"Date","iso":"2020-12-18T13:21:40.088Z"},"amount":444,"category":{"__type":"Pointer","className":"Categories","objectId":"SREwh7FEwF"},"parent":{"__type":"Pointer","className":"ParentCategory","objectId":"yomXRG8Hcj"}}} {"className":"Spendings","triggerType":"beforeSave"}
info: afterSave triggered for Spendings for user undefined:
  Input: {"date":{"__type":"Date","iso":"2020-12-18T13:21:40.088Z"},"amount":444,"createdAt":"2020-12-18T13:21:49.770Z","category":{"__type":"Pointer","className":"Categories","objectId":"SREwh7FEwF"},"parent":{"__type":"Pointer","className":"ParentCategory","objectId":"yomXRG8Hcj"},"updatedAt":"2020-12-18T13:21:49.770Z","objectId":"3MdGVC5NuY"} {"className":"Spendings","triggerType":"afterSave"}
===> request.object {"date":{},"amount":444,"createdAt":"2020-12-18T13:21:49.770Z","category":{"__type":"Pointer","className":"Categories","objectId":"SREwh7FEwF"},"parent":{"__type":"Pointer","className":"ParentCategory","objectId":"yomXRG8Hcj"},"updatedAt":"2020-12-18T13:21:49.770Z","objectId":"3MdGVC5NuY"}
NuclleaR commented 3 years ago

Flutter SDK throws Unhandled Exception: type '_InternalLinkedHashMap<String, dynamic>' is not a subtype of type 'DateTime'

Issue just with live query. with REST request all works as expected

mtrezza commented 3 years ago

Thanks for reporting.

Issue just with live query. with REST request all works as expected

Do you mean that the issue has only been observed with the Flutter SDK? Then this may be an SDK issue rather than a Parse Server issue. It would help if you could try to write a failing test case here, then we would know whether this issue also exists for the Parse JS SDK and/or for Parse Server in general.

NuclleaR commented 3 years ago

@mtrezza Hello! Logs that I provided I grabbed from Node Js server logs. So, I believe this is could be server issue, not Flutter SDK.

mtrezza commented 3 years ago

Thanks for clarifying, and what exactly did you mean by:

with REST request all works as expected

Can you post the REST request?

dblythy commented 3 years ago

I will write a test case in an hour or so to see what's going on. The afterLiveQueryEvent trigger should be able to handle dates so it seems strange to me.

I would recommend using Promise.all in your code tho so getCategory and getStore run in parallel (i.e, so they run at the same time, rather than one by one)

Parse.Cloud.afterLiveQueryEvent('Spendings', async function(request) {
  const [category, store] = await Promise.all([getCategory(request.object.get('category')),getStore(request.object.get('store')]);
  request.object.set('category', category);
  request.object.set('store', store);
});
dblythy commented 3 years ago
it('afterEvent should work with date', async done => {
    await reconfigureServer({
      liveQuery: {
        classNames: ['TestObject'],
      },
      startLiveQueryServer: true
    });
    Parse.Cloud.afterLiveQueryEvent('TestObject', request => {
      const date = request.object.get('date');
      expect(date).toBeDefined();
      expect(date).toBeInstanceOf(Date);
      console.log('===> request.object', JSON.stringify(request.object));
      done();
    });

    const query = new Parse.Query(TestObject);
    await query.subscribe(); 

    const object = new TestObject();
    object.set('date', new Date());
    await object.save();
});

This test passes for me

NuclleaR commented 3 years ago

@dblythy Hello! Seems like I found case when issue reproduces in 100% cases. In your test everything works coz you change date and save, but if you update object with any another field and don't touch date, date will come empty object. at least it reproduces in my project in 100% cases

dblythy commented 3 years ago

So is this your use case?

it('afterEvent should work with date', async done => {
    await reconfigureServer({
      liveQuery: {
        classNames: ['TestObject'],
      },
      startLiveQueryServer: true
    });
    let calls = 0;
    Parse.Cloud.afterLiveQueryEvent('TestObject', request => {
      const date = request.object.get('date');
      expect(date).toBeDefined();
      expect(date).toBeInstanceOf(Date);
      console.log('===> request.object', JSON.stringify(request.object));
      calls++;
      if (calls == 2) {
        done();
      }
    });

    const query = new Parse.Query(TestObject);
    await query.subscribe(); 

    const object = new TestObject();
    object.set('date', new Date());
    await object.save();
    object._clearServerData();

    const obj = await query.first();
    obj.set('foo','bar');
    await obj.save();
});

If you could write a failing test it would really help us get to the bottom of the bug.

NuclleaR commented 3 years ago

I found real issue. I debug a lilbit this method and figure out

export async function maybeRunAfterEventTrigger(triggerType, className, request) {
  const trigger = getTrigger(className, triggerType, Parse.applicationId);
  if (!trigger) {
    return;
  }
  if (request.object) {
    // Here is the issue where date become '{}'
    request.object = Parse.Object.fromJSON(request.object);
  }
  if (request.original) {
    request.original = Parse.Object.fromJSON(request.original);
  }
  request.user = await userForSessionToken(request.sessionToken);
  await maybeRunValidator(request, `${triggerType}.${className}`);
  if (request.skipWithMasterKey) {
    return;
  }
  return trigger(request);
}

I wrote test, and test fails

const Parse = require('parse/node');

describe('From JSON', () => {
  it('should return object from JSON', () => {
    const TestObject = {
      className: 'TestObject',
      __type: 'Object',
      Hello: 'World',
      date: new Date(),
      someValue: 123,
    };

    const parsed = Parse.Object.fromJSON(TestObject);

    expect(parsed.get('someValue')).toBeInstanceOf(Number); // => true
    expect(parsed.get('date')).toBeInstanceOf(Date); // => false
  });
});
NuclleaR commented 3 years ago

@dblythy could you take a look, please, and confirm?

here is my real log from real app date comes like JS Date object not a { __type: 'Date' }

date: 2021-01-15T17:51:39.403Z, amount: 551, category: { type: 'Pointer', className: 'Categories', objectId: 'WSRUJzVwOE' }, parent: { type: 'Pointer', className: 'ParentCategory', objectId: 'mTDh4FXkno' }, createdAt: '2021-01-15T15:51:45.343Z', updatedAt: '2021-01-15T16:09:43.181Z', __type: 'Object', className: 'Spendings', objectId: 'VC8PHzCW0i' }

mtrezza commented 3 years ago

This has been classified as a bug with severity 2:

mtrezza commented 3 years ago

I was able to reproduce this issue and create a failing test case.

There seems to be two issues here:

1)

let object = new TestObject();
object.set('date', new Date());
const json = object.toJSON();
object = Parse.Object.fromJSON(json);

Fails because toJSON() does not encode the className and __type. That is maybe not directly related to this issue, but I stumbled upon it anyway.

2)

it('afterEvent should work with date', async done => {
  await reconfigureServer({
    liveQuery: {
      classNames: ['TestObject'],
    },
    startLiveQueryServer: true,
  });
  Parse.Cloud.afterLiveQueryEvent('TestObject', request => {
    const date = request.object.get('date');
    expect(date).toBeDefined();
    expect(date).toBeInstanceOf(Date);
    done();
  });

  const query = new Parse.Query(TestObject);
  await query.subscribe();

  let object = new TestObject();
  object.set('date', new Date());
  const json = object.toJSON();
  json.className = 'TestObject';
  json.__type = 'Object';
  object = Parse.Object.fromJSON(json);
  await object.save();
});

Fails, date is undefined in the trigger. I was not able to get a date passed to the afterLiveQueryEvent, no matter how date is encoded in the object.

dblythy commented 3 years ago

Sorry @mtrezza! I was looking at this too as it made me sad to see that my feature afterLiveQuery event wasn't working the way we expected.

I think it's an issue with the JS SDK as this fails:

it('can be inflated from server JSON', () => {
    const date = new Date();
    const json = {
      className: 'Item',
      createdAt: '2013-12-14T04:51:19Z',
      objectId: 'I1',
      size: 'medium',
      date: date
    };
    const o = ParseObject.fromJSON(json);
    expect(o.className).toBe('Item');
    expect(o.id).toBe('I1');
    expect(o.attributes).toEqual({
      size: 'medium',
      createdAt: new Date(Date.UTC(2013, 11, 14, 4, 51, 19)),
      updatedAt: new Date(Date.UTC(2013, 11, 14, 4, 51, 19)),
      date
    });
    expect(o.dirty()).toBe(false);
    expect(o.get('date')).toBeInstanceOf(Date);
  });

I guess either way works, but I think Parsing a Date field in JSON should set the Parse.Object field to a date too.

mtrezza commented 3 years ago

Do you want to look into this? Now that we have a failing test and identified 2 issues, it seems the fix is not far.

dblythy commented 3 years ago

I'm expecting addressing the SDK not being able to convert JS dates in JSON objects to a Parse.Object should fix this issue.

I guess for the most part dates are decoded from the DB as objects with __type as date, however I think it could still cause some problems for some JS developers who are using the fromJSON method with JS dates.

I've opened a PR in the SDK 😊

dblythy commented 3 years ago

@NuclleaR would you mind checking if this is fixed on master?

dblythy commented 3 years ago

I think this can be closed @mtrezza, as this passes on the master version:

it('afterEvent should work with date', async done => {
    await reconfigureServer({
      liveQuery: {
        classNames: ['TestObject'],
      },
      startLiveQueryServer: true,
    });
    Parse.Cloud.afterLiveQueryEvent('TestObject', request => {
      const date = request.object.get('date');
      expect(date).toBeDefined();
      expect(date).toBeInstanceOf(Date);
      done();
    });

    const query = new Parse.Query(TestObject);
    await query.subscribe();

    let object = new TestObject();
    object.set('date', new Date());
    const json = object.toJSON();
    json.className = 'TestObject';
    json.__type = 'Object';
    object = Parse.Object.fromJSON(json, true, true);
    await object.save();
  });
mtrezza commented 3 years ago

I see that https://github.com/parse-community/Parse-SDK-JS/pull/1293 has been merged to fix the issue, but I don't see a PR in parse server to add a test. Do you think it's enough if the JS SDK has a test, or do you think parse server also needs one?

dblythy commented 3 years ago

Nothing wrong with being extra sure and adding the test in, right?

mtrezza commented 3 years ago

Only it it brings a benefit. I think the fixed bug in the SDK was not specific to afterLiveQueryEvent but a general date conversion issue, right?

dblythy commented 3 years ago

Yeah, which was the underlying cause of the bug relating to the afterLiveQueryEvent (.fromJSON wasn't encoding the date properly)

mtrezza commented 3 years ago

I guess if this issue cannot occur with other SDKs again and it was a JS SDK specific bug, then a server side test is unnecessary. What do you think?

dblythy commented 3 years ago

I agree, @NuclleaR please let us know if this issue is still occurring on new versions of Parse Server.