Page MenuHomePhabricator

Diffusion - be sure to properly unserialize result from conduit query
ClosedPublic

Authored by btrahan on Feb 17 2015, 9:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Sep 1, 1:46 PM
Unknown Object (File)
Wed, Aug 28, 12:28 PM
Unknown Object (File)
Sat, Aug 24, 7:43 AM
Unknown Object (File)
Mon, Aug 19, 3:11 AM
Unknown Object (File)
Thu, Aug 15, 11:40 AM
Unknown Object (File)
Wed, Aug 14, 1:07 AM
Unknown Object (File)
Sun, Aug 11, 5:06 PM
Unknown Object (File)
Sun, Aug 11, 5:04 PM
Subscribers

Details

Summary

Fixes T7256.

Test Plan

Looked at rXPRF0a7a5f69f5d7 in a local instance. things looked great both pre and post patch.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

btrahan retitled this revision from to Diffusion - be sure to properly unserialize result from conduit query.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
epriestley edited edge metadata.

Hrrm -- I would expect this to never work, because the Git and Mercurial pathways in DiffusionMergedCommitsQueryConduitAPIMethod end with:

return DiffusionQuery::loadHistoryForCommitIdentifiers(...)

..and it looks like that always returns objects (DiffusionPathChange objects). Then this will try to decode them, so I'd expect that it breaks the working case on its own.

In particular, it looks like this method does not work when called over HTTP currently (e.g., callsign=XPRF, commit=0a7a5f69f5d7 in the console on this install), so I think DiffusionMergedCommitsQueryConduitAPIMethod needs some return mpull($objects, 'toDictionary'); sort of stuff? Or am I crazy?

This revision now requires changes to proceed.Feb 17 2015, 9:36 PM
btrahan edited edge metadata.

serialize in the conduit endpoint

This revision is now accepted and ready to land.Feb 17 2015, 9:54 PM
btrahan edited edge metadata.
This revision was automatically updated to reflect the committed changes.