Add ability to override just the metadata in a struct.

Add ability to override just the metadata in a struct.

This is useful when the '$id' is actually multiple fields packed into a string,
but it's generally available for any metadata field.

R=dnj@chromium.org, estaab@chromium.org, maruel@chromium.org, tandrii@chromium.org, vadimsh@chromium.org
BUG=https://github.com/luci/gae/issues/15

Committed: https://github.com/luci/gae/commit/bd65a9b4e2bd64035cc79b8a8e85f96c67e0381b

[Vadim Sh., 2015-10-13 04:27:49]

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pl...
File service/datastore/pls.go (right):

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pl...
service/datastore/pls.go:91: // sophistocated metadata values.
note that explicitly defined fields take precedence over MGS

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pl...
File service/datastore/pls_impl.go (right):

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pl...
service/datastore/pls_impl.go:47: func (p *structPLS) getMGS() MetaGetterSetter
{
:) https://goo.gl/ik3wWF

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pr...
File service/datastore/properties.go (right):

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pr...
service/datastore/properties.go:709: GetAllMeta() PropertyMap
hm, why this isn't in MetaGetter?

[iannucci, 2015-10-13 04:34:39]

The CQ bit was checked by iannucci@chromium.org

[iannucci, 2015-10-13 04:34:40]

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pl...
File service/datastore/pls.go (right):

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pl...
service/datastore/pls.go:91: // sophistocated metadata values.
On 2015/10/13 at 04:27:48, Vadim Sh. wrote:
> note that explicitly defined fields take precedence over MGS

Done.

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pl...
File service/datastore/pls_impl.go (right):

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pl...
service/datastore/pls_impl.go:47: func (p *structPLS) getMGS() MetaGetterSetter
{
On 2015/10/13 at 04:27:48, Vadim Sh. wrote:
> :) https://goo.gl/ik3wWF

:)

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pr...
File service/datastore/properties.go (right):

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pr...
service/datastore/properties.go:709: GetAllMeta() PropertyMap
On 2015/10/13 at 04:27:48, Vadim Sh. wrote:
> hm, why this isn't in MetaGetter?

It's... for not a super great reason. Basically when you to GetMulti, you have
the option to pass down a restricted subset of PLS... just the bits to Get
metadata (a MultiMetaGetter). To make this object as simple as possible,
GetAllMeta isn't included.

This could be revised at some point, but I'm inclined to leave it alone for now.
If someone needed GetAllMeta in e.g. a filter, then it could be moved into
MetaGetter, but at that point it's very likely an abstraction violation; filters
should be able to know precisely what metadata they need.

[commit-bot: I haz the power, 2015-10-13 04:34:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1401533007/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1401533007/20001

[commit-bot: I haz the power, 2015-10-13 04:34:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-13 04:34:48]

No L-G-T-M from a valid reviewer yet. Only full committers are accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[iannucci, 2015-10-13 04:35:32]

On 2015/10/13 at 04:34:48, commit-bot wrote:
> No L-G-T-M from a valid reviewer yet. Only full committers are accepted.
> Even if an L-G-T-M may have been provided, it was from a non-committer,
> _not_ a full super star committer.
> See http://www.chromium.org/getting-involved/become-a-committer
> Note that this has nothing to do with OWNERS files.

oops! jumped the gun there a bit.

[Vadim Sh., 2015-10-13 04:39:59]

lgtm

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pr...
File service/datastore/properties.go (right):

https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pr...
service/datastore/properties.go:709: GetAllMeta() PropertyMap
On 2015/10/13 04:34:40, iannucci wrote:
> On 2015/10/13 at 04:27:48, Vadim Sh. wrote:
> > hm, why this isn't in MetaGetter?
> 
> It's... for not a super great reason. Basically when you to GetMulti, you have
> the option to pass down a restricted subset of PLS... just the bits to Get
> metadata (a MultiMetaGetter). To make this object as simple as possible,
> GetAllMeta isn't included.
> 
> This could be revised at some point, but I'm inclined to leave it alone for
now.
> If someone needed GetAllMeta in e.g. a filter, then it could be moved into
> MetaGetter, but at that point it's very likely an abstraction violation;
filters
> should be able to know precisely what metadata they need.

It just looks odd: *Setter (that extends Getter) adding another Get-only method.

[iannucci, 2015-10-13 04:43:03]

On 2015/10/13 at 04:39:59, vadimsh wrote:
> lgtm
> 
>
https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pr...
> File service/datastore/properties.go (right):
> 
>
https://chromiumcodereview.appspot.com/1401533007/diff/1/service/datastore/pr...
> service/datastore/properties.go:709: GetAllMeta() PropertyMap
> On 2015/10/13 04:34:40, iannucci wrote:
> > On 2015/10/13 at 04:27:48, Vadim Sh. wrote:
> > > hm, why this isn't in MetaGetter?
> > 
> > It's... for not a super great reason. Basically when you to GetMulti, you
have
> > the option to pass down a restricted subset of PLS... just the bits to Get
> > metadata (a MultiMetaGetter). To make this object as simple as possible,
> > GetAllMeta isn't included.
> > 
> > This could be revised at some point, but I'm inclined to leave it alone for
now.
> > If someone needed GetAllMeta in e.g. a filter, then it could be moved into
> > MetaGetter, but at that point it's very likely an abstraction violation;
filters
> > should be able to know precisely what metadata they need.
> 
> It just looks odd: *Setter (that extends Getter) adding another Get-only
method.

Yeah, agreed... not sure what to do about it w/o opening a can of worms tho.

[iannucci, 2015-10-13 04:43:04]

The CQ bit was checked by iannucci@chromium.org

[commit-bot: I haz the power, 2015-10-13 04:43:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1401533007/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1401533007/20001

[commit-bot: I haz the power, 2015-10-13 04:44:56]

Committed patchset #2 (id:20001) as
https://github.com/luci/gae/commit/bd65a9b4e2bd64035cc79b8a8e85f96c67e0381b