sproutcore / sproutcore

JavaScript Application Framework - JS library only
sproutcore.com
Other
2.15k stars 291 forks source link

SC.Store#loadRecord() should handle non-nested records whose hashes arrive embedded. #798

Closed dcporter closed 10 years ago

dcporter commented 12 years ago

Nested records exist for the purpose of managing complex, self-contained data, for example a driver's license that only belongs to a single person:

Person |-- ID |-- Name |-- Driver's License .....|-- State .....|-- Status .....|-- Number

The Store can keep all that data together in one hash in the store.

But there's another common case: servers often embed the hashes for NON-nested records inside each other. For example, if you ask the server for information about a TV show, and it replies with the following JSON:

TV Series |-- ID |-- Title |-- Creator |....|-- ID |....|-- Name |....|-- Age |-- Actors .....|-- 0 .....|..|-- ID .....|..|-- Name .....|..|-- Age .....|-- 1 ........|-- ID ........|-- Name ........|-- Age

The people in this model aren't nested records, since a Person could create and act in multiple shows. While servers ought to split these Person records out and relate them to each other by reference, they don't always, and the server guy isn't always right there to change it for you. If you're dealing with a legacy server, forget it.

Currently the workaround is to scrub your raw hashes as they arrive, in the DataSource -- you have to strip the embedded hashes out, load them separately, and replace them in the parent record with an ID or list of IDs. That can be a lot of work, and is definitely not friendly behavior. SproutCore could definitely be friendlier here.

I would like to propose an implementation wherein:

1) A new record attribute option, similar to isNested, is added to flag this situation on a per-property basis. For example, in the above example, the show's creator and the list of actors would be flagged. (Hell if I can think what the flag should be called though. hashIsNested? needsUnnesting?) (Edit 2: These are embedded hashes rather than nested records. The flag could be isEmbedded, or hashIsEmbedded?)

2) Subclasses of SC.Record keep a private list of attributes which have been thusly flagged.

3) In loadRecord, this private list is iterated over, and the corresponding hashes are unnested and separately loaded.

Thoughts on the idea? Thoughts on the flag name, since isNested is already taken?

Cheers, Dave

(Edit: Sorry for the ugly diagrams.)

mauritslamers commented 12 years ago

The problem in this case is that these kinds of complex hashes are not generic. Nested records are different in this respect because the model hierarchy can be described as models, and the data will be handled as one when saving.

The kind of hash you show is something serves do to minimize the number of requests. The store's only purpose is to handle records though, and any conversion that needs to be done from the server format to the stores format is the responsibility of the layer in between, whether that is a data source or hand-rolled functions.

My view is that the proper solution here is to write a generic solution (an extended data source?) that can handle these kinds of hashes.

publickeating commented 12 years ago

I can see how it would be nice for the store to figure this out, but @mauritslamers has a point that the data source's role is to take exactly this kind of variable input and generate the proper generic output for the store to consume. Because the server response is totally up to the implementor, I can see this working great for one situation and failing miserably for another, so I feel that this kind of functionality is still better suited for some kind of data source plugin or left to the individual developer.

Aside, there is an implementation problem that could be a roadblock to doing this in the store as well, and that is that the record attributes aren't used by the store to manage data hashes, the attributes really only apply when accessing those properties on the record.

Anyhow, I've made a few data sources, some very similar and some vastly different. Anyone have an idea how we could solve this problem? Maybe at least throw some common data sources in the new extensions repo?

dcporter commented 12 years ago

Two points, regarding the genericness of this situation and location of the logic.

First, genericness. I disagree that this isn't a generic problem - having non-nested records coming down from the server as embedded hashes is an easy decision to make on the server side, and I don't think it's a wrong one. Two of the last two projects where I've gotten my hands dirty in the hashes have used the exact same setup, and in one of them, I got some pretty dismissive lip from the server guy that the client side oughtta be able to handle breaking it out. He's right, and it's common, and there's no reason that we shouldn't have a go at helping folks out with it if we can.

Regarding the location of the logic: I agree that philosophically it certainly OUGHT to live in the data source, since the data source is supposed to be the mediator between server and store. But I have a straightforward, transparent implementation in mind for the store, and absolutely no idea how you'd do it in the data source. Does anyone have any ideas on how you would? -- transparently, so that developers don't have to become data store experts in order to handle a common case? I'm wide open to suggestions, but absent them, I'd like to defend breaking store philosophy slightly and having it use record attributes to change hash structure in this one practically useful, flag-initiated, clearly-delineated situation.

dcporter commented 12 years ago

@mauritslamers, is the problem with putting the flag on the model, or with changing the hash in the store? I have no problem moving the hash-hacking into the data source, but I don't see an alternative to defining the flag on the model. Thoughts?

mauritslamers commented 12 years ago

No, the main problem is that the store is the most effective when you have single models with single hashes. You can use nested hashes, but it should better not be a separate model.

I didn't write the store, but as far as I understand the idea behind the store is to have clean data hashes which can easily be stringified into JSON. One of the reasons of having clean hashes in there is that sproutcore objects contain circular references and cannot be stringified. In an application you want nice things as bindings, computed properties and observers, in the database you want clean hashes. There needs to be some place to put the boundary, and this happens to be the store. The store keeps the hashes clean, and gives out sproutcore objects to any searches, so your records are observable inside the application. When for some reason something changes inside the store, the materialized records (the sproutcore objects containing the record hashes) are automatically signaled and will cause controllers to update, which causes views to update.

The nested record stuff already breaks this boundary by attaching observers to the hashes, in order to be notified when things change in the nested record, and this causes its own kind of issues.

Ergo: as far as I can see it, the store is designed and written for, and should therefore only be used with straight models and hashes. Any other use requires a redesign of the store or accept nasty consequences such as possibly having to clean observer related stuff off the hashes (with all possible related problems) before converting them to JSON.

What many people don't realise is that you don't have to use the store. You can easily write something simple that allows the model layer to function on its own and use the rest of sproutcore on top of that. If you do want to use the store, you don't have to use a datasource to retrieve or store data. You can write your own framework with functions which retrieve the necessary data and pushRetrieve this into the store, or assemble complex objects back together in order to store it again on the server. (see: https://github.com/erichocean/otherinbox, CoreOI framework)

The problem you have is that your JSON is complex, and the resulting JS object structure is therefore complex too. The problem sproutcore has with that, is that it would require a very complex web of observers in order to consistently and correctly handle any changes to this object in order to have a consistent object across the application. Any change would also cause a waterfall of observer invocations, computed property reruns etc. As observers and bindings are not really cheap, a sproutcore application would quickly crawl to a halt if this would be supported (as would any web application that would use bindings for complex objects). I am also afraid that supporting this would also result in people trying to cram in thousands of complex objects, and then complaining that the app doesn't run snappy. :)

I think that the store is opinionated in this: if you have complex objects, untangle them before you put them into the store. :-)

I agree with you that it would be nice to have such a feature, but that would need quite a bit of thought in order to not end up in an endless amount of observers.

cheers

Maurits

On 25 jun. 2012, at 17:15, Dave Porter wrote:

@mauritslamers, is the problem with putting the flag on the model, or with changing the hash in the store? I have no problem moving the hash-hacking into the data source, but I don't see an alternative to defining the flag on the model. Thoughts?


Reply to this email directly or view it on GitHub: https://github.com/sproutcore/sproutcore/issues/798#issuecomment-6550103

dcporter commented 12 years ago

Thanks for the writeup, Maurits. I'm not sure I'm expressing myself clearly though - I'm not envisioning any changes to the store other than a one-time hash modification on load. The loadRecord method would extract any embedded hashes, load them as separate records, and replacing them in the parent hash with an ID before loading as usual. The hash is modified into a completely normal set of SproutCore records, with no further behavioral changes needed -- nothing like the crazy situation with nested records. All we're doing is tweaking the hash slightly, prior to loading, to account for a common server convention. I don't believe that any new observers whatsoever would be required.

dcporter commented 10 years ago

Closing this out. I've been convinced that the store should sport less magical behavior, not more. I still want to see magical behavior! But probably defined on models and handled by more-abstract data sources.

mauritslamers commented 10 years ago

Would it make sense to put this kind of behaviour on the data source, and create a set of template data sources to handle these kinds of situations?

On 20 nov. 2013, at 20:32, Dave Porter notifications@github.com wrote:

Closing this out. I've been convinced that the store should sport less magical behavior, not more. I still want to see magical behavior! But probably defined on models and handled by more-abstract data sources.

— Reply to this email directly or view it on GitHub.

dcporter commented 10 years ago

Yup.