storeAs option for references #1349

coudenysj · 2016-01-22T19:50:33Z

As discusses in #807, this pull requests starts using a storeAs option to support different storage formats:

simple: $id
partial: $id, $ref
full: $id, $ref, $db

With full being the default.

malarzm · 2016-01-22T23:04:20Z

lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php

+    /**#@+
+     * The types of storeAs references
+     *
+     * TODO move this to a different class?


I think it's a good place 👍

alcaeus · 2016-01-23T06:47:16Z

Thanks for getting started with this, this should clean up reference mapping a bit!

I know I'm the one to come up with "partial" and "full", but I've "seen some things" that make me want to change the naming to include "dbref" in the name, especially since that's the name of the structure we're actually storing. Likewise, we could rename the "simple" option to "id" as a reference of that type would only store the ID of the reference object, not a DBRef object.

I'm really looking forward to this 👍

coudenysj · 2016-01-28T15:02:03Z

I'm not sure if we should include the dbRef name, because the "simple" option would not use the the DBRef structure.

The 'simple' vs 'id' argument I can follow.

alcaeus · 2016-01-28T15:23:49Z

@coudenysj the "simple" option does not store it as DBRef and thus doesn't have "dbref" in its name. If we store it as "dbref" we might as well rename the storage option to include "dbref" in the name ;)

coudenysj · 2016-01-28T15:29:08Z

@alcaeus Are you talking about the "storeAs" name? Or the different values it can have?

alcaeus · 2016-01-28T16:07:58Z

Sorry, I was talking about the values it can have. IMO, "dbref" is more descriptive than "full" - same goes for partial as it's actually a partial dbref object (still dislike the wording "partial" in there, don't have an alternative though)

malarzm · 2016-01-28T16:27:20Z

Gotta love naming things

coudenysj · 2016-01-28T21:06:38Z

So you want to make the values really descriptive.

Some more brain dumps:

id: $id
dbRef: $id, $ref (as the $db option is optional: http://php.net/manual/en/mongodbref.create.php)
dbRef_X?_: $id, $ref, $db

alcaeus · 2016-03-12T16:28:22Z

@coudenysj what's the state of this? I could use some of this stuff for some sharding magic in references but want to hold off until we're done changing the whole simple vs. full thing. Any progress you can report?

coudenysj · 2016-03-16T06:46:38Z

Hi @alcaeus, no further progress on this. If we can agree upon naming, I can do some more programming to finalize this.

alcaeus · 2016-03-16T06:57:37Z

Right. How about id, dbRef and dbRefWithDb? Sounds silly but in the end it's the term that best explains the difference between dbRef and dbRefWithDb. What do you think?

coudenysj · 2016-03-18T18:55:38Z

I can live with that :). Let's see if I can work on it.

coudenysj · 2016-03-18T20:18:06Z

Owkay, so I've added the option to all drivers and scanned the code for the "simple" reference.

Is it the correct approach if I try to "redirect" the "simple" option to the "storeAs: id" option, and refactor all the "simple" checks?

alcaeus · 2016-03-19T06:26:36Z

Yes. Within ODM we can change all checks from simple to storeAs == 'ID'. I don't think we need to keep the simple flag around in the mapping for BC reasons - opinions @malarzm @jmikola?

malarzm · 2016-03-19T11:04:05Z

I think we should keep it

coudenysj · 2016-03-19T13:06:40Z

I will try to support both (simple=true triggers storeAs=id and storeAs=id triggers simple=true). I will however refactor all "simple" checks to "storeAs" checks in the code.

coudenysj · 2016-04-15T17:14:55Z

If this TravisCI check comes back positive, I think this pull request is ready.

coudenysj · 2016-04-15T18:44:36Z

@alcaeus @malarzm Any suggestions of unit tests that should be added?

malarzm · 2016-04-16T10:19:22Z

docs/en/reference/reference-mapping.rst

@@ -314,6 +314,8 @@ a certain class, you can optionally specify a default discriminator value:
 Simple References
 -----------------

+TODO: Rewrite this to reflect the new ``storeAs`` approach.


Commenting on this so we don't forget to revisit before merging

alcaeus · 2016-05-11T19:02:02Z

#1406 has been merged into 1.0.x and master - you can rebase this PR and update the fix.

…full)

… the deprecated "simple" key

…rds compatibility

…s scenarios

…maManagerTest::testEnsureIndexes

…hanges

…upport the "storeAs" method

… attribute

…erences")

coudenysj · 2016-05-11T19:43:10Z

Processed changes from #1406, so this PR is ready for final review.

malarzm · 2016-05-12T21:27:34Z

docs/en/reference/reference-mapping.rst

+
+.. note::
+
+    The ``storeAs=id`` option used to call a "simple reference". The old syntax is


"used to be called"

malarzm · 2016-05-12T21:35:36Z

LGTM, thanks @coudenysj for working on this!

alcaeus · 2016-05-13T08:41:14Z

Thank you @coudenysj!

malarzm reviewed Jan 22, 2016
View reviewed changes

This was referenced Mar 14, 2016

Introduce support for sharded collections #1385

Merged

Inline field denormalization for Reference* mapping #613

Open

coudenysj pushed a commit to coudenysj/mongodb-odm that referenced this pull request Mar 18, 2016

Update the 'storeAs' values as discussed in doctrine#1349

c7e8a2e

coudenysj force-pushed the feature/reference-storeAs branch from dc3b386 to 7ccc6bf Compare April 15, 2016 17:13

coudenysj pushed a commit to coudenysj/mongodb-odm that referenced this pull request Apr 15, 2016

Update the 'storeAs' values as discussed in doctrine#1349

3db4187

coudenysj changed the title ~~WIP: storeAs option for references~~ storeAs option for references Apr 15, 2016

malarzm reviewed Apr 16, 2016
View reviewed changes

malarzm added the Feature label Apr 16, 2016

malarzm added this to the 1.1 milestone Apr 16, 2016

Jachim Coudenys added 19 commits May 11, 2016 21:02

Add 3 different types to store MongoDB references (simple, partial & …

d81f1e9

…full)

Correct the coding standards (empty line between namespace/import)

11f7a2a

Update the 'storeAs' values as discussed in doctrine#1349

a78ff55

Add the storeAs key to all the drivers and add some todos to redirect…

3b4851c

… the deprecated "simple" key

Correct the mapping variable

c8f2afb

Synchronize the "simple" and "storeAs" mapping information for backwa…

60fad18

…rds compatibility

Introduce a new ReferenceUser test class to test the different storeA…

003dccf

…s scenarios

Remove he field references, it breaks Doctrine\ODM\MongoDB\Tests\Sche…

c6b20e2

…maManagerTest::testEnsureIndexes

Expand the unit tests for the ClassMetadataInfo::mapField "storeAs" c…

618b833

…hanges

Update the DM::createDBRef code to use the "storeAs" value

14b921b

Remove the "simple" reference handling throughout the code and only s…

f2ceef5

…upport the "storeAs" method

Use a custom type in the XML schema mapping and remove the camel case…

2a3f77a

… attribute

Check the availability of the 'storeAs' key once

b0a7256

Remove the group docblock formatting

ed0a32c

Update the "simple reference" documentation (and call it "storing ref…

7e2ee3c

…erences")

Correct the dbrefs link to the Mongo documentation

2c6d6ab

Add an extra backwards compatibility note for storing references

4e1a748

Add unit tests to test references and includesReferenceTo

07082a5

Correct the annotation formatting (string should be quoted)

191c63f

coudenysj force-pushed the feature/reference-storeAs branch from 916cb66 to 191c63f Compare May 11, 2016 19:19

malarzm reviewed May 12, 2016
View reviewed changes

Update the "simple reference" docs as suggested by @malarzm

bde7416

alcaeus merged commit a4d5226 into doctrine:master May 13, 2016

alcaeus mentioned this pull request Jun 6, 2016

Store simple field in mapping #1427

Merged

malarzm mentioned this pull request Jun 9, 2016

Partial reference support (without $db property) #807

Closed

alcaeus mentioned this pull request Feb 28, 2017

Suggestion: New type of reference to deprecate usage of dbref #1560

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

storeAs option for references #1349

storeAs option for references #1349

coudenysj commented Jan 22, 2016

malarzm Jan 22, 2016

alcaeus commented Jan 23, 2016

coudenysj commented Jan 28, 2016

alcaeus commented Jan 28, 2016

coudenysj commented Jan 28, 2016

alcaeus commented Jan 28, 2016

malarzm commented Jan 28, 2016

coudenysj commented Jan 28, 2016

alcaeus commented Mar 12, 2016

coudenysj commented Mar 16, 2016

alcaeus commented Mar 16, 2016

coudenysj commented Mar 18, 2016

coudenysj commented Mar 18, 2016

alcaeus commented Mar 19, 2016

malarzm commented Mar 19, 2016

coudenysj commented Mar 19, 2016

coudenysj commented Apr 15, 2016

coudenysj commented Apr 15, 2016

malarzm Apr 16, 2016

alcaeus commented May 11, 2016 •

edited

Loading

coudenysj commented May 11, 2016

malarzm May 12, 2016

malarzm commented May 12, 2016

alcaeus commented May 13, 2016


		.. note::

		The ``storeAs=id`` option used to call a "simple reference". The old syntax is

storeAs option for references #1349

storeAs option for references #1349

Conversation

coudenysj commented Jan 22, 2016

malarzm Jan 22, 2016

Choose a reason for hiding this comment

alcaeus commented Jan 23, 2016

coudenysj commented Jan 28, 2016

alcaeus commented Jan 28, 2016

coudenysj commented Jan 28, 2016

alcaeus commented Jan 28, 2016

malarzm commented Jan 28, 2016

coudenysj commented Jan 28, 2016

alcaeus commented Mar 12, 2016

coudenysj commented Mar 16, 2016

alcaeus commented Mar 16, 2016

coudenysj commented Mar 18, 2016

coudenysj commented Mar 18, 2016

alcaeus commented Mar 19, 2016

malarzm commented Mar 19, 2016

coudenysj commented Mar 19, 2016

coudenysj commented Apr 15, 2016

coudenysj commented Apr 15, 2016

malarzm Apr 16, 2016

Choose a reason for hiding this comment

alcaeus commented May 11, 2016 • edited Loading

coudenysj commented May 11, 2016

malarzm May 12, 2016

Choose a reason for hiding this comment

malarzm commented May 12, 2016

alcaeus commented May 13, 2016

alcaeus commented May 11, 2016 •

edited

Loading