Skip to content
This repository was archived by the owner on Jan 25, 2019. It is now read-only.

Conversation

@mathebox
Copy link

@mathebox mathebox commented Jan 27, 2017

Hi @wvteijlingen,

this is my attempt to change Resource to a protocol. Please have a look. The code is still kind of messy and really not ready to be merged. But the tests are passing 🎉

I made the following changes

  • Add lots of generics
  • Change Resource to protocol
  • Removed NSCoding conformance of Resource. Each implementation should handle this on its own.
  • Use ZeWo/Reflection (see Support value type attributes #152) for getting and setting properties. They do not provide a dynamic framework which is necessary for carthage. So I used carthage update --no-build and created a workspace for Spine and a project for Reflection.
  • Move lots of logic into Field instances due to generic types. This also prevents switch statements of fields of resources.
  • Change Field implementation to protocol-struct-combination. (We need to these protocols to hide the generic types, e.g. the Relationship protocol)
  • Since swift protocols do not support stored properties, every implementation of Resource needs a resourceData property.

Next steps / further improvements

  • Cleanup code inkl. tests (There is lots of "old" code.)
  • Documentation
  • Fix build for macOS, tvOS and watchOS
  • Improve access control
  • Add a FieldData struct (similar to ResourceData) to avoid code repetition
  • Fix travis-ci
  • Add description and debugDescription to Resource

Additional notes
We should distinguish between Attribute and Relationship of Resource objects. Currently they are returned as Field instances. Therefore the Field protocol contains some methods which are only used when working with relationships.

What's do we need to support CoreData? (This should probably be moved to a separate issue.)

  • ToMany-relationships in CoreData will not work with ResourceCollection objects. Maybe we could provide the links etc. via methods onResource.
  • CoreData can't store URL. We have to check the type of the property (String vs URL). This should be handled by the URLAttribute.

Hopefully I have covered every change I made. What are your thoughts?

@@ -1,2 +1,3 @@
github "SwiftyJSON/SwiftyJSON" ~> 3.1.4
github "Thomvis/BrightFutures" ~> 5.0
github "ZeWo/reflection" ~> 0.14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick thought: updating to Swift 3 was a pain, and it seemed like the pain was exponential to the amount of dependencies involved in the project. Is it really worth adding a new dependency here? Is it absolutely required versus implementing this PR's particular reflection needs and wrapping it in an internal class?

@markst
Copy link

markst commented Feb 28, 2017

#151

@markst
Copy link

markst commented Mar 1, 2017

Hey @mathebox, thanks for making a start at this. I was trying to pickup where you left off.
For starters I wasn't able to implement the protocol without having to conform entirely to Resource protocol. This seems to be due extension Resource not being public where as the Resource protocol is.


if let linkedResource = linkedResource {
if resource.value(forField: self.name) == nil || (resource.value(forField: self.name) as? Resource)?.isLoaded == false {
resource.setValue(linkedResource, forField: self.name)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mathebox: by the looks of it ToOneRelationship shouldn't be resolving the relationship here since the deserialise operation hasn't passed included yet...
https://github.com/mathebox/Spine/blob/741a0943fdbfd48ee149603eda1effe8678baa54/Spine/DeserializeOperation.swift#L82

@markst
Copy link

markst commented May 26, 2017

Also seems to break the ability to sequence a bunch of BrightFuture futures:
[self.spine.delete(self.fitting),self.spine.save(self.fitting)].sequence().onSuccess(callback: { (resources) in

@mmoonport
Copy link

Hello guys. I have worked on a a couple ORM's and wonder if there is anything I could lend a hand with. We are looking at using this library but the "offline mode" abilities really should be moved to CoreData and thus, here we are.

@mmoonport
Copy link

mmoonport commented Jun 20, 2017

After doing some research into this issue for our own uses. We have come up with an idea of using spine in concert with core data, abstracted behind NSIncrementalStore. This will allow us to use core data natively with the ability to have our "back-end" be a spine-compliant API. AFNetworking once had a AFIncrementalStore that used AFRESTClient. I believe this could be easily ported to be used with spine. The only thing that would make sense to me is allow the incremental store the ability to create a "Resource" Class by inspecting a NSManagedObject for which fields it has and what types they are when it begins the store in loadMetadata. This would prevent the need for both and give a user wanting to persist with core-data a backend connected to their api. I am hoping that @wvteijlingen could guide me in the right direction as to the dynamic creation of a resource.

@markst
Copy link

markst commented Aug 14, 2017

@mmoonport hey. I've forked this PR branch into our own & have written persistence using Realm. It isn't amazing; but it works for our needs. I'm keen to write a more robust persistence layer on-top of Spine; but it seems development & activity has gone stale.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants