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

CBLModel has trouble with dynamic properties in Swift classes #648

Closed
24601 opened this issue Apr 8, 2015 · 20 comments
Closed

CBLModel has trouble with dynamic properties in Swift classes #648

24601 opened this issue Apr 8, 2015 · 20 comments
Assignees
Milestone

Comments

@24601
Copy link

24601 commented Apr 8, 2015

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

@snej
Copy link
Contributor

snej commented Apr 8, 2015

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.

@snej snej added this to the 1.1.0 milestone Apr 8, 2015
@snej snej self-assigned this Apr 8, 2015
@snej snej closed this as completed in d4fe3c5 Apr 8, 2015
@snej
Copy link
Contributor

snej commented Apr 8, 2015

Fixed — for some reason the property type got encoded as id not NSString, and that MYDynamicObject change broke support for such properties.

snej added a commit that referenced this issue Apr 8, 2015
@24601
Copy link
Author

24601 commented Apr 9, 2015

Jens,

Thank you so much, again! Will pull and rebuild and try again!

Cheers!

Basit

@24601
Copy link
Author

24601 commented Apr 9, 2015

@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):

19:45:35.726| WARNING: Dynamic property VTCase.caseName has type '@' unsupported by VTCase
2015-04-08 19:45:35.727 Voltaire[13066:128226] -[VTCase caseName]: unrecognized selector sent to instance 0x7afdb8b0
(lldb) bt
* thread #1: tid = 0x1f4e2, 0x04caa69a libsystem_kernel.dylib`__pthread_kill + 10, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
    frame #0: 0x04caa69a libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x04cd8f19 libsystem_pthread.dylib`pthread_kill + 101
    frame #2: 0x04a61a5a libsystem_sim_c.dylib`abort + 156
    frame #3: 0x00360f97 Voltaire`uncaught_exception_handler + 38
    frame #4: 0x00361443 Voltaire`-[APLPLCrashReporter handleException:] + 17
  * frame #5: 0x002e5b79 Voltaire`-[APLCrashController handleException:](self=0x7aee95c0, _cmd=0x089bdd41, exception=0x7aff2570) + 81 at APLCrashController.m:73
    frame #6: 0x002daa69 Voltaire`-[APLCurrentSession applicationCrahsedWithHighLevelException:](self=0x7aee60b0, _cmd=0x00397887, notification=0x7bda1650) + 247 at APLCurrentSession.m:196
    frame #7: 0x026b8079 Foundation`__57-[NSNotificationCenter addObserver:selector:name:object:]_block_invoke + 40
    frame #8: 0x040b5be4 CoreFoundation`__CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 20
    frame #9: 0x03fa211b CoreFoundation`_CFXNotificationPost + 3051
    frame #10: 0x026a75d6 Foundation`-[NSNotificationCenter postNotificationName:object:userInfo:] + 98
    frame #11: 0x002ef28e Voltaire`APLInternalUncaughtExceptionHandler(exception=0x7aff2570) + 218 at APLInternalLogger.m:30
    frame #12: 0x040ebb80 CoreFoundation`__handleUncaughtException + 832
    frame #13: 0x03d74d3d libobjc.A.dylib`_objc_terminate() + 100
    frame #14: 0x04878cf0 libc++abi.dylib`std::__terminate(void (*)()) + 14
    frame #15: 0x04878922 libc++abi.dylib`__cxa_rethrow + 103
    frame #16: 0x03d74c38 libobjc.A.dylib`objc_exception_rethrow + 47
    frame #17: 0x04001b2d CoreFoundation`CFRunLoopRunSpecific + 653
    frame #18: 0x0400188b CoreFoundation`CFRunLoopRunInMode + 123
    frame #19: 0x0824c2c9 GraphicsServices`GSEventRunModal + 192
    frame #20: 0x0824c106 GraphicsServices`GSEventRun + 104
    frame #21: 0x01182106 UIKit`UIApplicationMain + 1526
    frame #22: 0x00117a74 Voltaire`main + 180 at AppDelegate.swift:23
    frame #23: 0x049bdac9 libdyld.dylib`start + 1
(lldb) 

@24601
Copy link
Author

24601 commented Apr 9, 2015

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!

@snej
Copy link
Contributor

snej commented Apr 9, 2015

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 id-valued property, though...

@snej snej reopened this Apr 9, 2015
@24601
Copy link
Author

24601 commented Apr 9, 2015

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!

@snej
Copy link
Contributor

snej commented Apr 9, 2015

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.

@24601
Copy link
Author

24601 commented Apr 10, 2015

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!).

@24601
Copy link
Author

24601 commented Apr 14, 2015

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!

@snej
Copy link
Contributor

snej commented Apr 14, 2015

Thanks! The code in those methods should treat [NSObject class] as equivalent to Nil when testing propertyClass. See if that fixes your problem. (Sorry, I've been working on a branch for the past week and haven't gotten around to checking out master and making this change.)

@24601
Copy link
Author

24601 commented Apr 14, 2015

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 :).

@24601
Copy link
Author

24601 commented Apr 20, 2015

Been spending a fair amount of time playing in these methods and can't seem to get things to work.

Been doing things like:

  1. adding

else if ( [propertyClass isSubclassOfClass: [NSObject class]])
{
return [super impForGetterOfProperty: property ofClass: propertyClass];
}

right before the last catch-all else in the getter method but property fields of CBLModel subclasses still seem to be broken...

  1. Doing commensurate updates on the setter side

Still playing, any pseudocode or explanation could be helpful, still working with this....

@24601
Copy link
Author

24601 commented Apr 20, 2015

(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

@24601
Copy link
Author

24601 commented Apr 20, 2015

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
11:27:58.403| WARNING: Dynamic property VTCase.currentReport has type '@"VTJuryReport"' unsupported by VTCase

@24601
Copy link
Author

24601 commented Apr 20, 2015

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....

24601 added a commit to 24601/couchbase-lite-ios that referenced this issue Apr 20, 2015
@24601
Copy link
Author

24601 commented Apr 20, 2015

Submitted PR above

@24601
Copy link
Author

24601 commented Apr 20, 2015

Improved fix based on my own commentary on the original PR after thinking about a better/cleaner/more maintainable way of doing/implementing this.

@snej
Copy link
Contributor

snej commented Apr 22, 2015

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.

@24601
Copy link
Author

24601 commented Apr 23, 2015

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:

https://github.com/24601/CBL-Bug-Demo

@zgramana zgramana modified the milestones: 1.2, 1.1.0 Apr 24, 2015
pasin added a commit that referenced this issue Apr 30, 2015
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
@snej snej closed this as completed in eb4ff92 May 5, 2015
snej added a commit that referenced this issue May 12, 2015
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
@snej snej changed the title Dynamic Property has type '@' which is not a known class or protocol issue CBLModel has trouble with dynamic properties in Swift classes Feb 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants