Changes in preparation for version 0.3.0 #89

Merged
merged 62 commits into from Jul 17, 2012

4 participants

@Marak

In preparation of the resourceful 0.3.0 release, this pull request contains the following changes:

  • Fixes unique key issue #83
  • Fixes nasty reference bug and cache issue in memory engine #87 #78
  • Fixes several issues related to relationships and relational resources
  • Made resourceful more engine agnostic
    • Cleaned up Resource._request
    • Decoupled any remaining couchdb logic from core
    • Decoupled any remaining couchdb logic from memory engine
  • Refactor of test suite
    • Relationship tests are now unified across engines
    • Improves general resource coverage
    • Improves fixtures and re-usable macros
  • Changes default primary key for resources from _id to id
  • Added several basic code /examples/

After reviewing and merging this request, we'll update the documentation and do the 0.3.0 release.

@pksunkara

Shouldn't the above 3 be prepended by book?

{ _id: 'book/bob/1' //...
{ _id: 'book/tim/1' //...
{ _id: 'book/bob/2' //...

I think so, yes.

If you have a resource with multiple children, their ids will conflict if not pre-pended with their resource name.

I'm going to expand coverage for relationships tests first, then will update this accordingly.

@pksunkara

Here t = t[t.length-1] is wrong. It should be

t = t.slice(1).join('/');

Take nodejitsu for example, we store the app document as username/appname, so when we prefix it with app, the doc id becomes app/username/appname in which case taking the last item in split array is wrong.

Agreed. Good catch.

I'll update the test coverage and fix this.

@pksunkara

Same here as mentioned above

res[r]._id = res[r]._id.slice(1).join('/');
@pksunkara

shouldn't articles be prefixed by article instead of author ?

No, read the code.

There's no need to prefix the resource name to the _id when using the new API calls, since the function is already aware it's inside the context of that resource.

yeah, sorry, kinda missed it.

@Marak

What's the reason for not testing the memory engine in this test?

Seems like we should be testing that.

pksunkara added some commits Jul 2, 2012
@pksunkara pksunkara [test] Finished fixture and models for all kinds of cases 0bdc42f
@pksunkara pksunkara [test] Finished tests for listing and creating children 5f90fef
@pksunkara pksunkara [test] Sometimes memory engine is getting tested instead of couchdb 259c353
@pksunkara pksunkara [test] Added tests for listing and creating children from the Parent …
…resource
555e8a4
@pksunkara pksunkara [test] Finished testing all the relationship methods and destroying e…
…ffects

Still need to test the cascading destruction
7af1458
@pksunkara pksunkara [fix] Append resource name when child is requested directly 787c1bc
@pksunkara pksunkara [minor] Added factory.lowerResource to avoid many calls toLowerCase
Also fixed some indentation error
11239a7
@pksunkara pksunkara [refactor] Refactored Couchdb._byParent method to use factory.get
Removed unneccessary usage of filter for byParent method when we
can easily get the parent obj and get that children list
7b558e1
@pksunkara pksunkara [test fix] Fix some typo bugs in relationship tests 3221a21
@pksunkara pksunkara [refactor][fix] Change createChild and fix bug in notifyParent b130580
@pksunkara pksunkara [minor] Disable buggy cache for the meanwhile bf82f0b
@pksunkara pksunkara [test fix] Fix one more typo in tests e7997e9
@pksunkara pksunkara [minor] Change all toLowerCase to lowerResource 19bc82d
@pksunkara pksunkara [api] Implemented cascading destruction but need to fix some scopes e99e621
@pksunkara pksunkara [test] Test to make sure atleast the children are destroyed 3aeec9b
@pksunkara pksunkara [minor] Make sure Factory.key respects engine's wishes 5ac327a
@pksunkara pksunkara [minor] Use resource.key to make resource methods a bit agnostic 3218c36
@pksunkara pksunkara [minor] Replace all hardcoded id keys with resource.key 2493d1e
@pksunkara pksunkara [major] Cleaned up _request and made basic method agnostic with couchdb 3a1ab10
@pksunkara pksunkara Merge branch 'agnostic' into couchids 6964dba
@pksunkara pksunkara [test minor] Make tests agnostic using Factory.key f393bc5
@pksunkara pksunkara [minor] Made cache use JSON encoding & decoding
Also enabled caching in Resource._request
c1c12e0
@pksunkara pksunkara [minor] Made use of JSON encoding & decoding in memory engine
Also, now engines-test.js and relationships-test.js
tests memory engine too
a2a9088
@pksunkara pksunkara [fix] Check if a parent already exists before appending 7d5a31c
@pksunkara pksunkara [test] Tests for cascading destruction 3050990
@pksunkara pksunkara [fix] Fix a bug when parent_id is null c4015ae
@pksunkara pksunkara [test fix] Unregister previous engine resources 8a5aec0
@pksunkara pksunkara [fix test] Fixed bug in destroy hook when parent is null
Added tests for creating and destroying of forums
(self-parent) resources
0e3b28f
@pksunkara pksunkara [test] Move all uses of _id to id 4f7f7b1
@pksunkara pksunkara [minor] Use id for public API in couchdb engine 14744af
@pksunkara pksunkara [examples] Update all examples wrt to new api fc0671e
@pksunkara pksunkara [fix] Fixed a bug which save .id in couchdb engine fbbb305
@pksunkara pksunkara [fix] Remove temp hack from Resource.view and add filter tests back 6cf7553
@pksunkara pksunkara [fix] Convert _id to id in couch views 1e31dad
@indexzero
a decoupled application framework member

@Marak do we still store the ._rev in the document?

@pksunkara

Default, We don't store ._rev in the document. We store it only if it is provided by couchdb engine. You might want to change #90 regarding this.

@indexzero
a decoupled application framework member

I've been giving this more thought and #90 should land in resourceful@0.2.2 before we publish 0.3.0

@pksunkara

ok with me if you want to release resourceful@0.2.3

@indexzero
a decoupled application framework member

Will do

@indexzero
a decoupled application framework member

#90 has been merged and published in resourceful@0.2.2

@Marak

Unless anyone has any objections, this is getting merged into master over the weekend.

@Marak Marak merged commit 680b02c into master Jul 17, 2012
@indexzero
a decoupled application framework member

@Marak Looks like we might have a bad merge here: why is this in the commit history twice?

History is messed up, fix it if you want. I don't know how.

Code should all be working.

@indexzero
a decoupled application framework member

Looking back at the commit history I think this merge may have corrupted the commit history for previous versions. @Marak did you rebase? Or did this get straight-up merged in?

What I'm seeing is that commits from this branch are interlaced with commits during the 0.2.0 - 0.2.2 timeframe. SInce these commits did not get published in these versions I'm trying to figure out what went wrong.

@indexzero
a decoupled application framework member

Digging in here the history for the 0.2.x tags does not include these commits which is really all we want to preserve a good history.

e.g. v0.2.2...master

Still curious if you rebased or not.

@pksunkara

I can fix the history. But the question is do we want to fix it and force publish v0.3.0 again?

@indexzero
a decoupled application framework member

@pksunkara No. The tags have the correct history so this is fine.

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