Pass parameter "transaction" to the Parse Server

[mfkenson]

https://github.com/parse-community/Parse-SDK-JS/issues/922

Did I miss anything?

[davimacedo]

Nice! Thanks for contributing. Could you please add a test case?

[dplewis]

@davimacedo Can you take this over and add the test cases? You know more about how transactions in parse work better than anyone.

[davimacedo]

Sure. I will work on this.

[dplewis]

@davimacedo Any updates?

[codecov[bot]]

Codecov Report

Merging #1090 into master will decrease coverage by 0.07%. The diff coverage is 42.85%.

@@            Coverage Diff            @@
##           master   #1090      +/-   ##
=========================================
- Coverage   92.28%   92.2%   -0.08%     
=========================================
  Files          54      54              
  Lines        5235    5223      -12     
  Branches     1172    1172              
=========================================
- Hits         4831    4816      -15     
- Misses        404     407       +3

Impacted Files	Coverage Δ
src/ParseObject.js	89.2% <25%> (-0.4%)	:arrow_down:
src/RESTController.js	83.66% <66.66%> (-0.34%)	:arrow_down:
src/ParseQuery.js	95.97% <0%> (-0.24%)	:arrow_down:
src/CryptoController.js	100% <0%> (+21.42%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9a33456...61594b4. Read the comment docs.

[davimacedo]

Sorry for the delay. I am working on this now.

[davimacedo]

@dplewis @mfkenson I started worked on the tests here but there is a problem with this implementation. In the case you have more objects to be saved/destroyed than the batch size, the SDK will split it in more than one batch and it will be generated two transactions in the server side. I think we can solve it using two different approaches:

1. Change the sdk to never split into batches when the transaction option is set to true
2. Throw an error message when the number of objects to be saved/destroyed is bigger than the batch size and the transaction option is set to true.

Thoughts?

[dplewis]

@davimacedo I think that would be a great approach. I tried that here and ran into an issue. The issue is that we wouldn't be be able to find cycles in SaveAll if we never split into batches.

[davimacedo]

I've just checked this problem and it will be very hard to be solved now. The not saved dependent objects need to go in a first batch because there is no warranty they will be executed in first place in the server side if we send them all together with the objects that depend on them. And if we have more batched sent to the server separately we cannot ensure the transaction. So, to simplify the solution and have a first version of this feature in the next release I suggest we simply do two verifications:

• Verify if there is any not saved dependent object when transaction=true and throw an error in the case they exist.
• Verify if there are more objects to be saved than the batch size and throw an error in the case there are.

Do you agree @dplewis ?

[dplewis]

Thats the only solution I can come up with. We will have to document it well.

[andreasanta]

@mfkenson Can you please grant me access to this PR so I can fix linting and maybe try and perform some tests?

@davimacedo Any suggestions on how to start writing tests for this? :)

[davimacedo]

Hi guys. I am working on this (to include the validations and the tests) but I found out a bug in Parse Server when deleting objects in a transaction. I am working now on a first PR to Parse Server before fixing this one here.

[andreasanta]

@davimacedo Thanks. Please let me know whether I can be of assistance.

[vahidalizad]

Hi guys, Great work so far I really had to use this feature so I came up with the following code:

function getServerUrlPath() {
  let serverUrl = SERVER_URL;
  if (serverUrl[serverUrl.length - 1] !== '/') {
    serverUrl += '/';
  }
  const url = serverUrl.replace(/https?:\/\//, '');
  return url.substr(url.indexOf('/'));
}

const MyCustom = Parse.Object.extend('MyCustom');
let obj1 = new MyCustom({name: 'just a name1'});
let obj2 = new MyCustom({name: 'just a name2'});

let batch = [obj1, obj2];
let options = {};

Parse._request(
  'POST',
  'batch',
  {
    requests: batch.map(obj => {
      const params = obj._getSaveParams();
      params.path = getServerUrlPath() + params.path;
      return params;
    }),
    transaction: true,
  },
  options
)
  .then(res => console.log('success', res))
  .catch(e => console.log('failed', e));

But there is something I don't understand when it fails, it sends up to REQUEST_ATTEMPT_LIMIT retries request but when I save only one file with await obj1.save() it only does it once and log it once. maybe this is because the batch request error does not return Parse.Error and the result has a code above 500. Is this intended and I have to change REQUEST_ATTEMPT_LIMIT somehow?

[davimacedo]

Maybe. You can set it via CoreManager.set('REQUEST_ATTEMPT_LIMIT', retry);

[vahidalizad]

Parse.CoreManager.set('REQUEST_ATTEMPT_LIMIT', 1); changes every request retry times to 1 and it works, but there is no way to set a retry value for a single request. but thanks anyway it works. Maybe you could change the src/ParseServerRESTController.js in the definition of ParseServerRESTController like bellow to return the error like the normal batch requests

if (path === "/batch") {
    let initialPromise = Promise.resolve();
    if (data.transaction === true) {
    initialPromise = config.database.createTransactionalSession();
    }
    return initialPromise.then(() => {
    const promises = data.requests.map((request) => {
        return handleRequest(
        request.method,
        request.path,
        request.body,
        options,
        config
        ).then(
        (response) => {
            return Promise.resolve({ success: response });
        },
        (error) => {
            return Promise.resolve({
            error: { code: error.code, error: error.message },
            });
        }
        );
    });
    return Promise.all(promises)
        .catch((error) => {
        if (data.transaction === true) {
            return config.database.abortTransactionalSession().then(() => {
            throw error;
            });
        } else {
            throw error;
        }
        })
        .then((result) => {
        if (data.transaction === true) {
            let error = result.find((t) => t.error && t.code);
            if (error) {
            return config.database.abortTransactionalSession().then(() => {
                return error;
            });
            }
            return config.database.commitTransactionalSession().then(() => {
            return result;
            });
        } else {
            return result;
        }
        });
    });
}

In this way not only it would not retry it again, but it also shows the problem as well. if you are agree with this and don't have the time for it I can create a pull request for it as well.

[davimacedo]

That makes sense. Would you be willed to continue this PR?

[vahidalizad]

That makes sense. Would you be willed to continue this PR?

Sorry my bad I just checked this FIX: Transaction should only abort when all promises finished #5878 PR and you fixed this problem there but it still lacks a retry option which is always 5 times but it does not log into the server which is ok I guess I create a fork of the parse-server project and check this one again if it's ok on the master branch I'm gonna join you in this one if you haven't finished it yet.

[RahulLanjewar93]

@davimacedo Hi guys, any updates on this?

[araskinwhichbnb]

Its Sep 2023 and there is an ominous comment above that says someone deleted this feature. Does that mean we are not going to get Transactional Integrity in the JS SDK? @davimacedo

[RahulLanjewar93]

@mfkenson just wanted to know if there's any other pr/issue for reference since you closed this one.