Commit Graph

668 Commits

Author SHA1 Message Date
Arun Kulshreshtha
6b67d820bd auth: remove use of unwrap
Reviewed By: quark-zju

Differential Revision: D22467292

fbshipit-source-id: d645d437a3dc80b1a7f29841067aa05b0e48df17
2020-07-09 19:05:55 -07:00
Arun Kulshreshtha
14a7fe636f cpython-ext: Add ExtractInnerRef trait
Summary: Per comments on D22429347, add a new `ExtractInnerRef` trait that is similar to `ExtractInner`, but returns a reference to the underlying value. A default implementation is provided for types whose inner value is `Clone + 'static`, so in practice most types will only need to implement `ExtractInnerRef`, whereas the callsite may choose whether it needs a reference or an owned value.

Reviewed By: quark-zju

Differential Revision: D22464158

fbshipit-source-id: 7b97329aedcddb0e51fd242b519e79eba2eed350
2020-07-09 19:05:55 -07:00
Arun Kulshreshtha
5cb7bdd3c0 edenapi: use EdenApiError as error type for StatsFuture
Summary: Ensure that all of the components of an EdenAPI response use the same error type.

Reviewed By: quark-zju

Differential Revision: D22443029

fbshipit-source-id: 3e00a8b83677beb5ef2d90630fe9b85760874186
2020-07-09 19:05:55 -07:00
Arun Kulshreshtha
cb16831e6d revisionstore: add add_entry method to HgIdMutableDeltaStore
Summary: Add an `add_entry` convenience method to `HgIdMutableDeltaStore`, similar to the one present in `HgIdMutableHistoryStore`.

Reviewed By: quark-zju

Differential Revision: D22443031

fbshipit-source-id: 84fdaae9fbd51e6f2df466b0441ec5f7ce6715f7
2020-07-09 19:05:55 -07:00
Arun Kulshreshtha
7ae097e8da cpython-ext: add ExtractInner trait
Summary:
A common pattern in Mercurial's data storage layer Python bindings is to have a Python object that wraps a Rust object. These Python objects are often passed across the FFI boundary to Rust code, which then may need to access the underlying Rust value.

Previously, the objects that used this pattern did so in an ad-hoc manner, typically by providing an `into_inner` or `to_inner` inherent method. This diff introduces a new `ExtractInner` trait that standardizes this pattern into a single interface, which in turn allows this pattern to be used with generics.

Reviewed By: quark-zju

Differential Revision: D22429347

fbshipit-source-id: cab4c24b8b98c6ef8307f72a9b4726aabdc829cc
2020-07-09 19:05:55 -07:00
Arun Kulshreshtha
9c9d27d95f edenapi: reuse HttpClient
Summary:
D22396026 made it so that `HttpClient::send_async` no longer consumes `self`. This means that instead of creating a new HTTP client for each request, we can reuse the same one.

This has the benefit of allowing for connection reuse (which was the point of D22396026), resulting in lower latency for serial fetches.

Reviewed By: quark-zju

Differential Revision: D22397768

fbshipit-source-id: 9d066c1ec64a6aa1b36ec674ef294030c1f90b41
2020-07-09 13:08:28 -07:00
Arun Kulshreshtha
ce69772ec7 edenapi: support sending serial requests from CLI
Summary: Allow passing multiple JSON requests to the EdenAPI CLI. The requests will be performed serially, which allows for testing the performance of serial EdenAPI calls.

Reviewed By: quark-zju

Differential Revision: D22397769

fbshipit-source-id: c59e5abf53eee9c2014010672183e202b6f180fc
2020-07-09 13:08:27 -07:00
Arun Kulshreshtha
0b05d4aefe http_client: add pool for Multi handles
Summary:
Add a pool of `Multi` handles that the client can reuse across requests.

Previously, `HttpClient`'s async functions had to consume the client in order to have a `'static` lifetime (since `Future`s generally cannot hold references to things outside of themselves). This meant that the each async operation would use its own `Multi` handle, preventing connection reuse across operations since the `Multi` handle maintains a connection cache internally.

With this change, the client can reuse the `Multi` session after an async operation, thereby benefitting from libcurl's caching. Note that the same `Multi` handle still cannot be used by concurrently running `Future`s (as this [would not be thread safe](https://curl.haxx.se/libcurl/c/threadsafe.html)), but once a `Future` has completed its `Multi` handle will return to the pool for use by subsequent requests.

 ---
(Somewhat tangential)

As is noted in the code comments, `libcurl`'s C API provides a way to share caches across multiple multi sessions: [the "share" interface](https://curl.haxx.se/libcurl/c/libcurl-share.html).

While using this would seems preferable to an ad-hoc solution like this diff, it turns out that the `curl` crate does not provide safe bindings to the share interface. This means that in order to use the share interface, we'd need to directly use the unsafe bindings from `curl-sys`.

In addition to the difficulty of working with unsafe FFI code, the API expects the application to handle synchronization by passing it function pointers to handle locking/unlocking shared resources.

Ultimately, I came to the conclusion that managing lifetimes and synchronization in unsafe code across an FFI boundary would be nontrivial, and ensuring correctness would require a lot of effort that could be avoided by implementing an ad-hoc solution on top of the safe API instead. However, it might make sense to change this to use the share interface in the future.

Reviewed By: quark-zju

Differential Revision: D22396026

fbshipit-source-id: 06eea2ffacdc791527eac9ce4becc457af5c0480
2020-07-09 13:08:27 -07:00
Arun Kulshreshtha
1323b17436 edenapi: delete old client
Summary: Delete the old EdenAPI client.

Reviewed By: quark-zju

Differential Revision: D22379475

fbshipit-source-id: 99f76ea170ec9db8d79727fbdfd441afd2de3899
2020-07-09 13:08:27 -07:00
Arun Kulshreshtha
d1d3224ba1 revisionstore: use new EdenAPI crate
Summary: Update the `revisionstore` and `backingstore` crates to use the new EdenAPI crate.

Reviewed By: quark-zju

Differential Revision: D22378330

fbshipit-source-id: 989f34827b744ff4b4ac0aa10d004f03dbe9058f
2020-07-09 13:08:27 -07:00
Arun Kulshreshtha
41e68f46d3 edenapi: add blocking API
Summary: Add a new `EdenApiBlocking` trait that exposes blocking versions of the `EdenApi` trait's methods, for use in non-async code.

Reviewed By: quark-zju

Differential Revision: D22305396

fbshipit-source-id: d0f3a73cad1a23a4f0892a17f18267374e63108e
2020-07-09 13:08:27 -07:00
Arun Kulshreshtha
30a6cad591 edenapi: add EdenAPI testing CLI
Summary:
This diff adds an EdenAPI CLI program that allows manually sending requests to the server.

Requests are read from stdin in a JSON format (the same format used by the `make_req` tool and the EdenAPI server integration tests). This makes it easy to create and edit requests during debugging.

Responses are re-serialized as CBOR and written to stdout. (The program will refuse to write output if stdout is a TTY.) These responses can then be analyzed using the `read_res` tool (also used by the EdenAPI server integration tests).

The program prints real-time download statistics during data fetching, allow the user to debug performance in addition to correctness.

The program uses standard `hgrc` files to configure the EdenAPI client, which means that one can simulate production settings by specifying a production `hgrc`. By default, it will read from `~/.hgrc.edenapi` rather than `~/.hgrc` since the user will most likely want to configure this program independently of Mercurial.

Reviewed By: quark-zju

Differential Revision: D22370163

fbshipit-source-id: 5d9974bc05fa960d26cd2c87810f4646e2bc55b4
2020-07-09 13:08:27 -07:00
Viet Hung Nguyen
b7817ffbd8 xdiff: renamed third-party xdiff functions
Summary:
Renamed xdiff functions to avoid linking issues when using both libgit2-sys and xdiff.

When using repo_import tool (https://fburl.com/diffusion/8p6fhjt2) we have libgit2-sys dependency for importing git repos. However, when we derive blame data types, we need to use xdiff functionalities (from_no_parents: https://fburl.com/diffusion/pitukmyo -> diff_hunks: https://fburl.com/diffusion/9f8caan9 -> xdl_diff: https://fburl.com/diffusion/260x66hf). Both libgit2 and eden/scm have vendored versions of xdiff library. Therefore, libgit2-sys and eden/scm share functions with the same signatures, but have different behaviours and when we tried to derive blame, it used libgit2-sys's xdl_diff instead of eden's. This resulted in getting segfaults (https://fburl.com/paste/04gwalpo).
Note: repo_import is the first tool that has tried to import both and the first to run into this issue.

Reviewed By: StanislavGlebik

Differential Revision: D22432330

fbshipit-source-id: f2b965f3926a2dc45de1bf20e41dad70ca09cdfd
2020-07-09 01:20:32 -07:00
Arun Kulshreshtha
fdb8859422 edenapi: add new EdenAPI client
Summary:
This diff is a complete, ground-up rewrite of the EdenAPI client. Rather than attempting to use `libcurl` directly, it relies on the new `http_client` crate, which makes the code considerably simpler and allows for a proper async interface.

The most notable change is that `EdenApi` is now an async trait. A blocking API is added later in the stack for use in non-async contexts.

Reviewed By: quark-zju

Differential Revision: D22305397

fbshipit-source-id: 4c1e5d3091d6dd04cf13291e7b7a4217dfdd249f
2020-07-08 12:51:04 -07:00
Arun Kulshreshtha
014f1a5289 http_client: add buffering to CborStream
Summary:
As was pointed out in the review for D22280745 (d73c63d862), `CborStream` is inefficient in situations where the underlying stream produces chunks that are much smaller than the size of the serialized items. To avoid pathological behavior, make `CborStream` buffer the incoming data, and only attempt deserialization if enough data has accumulated.

For now, the buffer size is fixed (with a default of 1MB, chosen arbitrarily). In the future, it might make sense to have the stream adjust the buffer size based on the average size of observed deserialized values.

Reviewed By: quark-zju

Differential Revision: D22370164

fbshipit-source-id: ed940c56ca2cbbfc07f01d47becf6f1d71872872
2020-07-08 12:51:04 -07:00
Jun Wu
e440d3ce2b revlogindex: update nodemap even if it's non-symlink and mmaped on Windows
Summary: On Windows a mmap file cannot be replaced. Detect that and delete manually.

Reviewed By: farnz

Differential Revision: D22428731

fbshipit-source-id: 4d308a07aae02dcaf2aedb7b0267a535c2e09c92
2020-07-08 11:31:21 -07:00
Lukas Piatkowski
0eb9d79a46 rust/reindeer: update eventsource to 0.5
Summary: The 0.3 version (currently being used only in one crate eden/scm/lib/commitcloudsubscriber) is using an old openssl crate which doesn't work with openssl library installed on most machines (Both in FB and on GitHub Actions).

Reviewed By: mitrandir77

Differential Revision: D22430649

fbshipit-source-id: b8fa930841dbcdd4c085d8c9488d768b3526e1c4
2020-07-08 04:15:21 -07:00
Arun Kulshreshtha
5f0181f48c Regenerate all Cargo.tomls after upgrade to futures 0.3.5
Summary: D22381744 updated the version of `futures` in third-party/rust to 0.3.5, but did not regenerate the autocargo-managed Cargo.toml files in the repo. Although this is a semver-compatible change (and therefore should not break anything), it means that affected projects would see changes to all of their Cargo.toml files the next time they ran `cargo autocargo`.

Reviewed By: dtolnay

Differential Revision: D22403809

fbshipit-source-id: eb1fdbaf69c99549309da0f67c9bebcb69c1131b
2020-07-06 20:49:43 -07:00
Xavier Deguillard
48d97384d0 revisionstore: fix typo in memcache trace
Summary:
The key is "hit_count" not "hits". This typo caused the trace to always claim
that no data was fetched from memcache, which is obviously not true as the
getpack trace that follows listed significantly less requested keys.

Reviewed By: kulshrax

Differential Revision: D22401592

fbshipit-source-id: ab2ea3e7f8ff3a9c7322678afc8a174e09d6dc09
2020-07-06 17:57:16 -07:00
Jun Wu
b1ae5b2874 revlogindex: skip flushing duplicated entries
Summary:
If the revlog on disk was changed to include new commits, read them and avoid
writing duplicated commits (which breaks nodemap building).

Reviewed By: sfilipco

Differential Revision: D22323187

fbshipit-source-id: cdd65f31e65865d9f3868e43416633297896c0f9
2020-07-06 15:51:00 -07:00
Jun Wu
e53be6d0fc hgcommits: make HgCommit serializable
Summary: This is used by the next diff.

Reviewed By: sfilipco

Differential Revision: D21944139

fbshipit-source-id: 184c4e97aaeca36c3608665defd1473c9300fb5b
2020-07-06 15:51:00 -07:00
Jun Wu
132f046f11 hgcommits: add revlog-based commits
Summary: Use `RevlogIndex` to implement the HgCommits interface.

Reviewed By: sfilipco

Differential Revision: D21854226

fbshipit-source-id: 05ff242858ac879d3b40b35ba8db5044135604be
2020-07-06 15:50:59 -07:00
Jun Wu
fa62891e30 hgcommits: add in-memory version of HgCommits
Summary: This will satisfy some use-cases.

Reviewed By: sfilipco

Differential Revision: D21854225

fbshipit-source-id: 76758716b35cfd31dc3843c118917c0fb7609027
2020-07-06 15:50:59 -07:00
Jun Wu
ea67a2168e hgcommits: new crate for hybrid commit data + dag structure
Summary: This will help move more Python logic to Rust.

Reviewed By: sfilipco

Differential Revision: D21854224

fbshipit-source-id: b03cbacedc11d77e8c56262437a8d10bd9a89e59
2020-07-06 15:50:59 -07:00
Jun Wu
a0c5b1b3a5 revlogindex: is_ancestor(x, x) should return true
Summary: This is discovered by using it in Python world.

Reviewed By: sfilipco

Differential Revision: D22323186

fbshipit-source-id: 295811e0950b94ad2ad73ad242228b6a3f9765d0
2020-07-06 15:50:59 -07:00
Jun Wu
52668752d8 revlogindex: de-duplicate insertions
Summary: Adding a same commit multiple times is a no-op.

Reviewed By: sfilipco

Differential Revision: D22323190

fbshipit-source-id: 61a06335581a9cad32dc7e929b841ec69b551a9c
2020-07-06 15:50:59 -07:00
Jun Wu
b86f3bd6e2 revlogindex: use tests from the dag crate
Summary: This adds some test coverage for the revlog DagAlgorithm implementation.

Reviewed By: sfilipco

Differential Revision: D22249157

fbshipit-source-id: a1d347b4d90d0e7f8fb229c317cc75c2b8e16242
2020-07-06 15:50:59 -07:00
Jun Wu
fcbe821dd1 revlogindex: impl DagAddHeads for RevlogIndex
Summary:
This makes RevlogIndex compatible with the generic DAG testing API from the dag
crate.

Reviewed By: sfilipco

Differential Revision: D22249156

fbshipit-source-id: 54a3c458e85804968964174eab674e494a6fa8a2
2020-07-06 15:50:59 -07:00
Jun Wu
cf1bc37007 dag: avoid using > 2 parents in generic DAG tests
Summary: Some DAG implementations does not support it.

Reviewed By: sfilipco

Differential Revision: D22249158

fbshipit-source-id: ebcdf164677ee647ef44aa1ee3cfd318bac658b0
2020-07-06 15:50:59 -07:00
Jun Wu
9a17be7ce0 dag: do not test the order of vertexes in generic tests
Summary:
Different implementation might return different orders. They should be
considered correct.

Reviewed By: sfilipco

Differential Revision: D22249159

fbshipit-source-id: 36e4cadf814366f7ee2ed8a778948ff810760550
2020-07-06 15:50:58 -07:00
Jun Wu
f24dc621cb dag: make part of the tests generic
Summary: This makes it possible to run tests for other DAGs, like the revlog.

Reviewed By: sfilipco

Differential Revision: D22249155

fbshipit-source-id: 205579eeaccd42a21297d965973957168bb8726e
2020-07-06 15:50:58 -07:00
Jun Wu
5d9baa2f07 revlogindex: implement fast path for only
Summary:
For revlog, calculating `only` can have some fast paths that do not scan the
entire changelog.

Reviewed By: sfilipco

Differential Revision: D21944136

fbshipit-source-id: 58391636350f8f19643d59c46d663f55861d6de3
2020-07-06 15:50:58 -07:00
Jun Wu
7d75f6046f revlogindex: implement fast path for only_both
Summary:
This will be used to maintain narrow-heads phase calculation and sunsetting the
revlog-specific changelog.index2.

Reviewed By: sfilipco

Differential Revision: D21944131

fbshipit-source-id: a8bbd1fd24546f4891ffa677476bff750c3faf5f
2020-07-06 15:50:58 -07:00
Jun Wu
40cd0f8f06 revlogindex: fix an offset-by-one error
Summary: The values of `pending_nodes_index` should start from 0 instead of 1.

Reviewed By: sfilipco

Differential Revision: D21944133

fbshipit-source-id: a2a332868f16b398037289c81bf8076d1400c0a7
2020-07-06 15:50:58 -07:00
Jun Wu
bd0a35f2a0 revlogindex: do not raise errors on ambiguous prefix
Summary: This matches the interface of segmented changelog.

Reviewed By: sfilipco

Differential Revision: D21944134

fbshipit-source-id: 75f68b2838de4abe95f13cb3c62dc68af132fff7
2020-07-06 15:50:58 -07:00
Jun Wu
4670200c21 revlogindex: maintain revlog.d file handler transparently
Summary:
This drops the `file` parameter from the `raw_data` API, making
RevlogIndex easier to use.

Reviewed By: sfilipco

Differential Revision: D21854228

fbshipit-source-id: 259726524d1cc6a1f9d00783e22f9502c7decdeb
2020-07-06 15:50:58 -07:00
Jun Wu
137fa3cd34 revlogindex: implement writing to revlog data
Summary: Extend RevlogIndex to support writing to revlog data.

Reviewed By: sfilipco

Differential Revision: D21854227

fbshipit-source-id: 11b6bf3b706b316f23c33ab07144530c9db92d58
2020-07-06 15:50:58 -07:00
Jun Wu
f005d92f07 revlogindex: implement reading from revlog data
Summary: Extend RevlogIndex to support reading from revlog data.

Reviewed By: sfilipco

Differential Revision: D21854229

fbshipit-source-id: 4cbc08762fd236a97370d5d55c59a222f935b262
2020-07-06 15:50:58 -07:00
Jun Wu
2bc4dd01ca dag: add a trait to convert IdSet to Set
Summary:
The reverse `to_id_set` exists.
It turns out that the Python land wants this in many places.

Reviewed By: sfilipco

Differential Revision: D22240175

fbshipit-source-id: b6a3a3a3869dc0c521a21b1d86394421b816632b
2020-07-06 15:50:58 -07:00
Jun Wu
07b3d60f80 dag: add "only(x, y)" to DagAlgorithm
Summary:
This provides a way for implementations to optimize the operation.

For segmented changelog, the default implementation is good enough.

For revlog, `only` can have a fast path that does not iterate through the
entire changelog.

A related API `only_both` is added. For revlog it has multiple use-cases,
including narrow-heads phase calculation and revlog.findcommonmissing used by
discovery.

Reviewed By: markbt

Differential Revision: D21944132

fbshipit-source-id: d11660dae85ea6158977eb00d1ceaceddf1d8234
2020-07-06 15:50:57 -07:00
Arun Kulshreshtha
7ae5344bb2 edenapi_types: improve DataEntry hash check API
Summary: Use `thiserror` to provide a more ergonomic API for `DataEntry` hash checking. The `.data()` method now simply returns a `Result` rather than a tuple with an ad-hoc enum.

Reviewed By: quark-zju

Differential Revision: D22376164

fbshipit-source-id: fc39cb212ec1ee5830292db4aa5eca18f2c16a2b
2020-07-06 14:47:48 -07:00
Arun Kulshreshtha
1b5283aa5a edenapi_types: improve comments
Summary: Tidy up some comments in this file.

Reviewed By: ikostia

Differential Revision: D22376165

fbshipit-source-id: ce4760776048aa8e72809b4f828d0ea426fcf878
2020-07-03 12:29:19 -07:00
Arun Kulshreshtha
dc98c085ad edenapi_types: split redaction tombstone string across multiple lines
Summary: Make this line less long.

Reviewed By: ikostia

Differential Revision: D22372492

fbshipit-source-id: cfc1ab6a296aa2056a908bf786e4f498f3a688b4
2020-07-03 12:15:01 -07:00
Lukas Piatkowski
c763ab4b40 eden/scm: provide getdeps.py way of building eden/scm on GitHub
Summary:
In order to do what the title says, this diff does:
1. Add the `eden/oss/.../third-party/rust/.../Cargo.toml` files. As mentioned in the previous diff, those are required by GitHub so that the third party dependencies that are local in fbsource are properly defined with a "git" dependency in order for Cargo to "link" crates properly.
2. Changes to `eden/scm/Makefile` to add build/install commands for getdeps to invoke. Those command knowing that they are called from withing getdeps context they link the dependencies brought by getdeps into their proper places that match their folder layout in fbsource. Those Makefile commands also pass a GETDEPS_BUILD env to the setup.py invocations so that it knows it is being called withing a getdeps build.
3. Changes to `eden/scm/setup.py` that add "thriftasset" that makes use of the getdeps.py provided "thrift" binary to build .py files out of thrift files.
4. Changes to `distutils_rust` to use the vendored crates dir provided by getdeps.
5. Changes to `getdeps/builder.py` and `getdeps/manifest.py` that enable more fine-grained configuratior of how Makefile builds are invoked.
6. Changes to `getdeps/buildopts.py` and `getdeps/manifest.py` to disable overriding PATH and pkgconfig env, so that "eden/scm" builds in getdeps using system libraries rather than getdeps-provided ones (NOTE: I've tried to use getdeps provided libraries, but the trickiest bit was that Rust links with Python, which is currently not providable by getdeps, so if you try to build everything the system provided Python libraries will collide with getdeps provided ones)
7. Added `opensource/fbcode_builder/manifests/eden_scm` for the getdeps build.

Reviewed By: quark-zju

Differential Revision: D22336485

fbshipit-source-id: 244d10c9e06ee83de61e97e62a1f2a2184d2312f
2020-07-02 17:53:37 -07:00
Jun Wu
868c2b0108 mutationstore: copy entries automatically on flush
Summary:
Similar to D7121487 (af8ecd5f80) but works for mutation store. This makes sure at the Rust
layer, mutation entries won't get lost after rebasing or metaeditting a set of
commits where a subset of the commits being edited has mutation relations.

Unlike the Python layer, the Rust layer works for mutation chains. Therefore
some of the tests changes.

Reviewed By: markbt

Differential Revision: D22174991

fbshipit-source-id: d62f7c1071fc71f939ec8771ac5968b992aa253c
2020-07-02 13:22:34 -07:00
Jun Wu
2f1d35b06e mutationstore: break cycles for get_dag
Summary: Avoids infinite loops creating the DAG.

Reviewed By: markbt

Differential Revision: D22174978

fbshipit-source-id: ec54665a9d5b88a97fce988456041f9aabc824d6
2020-07-02 13:22:34 -07:00
Jun Wu
d647045a5f mutation: lift "1:1" restriction for get_dag
Summary:
Enforcing 1:1 handling is causing trouble for multiple reasons:
- Test cases like "Metaedit with descendant folded commits" in
  test-mutation.t requires handling non-1:1 entries.
- Currently operations like `metaedit-copy` uses two predecessors (edited,
  copied). It is considered 1:1 rewrite (rewriting 1 commit to
  another 1 commit). However, it does not use multiple records, therefore
  cannot be distinguished from `fold`, which is not 1:1 rewrite and also
  has multiple predecessors. We want to include the `metaedit-copy`
  entries in the 1:1 DAG too.

Therefore lift the 1:1 restriction and let's see how it works.

Reviewed By: markbt

Differential Revision: D22175008

fbshipit-source-id: 84d870dbcc433a0d0e5611a83c93781bfa59d035
2020-07-02 13:22:34 -07:00
Jun Wu
d745424bf9 dag: add a utility to help break cycles
Summary:
This makes it easier to remove cycles in other places.

There are probably fancier and more efficient algorithm for this.
For now I just wrote one that is easy to verify correctness.

Reviewed By: markbt

Differential Revision: D22174975

fbshipit-source-id: 8a2dc755e4bc0b066eda5f42a51208c92409f2f9
2020-07-02 13:22:34 -07:00
Arun Kulshreshtha
b9d35dc854 edenapi_types: add ToJson trait
Summary: Add a `ToJson` trait as a counterpart to the `FromJson` trait introduced in the last diff. The primary use case for this will be to allow recording and replaying the data fetches that occur during Mercurial operations. A JSON representation was chosen so that the format will be directly compatible with tools like `make_req` used in EdenAPI integration tests.

Reviewed By: markbt

Differential Revision: D22344599

fbshipit-source-id: 52c888bde93a8e86b6dd76cb862337f716b007eb
2020-07-02 13:11:48 -07:00
Arun Kulshreshtha
58984e1378 edenapi_types: add FromJson trait
Summary:
Add a `FromJson` trait to `edenapi_types` and use it instead of the parsing functions when parsing requests from JSON.

Some design commentary:

I've created a custom trait rather than using `TryFrom<serde_json::Value>` for two reasons:

- From a design standpoint, I'd like users to have to explicitly opt-in to this functionality by importing this `FromJson` trait, since this is different from the usual way of deserializing with serde, and it might cause confusion.
- From an implementation standpoint, it turns out that using `TryFrom` in a trait bound causes difficulties with type inference in some situations (in particular, around the associated `Error` type), which necessitates type annotations, making the API less ergonomic to use.

Why not just use `serde::Deserialize` directly?

- The representation here doesn't actually directly match the structure of the underlying type. Instead, the JSON representation has been designed to be easy for humans to edit when writing tests. (For example, keys are replaced with simply arrays, and hashes are represented as hex rather than as byte arrays.)

- In this case, we'd like to work with `serde_json::Value` rather than going directly between bytes and structs, since this allows for a level of dynamism that will be useful later in the stack.

Reviewed By: markbt

Differential Revision: D22320576

fbshipit-source-id: 64b6bed42e1ec599a0da61ae5d55feb7c90101a4
2020-07-02 13:11:48 -07:00