Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New issue with .saveAll batch operation #47

Closed
pmmlo opened this issue Jan 7, 2021 · 12 comments · Fixed by #48
Closed

New issue with .saveAll batch operation #47

pmmlo opened this issue Jan 7, 2021 · 12 comments · Fixed by #48
Labels
type:bug Impaired feature or lacking behavior that is likely assumed

Comments

@pmmlo
Copy link
Contributor

pmmlo commented Jan 7, 2021

Sorry @cbaker6 but I'm having a new issue with .saveAll batch save/update operation since one of the last two commits.

I can't imagine it's because of the cloudcode name changes from #46 but maybe there is a cloud code that runs in parse-server source that I am not aware of. I scanned through #43 and I'm going to take a flyer and say it might be the skippedKeys or other changes to the batch command in API+Commands.swift. I don't see the error but I don't know enough about the return headers and body to know how it is encoded.

Also, I think Parse has always done this, but I get a success response with an array of parseerrors. So, it's a success but the saveAll failed.

Here's the code:

/// I have something like this where GameScore.scores is a [String]?
let score = GameScore(score: ["2"])
let score2 = GameScore(score: ["3"])
let scoresArray = [score, score2]

scoresArray.saveAll { results in
    switch results {
    case .success(let success):
        var index = 0
        success.forEach { success in
            switch success {
            case .success(let savedScore):
                print("Saved \"\(savedScore.className)\" with score \(savedScore.score) successfully")
                if index == 1 {
                    print("\(savedScore)")
                }
                index += 1
            case .failure(let error):
                print("Error saving: \(error)") // <---All the objects fail now after the recent commits and scores are not updated remotely.
            }
        }

    case .failure(let error):
        print("Error batch saving: \(error)")
    }
}
// This is the response from the `error` in the first try-catch
[Swift.Result<ParseApp.GameScore, ParseSwift.ParseError>.failure(ParseSwift.ParseError(code: ParseSwift.ParseError.Code.invalidKeyName, message: "Invalid field name: __type.")), Swift.Result<ParseApp.GameScore, ParseSwift.ParseError>.failure(ParseSwift.ParseError(code: ParseSwift.ParseError.Code.invalidKeyName, message: "Invalid field name: __type."))]

I thought there may be some issue with a pointer, missing column, or my key strings in query, because Invalid field name:__type. in the response, but after some inspection it seems to be a potential issue with encoding the query. Everything is in order and looking at history, it seemed to work before. I could be wrong of course. I may try rolling back and testing, but for now, it would be nice to have another pair of eyes who knows Parse-Swift well.

There are no issues updating remote objects, if I do a for-loop and do individual .save.

@cbaker6
Copy link
Contributor

cbaker6 commented Jan 7, 2021

There were definitely some changes to batch so if save works in a for loop, I probably introduced an bug some where

@pmmlo
Copy link
Contributor Author

pmmlo commented Jan 7, 2021

No worries. I'll run through it later tonight.

I just confirmed commit: 3f1c897 works without issue.

@cbaker6
Copy link
Contributor

cbaker6 commented Jan 7, 2021

Also, I think Parse has always done this, but I get a success response with an array of parseerrors. So, it's a success but the saveAll failed.

This part seems correct (ignoring the encoding error from earlier) the first success/failure will return a failure if there's a connection issue or something that didn't come from the server. The inside success/failure pertain to your objects directly.

More details about how the server handles batch responses is here: https://docs.parseplatform.org/rest/guide/#batch-operations

@pmmlo
Copy link
Contributor Author

pmmlo commented Jan 7, 2021

I agree. Makes sense - cheers.

Saw your pending LiveQuery PR. You are a rockstar.

@cbaker6
Copy link
Contributor

cbaker6 commented Jan 7, 2021

Thanks! When it's ready, will you be able to test it since you have LiveQuery experience?

@pmmlo
Copy link
Contributor Author

pmmlo commented Jan 7, 2021

My experience is from ~5 years ago, but will definitely contribute the little I can and test.

@cbaker6
Copy link
Contributor

cbaker6 commented Jan 8, 2021

I'm not able to replicate this. What version of the parse-server are you using?

Below is the code I used in Playgrounds and didn't get any errors:

struct GameScore1: ParseObject {
    //: Those are required for Object
    var objectId: String?
    var createdAt: Date?
    var updatedAt: Date?
    var ACL: ParseACL?

    //: Your own properties
    var score: [String]?

    //custom initializer
    init(score: [String]) {
        self.score = score
    }
}
let score = GameScore1(score: ["2"])
let score2 = GameScore1(score: ["3"])
let scoresArray = [score, score2]

scoresArray.saveAll { results in
    switch results {
    case .success(let success):
        var index = 0
        success.forEach { success in
            switch success {
            case .success(let savedScore):
                print("Saved \"\(savedScore.className)\" with score \(savedScore.score) successfully")
                if index == 1 {
                    print("\(savedScore)")
                }
                index += 1
            case .failure(let error):
                print("Error saving: \(error)") // <---All the objects fail now after the recent commits and scores are not updated remotely.
            }
        }

    case .failure(let error):
        print("Error batch saving: \(error)")
    }
}

@cbaker6
Copy link
Contributor

cbaker6 commented Jan 8, 2021

Also, you mentioned a query, but I only see fetchAll in your example.

@pmmlo
Copy link
Contributor Author

pmmlo commented Jan 8, 2021

I'm running on a staging parse-server 4.5.0 with TLS. You're right - the example is wrong. I was trying to write it on the fly without referencing how I wrote it in staging.

I think this is closer and runnable:

struct GameScore: ParseObject {
    //: Those are required for Object
    var objectId: String?
    var createdAt: Date?
    var updatedAt: Date?
    var ACL: ParseACL?

    //: Your own properties
    var score: Int?
    var scoresStrArray: [String]?
    
    init(score: Int?, scoresStrArray: [String]?) {
        self.score = score
        self.scoresStrArray = scoresStrArray
    }

    init(objectId: String?) {
        self.objectId = objectId
    }
}

//: Define initial GameScores
let score = GameScore(score: 10, scoresStrArray: ["10"])
let score2 = GameScore(score: 3, scoresStrArray: ["3"])

var scoreId = ""
var score2Id = ""
/*: Save asynchronously (preferred way) - Performs work on background
    queue and returns to designated on designated callbackQueue.
    If no callbackQueue is specified it returns to main queue.
*/
score.save { result in
    switch result {
    case .success(let savedScore):
        assert(savedScore.objectId != nil)
        assert(savedScore.createdAt != nil)
        assert(savedScore.updatedAt != nil)
        assert(savedScore.ACL == nil)
        scoreId = savedScore.objectId!
    case .failure(let error):
        assertionFailure("Error saving: \(error)")
    }
}

score2.save { result in
    switch result {
    case .success(let savedScore):
        assert(savedScore.objectId != nil)
        assert(savedScore.createdAt != nil)
        assert(savedScore.updatedAt != nil)
        assert(savedScore.ACL == nil)
        score2Id = savedScore.objectId!
    case .failure(let error):
        assertionFailure("Error saving: \(error)")
    }
}

var updatedScore = GameScore(objectId: scoreId)
var updatedScore2 = GameScore(objectId: score2Id)
updatedScore.scoresStrArray = ["10", "2"]
updatedScore2.scoresStrArray = ["6", "3"]
let scoresArray = [updatedScore, updatedScore2]

// I think this should PUT the new score values into the array.  Did I get this wrong?
scoresArray.saveAll { results in
    switch results {
    case .success(let success):
        var index = 0
        success.forEach { success in
            switch success {
            case .success(let savedScore):
                print("Saved \"\(savedScore.className)\" with score \(savedScore.scoresStrArray) successfully")
                if index == 1 {
                    print("\(savedScore)")
                }
                index += 1
            case .failure(let error):
                print("Error saving: \(error)") // <---All the objects fail now after the recent commits and scores are not updated remotely.
            }
        }

    case .failure(let error):
        print("Error batch saving: \(error)")
    }
}

The error I get running this in Playground:

ParseError(code: ParseSwift.ParseError.Code.unknownError, message: "couldn\'t encode body BatchCommand<GameScore, GameScore>(requests: [ParseSwift.API.Command<__lldb_expr_67.GameScore, __lldb_expr_67.GameScore>(method: ParseSwift.API.Method.PUT, path: ParseSwift.API.Endpoint.any(\"/parse/classes/GameScore/\"), body: Optional(GameScore ({\"scoresStrArray\":[\"10\",\"2\"],\"objectId\":\"\"})), mapper: (Function), params: nil, uploadData: nil, uploadFile: nil, parseURL: nil, otherURL: nil, stream: nil), ParseSwift.API.Command<__lldb_expr_67.GameScore, __lldb_expr_67.GameScore>(method: ParseSwift.API.Method.PUT, path: ParseSwift.API.Endpoint.any(\"/parse/classes/GameScore/\"), body: Optional(GameScore ({\"scoresStrArray\":[\"6\",\"3\"],\"objectId\":\"\"})), mapper: (Function), params: nil, uploadData: nil, uploadFile: nil, parseURL: nil, otherURL: nil, stream: nil)])")

Makes me really think there may be something going on in the encoder. I'll try to dig in when I have some time.

@pmmlo
Copy link
Contributor Author

pmmlo commented Jan 8, 2021

Also, you mentioned a query, but I only see fetchAll in your example.

I may have gotten this wrong. Is this a fetchAll? Sorry - not query but batch operation.

This worked in the past commit I previously mentioned, but could easily have gotten this wrong.

Playing around with the code, my individual HTTP methods seem to be POST, as well as the POST for the batch operation. Is this correct? I would have expected PUT for the individual methods with a POST for the overall batch operation.

How do I properly batch update? Cheers!

@cbaker6
Copy link
Contributor

cbaker6 commented Jan 8, 2021

I may have gotten this wrong. Is this a fetchAll? Sorry - not query but batch operation.

I meant to say all I see is saveAll, sorry about that.

This worked in the past commit I previously mentioned, but could easily have gotten this wrong.

The objects were most likely going through the wrong encoder in the older commits.

Playing around with the code, my individual HTTP methods seem to be POST, as well as the POST for the batch operation. Is this correct? I would have expected PUT for the individual methods with a POST for the overall batch operation.

It matters which ParseObject command you are looking at. When the save of an unsaved object is sent, it's POST, when the save of an already save object is sent (update), it's a PUT. During the batch, it was sending PUT correctly when you were getting errors, it was just trying to PUT pointers instead of objects.

@pmmlo
Copy link
Contributor Author

pmmlo commented Jan 8, 2021

It matters which ParseObject command you are looking at. When the save of an unsaved object is sent, it's POST, when the save of an already save object is sent (update), it's a PUT. During the batch, it was sending PUT correctly when you were getting errors, it was just trying to PUT pointers instead of objects.

Makes sense. Thanks @cbaker6

@cbaker6 cbaker6 added the type:bug Impaired feature or lacking behavior that is likely assumed label Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Impaired feature or lacking behavior that is likely assumed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants