Custom init() is not recognized anymore

vdkdamian commented 3 years ago

New Issue Checklist

[x] I am not disclosing a vulnerability.
[x] I am not just asking a question.
[x] I have searched through existing issues.
[x] I can reproduce the issue with the latest versions of Parse Server and the Parse Swift SDK.

Issue Description

There is a problem when using custom init() code on a ParseObject class. I get the error Type 'User' does not conform to protocol 'ParseObject' on every ParseObject struct I use. My code used to work, and all of a sudden it is broken.

Steps to reproduce

Add a init() function to your Parse struct, and add variables to it.

Actual Outcome

Unexpected error

Expected Outcome

Nothing

Environment

Client

Parse Swift SDK version: 1.10.0
Xcode version: 13.0
Operating system (iOS, macOS, watchOS, etc.): SPM
Operating system version: 12.0

Server

Parse Server version: 4.4.0
Operating system: Back4App
Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): AWS

Database

System (MongoDB or Postgres): MongoDB
Database version: 3.6
Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): ?

Logs

parse-github-assistant[bot] commented 3 years ago

Thanks for opening this issue!

🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

cbaker6 commented 3 years ago

This is due to you updating to 1.10.0, please see the "Breaking Change" notes: https://github.com/parse-community/Parse-Swift/releases/tag/1.10.0 which point to https://github.com/parse-community/Parse-Swift/pull/243#issue-1005536964. You should also take note of the "placing inits in extensions" comments.

From the code you posted you need to either userId:String and optional, userId:String? or initialize it in your new init():

public init() {
  userId = UUID().uuidString // Default value
}

Basically, the rules of any initializer apply, you need to initialize all of your properties to initialize an instance or make it optional.

vdkdamian commented 3 years ago

This is due to you updating to 1.10.0, please see the "Breaking Change" notes: https://github.com/parse-community/Parse-Swift/releases/tag/1.10.0 which point to #243 (comment). You should also take note of the "placing inits in extensions" comments.

From the code you posted you need to either userId:String and optional, userId:String? or initialize it in your new init():
public init() {
  userId = UUID().uuidString // Default value
}
Basically, the rules of any initializer apply, you need to initialize all of your properties to initialize an instance or make it optional.

Yes, but I initialize my variables when I call my custom init() method. I also don't want to make them optional since they should never be optional.

Optional variables need to be unwrapped and that's not necessary for me.

cbaker6 commented 3 years ago

Yes, but I initialize my variables when I call my custom init() method. I also don't want to make them optional since they should never be optional.

Optional variables need to be unwrapped and that's not necessary for me.

I recommend reading the PR I linked and the breaking change comment more thoroughly along with linked https://github.com/parse-community/Parse-Swift/issues/242, it's now a requirement to do as I mentioned because of the change. There's no way around it if you want to upgrade to >= 1.10.0.

You can choose to never use the init(), but the SDK requires it for emptyObject.

vdkdamian commented 3 years ago

So there is no other way then making my variables optional, or setting a default value?

I don't like this, since I can't set a default value. That would create a volnerability in my opinion.

And making every variable optional makes things complicated for nothing. More unwrapping needed for variables I use a lot (and that actually can't be optional. The server has JavaScript code that checks for that.)

You have any other idea i can use to accomplish what I want? Or do I have no option other then creating them as optionals?

Sorry for me being critical and annoying but it's really something that seems a little of to me 🙂

Thanks in advance

cbaker6 commented 3 years ago

I don't like this, since I can't set a default value. That would create a volnerability in my opinion.

I'm not happy with this either.

You have any other idea i can use to accomplish what I want? Or do I have no option other then creating them as optionals?

Unfortunately, I don't know of another way. In the past, I left the init process up to the developer as mentioned here which is why you didn't have to adhere to this requirement. If you have issues with this, I recommend responding to @dblythy as he's the one that opened the issue pushed for the change. If you are worried about those changes, you should be even more concerned about the changes dblythy and others are proposing, though the feasibility of them developing such solutions seems highly unlikely IMO from what they've demonstrated so far. I recommend commenting on the respective issues with your concerns/comments if any.

cbaker6 commented 3 years ago

@novemTeam I want to highlight that with the current setup of your User, you can never have a ParsePointer because of your client-side requirement of userId, this is not typical in Parse client side apps as developers usually will want a pointer to User at some point.

vdkdamian commented 3 years ago

@novemTeam I want to highlight that with the current setup of your User, you can never have a ParsePointer because of your client-side requirement of userId, this is not typical in Parse client side apps as developers usually will want a pointer to User at some point.

Good point, I'll probably change it.

I now changed all my structs to optional variables, and decided to force unwrap them when needed. Not really ideal, but yeah. It'll do the job

cbaker6 commented 3 years ago

I now changed all my structs to optional variables, and decided to force unwrap them when needed. Not really ideal, but yeah. It'll do the job

You probably don't need to force unwrap everything. Most types you can use and compare against as optionals directly with the exception of collections, you will want to make sure those are valid before adding or modifying.

dblythy commented 3 years ago

As I posted in the other thread, I wasn't aware the emptyObject solution would be a breaking change, one that would invoke this issue here.

@cbaker6 You're welcome to revert the emptyObject changes and the function can be added manually to the struct's that need it - I wouldn't suggest to sacrifice one existing feature in the way of others, without properly weighing up the pros/cons.

If you are worried about those changes, you should be even more concerned about the changes dblythy and others are proposing, though the feasibility of them developing such solutions seems highly unlikely IMO from what they've demonstrated so far

I would remind you that all of us donate our time to Parse / Parse Server for the overall benefit of the community and the project. @cbaker6, as our valued ParseSwift expert, you've added your opinion as to why you don't endorse the improvement, and chosen not to offer any suggestions for OOTB solutions, which is your prerogative. It goes without saying that we appreciate your commitment to this SDK, and I would hope that you could eventually help us work towards improving the cost efficiency of this great SDK that you’ve built, and weed out any breaking changes.

@novemTeam if you would like to join the discussion for how we improve the core efficiency of the project, as well as being backwards compatible, we would value your input on the other thread. Wholesale changes have always been a headache for our community and we would want to be sure that any changes are providing an overall benefit, and/or, introduce as little breaking changes as possible. We welcome all discussion and contributions, regardless of technical ability.

cbaker6 commented 3 years ago

@novemTeam 249 should address this issue. I recommend reading the issue description there as I explain why developers should feel empowered to add their own properties and extensions to their own objects. Sending modified keys is only needed for particular use-cases, not “all”. If developers want to send modified keys, they can add simple things to accomplish it, and should feel empowered to do any customization they see fit for their respective apps.

If developers ever plan on using ParseObject as ParsePointers, they are required to have all fields as optional. This doesn’t open up any more/less problems with security. Particularly, if you are making an app as opposed to a framework based on the Swift SDK. If a developer really wants to have a required key, they should require it on the server-side or do an If check on the client-side before saving objects.

I’ve also adde the comment below to 249 as having non-optional keys causes those required keys to always be sent which is probably not what you want when only plan to send a subset of the keys.

// Or any init of your object you want. Note, ideally you want an empty init so you don’t send unnecessary keys.

parse-community / Parse-Swift