Refactor multiarg, split MGS/PLS.

Refactor multiarg, split MGS/PLS.

Refactor the multiarg construction method to handle the various cases
inline, rather than defining and invoking methods for specific
scenarios. This enables PropertyLoadSaver-based methods to be handled
separately from MetaGetterSetter-based methods, which, in turn, enables
the two to be implemented independently.

A bit of boilerplate is removed and the odd case of MGS/PLS Map/Chan fits
better. Additionally, a few double-reflection code paths are optimized
along the way.

BUG=None

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

[dnj (Google), 2016-06-08 18:21:21]

dnj@google.com changed reviewers:
+ dnj@google.com, iannucci@chromium.org

[dnj (Google), 2016-06-08 18:21:22]

PTAL, same CL minus the non-pointer-to-struct part.

[dnj, 2016-06-13 15:53:07]

(Ping, this actually fixes some bugs so it'd be good to land).

[iannucci, 2016-06-14 00:19:51]

lgtm, this is good cleanup

https://chromiumcodereview.appspot.com/2048933004/diff/1/service/datastore/in...
File service/datastore/interface.go (right):

https://chromiumcodereview.appspot.com/2048933004/diff/1/service/datastore/in...
service/datastore/interface.go:145: //	  be non-nil, and its underlying type
must be either *S or *P.
commas for these too?

https://chromiumcodereview.appspot.com/2048933004/diff/1/service/datastore/in...
service/datastore/interface.go:212: //	  IDs will NOT be written back.
whoa why? Even if it implements MetaGetterSetter?

https://chromiumcodereview.appspot.com/2048933004/diff/1/service/datastore/in...
service/datastore/interface.go:215: //	- []I where i is some interface type.
Each element of the slice must
and everywhere

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

https://chromiumcodereview.appspot.com/2048933004/diff/1/service/datastore/pl...
service/datastore/pls_test.go:1817: "cannot GetPLS(*[]string): pointer-to-struct
is nil")
hm, I think the spirit of this test was lost?

[dnj (Google), 2016-06-14 01:49:43]

https://chromiumcodereview.appspot.com/2048933004/diff/1/service/datastore/in...
File service/datastore/interface.go (right):

https://chromiumcodereview.appspot.com/2048933004/diff/1/service/datastore/in...
service/datastore/interface.go:145: //	  be non-nil, and its underlying type
must be either *S or *P.
On 2016/06/14 00:19:51, iannucci wrote:
> commas for these too?

Done.

https://chromiumcodereview.appspot.com/2048933004/diff/1/service/datastore/in...
service/datastore/interface.go:212: //	  IDs will NOT be written back.
On 2016/06/14 00:19:51, iannucci wrote:
> whoa why? Even if it implements MetaGetterSetter?

oops, that's leftover from when they were structs.

https://chromiumcodereview.appspot.com/2048933004/diff/1/service/datastore/in...
service/datastore/interface.go:215: //	- []I where i is some interface type.
Each element of the slice must
On 2016/06/14 00:19:51, iannucci wrote:
> and everywhere

Done.

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

https://chromiumcodereview.appspot.com/2048933004/diff/1/service/datastore/pl...
service/datastore/pls_test.go:1817: "cannot GetPLS(*[]string): pointer-to-struct
is nil")
On 2016/06/14 00:19:51, iannucci wrote:
> hm, I think the spirit of this test was lost?

Hah true that. Added separate tests for "not a pointer-to-struct" and "pointer
is nil".

[dnj (Google), 2016-07-01 01:42:16]

The CQ bit was checked by dnj@google.com

[dnj (Google), 2016-07-01 01:42:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org
Link to the patchset: https://codereview.chromium.org/2048933004/#ps40001
(title: "Rebarse?")

[commit-bot: I haz the power, 2016-07-01 01:42:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-01 01:48:21]

Description was changed from

==========
Refactor multiarg, split MGS/PLS.

Refactor the multiarg construction method to handle the various cases
inline, rather than defining and invoking methods for specific
scenarios. This enables PropertyLoadSaver-based methods to be handled
separately from MetaGetterSetter-based methods, which, in turn, enables
the two to be implemented independently.

A bit of boilerplate is removed and the odd case of MGS/PLS Map/Chan fits
better. Additionally, a few double-reflection code paths are optimized
along the way.

BUG=None
==========

to

==========
Refactor multiarg, split MGS/PLS.

Refactor the multiarg construction method to handle the various cases
inline, rather than defining and invoking methods for specific
scenarios. This enables PropertyLoadSaver-based methods to be handled
separately from MetaGetterSetter-based methods, which, in turn, enables
the two to be implemented independently.

A bit of boilerplate is removed and the odd case of MGS/PLS Map/Chan fits
better. Additionally, a few double-reflection code paths are optimized
along the way.

BUG=None

Committed:
https://github.com/luci/gae/commit/f11b1de0154fa1f3dc0f06ff77fba94949123326
==========

[commit-bot: I haz the power, 2016-07-01 01:48:21]

Committed patchset #3 (id:40001) as
https://github.com/luci/gae/commit/f11b1de0154fa1f3dc0f06ff77fba94949123326