-
Notifications
You must be signed in to change notification settings - Fork 298
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
CBLModel has trouble with dynamic properties in Swift classes #648
Comments
Let's see. The last commit to MYDynamicObject was on Feb 19, but it didn't get merged into CBL until March 12 (commit 82ec99c). That commit affected property type parsing, to handle protocol types, but I'm surprised it would break with a common type like NSString — our unit tests would have caught that. Unless there's something weird about the property type encoded by Swift. |
Fixed — for some reason the property type got encoded as |
Jens, Thank you so much, again! Will pull and rebuild and try again! Cheers! Basit |
@snej - Just pulled the latest version from master and built it and still seem to be getting the issue (al beit without the MYDynamicObject warning line):
|
And, the nuance now with the released version of Xcode 6.3 is that with Swift 1.2 nullability modifiers, reverting to Feb 27 known-good does not work since those builds error out with nullability errors (that the newer builds do not suffer from, of course even in Xcode 6.3 betas these weren't an issue, only with the GM release today has that become an issue, so kind of between a rock and a hard place, happy to help with diagnosing this further as you need! |
Looks like CBLModel's +impForGetterOfProperty: (and ...setter... too probably) need to check for NSObject as the property class. It's weird that this didn't trigger in the unit tests after I added an |
Thank you, @snej, as myself and my team gain more experience with CBL we look forward to being able to help and be more self-sufficient on diagnosis and fix of these issues, much rather have us come with a PR for you than just an "ugh, this is broken" issue! |
No problem. MYDynamicObject is some really weird low-level stuff; definitely not one of the more approachable parts of the source code! Thanks a lot for working with the master branch — it really helps us find problems before we release. |
You are absolutely welcome, @snej - if that's the little we can do to contribute for now/until we can learn the intricacies of the code, happy to be able to do it (we are meticulous about keeping known-good-to-us/for-our-use-case binaries for use just in situations like this, although with the nullability issue with Swift 1.2 release, it's actually forced us to revert to the Xcode-beta for development so we can use the older known-good binary until this is fixed, hopefully the expiration date of the beta is still a ways out - that makes me nervous, though!). |
Hi! With a bit of guidance on exactly what to work on in CBLModel's +impForGetterOfProperty (and setter?) I am happy to try and work on a fix for this! |
Thanks! The code in those methods should treat |
Sure, I absolutely understand and want to help and not just complain "hey! it's broken!", so I'll give it a shot later today or tomorrow and report back with (hopefully?) a PR or my hands up :). |
Been spending a fair amount of time playing in these methods and can't seem to get things to work. Been doing things like:
else if ( [propertyClass isSubclassOfClass: [NSObject class]]) right before the last catch-all else in the getter method but property fields of CBLModel subclasses still seem to be broken...
Still playing, any pseudocode or explanation could be helpful, still working with this.... |
(On these properties, for example, I'm getting similar errors to earlier like:) WARNING: MYDynamicObject: VTCase.currentReport has type @"VTJuryReport" which is not a known class or protocol |
Should I be looking into why the error messages have quoted string with Obj-c style strings? For example, calling MYDynamicObject, if we're looking for class (klass as in the code added on the Feb 19 commit) is looking for an actual class named '!@"VTJuryReport"', it certainly won't find it b/c of the @" preceding it and " suffix on it, is this an issue? Is there some string handling nuance between Swift-ObjC that could be the issue here? 11:27:58.403| WARNING: MYDynamicObject: VTCase.currentReport has type @"VTJuryReport" which is not a known class or protocol |
Hmmmmm, looks like, at least for the previous two issues I've been grappling with, that the issue was a missing @objc(VTJuryReport) declaration in the Swift file. Doing some testing to see if my fix works.... |
Submitted PR above |
Improved fix based on my own commentary on the original PR after thinking about a better/cleaner/more maintainable way of doing/implementing this. |
Can you show me a minimal test case that reproduces the problem? Re-reading this thread, I'm not clear on what is still going wrong after my initial fix. |
Absolutely, @snej - it happens whenever trying to access a property on a CBLModel. Here's a minimal test case that demonstrates the issue, launch the app, create a record, and then try to view it, should crash with the trace in the README on the repo: |
commit feb7ff5 Merge: 5b98b28 d7047ed Author: Jens Alfke <jens@couchbase.com> Date: Fri Apr 24 15:30:53 2015 -0700 Merge pull request #673 from couchbase/feature/issue_672_stub_attachment Fix pulling documents with sub attachments failure commit d7047ed Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Fri Apr 24 13:59:08 2015 -0700 Make -processAttachmentsForRevision ignore the return value of -mutateAttachments Per code review, Make -processAttachmentsForRevision ignore the return value of -mutateAttachments. commit 5b98b28 Author: Jens Alfke <jens@couchbase.com> Date: Thu Apr 23 15:49:57 2015 -0700 Major optimization for updating docs with large numbers of revisions Use an O(1) NSDictionary instead of an O(n) CBL_RevisionList to look up locally-known revisions by revID when inserting an existing revision. In the database I'm testing with, this sped up the entire replication by a factor of 8x! (From ~55sec down to ~7sec.) commit 2b214e4 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Wed Apr 22 17:07:20 2015 -0700 Fix pulling documents with sub attachments failure A few related issues have been fixed: 1. Correct minRevPos value when stripping the (non modified) attachments. This corrects the performance optimization when pushing documents that have non-modified attachments. 2. Applying a patch from @snej that "if the attachment is a stub, and the parent revision's JSON isn't known, but the attachment exists locally (based on its digest), just leave the stub alone". 3. Fix the issue that atts_since parameter has never been sent. #672 commit 5c6eed4 Merge: fbbe006 a26dec8 Author: Jens Alfke <jens@couchbase.com> Date: Wed Apr 22 10:29:20 2015 -0700 Merge pull request #666 from couchbase/feature/multipart_retry Fix multipart streams need to be recreated when doing retry commit fbbe006 Merge: 6d6cf16 69c83b1 Author: Jens Alfke <jens@couchbase.com> Date: Wed Apr 22 10:25:32 2015 -0700 Merge pull request #647 from couchbase/feature/issue_638_cblis_many_to_many Add CBLIS support for many-to-many relationship commit 6d6cf16 Merge: 67e591b 1b6d000 Author: Pasin Suriyentrakorn <phasin@gmail.com> Date: Fri Apr 17 08:48:56 2015 -0700 Merge pull request #667 from couchbase/feature/issue_655 Updated CBL Mac Test app scheme to include mac unit tests commit 1b6d000 Author: ashvinder <ashvinder@couchbase.com> Date: Thu Apr 16 16:16:12 2015 -0700 Updated CBL Mac Test app scheme to include mac unit tests commit 67e591b Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Thu Apr 16 15:49:10 2015 -0700 Fix Mac build issue on OSX10.9 and XCode 6.0 commit 5c80e70 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Thu Apr 16 15:18:00 2015 -0700 Fix CBLIS NSRelationshipDescription.ordered property mac build failed The ordered property has been available since OSX v10.7 so we need to guared that for OSX version less than 10.7. commit a26dec8 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Thu Apr 16 13:26:43 2015 -0700 Fix multipart streams need to be recreated when doing retry Refactor the CBLMultipartUploader's init method to accept a block for creating a new mutipart writer object instead of a multipart writer object that cannot be reused when doing retry. #665 commit e495866 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Wed Apr 15 20:19:43 2015 -0700 Fix CBL Mac Test App Build Fail Removed old and missing CBLJSONValidator.m and CBLJSONValidatorTests.m from the CBL Mac Test App Compile Source. commit 25f98a7 Merge: 4cf8324 611dca4 Author: Pasin Suriyentrakorn <phasin@gmail.com> Date: Wed Apr 15 12:05:51 2015 -0700 Merge pull request #663 from couchbase/feature/issue_661_docid Change docid prefix commit 611dca4 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Wed Apr 15 11:53:51 2015 -0700 Change docid prefix Change docid prefix from '!' to '-'. Using '!' doesn't work with sync function for the case of using the docid to construct a channel name. Sync gateway doesn't allow '!' as part of the channel name. #661 commit 4cf8324 Merge: 02eb6f6 c073243 Author: Jens Alfke <jens@couchbase.com> Date: Tue Apr 14 10:14:39 2015 -0700 Merge pull request #657 from couchbase/feature/issue_649_mapped_file Remove NSDataReadingMappedIfSafe when reading attachment content commit c073243 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Mon Apr 13 16:17:16 2015 -0700 Remove NSDataReadingMappedIfSafe from CBL_BlobStore.blobForKey Remove NSDataReadingMappedIfSafe and use NSDataReadingUncached option when reading blob data for a key. #649 commit 02eb6f6 Merge: 868a9b0 79ada94 Author: Jens Alfke <jens@couchbase.com> Date: Mon Apr 13 17:17:53 2015 -0700 Merge pull request #659 from couchbase/feature/attachment_memleak Fix circular reference between CBLUnsavedRevision and CBLAttachment commit 79ada94 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Mon Apr 13 17:12:05 2015 -0700 Removed body property from CBLAttachment per code review commit ed0fc74 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Mon Apr 13 16:45:47 2015 -0700 Remove overriden attachmentNamed: from CBLUnsavedRevision - Per code review, removed the overriden attachmentNamed: from the CBLUnsavedRevision class. - The new added CBLAttachments get from the attachmentNamed: of the CBLUnsavedRevision will not have revision object set. Updated the unit test accordingly. commit 868a9b0 Merge: bbd3b72 066d328 Author: Jens Alfke <jens@couchbase.com> Date: Mon Apr 13 16:02:24 2015 -0700 Merge pull request #658 from couchbase/feature/issue_649_file_handlers Not pre-open stream in CBL_BlobStore's blobInputStreamForKey commit a3f6388 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Mon Apr 13 14:58:36 2015 -0700 Fix circular reference between CBLUnsavedRevision and CBLAttachment - As CBLUnsavedRevision keeps CBLAttachment objects in an array so setting the revision (self) object to the CBLAttachment objects will cause circular reference memory leak. - Instead of setting the revision (self) object when adding a CBLAttachment to the CBLUnsavedRevision object, overiding the attachmentNamed: method and creating a new CBLAttachment object and adding a revision object there. commit d7f0855 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Mon Apr 13 11:47:17 2015 -0700 Remove NSDataReadingMappedIfSafe when reading attachment content NSDataReadingMappedIfSafe option can keep the file descriptors of the attachments open if the client application keeps holding the NSData object returned by CBLAttachment.content. For a very big attachment, alternatively, users can call contentURL and read the NSData with the NSDataReadingMappedIfSafe option themselves. #649 commit 066d328 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Fri Apr 10 16:23:03 2015 -0700 Not pre-open stream in CBL_BlobStore's blobInputStreamForKey There is no needs to pre-open the stream except when decrypting the stream. This helps reduce number of opened file descriptors when pushing high volumn of documents with attachments in multipart mode. #649 commit bbd3b72 Merge: 0586286 94f5107 Author: Jens Alfke <jens@couchbase.com> Date: Wed Apr 8 20:27:27 2015 -0700 Merge pull request #650 from couchbase/feature/issue_649_attachment_crash Handle unable to create a blob store writer for the attachment For #649 (should prevent the crash, at least) commit 0586286 Merge: fcbed11 a61bd33 Author: Pasin Suriyentrakorn <phasin@gmail.com> Date: Wed Apr 8 17:45:51 2015 -0700 Merge pull request #651 from couchbase/feature/issue_356_all_docs Add all_docs with keys body test case commit a61bd33 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Wed Apr 8 17:43:49 2015 -0700 Add all_docs with keys body test case Add POST all_docs with keys in the body to confirm that the issue doesn't exist. #356 commit 94f5107 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Wed Apr 8 16:52:01 2015 -0700 Handle unable to create a blob store writer for the attachment - Return AttachmentError when cannot create a blob store writer for the attachment. - Add warning messages to pin point where the issue is. commit fcbed11 Author: Jens Alfke <jens@couchbase.com> Date: Wed Apr 8 09:24:54 2015 -0700 Added test for CBLModel properties of type "id" For #648 commit d4fe3c5 Author: Jens Alfke <jens@couchbase.com> Date: Wed Apr 8 09:09:07 2015 -0700 Fixed regression with CBLModel properties of type "id" Fixes #648 commit 69c83b1 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Tue Apr 7 17:14:31 2015 -0700 Add CBLIS support for many-to-many relationship - Store embeded array of doc ids in the documents for many-to-many relationship. - Bug fix: add relationship name as part of to-many inverse key view name. - Small code clean up. #638 commit b4850d3 Merge: 38dd697 ae6022a Author: Jens Alfke <jens@couchbase.com> Date: Sun Apr 5 11:02:59 2015 -0700 Merge pull request #645 from couchbase/feature/issue_637_cblis_ordered_relationship Add ordered relationship warning message to CBLIS commit ae6022a Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Fri Apr 3 14:03:31 2015 -0700 Add ordered relationship warning message to CBLIS With the current approach of using inverse relationship view for storing to-many relationship, we are not supporting ordered many relationship. This commit prints warning messages when detecting that the core data model uses ordered many relationship. #637 commit 38dd697 Merge: 3f8655c 5438f47 Author: Jens Alfke <jens@couchbase.com> Date: Fri Apr 3 08:33:20 2015 -0700 Merge pull request #642 from couchbase/feature/issue_639_replicator_stop Fix stop idle replicator not getting change notification commit 5438f47 Author: Pasin Suriyentrakorn <pasin@couchbase.com> Date: Mon Mar 30 18:23:11 2015 -0700 Fix stop idle replicator not getting change notification - In CBL_Replicator's stop method, post progress changed notification after calling stopped. - Refactor CBLReplication's stop method to just calling stop method on the background replicator. - The rest of the logic including forgetting the replicator and setting _started variable will be done in the updateStatus:error:processed:ofTotal: serverCert: method after getting the progress changed notitification after the replication get stopped. #639
For some reason the prior fix was only implemented for setter methods, not getters, and the unit tests didn't notice because they only tested the untyped setter... Fixes #648
Just updated to the current master build of couchbase-lite-ios from my previous known-good-in-my-app version built on Feb 27 and launching the app causes this error.
I have an class named VTCase, of course, with a property caseName which is an NSString (Swift project), @NSManaged property, of course, displaying this object in a form and loading that view controller tries to access this property and this is where it croaks.
Going back to the Feb 27 build of the library fixes the problem.
Swift 1.2 project with Xcode 6.3 6D543q on OS x 10.10.3 (14D130a).
23:54:38.118| WARNING: MYDynamicObject: VTCase.caseName has type @ which is not a known class or protocol
23:54:38.119| WARNING: Dynamic property VTCase.caseName has type '@' unsupported by VTCase
2015-04-07 23:54:38.119 Voltaire[49935:1641226] -[VTCase caseName]: unrecognized selector sent to instance 0x788a72f0
The text was updated successfully, but these errors were encountered: