From 1370df9d7c2e6d656f50332b3f8615faafacead0 Mon Sep 17 00:00:00 2001 From: jrconlin Date: Fri, 31 Jul 2020 18:01:18 +0000 Subject: [PATCH 01/17] add debug_client check to BsoBodies for batch operations. Issue #757 --- src/web/extractors.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/web/extractors.rs b/src/web/extractors.rs index bc8bac010c..c93abd85e6 100644 --- a/src/web/extractors.rs +++ b/src/web/extractors.rs @@ -227,6 +227,34 @@ impl FromRequest for BsoBodies { )); } }; + + // ### debug_client + if let Some(uids) = &state.limits.debug_client { + for uid in uids.split(',') { + debug!("### checking uaid: {:?}", &uid); + match u64::from_str(uid.trim()) { + Ok(v) => { + if v == HawkIdentifier::uid_from_path(req.uri(), None).unwrap_or(0) { + debug!("### returning quota exceeded."); + error!("Returning over quota for {:?}", v); + return Box::pin(future::err( + ValidationErrorKind::FromDetails( + "size-limit-exceeded".to_owned(), + RequestErrorLocation::Unknown, + Some("size-limit-exceeded".to_owned()), + None, + ) + .into(), + )); + } + } + Err(_) => { + debug!("{:?} is not a u64", uid); + } + }; + } + } + let max_payload_size = state.limits.max_record_payload_bytes as usize; let max_post_bytes = state.limits.max_post_bytes as usize; From 655c02e501637ba9cd3d786d2ddcc6df8fc88204 Mon Sep 17 00:00:00 2001 From: Rachel Tublitz Date: Tue, 4 Aug 2020 11:58:54 -0400 Subject: [PATCH 02/17] chore: tag 0.5.4 release (#760) --- CHANGELOG.md | 117 +++++++++++++++++++++++++++++++++++++++++++++++++++ Cargo.lock | 2 +- Cargo.toml | 2 +- 3 files changed, 119 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a2561c217..985af597ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,120 @@ + +### 0.5.4 (2020-08-04) + + +#### Chore + +* Update vendored SDK to use protobuf 2.16.2 (#747) ([39519bb8](https://github.com/mozilla-services/syncstorage-rs/commit/39519bb821fdf58ecf5842c6b818a58d53167135)) +* Update Docker rust to 1.45 (#734) ([538abe4b](https://github.com/mozilla-services/syncstorage-rs/commit/538abe4badf7a17200cd1400ed85b0504dadc865)) +* tag 0.4.2 ([c5d5d7dd](https://github.com/mozilla-services/syncstorage-rs/commit/c5d5d7ddd9c2dc6a39f4423956938662a2cb1f4b)) +* tag 0.4.1 ([0d140d7a](https://github.com/mozilla-services/syncstorage-rs/commit/0d140d7ac30b2867498968b5719d6e49b644e864)) +* tag 0.4.0 ([17787c50](https://github.com/mozilla-services/syncstorage-rs/commit/17787c50aafc0e55efa7f827531235273b953cd0)) +* tag 0.3.4 ([de9ccede](https://github.com/mozilla-services/syncstorage-rs/commit/de9ccede5631d2e446da053aa97b8141181ea7c6)) +* tag 0.3.3 ([a1ebf297](https://github.com/mozilla-services/syncstorage-rs/commit/a1ebf2976af916a6189716d32f7b2c0bf5e4fe56)) +* tag 0.3.2 ([ddababa5](https://github.com/mozilla-services/syncstorage-rs/commit/ddababa5a85da1ea0942ee52a11e1c8a875bc1a4)) +* cargo fmt/clippy ([c17682fa](https://github.com/mozilla-services/syncstorage-rs/commit/c17682fa464c89faea4cb2e384a6c8747834d2dc)) +* tag 0.3.1 ([ba6aeb00](https://github.com/mozilla-services/syncstorage-rs/commit/ba6aeb00a93865016bc852c088969003820a7cca)) +* default-run syncstorage ([24b600dd](https://github.com/mozilla-services/syncstorage-rs/commit/24b600dd45b883563d06a2545f8c305ad1331fd3)) +* tag 0.3.0 ([0fab270b](https://github.com/mozilla-services/syncstorage-rs/commit/0fab270b97b788658992a9ca53a65c532eec621d)) +* remove unused dependencies ([382f342a](https://github.com/mozilla-services/syncstorage-rs/commit/382f342a4c95641e8de1c0700648c922a6abc095)) +* Update dependencies 2020-03 ([7825ead1](https://github.com/mozilla-services/syncstorage-rs/commit/7825ead15313c50fcb41d2a48c0f13245a5c6024), closes [#537](https://github.com/mozilla-services/syncstorage-rs/issues/537)) +* move `insert into` to the bottom of ddl ([0203261e](https://github.com/mozilla-services/syncstorage-rs/commit/0203261ea6967bf5bda7a6284e1c3fc5edcd1238), closes [#473](https://github.com/mozilla-services/syncstorage-rs/issues/473)) +* remove custom async_test implementation ([3cbc3a1c](https://github.com/mozilla-services/syncstorage-rs/commit/3cbc3a1cf1f0137c8d23c8592b5ac805151413e9), closes [#461](https://github.com/mozilla-services/syncstorage-rs/issues/461)) +* re-add gcp-grpc deps setup ([aa7495d9](https://github.com/mozilla-services/syncstorage-rs/commit/aa7495d9151950431c5f67a5c61e16bdf02efcae)) +* kill checkout-gcp-grpc ([625a1c9f](https://github.com/mozilla-services/syncstorage-rs/commit/625a1c9f8b3e6779352dd97d5bffeaaff5df45ee)) +* add minumum supported rust version ([9740221a](https://github.com/mozilla-services/syncstorage-rs/commit/9740221aea93f4872e6369522aa55f0a93c3742a)) +* add a badge for matrix ([cd23e152](https://github.com/mozilla-services/syncstorage-rs/commit/cd23e15288ba6f9295ab7d0083b21edbdaa464b6)) +* Update to actix-web 2.0. ([a79434a9](https://github.com/mozilla-services/syncstorage-rs/commit/a79434a9e721f639bdda339bc601dc152451a1bb)) + +#### Doc + +* update per sentry dev's rename to local (#628) ([456c857d](https://github.com/mozilla-services/syncstorage-rs/commit/456c857dc06192d671516bd17f474d59f51cae30)) +* Update instructions for running syncstorage-rs via Docker (#624) ([eb5fa003](https://github.com/mozilla-services/syncstorage-rs/commit/eb5fa003d183b81b146c12afd498e8bf3555f334)) +* fix typos in README.md files Fixed typos in README.md files to improve readiblity. ([7da2154b](https://github.com/mozilla-services/syncstorage-rs/commit/7da2154bcc2bc7618bf414d60212c2c2d2cfac5a), closes [#529](https://github.com/mozilla-services/syncstorage-rs/issues/529)) +* fix URL rendering in README ([bcb0e2e2](https://github.com/mozilla-services/syncstorage-rs/commit/bcb0e2e212554160978f206970e0856508840eb2), closes [#496](https://github.com/mozilla-services/syncstorage-rs/issues/496)) +* add system dependencies to README ([f0183495](https://github.com/mozilla-services/syncstorage-rs/commit/f01834957e5ced9989969f28ff4c3e6f23b2bf29), closes [#255](https://github.com/mozilla-services/syncstorage-rs/issues/255)) + +#### Features + +* force client to rec'v over quota error ([81c00c31](https://github.com/mozilla-services/syncstorage-rs/commit/81c00c31b89c21d20563aef9d31a351a7d581c3c), closes [#746](https://github.com/mozilla-services/syncstorage-rs/issues/746)) +* add metric for db conflicts ([1595f27f](https://github.com/mozilla-services/syncstorage-rs/commit/1595f27f4d4061c610078cb569790a1bdc52fc50)) +* make migrations play nice with existing databases. (#721) ([40b97fc3](https://github.com/mozilla-services/syncstorage-rs/commit/40b97fc331d088462e09cbc5949b961ef5b6d4a5), closes [#663](https://github.com/mozilla-services/syncstorage-rs/issues/663)) +* option to limit purgettl to range of fxa_uids ([695722a9](https://github.com/mozilla-services/syncstorage-rs/commit/695722a9b5286eab62b7f541a3479da5f2dd0a07), closes [#713](https://github.com/mozilla-services/syncstorage-rs/issues/713)) +* limit purge ttl to prior midnight (#708) ([198eb816](https://github.com/mozilla-services/syncstorage-rs/commit/198eb816bc4a090d987aa933b492ec187de1e8e8), closes [#707](https://github.com/mozilla-services/syncstorage-rs/issues/707)) +* add conditions, args to purge_ttl script (#668) ([2a14eb29](https://github.com/mozilla-services/syncstorage-rs/commit/2a14eb2973997e2637ff0894e593642ba9a729f3)) +* build spanner python utils image (#661) ([2060601c](https://github.com/mozilla-services/syncstorage-rs/commit/2060601c483a09c50ae6c7809d5b658980ad3ad8)) +* log messages from middleware to sentry (#604) ([b6ced47a](https://github.com/mozilla-services/syncstorage-rs/commit/b6ced47a39c5932cfc25a37008f78ba03c3e2655), closes [#504](https://github.com/mozilla-services/syncstorage-rs/issues/504)) +* Allow for failure "replay" from failure file (#644) ([b0f1590f](https://github.com/mozilla-services/syncstorage-rs/commit/b0f1590f4a289163b7043d01af06968b082d02ac), closes [#642](https://github.com/mozilla-services/syncstorage-rs/issues/642)) +* Don't report Conflict errors to sentry (#623) ([b2d93418](https://github.com/mozilla-services/syncstorage-rs/commit/b2d9341824d3bb7b722e75a5aaaa2e4096007e20), closes [#614](https://github.com/mozilla-services/syncstorage-rs/issues/614)) +* add async to `delete_all` (#621) ([fdb366da](https://github.com/mozilla-services/syncstorage-rs/commit/fdb366da3837ad74ec7fe6e67ad02c62af790c85), closes [#615](https://github.com/mozilla-services/syncstorage-rs/issues/615)) +* include a hostname tag w/ pool metrics (#627) ([f11c04b5](https://github.com/mozilla-services/syncstorage-rs/commit/f11c04b530ef738703d87b8ea9c882bbfe21df80), closes [#555](https://github.com/mozilla-services/syncstorage-rs/issues/555)) +* emit Db pool metrics periodically (#605) ([1761f7c7](https://github.com/mozilla-services/syncstorage-rs/commit/1761f7c7f1ee40de0563ebca2a23d50b0995fcee), closes [#406](https://github.com/mozilla-services/syncstorage-rs/issues/406)) +* emit Db pool metrics periodically (#605) ([c3d6946e](https://github.com/mozilla-services/syncstorage-rs/commit/c3d6946e041a321fc1e11783a02b767f8e73dbe1), closes [#406](https://github.com/mozilla-services/syncstorage-rs/issues/406)) +* add a --wipe_user mode ([16058f20](https://github.com/mozilla-services/syncstorage-rs/commit/16058f20a42564398f0f27a6adfc686ed774531d), closes [#596](https://github.com/mozilla-services/syncstorage-rs/issues/596)) +* latest ops requests ([edd0017d](https://github.com/mozilla-services/syncstorage-rs/commit/edd0017d2cf7cbade3225fc640d2df8377d55938)) +* Enable circleci remote docker layer caching, speeding up the ci builds. ([7d9d521a](https://github.com/mozilla-services/syncstorage-rs/commit/7d9d521ab675db112f9ec66fe54ba028543c8ead)) +* reject firefox-ios < 20 w/ a 503 ([337275c3](https://github.com/mozilla-services/syncstorage-rs/commit/337275c349c9acaa4965a755ba38126fadd53f38), closes [#293](https://github.com/mozilla-services/syncstorage-rs/issues/293)) +* specify database in user_migration/fix_collections.sql to help running from automation ([cbe3452c](https://github.com/mozilla-services/syncstorage-rs/commit/cbe3452c9d7cc9d968e49b075c8110b65d63fc4e)) +* add `--user_percent` option ([08a646a3](https://github.com/mozilla-services/syncstorage-rs/commit/08a646a36e9d1eda589dd21586ad3b3e4fe41f15)) +* add an extra sanity check of the db url ([f58b3bc9](https://github.com/mozilla-services/syncstorage-rs/commit/f58b3bc9b7bd069fb17090ff8cb440f4126610b5)) +* Add `--abort` and `--user_range` flags ([a65123bc](https://github.com/mozilla-services/syncstorage-rs/commit/a65123bcf2756cf2c6212cb683918c2bd83d692e)) +* more user_migration stuff (#450) ([ecfca9fd](https://github.com/mozilla-services/syncstorage-rs/commit/ecfca9fdf5b040abfa34b0c60daf19e0136adabf)) +* separately metric batch update/insert ([33065a8f](https://github.com/mozilla-services/syncstorage-rs/commit/33065a8f78fa978b990df043c841f663f4682157), closes [#454](https://github.com/mozilla-services/syncstorage-rs/issues/454)) + +#### Test + +* move db-tests back into the main crate (#465) ([f6990853](https://github.com/mozilla-services/syncstorage-rs/commit/f699085363b28bd0ea5c71f6f4231fa1df068fc0), closes [#410](https://github.com/mozilla-services/syncstorage-rs/issues/410)) + +#### Refactor + +* clear new clippy warnings ([d918550a](https://github.com/mozilla-services/syncstorage-rs/commit/d918550a8cf5b72631d79fc2232050418dd101ec)) +* quiet clippy warnings ([b08a90f1](https://github.com/mozilla-services/syncstorage-rs/commit/b08a90f14ab8db1bf1c7dedfc35d59d0fb05d2ee)) +* Convert actix-web frontend *_bso calls to async await (#638) ([7203b8fb](https://github.com/mozilla-services/syncstorage-rs/commit/7203b8fb7f4ccaf6bfbd47cd5d21876ad641f653), closes [#543](https://github.com/mozilla-services/syncstorage-rs/issues/543)) +* convert actix-web front-end calls to async ([300f2852](https://github.com/mozilla-services/syncstorage-rs/commit/300f28524677c0d4200ed3f440ed48f06dd21899), closes [#541](https://github.com/mozilla-services/syncstorage-rs/issues/541), [#541](https://github.com/mozilla-services/syncstorage-rs/issues/541), [#541](https://github.com/mozilla-services/syncstorage-rs/issues/541), [#541](https://github.com/mozilla-services/syncstorage-rs/issues/541), [#541](https://github.com/mozilla-services/syncstorage-rs/issues/541)) +* use u64 instead of i64 for Offset.offset ([8f4f4407](https://github.com/mozilla-services/syncstorage-rs/commit/8f4f4407a6f03d8d3ee90539dff8b8e6836198a1), closes [#414](https://github.com/mozilla-services/syncstorage-rs/issues/414)) +* Remove python dependency from the dockerfile. ([3cd80947](https://github.com/mozilla-services/syncstorage-rs/commit/3cd809474573588471611c0e13e640530cbc588e), closes [#567](https://github.com/mozilla-services/syncstorage-rs/issues/567)) +* rewrite purge_ttl in Rust ([5d6d7c1a](https://github.com/mozilla-services/syncstorage-rs/commit/5d6d7c1a8aef941134aae2ea24a8d3ed0c4a0c15)) +* Convert the rest of the spanner module to async await ([e2017bbc](https://github.com/mozilla-services/syncstorage-rs/commit/e2017bbc2aee60399da2e9b750b7ecce856c4559)) +* Fix #442 Use map_ok in handlers to simplify the code and improve error reporting. ([c50b4cca](https://github.com/mozilla-services/syncstorage-rs/commit/c50b4cca22dc1a6c83757c2c63d719f2753054bf)) +* Fix #453 Convert straggler functions to async await ([69d50d2a](https://github.com/mozilla-services/syncstorage-rs/commit/69d50d2a3cdcf8f2b50acdd20c61743c50c014bc)) +* Fix #435 Convert db batch calls to async await. ([a9eeddb1](https://github.com/mozilla-services/syncstorage-rs/commit/a9eeddb14cdd0ecfc084307d751970656e2f842b)) +* Fix #433 Convert database bso calls to async await ([9279782f](https://github.com/mozilla-services/syncstorage-rs/commit/9279782f607fa87577f49f86a6017515f7c5d2b0)) +* Fix #434 Convert db collectioon calls to async await. ([e0b1c1cd](https://github.com/mozilla-services/syncstorage-rs/commit/e0b1c1cd1d6cfa227554fe670486525b413aa4bf)) + +#### Bug Fixes + +* defer grpc auth to actix-web's thread pool ([7a79fe07](https://github.com/mozilla-services/syncstorage-rs/commit/7a79fe0766790d2e799070046ffa7aa21e06cbd5), closes [#745](https://github.com/mozilla-services/syncstorage-rs/issues/745)) +* avoid unneeded clones ([9c1c19f2](https://github.com/mozilla-services/syncstorage-rs/commit/9c1c19f262afb4057f1bc3473d77bc4c84592d35), closes [#736](https://github.com/mozilla-services/syncstorage-rs/issues/736)) +* switch create_session to async (#733) ([7cd04bc9](https://github.com/mozilla-services/syncstorage-rs/commit/7cd04bc9b4245bfb2ffca5e09de99cf3dd5753a8), closes [#731](https://github.com/mozilla-services/syncstorage-rs/issues/731)) +* remove report_error from the transaction handler ([f0e4c62e](https://github.com/mozilla-services/syncstorage-rs/commit/f0e4c62e3cff366edc9fc798cbe7c94377cc4a8a), closes [#723](https://github.com/mozilla-services/syncstorage-rs/issues/723)) +* Replace batch index to resolve hot table problem (#720) ([c3ca80e6](https://github.com/mozilla-services/syncstorage-rs/commit/c3ca80e66e4084ebc9b6c6efd41dff361b466fb8), closes [#719](https://github.com/mozilla-services/syncstorage-rs/issues/719)) +* don't reject firefox-ios dev builds ([f6f4a15e](https://github.com/mozilla-services/syncstorage-rs/commit/f6f4a15e3325f8dec18ee0e9b705a0eaf9ceafa8), closes [#683](https://github.com/mozilla-services/syncstorage-rs/issues/683)) +* don't call begin twice for mysql's delete_all (#673) ([c93db759](https://github.com/mozilla-services/syncstorage-rs/commit/c93db75976eaaf262c6c972566e80cfc3809e810), closes [#639](https://github.com/mozilla-services/syncstorage-rs/issues/639), [#441](https://github.com/mozilla-services/syncstorage-rs/issues/441)) +* python image build needs stable docker git container ([93edc9f6](https://github.com/mozilla-services/syncstorage-rs/commit/93edc9f6d20300dc2355cf80850ebf6d67143f5c)) +* range check the header to avoid a panic (#664) ([b73e6ee2](https://github.com/mozilla-services/syncstorage-rs/commit/b73e6ee2c7bd0aef080fa04af1d60fb41946837f), closes [#647](https://github.com/mozilla-services/syncstorage-rs/issues/647)) +* Make `bso_num` in migrate_node less truthy (#637) ([fa96964f](https://github.com/mozilla-services/syncstorage-rs/commit/fa96964f0703c731ea11f4a05d31a81c16669ce7), closes [#636](https://github.com/mozilla-services/syncstorage-rs/issues/636)) +* don't classify AlreadyExists as a ConflictError (#635) ([07276667](https://github.com/mozilla-services/syncstorage-rs/commit/07276667a30bba299f1085a6c1b16465250894a2), closes [#633](https://github.com/mozilla-services/syncstorage-rs/issues/633)) +* don't consider expiry during batch commit (#632) ([90ff7485](https://github.com/mozilla-services/syncstorage-rs/commit/90ff74858f10f5e52f1acd60a57f6a2ead46c891)) +* Add retry and sleep to purge_ttl attempts (#620) ([38c3295b](https://github.com/mozilla-services/syncstorage-rs/commit/38c3295b16a3250d474ff2024e855675c803f1a4)) +* restore delete_bso's handling of errors ([c11e7894](https://github.com/mozilla-services/syncstorage-rs/commit/c11e78948ef507b7eb74743a02df95f907ba9a08), closes [#599](https://github.com/mozilla-services/syncstorage-rs/issues/599)) +* don't replace user_collections ([d6b2dc21](https://github.com/mozilla-services/syncstorage-rs/commit/d6b2dc2187de5a1877b79e2354aa5ac746ce823a)) +* add build_essential package to Dockerfile. ([05b20eca](https://github.com/mozilla-services/syncstorage-rs/commit/05b20eca8be5f3b5322d92cd73bcd42ddcfde2e3), closes [#572](https://github.com/mozilla-services/syncstorage-rs/issues/572)) +* convert user_id into bigint ([ab2606da](https://github.com/mozilla-services/syncstorage-rs/commit/ab2606daeb3f5a9def697b4f16ded02af4290329), closes [#470](https://github.com/mozilla-services/syncstorage-rs/issues/470)) +* convert user_id into bigint ([8b951137](https://github.com/mozilla-services/syncstorage-rs/commit/8b951137374218ac6d2ec23e5f2c975b45fc2105), closes [#470](https://github.com/mozilla-services/syncstorage-rs/issues/470)) +* do not populate mysql CollectionCache with invalid values ([0741104e](https://github.com/mozilla-services/syncstorage-rs/commit/0741104ec8d516b5ebe25399e2baa805a5d207a5), closes [#239](https://github.com/mozilla-services/syncstorage-rs/issues/239)) +* correct the test version of post_bsos ([f9842af9](https://github.com/mozilla-services/syncstorage-rs/commit/f9842af9fc7cc931d40205f5a7668cc1e5828d6b), closes [#533](https://github.com/mozilla-services/syncstorage-rs/issues/533)) +* Reduce log release_max_levels ([17ff2867](https://github.com/mozilla-services/syncstorage-rs/commit/17ff2867442e7600f121976c04af32fc4eb7632a)) +* `cargo clippy` for rust 1.42 ([546d96ca](https://github.com/mozilla-services/syncstorage-rs/commit/546d96ca2885003e4d912a72bccf33f2f6fcb1f2)) +* Convert erect_tombstone to async/await ([442c4c05](https://github.com/mozilla-services/syncstorage-rs/commit/442c4c05a1939b70d9632ce2228e036ef8d7589c)) +* revert unsupported config change ([f4cfcab1](https://github.com/mozilla-services/syncstorage-rs/commit/f4cfcab1771870674ad49e409ec33a43838c842f)) +* adapt to async ([fceea69e](https://github.com/mozilla-services/syncstorage-rs/commit/fceea69e324b3d4d33b8d06eb614f1e944996a9b)) +* Fix #444 invalid offset code that was lost in the actix 2 upgrade due to a bad merge ([efbf6594](https://github.com/mozilla-services/syncstorage-rs/commit/efbf65948fc42e0f7f23cfd051814dba3b399ded)) +* Fix #459 db-tests on master ([0cd2b9db](https://github.com/mozilla-services/syncstorage-rs/commit/0cd2b9db969cdf515ae46f939bdaee5a3a1edd4d)) +* Fix #457 remaining blocking execute ([3ed7ae62](https://github.com/mozilla-services/syncstorage-rs/commit/3ed7ae62d8ad0ccb5f765a7b8b6397ce110d30ea)) +* convert migration state to smallint (#429) ([b980b438](https://github.com/mozilla-services/syncstorage-rs/commit/b980b43872d8adca1c08ed56920b1da2d74fb329), closes [#428](https://github.com/mozilla-services/syncstorage-rs/issues/428)) + + + ## 0.4.2 (2020-06-24) diff --git a/Cargo.lock b/Cargo.lock index 2987002d25..cd7e2d434a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2706,7 +2706,7 @@ dependencies = [ [[package]] name = "syncstorage" -version = "0.4.2" +version = "0.5.4" dependencies = [ "actix-cors", "actix-http", diff --git a/Cargo.toml b/Cargo.toml index 3cd80063de..5122a5af15 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "syncstorage" -version = "0.4.2" +version = "0.5.4" license = "MPL-2.0" authors = [ "Ben Bangert ", From 76b577b0d23ec9d752ed1b1ab05b18efb862077d Mon Sep 17 00:00:00 2001 From: JR Conlin Date: Tue, 4 Aug 2020 13:52:35 -0700 Subject: [PATCH 03/17] dereference Data<> before calling `on_response()` (#754) * Set grpcio feature `"openssl"` on ubuntu platforms to address seg faulting on startup due to combination of openssl and boringssl. Issue #742 --- Cargo.toml | 3 +++ src/error.rs | 5 ++--- src/web/middleware/sentry.rs | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5122a5af15..a341142cf7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,6 +67,9 @@ validator = "0.10" validator_derive = "0.10" woothee = "0.11" +[target.'cfg(target_vendor="ubuntu")'.dependencies] +grpcio = { version="0.6", features=["openssl"] } + [dev-dependencies] tokio = { version = "0.2", features = ["macros"] } diff --git a/src/error.rs b/src/error.rs index 68eef6dc74..dbbfe21b1b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -13,7 +13,6 @@ use actix_web::{ error::ResponseError, http::StatusCode, middleware::errhandlers::ErrorHandlerResponse, - web::Data, HttpResponse, Result, }; use failure::{Backtrace, Context, Fail}; @@ -130,9 +129,9 @@ impl ApiError { true } - pub fn on_response(&self, state: &Data) { + pub fn on_response(&self, state: &ServerState) { if self.is_conflict() { - Metrics::from(state.as_ref()).incr("storage.confict") + Metrics::from(state).incr("storage.confict") } } diff --git a/src/web/middleware/sentry.rs b/src/web/middleware/sentry.rs index 4746133613..56d7c31cce 100644 --- a/src/web/middleware/sentry.rs +++ b/src/web/middleware/sentry.rs @@ -149,7 +149,7 @@ where Some(e) => { if let Some(apie) = e.as_error::() { if let Some(state) = sresp.request().app_data::>() { - apie.on_response(state); + apie.on_response(state.as_ref()); }; if !apie.is_reportable() { debug!("Not reporting error to sentry: {:?}", apie); From 71ab9b344601479de2b4ebcf3b221720577f6e74 Mon Sep 17 00:00:00 2001 From: JR Conlin Date: Tue, 4 Aug 2020 14:48:38 -0700 Subject: [PATCH 04/17] bug: normalize id elements to remove potential wrap characters (#748) * bug: normalize id elements to remove potential wrap characters * chore: Update vendored SDK to use protobuf 2.16.2 Remove potential "{}" wrapper from bsoids and collection ids. (See original issue for details) Test included. Try sending in a sync request with a bso_id wrapped and URLencoded. Closes #680 --- Cargo.lock | 7 ++ Cargo.toml | 1 + src/error.rs | 7 +- src/web/extractors.rs | 100 ++++++++++++++++-- .../googleapis/src/spanner.rs | 2 +- 5 files changed, 108 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cd7e2d434a..28ebd24496 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2755,6 +2755,7 @@ dependencies = [ "time 0.2.16", "tokio", "url 2.1.1", + "urlencoding", "uuid", "validator", "validator_derive", @@ -3139,6 +3140,12 @@ dependencies = [ "serde 1.0.114", ] +[[package]] +name = "urlencoding" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c9232eb53352b4442e40d7900465dfc534e8cb2dc8f18656fcb2ac16112b5593" + [[package]] name = "uuid" version = "0.8.1" diff --git a/Cargo.toml b/Cargo.toml index a341142cf7..a45be8584b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,6 +62,7 @@ slog-stdlog = "4.0" slog-term = "2.6" time = "0.2" url = "2.1" +urlencoding = "1.1" uuid = { version = "0.8.1", features = ["serde", "v4"] } validator = "0.10" validator_derive = "0.10" diff --git a/src/error.rs b/src/error.rs index dbbfe21b1b..2a985b2df2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -76,6 +76,9 @@ pub enum ApiErrorKind { #[fail(display = "{}", _0)] Validation(#[cause] ValidationError), + + #[fail(display = "Invalid Submission: {}", _0)] + InvalidSubmission(String), } impl ApiError { @@ -216,6 +219,7 @@ impl From> for ApiError { StatusCode::INTERNAL_SERVER_ERROR } ApiErrorKind::Validation(error) => error.status, + ApiErrorKind::InvalidSubmission(_) => StatusCode::BAD_REQUEST, }; Self { inner, status } @@ -269,7 +273,8 @@ impl Serialize for ApiErrorKind { match *self { ApiErrorKind::Db(ref error) => serialize_string_to_array(serializer, error), ApiErrorKind::Hawk(ref error) => serialize_string_to_array(serializer, error), - ApiErrorKind::Internal(ref description) => { + ApiErrorKind::Internal(ref description) + | ApiErrorKind::InvalidSubmission(ref description) => { serialize_string_to_array(serializer, description) } ApiErrorKind::Validation(ref error) => Serialize::serialize(error, serializer), diff --git a/src/web/extractors.rs b/src/web/extractors.rs index c93abd85e6..0995116935 100644 --- a/src/web/extractors.rs +++ b/src/web/extractors.rs @@ -28,7 +28,7 @@ use validator::{Validate, ValidationError}; use crate::db::transaction::DbTransactionPool; use crate::db::{util::SyncTimestamp, DbPool, Sorting}; -use crate::error::ApiError; +use crate::error::{ApiError, ApiErrorKind}; use crate::server::{metrics, ServerState, BSO_ID_REGEX, COLLECTION_ID_REGEX}; use crate::settings::{Secrets, ServerLimits}; use crate::web::{ @@ -66,6 +66,22 @@ pub struct UidParam { uid: u64, } +fn clean_entry(s: &str) -> Result { + // URL decode and check that the string is all ascii. + let decoded: String = match urlencoding::decode(s) { + Ok(v) => v, + Err(e) => { + debug!("unclean entry: {:?} {:?}", s, e); + return Err(ApiErrorKind::Internal(e.to_string()).into()); + } + }; + if !decoded.is_ascii() { + debug!("unclean entry, non-ascii value in {:?}", decoded); + return Err(ApiErrorKind::InvalidSubmission("invalid value".into()).into()); + }; + Ok(decoded) +} + #[derive(Clone, Debug, Deserialize, Validate)] pub struct BatchBsoBody { #[validate(custom = "validate_body_bso_id")] @@ -300,7 +316,20 @@ impl FromRequest for BsoBodies { } // Save all id's we get, check for missing id, or duplicate. let bso_id = if let Some(id) = bso.get("id").and_then(serde_json::Value::as_str) { - let id = id.to_string(); + let id = match clean_entry(&id.to_string()) { + Ok(v) => v, + Err(_) => { + return future::err( + ValidationErrorKind::FromDetails( + "Invalid BSO ID".to_owned(), + RequestErrorLocation::Body, + Some("bsos".to_owned()), + None, + ) + .into(), + ); + } + }; if bso_ids.contains(&id) { return future::err( ValidationErrorKind::FromDetails( @@ -518,8 +547,17 @@ impl BsoParam { ))?; } if let Some(v) = elements.get(5) { - let sv = String::from_str(v).map_err(|_| { - warn!("⚠️ Invalid BsoParam Error: {:?}", v; tags); + let sv = clean_entry(&String::from_str(v).map_err(|e| { + warn!("⚠️ Invalid BsoParam Error: {:?} {:?}", v, e; tags); + ValidationErrorKind::FromDetails( + "Invalid BSO".to_owned(), + RequestErrorLocation::Path, + Some("bso".to_owned()), + Some(tags.clone()), + ) + })?) + .map_err(|e| { + warn!("⚠️ Invalid BsoParam Error: {:?} {:?}", v, e; tags); ValidationErrorKind::FromDetails( "Invalid BSO".to_owned(), RequestErrorLocation::Path, @@ -585,7 +623,7 @@ impl CollectionParam { return Ok(None); } if let Some(v) = elements.get(4) { - let sv = String::from_str(v).map_err(|_e| { + let mut sv = String::from_str(v).map_err(|_e| { ValidationErrorKind::FromDetails( "Missing Collection".to_owned(), RequestErrorLocation::Path, @@ -593,6 +631,14 @@ impl CollectionParam { Some(tags.clone()), ) })?; + sv = clean_entry(&sv).map_err(|_e| { + ValidationErrorKind::FromDetails( + "Invalid Collection".to_owned(), + RequestErrorLocation::Path, + Some("collection".to_owned()), + Some(tags.clone()), + ) + })?; Ok(Some(Self { collection: sv })) } else { Err(ValidationErrorKind::FromDetails( @@ -1113,7 +1159,20 @@ impl HawkIdentifier { // path: "/1.5/{uid}" let elements: Vec<&str> = uri.path().split('/').collect(); if let Some(v) = elements.get(2) { - u64::from_str(v).map_err(|e| { + let clean = match clean_entry(v) { + Err(e) => { + warn!("⚠️ HawkIdentifier Error invalid UID {:?} {:?}", v, e); + return Err(ValidationErrorKind::FromDetails( + "Invalid UID".to_owned(), + RequestErrorLocation::Path, + Some("uid".to_owned()), + tags, + ) + .into()); + } + Ok(v) => v, + }; + u64::from_str(&clean).map_err(|e| { warn!("⚠️ HawkIdentifier Error invalid UID {:?} {:?}", v, e); ValidationErrorKind::FromDetails( "Invalid UID".to_owned(), @@ -1700,7 +1759,9 @@ fn validate_body_bso_sortindex(sort: i32) -> Result<(), ValidationError> { /// Verifies the BSO id string is valid fn validate_body_bso_id(id: &str) -> Result<(), ValidationError> { - if !VALID_ID_REGEX.is_match(id) { + let clean = + clean_entry(id).map_err(|_| request_error("Invalid id", RequestErrorLocation::Body))?; + if !VALID_ID_REGEX.is_match(&clean) { return Err(request_error("Invalid id", RequestErrorLocation::Body)); } Ok(()) @@ -2138,6 +2199,31 @@ mod tests { assert_eq!(&result.collection, "tabs"); } + #[test] + fn test_quoted_bso() { + let payload = HawkPayload::test_default(*USER_ID); + let altered_bso = format!("\"{{{}}}\"", *USER_ID); + let state = make_state(); + let uri = format!( + "/1.5/{}/storage/tabs/{}", + *USER_ID, + urlencoding::encode(&altered_bso) + ); + let header = create_valid_hawk_header(&payload, &state, "GET", &uri, TEST_HOST, TEST_PORT); + let req = TestRequest::with_uri(&uri) + .data(state) + .header("authorization", header) + .header("accept", "application/json,text/plain:q=0.5") + .method(Method::GET) + .to_http_request(); + req.extensions_mut().insert(make_db()); + let result = block_on(BsoRequest::extract(&req)) + .expect("Could not get result in test_valid_collection_request"); + // make sure the altered bsoid matches the unaltered one, without the quotes and cury braces. + assert_eq!(result.user_id.legacy_id, *USER_ID); + assert_eq!(altered_bso.as_str(), result.bso); + } + #[test] fn test_invalid_collection_request() { let hawk_payload = HawkPayload::test_default(*USER_ID); diff --git a/vendor/mozilla-rust-sdk/googleapis/src/spanner.rs b/vendor/mozilla-rust-sdk/googleapis/src/spanner.rs index fd8f8ce989..a75cfa2d5f 100644 --- a/vendor/mozilla-rust-sdk/googleapis/src/spanner.rs +++ b/vendor/mozilla-rust-sdk/googleapis/src/spanner.rs @@ -36,7 +36,7 @@ impl Client { let creds = ChannelCredentials::google_default_credentials()?; // Create a Spanner client. - let chan = ChannelBuilder::new(env.clone()) + let chan = ChannelBuilder::new(env) .max_send_message_len(100 << 20) .max_receive_message_len(100 << 20) .secure_connect(&endpoint, creds); From f1d88feae60d7fea15b7575ac2108f0f80ff42b4 Mon Sep 17 00:00:00 2001 From: JR Conlin Date: Thu, 6 Aug 2020 08:53:16 -0700 Subject: [PATCH 05/17] bug: set config env separator to double underscore. (#763) Closes #762 Issue #320 --- src/settings.rs | 7 ++++++- src/web/extractors.rs | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/settings.rs b/src/settings.rs index ba9d9b2748..e271929d2d 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -108,7 +108,12 @@ impl Settings { } // Merge the environment overrides - s.merge(Environment::with_prefix(PREFIX))?; + // While the prefix is currently case insensitive, it's traditional that + // environment vars be UPPERCASE, this ensures that will continue should + // Environment ever change their policy about case insensitivity. + // This will accept environment variables specified as + // `SYNC_FOO__BAR_VALUE="gorp"` as `foo.bar_value = "gorp"` + s.merge(Environment::with_prefix(&PREFIX.to_uppercase()).separator("__"))?; Ok(match s.try_into::() { Ok(s) => { diff --git a/src/web/extractors.rs b/src/web/extractors.rs index 0995116935..75d92554e5 100644 --- a/src/web/extractors.rs +++ b/src/web/extractors.rs @@ -1045,7 +1045,7 @@ impl FromRequest for ConfigRequest { max_request_bytes: data.max_request_bytes, max_total_bytes: data.max_total_bytes, max_total_records: data.max_total_records, - debug_client: None, + debug_client: data.debug_client.clone(), }, })) } From fccff2770536e148aa69f80e42ffc487cdf3e252 Mon Sep 17 00:00:00 2001 From: Rachel Tublitz Date: Thu, 6 Aug 2020 12:43:31 -0400 Subject: [PATCH 06/17] chore: tag 0.5.5 release (#765) --- CHANGELOG.md | 14 ++++++++++++++ Cargo.lock | 2 +- Cargo.toml | 2 +- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 985af597ba..b4b1f805ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,17 @@ + +### 0.5.5 (2020-08-06) + +#### Chore + +* tag 0.5.4 release (#760) ([655c02e5](https://github.com/mozilla-services/syncstorage-rs/commit/655c02e501637ba9cd3d786d2ddcc6df8fc88204)) + + +#### Bug Fixes + +* set config env separator to double underscore. (#763) ([f1d88fea](https://github.com/mozilla-services/syncstorage-rs/commit/f1d88feae60d7fea15b7575ac2108f0f80ff42b4), closes [#762](https://github.com/mozilla-services/syncstorage-rs/issues/762)) + + + ### 0.5.4 (2020-08-04) diff --git a/Cargo.lock b/Cargo.lock index 28ebd24496..9b0202a832 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2706,7 +2706,7 @@ dependencies = [ [[package]] name = "syncstorage" -version = "0.5.4" +version = "0.5.5" dependencies = [ "actix-cors", "actix-http", diff --git a/Cargo.toml b/Cargo.toml index a45be8584b..8fe30388d3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "syncstorage" -version = "0.5.4" +version = "0.5.5" license = "MPL-2.0" authors = [ "Ben Bangert ", From 36533f8566c39e8c82ccb5a2bc8ae62fb254129a Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Thu, 6 Aug 2020 10:29:10 -0700 Subject: [PATCH 07/17] fix: ensure an X-Last-Modified for /info/configuration (#761) Closes #759 Co-authored-by: JR Conlin Co-authored-by: jrconlin --- src/server/mod.rs | 17 ++++++++++---- src/server/test.rs | 21 +++++++++++++++-- src/web/extractors.rs | 53 ++----------------------------------------- src/web/handlers.rs | 36 +++++++++++++++++++++-------- 4 files changed, 59 insertions(+), 68 deletions(-) diff --git a/src/server/mod.rs b/src/server/mod.rs index 1cc2887c41..786bf6661e 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -2,11 +2,6 @@ use std::{sync::Arc, time::Duration}; -use crate::db::{pool_from_settings, spawn_pool_periodic_reporter, DbPool}; -use crate::error::ApiError; -use crate::server::metrics::Metrics; -use crate::settings::{Secrets, ServerLimits, Settings}; -use crate::web::{handlers, middleware, tokenserver}; use actix_cors::Cors; use actix_web::{ dev, http::StatusCode, middleware::errhandlers::ErrorHandlers, web, App, HttpRequest, @@ -14,6 +9,12 @@ use actix_web::{ }; use cadence::StatsdClient; +use crate::db::{pool_from_settings, spawn_pool_periodic_reporter, DbPool}; +use crate::error::ApiError; +use crate::server::metrics::Metrics; +use crate::settings::{Secrets, ServerLimits, Settings}; +use crate::web::{handlers, middleware, tokenserver}; + pub const BSO_ID_REGEX: &str = r"[ -~]{1,64}"; pub const COLLECTION_ID_REGEX: &str = r"[a-zA-Z0-9._-]{1,32}"; const MYSQL_UID_REGEX: &str = r"[0-9]{1,10}"; @@ -32,6 +33,9 @@ pub struct ServerState { /// Server-enforced limits for request payloads. pub limits: Arc, + /// limits rendered as JSON + pub limits_json: String, + /// Secrets used during Hawk authentication. pub secrets: Arc, @@ -154,6 +158,8 @@ impl Server { let metrics = metrics::metrics_from_opts(&settings)?; let db_pool = pool_from_settings(&settings, &Metrics::from(&metrics)).await?; let limits = Arc::new(settings.limits); + let limits_json = + serde_json::to_string(&*limits).expect("ServerLimits failed to serialize"); let secrets = Arc::new(settings.master_secret); let port = settings.port; @@ -164,6 +170,7 @@ impl Server { let state = ServerState { db_pool: db_pool.clone(), limits: Arc::clone(&limits), + limits_json: limits_json.clone(), secrets: Arc::clone(&secrets), metrics: Box::new(metrics.clone()), port, diff --git a/src/server/test.rs b/src/server/test.rs index 061ac4356a..e3619ad46f 100644 --- a/src/server/test.rs +++ b/src/server/test.rs @@ -23,8 +23,7 @@ use crate::db::pool_from_settings; use crate::db::results::{DeleteBso, GetBso, PostBsos, PutBso}; use crate::db::util::SyncTimestamp; use crate::settings::{Secrets, ServerLimits}; -use crate::web::auth::HawkPayload; -use crate::web::extractors::BsoBody; +use crate::web::{auth::HawkPayload, extractors::BsoBody, X_LAST_MODIFIED}; lazy_static! { static ref SERVER_LIMITS: Arc = Arc::new(ServerLimits::default()); @@ -70,6 +69,7 @@ async fn get_test_state(settings: &Settings) -> ServerState { .await .expect("Could not get db_pool in get_test_state"), limits: Arc::clone(&SERVER_LIMITS), + limits_json: serde_json::to_string(&**SERVER_LIMITS).unwrap(), secrets: Arc::clone(&SECRETS), metrics: Box::new(metrics), port: settings.port, @@ -604,3 +604,20 @@ async fn reject_old_ios() { let body = String::from_utf8(test::read_body(response).await.to_vec()).unwrap(); assert_eq!(body, "0"); } + +#[actix_rt::test] +async fn info_configuration_xlm() { + let mut app = init_app!().await; + let req = + create_request(http::Method::GET, "/1.5/42/info/configuration", None, None).to_request(); + let response = app.call(req).await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + let xlm = response.headers().get(X_LAST_MODIFIED); + assert!(xlm.is_some()); + assert_eq!( + xlm.unwrap() + .to_str() + .expect("Couldn't parse X-Last-Modified"), + "0.00" + ); +} diff --git a/src/web/extractors.rs b/src/web/extractors.rs index 75d92554e5..26c8b971e3 100644 --- a/src/web/extractors.rs +++ b/src/web/extractors.rs @@ -30,7 +30,7 @@ use crate::db::transaction::DbTransactionPool; use crate::db::{util::SyncTimestamp, DbPool, Sorting}; use crate::error::{ApiError, ApiErrorKind}; use crate::server::{metrics, ServerState, BSO_ID_REGEX, COLLECTION_ID_REGEX}; -use crate::settings::{Secrets, ServerLimits}; +use crate::settings::Secrets; use crate::web::{ auth::HawkPayload, error::{HawkErrorKind, ValidationErrorKind}, @@ -1001,56 +1001,6 @@ impl FromRequest for BsoPutRequest { } } -#[derive(Debug, Default, Serialize)] -pub struct ConfigRequest { - pub limits: ServerLimits, -} - -impl FromRequest for ConfigRequest { - type Config = (); - type Error = Error; - type Future = LocalBoxFuture<'static, Result>; - - fn from_request(req: &HttpRequest, _: &mut Payload) -> Self::Future { - let tags = { - let exts = req.extensions(); - match exts.get::() { - Some(t) => t.clone(), - None => Tags::from_request_head(req.head()), - } - }; - - let state = match req.app_data::>() { - Some(s) => s, - None => { - error!("⚠️ Could not load the app state"); - return Box::pin(future::err( - ValidationErrorKind::FromDetails( - "Internal error".to_owned(), - RequestErrorLocation::Unknown, - Some("state".to_owned()), - Some(tags), - ) - .into(), - )); - } - }; - - let data = &state.limits; - Box::pin(future::ok(Self { - limits: ServerLimits { - max_post_bytes: data.max_post_bytes, - max_post_records: data.max_post_records, - max_record_payload_bytes: data.max_record_payload_bytes, - max_request_bytes: data.max_request_bytes, - max_total_bytes: data.max_total_bytes, - max_total_records: data.max_total_records, - debug_client: data.debug_client.clone(), - }, - })) - } -} - #[derive(Clone, Debug)] pub struct HeartbeatRequest { pub headers: HeaderMap, @@ -1901,6 +1851,7 @@ mod tests { ServerState { db_pool: Box::new(MockDbPool::new()), limits: Arc::clone(&SERVER_LIMITS), + limits_json: serde_json::to_string(&**SERVER_LIMITS).unwrap(), secrets: Arc::clone(&SECRETS), port: 8000, metrics: Box::new(metrics::metrics_from_opts(&settings).unwrap()), diff --git a/src/web/handlers.rs b/src/web/handlers.rs index 92a4b5b453..9ea3e87883 100644 --- a/src/web/handlers.rs +++ b/src/web/handlers.rs @@ -1,18 +1,25 @@ //! API Handlers use std::collections::HashMap; -use actix_web::{http::StatusCode, Error, HttpRequest, HttpResponse}; +use actix_web::{http::StatusCode, web::Data, Error, HttpRequest, HttpResponse}; use serde::Serialize; use serde_json::{json, Value}; -use crate::db::transaction::DbTransactionPool; -use crate::db::{params, results::Paginated, util::SyncTimestamp, Db, DbError, DbErrorKind}; -use crate::error::{ApiError, ApiErrorKind, ApiResult}; -use crate::web::extractors::{ - BsoPutRequest, BsoRequest, CollectionPostRequest, CollectionRequest, ConfigRequest, - HeartbeatRequest, MetaRequest, ReplyFormat, TestErrorRequest, +use crate::{ + db::{ + params, results::Paginated, transaction::DbTransactionPool, util::SyncTimestamp, Db, + DbError, DbErrorKind, + }, + error::{ApiError, ApiErrorKind, ApiResult}, + server::ServerState, + web::{ + extractors::{ + BsoPutRequest, BsoRequest, CollectionPostRequest, CollectionRequest, HeartbeatRequest, + MetaRequest, ReplyFormat, TestErrorRequest, + }, + X_LAST_MODIFIED, X_WEAVE_NEXT_OFFSET, X_WEAVE_RECORDS, + }, }; -use crate::web::{X_LAST_MODIFIED, X_WEAVE_NEXT_OFFSET, X_WEAVE_RECORDS}; pub const ONE_KB: f64 = 1024.0; @@ -443,8 +450,17 @@ pub async fn put_bso( .await } -pub async fn get_configuration(creq: ConfigRequest) -> Result { - Ok(HttpResponse::Ok().json(creq.limits)) +pub fn get_configuration(state: Data) -> HttpResponse { + // With no DbConnection (via a `transaction_http` call) needed here, we + // miss out on a couple things it does: + // 1. Ensuring an X-Last-Modified (always 0.00) is returned + // 2. Handling precondition checks + // The precondition checks don't make sense against hardcoded to the + // service limits data + a 0.00 timestamp, so just ensure #1 is handled + HttpResponse::Ok() + .header(X_LAST_MODIFIED, "0.00") + .content_type("application/json") + .body(&state.limits_json) } /** Returns a status message indicating the state of the current server From 38cd5dddc36ae0aeda159fea88ba6128a8e85181 Mon Sep 17 00:00:00 2001 From: JR Conlin Date: Mon, 10 Aug 2020 11:45:25 -0700 Subject: [PATCH 08/17] bug: Return WeaveError::OverQuota for over quota responses (#773) Closes #769 --- src/error.rs | 6 ++++-- src/web/extractors.rs | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/error.rs b/src/error.rs index 2a985b2df2..85588b97ba 100644 --- a/src/error.rs +++ b/src/error.rs @@ -147,8 +147,10 @@ impl ApiError { name, ref _tags, ) => { - if description == "size-limit-exceeded" { - return WeaveError::SizeLimitExceeded; + match description.as_ref() { + "over-quota" => return WeaveError::OverQuota, + "size-limit-exceeded" => return WeaveError::SizeLimitExceeded, + _ => {} } let name = name.clone().unwrap_or_else(|| "".to_owned()); if *location == RequestErrorLocation::Body diff --git a/src/web/extractors.rs b/src/web/extractors.rs index 26c8b971e3..e366411eb7 100644 --- a/src/web/extractors.rs +++ b/src/web/extractors.rs @@ -255,9 +255,9 @@ impl FromRequest for BsoBodies { error!("Returning over quota for {:?}", v); return Box::pin(future::err( ValidationErrorKind::FromDetails( - "size-limit-exceeded".to_owned(), + "over-quota".to_owned(), RequestErrorLocation::Unknown, - Some("size-limit-exceeded".to_owned()), + Some("over-quota".to_owned()), None, ) .into(), @@ -460,9 +460,9 @@ impl FromRequest for BsoBody { error!("Returning over quota for {:?}", v); return Box::pin(future::err( ValidationErrorKind::FromDetails( - "size-limit-exceeded".to_owned(), + "over-quota".to_owned(), RequestErrorLocation::Unknown, - Some("size-limit-exceeded".to_owned()), + Some("over-quota".to_owned()), None, ) .into(), From 7d1061f7197a56936a6cff9a438997640892d6c6 Mon Sep 17 00:00:00 2001 From: JR Conlin Date: Mon, 10 Aug 2020 12:40:26 -0700 Subject: [PATCH 09/17] bug: remove ubuntu target for grpcio (#775) Closes #774 --- Cargo.toml | 6 +++--- README.md | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8fe30388d3..59629594b9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,9 @@ env_logger = "0.7.1" failure = "0.1.8" futures = { version = "0.3", features = ["compat"] } googleapis-raw = { version = "0", path = "vendor/mozilla-rust-sdk/googleapis-raw" } +# Some versions of OpenSSL 1.1.1 conflict with grpcio's built-in boringssl which can cause +# syncserver to either fail to either compile, or start. In those cases, try +# `cargo build --features grpcio/openssl ...` grpcio = { version = "0.6.0" } lazy_static = "1.4.0" hawk = "3.2" @@ -68,9 +71,6 @@ validator = "0.10" validator_derive = "0.10" woothee = "0.11" -[target.'cfg(target_vendor="ubuntu")'.dependencies] -grpcio = { version="0.6", features=["openssl"] } - [dev-dependencies] tokio = { version = "0.2", features = ["macros"] } diff --git a/README.md b/README.md index e14433c665..256098474d 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,7 @@ This requires access to the mozilla-rust-sdk which is now available at `/vendor/ ### Connecting to Firefox -This will walk you through the steps to connect this project to your local copy of Firefox. +This will walk you through the steps to connect this project to your local copy of Firefox. 1. Follow the steps outlined above for running this project using [MySQL](https://github.com/mozilla-services/syncstorage-rs#mysql). @@ -201,11 +201,13 @@ Open a PR after doing the following: Once your PR merges, then go ahead and create an official [GitHub release](https://github.com/mozilla-services/syncstorage-rs/releases). - ## Troubleshooting - `rm Cargo.lock; cargo clean;` - Try this if you're having problems compiling. +- Some versions of OpenSSL 1.1.1 can conflict with grpcio's built in BoringSSL. These errors can cause syncstorage to fail to run or compile. +If you see a problem related to `libssl` you may need to specify the `cargo` option `--features grpcio/openssl` to force grpcio to use OpenSSL. + ## Related Documentation - [API docs](https://mozilla-services.readthedocs.io/en/latest/storage/apis-1.5.html) From 59aa28a4e5fdcfe2acc3f767487066d30b998af0 Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Tue, 11 Aug 2020 15:05:11 -0400 Subject: [PATCH 10/17] feat: More purge_ttl features (#776) * Support a mode option in purge_ttl * Support an expiry mode option in purge_ttl * Support serially deleting prefixes by regex Also changes the collection IDs option from a JSON list to an args list (i.e. [item1,item2,item3]). Closes #735 Closes #743 --- tools/spanner/purge_ttl.py | 145 +++++++++++++++++++++++++------------ 1 file changed, 100 insertions(+), 45 deletions(-) diff --git a/tools/spanner/purge_ttl.py b/tools/spanner/purge_ttl.py index 24a0821bb3..2ee8e1d27a 100644 --- a/tools/spanner/purge_ttl.py +++ b/tools/spanner/purge_ttl.py @@ -5,15 +5,16 @@ # file, You can obtain one at https://mozilla.org/MPL/2.0/. import argparse -import json +import logging import os import sys -import logging from datetime import datetime -from statsd.defaults.env import statsd +from typing import List, Optional from urllib import parse from google.cloud import spanner +from google.cloud.spanner_v1.database import Database +from statsd.defaults.env import statsd # set up logger logging.basicConfig( @@ -41,18 +42,25 @@ def use_dsn(args): return args -def deleter(database, name, query): +def deleter(database: Database, name: str, query: str, prefix: Optional[str]): with statsd.timer("syncstorage.purge_ttl.{}_duration".format(name)): logging.info("Running: {}".format(query)) start = datetime.now() result = database.execute_partitioned_dml(query) end = datetime.now() logging.info( - "{name}: removed {result} rows, {name}_duration: {time}".format( - name=name, result=result, time=end - start)) - - -def add_conditions(args, query): + "{name}: removed {result} rows, {name}_duration: {time}, prefix: {prefix}".format( + name=name, result=result, time=end - start, prefix=prefix)) + + +def add_conditions(args, query: str, prefix: Optional[str]): + """ + Add SQL conditions to a query. + :param args: The program arguments + :param query: The SQL query + :param prefix: The current prefix, if given + :return: The updated SQL query + """ if args.collection_ids: query += " AND collection_id" if len(args.collection_ids) == 1: @@ -60,40 +68,58 @@ def add_conditions(args, query): else: query += " in ({})".format( ', '.join(map(str, args.collection_ids))) - if args.uid_starts: - query += " AND fxa_uid LIKE \"{}%\"".format(args.uid_starts) + if prefix: + query += ' AND REGEXP_CONTAINS(fxa_uaid, r"{}")'.format(prefix) return query -def spanner_purge(args, request=None): - instance = client.instance(args.instance_id) - database = instance.database(args.database_id) - - logging.info("For {}:{}".format(args.instance_id, args.database_id)) - batch_query = ( - 'DELETE FROM batches WHERE ' - 'expiry < TIMESTAMP_TRUNC(CURRENT_TIMESTAMP(), DAY, "UTC")' - ) - bso_query = add_conditions( - args, - 'DELETE FROM bsos WHERE ' - 'expiry < TIMESTAMP_TRUNC(CURRENT_TIMESTAMP(), DAY, "UTC")' - ) +def get_expiry_condition(args): + """ + Get the expiry SQL WHERE condition to use + :param args: The program arguments + :return: A SQL snippet to use in the WHERE clause + """ + if args.expiry_mode == "now": + return 'expiry < CURRENT_TIMESTAMP()' + elif args.expiry_mode == "midnight": + return 'expiry < TIMESTAMP_TRUNC(CURRENT_TIMESTAMP(), DAY, "UTC")' + else: + raise Exception("Invalid expiry mode: {}".format(args.expiry_mode)) - # Delete Batches. Also deletes child batch_bsos rows (INTERLEAVE - # IN PARENT batches ON DELETE CASCADE) - deleter( - database, - name="batches", - query=batch_query - ) - # Delete BSOs - deleter( - database, - name="bso", - query=bso_query - ) +def spanner_purge(args): + instance = client.instance(args.instance_id) + database = instance.database(args.database_id) + expiry_condition = get_expiry_condition(args) + prefixes = args.uid_prefixes if args.uid_prefixes else [None] + + for prefix in prefixes: + logging.info("For {}:{}, prefix = {}".format(args.instance_id, args.database_id, prefix)) + + if args.mode in ["batches", "both"]: + # Delete Batches. Also deletes child batch_bsos rows (INTERLEAVE + # IN PARENT batches ON DELETE CASCADE) + batch_query = 'DELETE FROM batches WHERE {}'.format(expiry_condition) + deleter( + database, + name="batches", + query=batch_query, + prefix=prefix + ) + + if args.mode in ["bsos", "both"]: + # Delete BSOs + bso_query = add_conditions( + args, + 'DELETE FROM bsos WHERE {}'.format(expiry_condition), + prefix + ) + deleter( + database, + name="bso", + query=bso_query, + prefix=prefix + ) def get_args(): @@ -120,25 +146,54 @@ def get_args(): ) parser.add_argument( "--collection_ids", + type=parse_args_list, default=os.environ.get("COLLECTION_IDS", "[]"), - help="JSON array of collection IDs to purge" + help="Array of collection IDs to purge" + ) + parser.add_argument( + "--uid_prefixes", + type=parse_args_list, + default=os.environ.get("PURGE_UID_PREFIXES", "[]"), + help="Array of regex strings used to limit purges based on UID. " + "Each entry is a separate purge run." + ) + parser.add_argument( + "--mode", + type=str, + choices=["batches", "bsos", "both"], + default=os.environ.get("PURGE_MODE", "both"), + help="Purge TTLs in batches, bsos, or both" ) parser.add_argument( - "--uid_starts", + "--expiry_mode", type=str, - help="Limit to UIDs starting with specified characters" + choices=["now", "midnight"], + default=os.environ.get("PURGE_EXPIRY_MODE", "midnight"), + help="Choose the timestamp used to check if an entry is expired" ) args = parser.parse_args() - collections = json.loads(args.collection_ids) - if not isinstance(collections, list): - collections = [collections] - args.collection_ids = collections + # override using the DSN URL: if args.sync_database_url: args = use_dsn(args) + return args +def parse_args_list(args_list: str) -> List[str]: + """ + Parse a list of items (or a single string) into a list of strings. + Example input: [item1,item2,item3] + :param args_list: The list/string + :return: A list of strings + """ + if args_list[0] != "[" or args_list[-1] != "]": + # Assume it's a single item + return [args_list] + + return args_list[1:-1].split(",") + + if __name__ == "__main__": args = get_args() with statsd.timer("syncstorage.purge_ttl.total_duration"): From e044858323297a95bcc903c7bc983b9093422fc7 Mon Sep 17 00:00:00 2001 From: Mark Drobnak Date: Thu, 13 Aug 2020 15:10:00 -0400 Subject: [PATCH 11/17] fix: Avoid implicit transactions in DbTransactionPool (#777) * Avoid implicit transactions in DbTransactionPool * Always determine if a transaction is read/write by checking HTTP method Also do some refactoring to simplify the code. * Remove the begin transaction call from delete_all Now that it's handled in TransactionDbPool. Closes #768 --- src/db/transaction.rs | 35 ++++++++++++++++------------------- src/web/handlers.rs | 5 ----- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/db/transaction.rs b/src/db/transaction.rs index f4b2154da1..e5f9e9eb48 100644 --- a/src/db/transaction.rs +++ b/src/db/transaction.rs @@ -21,7 +21,6 @@ use std::future::Future; #[derive(Clone)] pub struct DbTransactionPool { pool: Box, - lock_collection: Option, is_read: bool, tags: Tags, user_id: HawkIdentifier, @@ -48,10 +47,10 @@ impl DbTransactionPool { let db2 = db.clone(); // Lock for transaction - let result = match (self.lock_collection.clone(), self.is_read) { + let result = match (self.get_lock_collection(), self.is_read) { (Some(lc), true) => db.lock_for_read(lc).await, (Some(lc), false) => db.lock_for_write(lc).await, - _ => Ok(()), + (None, is_read) => db.begin(!is_read).await, }; // Handle lock error @@ -156,6 +155,16 @@ impl DbTransactionPool { }; Ok(resp) } + + /// Create a lock collection if there is a collection to lock + fn get_lock_collection(&self) -> Option { + self.collection + .clone() + .map(|collection| params::LockCollection { + collection, + user_id: self.user_id.clone(), + }) + } } impl FromRequest for DbTransactionPool { @@ -193,7 +202,7 @@ impl FromRequest for DbTransactionPool { } }; let collection = match col_result { - Ok(v) => v, + Ok(v) => v.map(|collection| collection.collection), Err(e) => { // Semi-example to show how to use metrics inside of middleware. Metrics::from(state.as_ref()).incr("sync.error.collectionParam"); @@ -212,25 +221,13 @@ impl FromRequest for DbTransactionPool { let bso = BsoParam::extrude(req.head(), &mut req.extensions_mut()).ok(); let bso_opt = bso.map(|b| b.bso); - let (lc, is_read) = if let Some(collection) = collection { - let lc = params::LockCollection { - user_id: user_id.clone(), - collection: collection.collection, - }; - let is_read = match method { - Method::GET | Method::HEAD => true, - _ => false, - }; - - (Some(lc), is_read) - } else { - (None, true) + let is_read = match method { + Method::GET | Method::HEAD => true, + _ => false, }; - let collection = lc.as_ref().map(|c| c.collection.clone()); let precondition = PreConditionHeaderOpt::extrude(&req.headers(), Some(tags.clone()))?; let pool = Self { pool: state.db_pool.clone(), - lock_collection: lc, is_read, tags, user_id, diff --git a/src/web/handlers.rs b/src/web/handlers.rs index 9ea3e87883..6b8194bd3e 100644 --- a/src/web/handlers.rs +++ b/src/web/handlers.rs @@ -96,11 +96,6 @@ pub async fn delete_all( db_pool .transaction_http(|db| async move { meta.metrics.incr("request.delete_all"); - // transaction_http won't implicitly begin a write transaction - // for DELETE /storage because it lacks a collection. So it's done - // manually here, partly to not further complicate the unit test's - // transactions - db.begin(true).await?; Ok(HttpResponse::Ok().json(db.delete_storage(meta.user_id).await?)) }) .await From af5234d4ceb9db479e550d06796d783d4cec33aa Mon Sep 17 00:00:00 2001 From: JR Conlin Date: Fri, 14 Aug 2020 08:51:45 -0700 Subject: [PATCH 12/17] chore: update protobuf to 2.17.0 (#783) Closes #782 --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- vendor/mozilla-rust-sdk/googleapis-raw/Cargo.toml | 2 +- vendor/mozilla-rust-sdk/googleapis-raw/src/empty.rs | 4 ++-- .../mozilla-rust-sdk/googleapis-raw/src/iam/v1/iam_policy.rs | 4 ++-- vendor/mozilla-rust-sdk/googleapis-raw/src/iam/v1/policy.rs | 4 ++-- vendor/mozilla-rust-sdk/googleapis-raw/src/lib.rs | 2 +- .../googleapis-raw/src/longrunning/operations.rs | 4 ++-- vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/code.rs | 4 ++-- .../mozilla-rust-sdk/googleapis-raw/src/rpc/error_details.rs | 4 ++-- vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/status.rs | 4 ++-- .../src/spanner/admin/database/v1/spanner_database_admin.rs | 4 ++-- .../src/spanner/admin/instance/v1/spanner_instance_admin.rs | 4 ++-- vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/keys.rs | 4 ++-- .../googleapis-raw/src/spanner/v1/mutation.rs | 4 ++-- .../googleapis-raw/src/spanner/v1/query_plan.rs | 4 ++-- .../googleapis-raw/src/spanner/v1/result_set.rs | 4 ++-- .../mozilla-rust-sdk/googleapis-raw/src/spanner/v1/spanner.rs | 4 ++-- .../googleapis-raw/src/spanner/v1/transaction.rs | 4 ++-- .../mozilla-rust-sdk/googleapis-raw/src/spanner/v1/type_pb.rs | 4 ++-- 20 files changed, 37 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9b0202a832..add81b9a3c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1994,9 +1994,9 @@ dependencies = [ [[package]] name = "protobuf" -version = "2.16.2" +version = "2.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d883f78645c21b7281d21305181aa1f4dd9e9363e7cf2566c93121552cff003e" +checksum = "cb14183cc7f213ee2410067e1ceeadba2a7478a59432ff0747a335202798b1e2" [[package]] name = "quick-error" diff --git a/Cargo.toml b/Cargo.toml index 59629594b9..ea17f1a7da 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,7 +47,7 @@ log = { version = "0.4.8", features = ["max_level_info", "release_max_level_info mime = "0.3" num_cpus = "1" # must match what's used by googleapis-raw -protobuf = "2.15" +protobuf = "2.17.0" rand = "0.7" regex = "1.3" sentry = { version = "0.19", features = ["with_curl_transport"] } diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/Cargo.toml b/vendor/mozilla-rust-sdk/googleapis-raw/Cargo.toml index 36151512a4..ccffc229b7 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/Cargo.toml +++ b/vendor/mozilla-rust-sdk/googleapis-raw/Cargo.toml @@ -7,7 +7,7 @@ edition = "2018" [dependencies] futures = "0.3.5" grpcio = "0.6.0" -protobuf = "2.16.2" +protobuf = "2.17.0" [dev-dependencies] slog = "2.5" diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/empty.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/empty.rs index b919d9ec86..6722a969cc 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/empty.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/empty.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct Empty { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/iam/v1/iam_policy.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/iam/v1/iam_policy.rs index a1303cc17e..b0fd32cf3b 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/iam/v1/iam_policy.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/iam/v1/iam_policy.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct SetIamPolicyRequest { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/iam/v1/policy.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/iam/v1/policy.rs index 827baba5d2..a8b55f9aab 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/iam/v1/policy.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/iam/v1/policy.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct Policy { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/lib.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/lib.rs index 627927eb5d..69e4a3e819 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/lib.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/lib.rs @@ -3,7 +3,7 @@ // This appears as a comment in each generated file. Add it once here // to save a bit of time and effort. -const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; pub mod empty; pub(crate) mod iam; diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/longrunning/operations.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/longrunning/operations.rs index 83585aa78f..3d36a6cf3c 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/longrunning/operations.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/longrunning/operations.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct Operation { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/code.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/code.rs index a68c1b7580..2162890d9f 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/code.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/code.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(Clone,PartialEq,Eq,Debug,Hash)] pub enum Code { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/error_details.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/error_details.rs index 5510510e6f..6f8094738e 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/error_details.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/error_details.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct RetryInfo { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/status.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/status.rs index 7e6545a65c..ec37a504c2 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/status.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/rpc/status.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct Status { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/admin/database/v1/spanner_database_admin.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/admin/database/v1/spanner_database_admin.rs index 4e5e661e7c..f67efcbeb8 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/admin/database/v1/spanner_database_admin.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/admin/database/v1/spanner_database_admin.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct Database { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/admin/instance/v1/spanner_instance_admin.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/admin/instance/v1/spanner_instance_admin.rs index 4e28f92181..a8786d3909 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/admin/instance/v1/spanner_instance_admin.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/admin/instance/v1/spanner_instance_admin.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct InstanceConfig { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/keys.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/keys.rs index 4b3dad4040..7060cb910e 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/keys.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/keys.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct KeyRange { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/mutation.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/mutation.rs index fd3836ff00..a1c4b6a95c 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/mutation.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/mutation.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct Mutation { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/query_plan.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/query_plan.rs index 6a550be61f..cc7ad4a38f 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/query_plan.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/query_plan.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct PlanNode { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/result_set.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/result_set.rs index 576bcf9543..00e1351c95 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/result_set.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/result_set.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct ResultSet { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/spanner.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/spanner.rs index c529de34ca..8e1d849ee5 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/spanner.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/spanner.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct CreateSessionRequest { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/transaction.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/transaction.rs index 7ea86d8e03..3a31f33e8e 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/transaction.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/transaction.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct TransactionOptions { diff --git a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/type_pb.rs b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/type_pb.rs index 9515febd14..be4eb2b86b 100644 --- a/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/type_pb.rs +++ b/vendor/mozilla-rust-sdk/googleapis-raw/src/spanner/v1/type_pb.rs @@ -1,4 +1,4 @@ -// This file is generated by rust-protobuf 2.16.2. Do not edit +// This file is generated by rust-protobuf 2.17.0. Do not edit // @generated // https://github.com/rust-lang/rust-clippy/issues/702 @@ -21,7 +21,7 @@ /// Generated files are compatible only with the same version /// of protobuf runtime. -// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_16_2; +// const _PROTOBUF_VERSION_CHECK: () = ::protobuf::VERSION_2_17_0; #[derive(PartialEq,Clone,Default)] pub struct Type { From ec25bc47e2eed88a6fdabc3d32d04d065a780e67 Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Mon, 17 Aug 2020 21:34:23 -0700 Subject: [PATCH 13/17] feat: emit internal bb8 Pool errors to logs/sentry - add a keepalive setting - fix: don't urldecode bso_ids from JSON - pass the user-agent to sentry as an extra Closes #786 Closes #785 Closes #764 Closes #787 --- src/db/spanner/pool.rs | 21 ++++++++++++++-- src/error.rs | 7 +----- src/server/mod.rs | 14 +++++++---- src/settings.rs | 3 +++ src/web/extractors.rs | 46 +++++++++--------------------------- src/web/middleware/sentry.rs | 3 --- src/web/tags.rs | 11 +++++---- 7 files changed, 49 insertions(+), 56 deletions(-) diff --git a/src/db/spanner/pool.rs b/src/db/spanner/pool.rs index 1d0311595d..610335c7be 100644 --- a/src/db/spanner/pool.rs +++ b/src/db/spanner/pool.rs @@ -1,5 +1,5 @@ use async_trait::async_trait; -use bb8::Pool; +use bb8::{ErrorSink, Pool}; use std::{ collections::HashMap, @@ -49,7 +49,8 @@ impl SpannerDbPool { let max_size = settings.database_pool_max_size.unwrap_or(10); let builder = bb8::Pool::builder() .max_size(max_size) - .min_idle(settings.database_pool_min_idle); + .min_idle(settings.database_pool_min_idle) + .error_sink(Box::new(LoggingErrorSink)); Ok(Self { pool: builder.build(manager).await?, @@ -164,3 +165,19 @@ impl Default for CollectionCache { } } } + +/// Logs internal bb8 errors +#[derive(Debug, Clone, Copy)] +pub struct LoggingErrorSink; + +impl ErrorSink for LoggingErrorSink { + fn sink(&self, e: E) { + error!("bb8 Error: {}", e); + let event = sentry::integrations::failure::event_from_fail(&e); + sentry::capture_event(event); + } + + fn boxed_clone(&self) -> Box> { + Box::new(*self) + } +} diff --git a/src/error.rs b/src/error.rs index 85588b97ba..199b6deb18 100644 --- a/src/error.rs +++ b/src/error.rs @@ -76,9 +76,6 @@ pub enum ApiErrorKind { #[fail(display = "{}", _0)] Validation(#[cause] ValidationError), - - #[fail(display = "Invalid Submission: {}", _0)] - InvalidSubmission(String), } impl ApiError { @@ -221,7 +218,6 @@ impl From> for ApiError { StatusCode::INTERNAL_SERVER_ERROR } ApiErrorKind::Validation(error) => error.status, - ApiErrorKind::InvalidSubmission(_) => StatusCode::BAD_REQUEST, }; Self { inner, status } @@ -275,8 +271,7 @@ impl Serialize for ApiErrorKind { match *self { ApiErrorKind::Db(ref error) => serialize_string_to_array(serializer, error), ApiErrorKind::Hawk(ref error) => serialize_string_to_array(serializer, error), - ApiErrorKind::Internal(ref description) - | ApiErrorKind::InvalidSubmission(ref description) => { + ApiErrorKind::Internal(ref description) => { serialize_string_to_array(serializer, description) } ApiErrorKind::Validation(ref error) => Serialize::serialize(error, serializer), diff --git a/src/server/mod.rs b/src/server/mod.rs index 786bf6661e..d9644ef299 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -165,7 +165,7 @@ impl Server { spawn_pool_periodic_reporter(Duration::from_secs(10), metrics.clone(), db_pool.clone())?; - let server = HttpServer::new(move || { + let mut server = HttpServer::new(move || { // Setup the server state let state = ServerState { db_pool: db_pool.clone(), @@ -177,10 +177,14 @@ impl Server { }; build_app!(state, limits) - }) - .bind(format!("{}:{}", settings.host, settings.port)) - .expect("Could not get Server in Server::with_settings") - .run(); + }); + if let Some(keep_alive) = settings.actix_keep_alive { + server = server.keep_alive(keep_alive as usize); + } + let server = server + .bind(format!("{}:{}", settings.host, settings.port)) + .expect("Could not get Server in Server::with_settings") + .run(); Ok(server) } } diff --git a/src/settings.rs b/src/settings.rs index e271929d2d..17b22d35b3 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -32,6 +32,8 @@ pub struct Settings { #[cfg(test)] pub database_use_test_transactions: bool, + pub actix_keep_alive: Option, + /// Server-enforced limits for request payloads. pub limits: ServerLimits, @@ -57,6 +59,7 @@ impl Default for Settings { database_pool_min_idle: None, #[cfg(test)] database_use_test_transactions: false, + actix_keep_alive: None, limits: ServerLimits::default(), master_secret: Secrets::default(), statsd_host: None, diff --git a/src/web/extractors.rs b/src/web/extractors.rs index e366411eb7..6ac8ebcf8e 100644 --- a/src/web/extractors.rs +++ b/src/web/extractors.rs @@ -66,19 +66,11 @@ pub struct UidParam { uid: u64, } -fn clean_entry(s: &str) -> Result { - // URL decode and check that the string is all ascii. - let decoded: String = match urlencoding::decode(s) { - Ok(v) => v, - Err(e) => { - debug!("unclean entry: {:?} {:?}", s, e); - return Err(ApiErrorKind::Internal(e.to_string()).into()); - } - }; - if !decoded.is_ascii() { - debug!("unclean entry, non-ascii value in {:?}", decoded); - return Err(ApiErrorKind::InvalidSubmission("invalid value".into()).into()); - }; +fn urldecode(s: &str) -> Result { + let decoded: String = urlencoding::decode(s).map_err(|e| { + debug!("unclean entry: {:?} {:?}", s, e); + ApiErrorKind::Internal(e.to_string()) + })?; Ok(decoded) } @@ -316,20 +308,7 @@ impl FromRequest for BsoBodies { } // Save all id's we get, check for missing id, or duplicate. let bso_id = if let Some(id) = bso.get("id").and_then(serde_json::Value::as_str) { - let id = match clean_entry(&id.to_string()) { - Ok(v) => v, - Err(_) => { - return future::err( - ValidationErrorKind::FromDetails( - "Invalid BSO ID".to_owned(), - RequestErrorLocation::Body, - Some("bsos".to_owned()), - None, - ) - .into(), - ); - } - }; + let id = id.to_string(); if bso_ids.contains(&id) { return future::err( ValidationErrorKind::FromDetails( @@ -532,13 +511,12 @@ pub struct BsoParam { } impl BsoParam { - pub fn bsoparam_from_path(uri: &Uri, tags: &Tags) -> Result { + fn bsoparam_from_path(uri: &Uri, tags: &Tags) -> Result { // TODO: replace with proper path parser // path: "/1.5/{uid}/storage/{collection}/{bso}" let elements: Vec<&str> = uri.path().split('/').collect(); let elem = elements.get(3); if elem.is_none() || elem != Some(&"storage") || elements.len() != 6 { - warn!("⚠️ Unexpected BSO URI: {:?}", uri.path(); tags); return Err(ValidationErrorKind::FromDetails( "Invalid BSO".to_owned(), RequestErrorLocation::Path, @@ -547,7 +525,7 @@ impl BsoParam { ))?; } if let Some(v) = elements.get(5) { - let sv = clean_entry(&String::from_str(v).map_err(|e| { + let sv = urldecode(&String::from_str(v).map_err(|e| { warn!("⚠️ Invalid BsoParam Error: {:?} {:?}", v, e; tags); ValidationErrorKind::FromDetails( "Invalid BSO".to_owned(), @@ -631,7 +609,7 @@ impl CollectionParam { Some(tags.clone()), ) })?; - sv = clean_entry(&sv).map_err(|_e| { + sv = urldecode(&sv).map_err(|_e| { ValidationErrorKind::FromDetails( "Invalid Collection".to_owned(), RequestErrorLocation::Path, @@ -1109,7 +1087,7 @@ impl HawkIdentifier { // path: "/1.5/{uid}" let elements: Vec<&str> = uri.path().split('/').collect(); if let Some(v) = elements.get(2) { - let clean = match clean_entry(v) { + let clean = match urldecode(v) { Err(e) => { warn!("⚠️ HawkIdentifier Error invalid UID {:?} {:?}", v, e); return Err(ValidationErrorKind::FromDetails( @@ -1709,9 +1687,7 @@ fn validate_body_bso_sortindex(sort: i32) -> Result<(), ValidationError> { /// Verifies the BSO id string is valid fn validate_body_bso_id(id: &str) -> Result<(), ValidationError> { - let clean = - clean_entry(id).map_err(|_| request_error("Invalid id", RequestErrorLocation::Body))?; - if !VALID_ID_REGEX.is_match(&clean) { + if !VALID_ID_REGEX.is_match(id) { return Err(request_error("Invalid id", RequestErrorLocation::Body)); } Ok(()) diff --git a/src/web/middleware/sentry.rs b/src/web/middleware/sentry.rs index 56d7c31cce..20c5d741a0 100644 --- a/src/web/middleware/sentry.rs +++ b/src/web/middleware/sentry.rs @@ -100,7 +100,6 @@ where fn call(&mut self, sreq: ServiceRequest) -> Self::Future { let mut tags = Tags::from_request_head(sreq.head()); - let uri = sreq.head().uri.to_string(); sreq.extensions_mut().insert(tags.clone()); Box::pin(self.service.call(sreq).and_then(move |mut sresp| { @@ -119,8 +118,6 @@ where tags.tags.insert(k, v); } }; - // add the uri.path (which can cause influx to puke) - tags.extra.insert("uri.path".to_owned(), uri); match sresp.response().error() { None => { // Middleware errors are eaten by current versions of Actix. Errors are now added diff --git a/src/web/tags.rs b/src/web/tags.rs index f38401003d..d5923331ad 100644 --- a/src/web/tags.rs +++ b/src/web/tags.rs @@ -57,6 +57,7 @@ impl Tags { // Return an Option<> type because the later consumers (ApiErrors) presume that // tags are optional and wrapped by an Option<> type. let mut tags = HashMap::new(); + let mut extra = HashMap::new(); if let Some(ua) = req_head.headers().get(USER_AGENT) { if let Ok(uas) = ua.to_str() { let (ua_result, metrics_os, metrics_browser) = parse_user_agent(uas); @@ -65,14 +66,14 @@ impl Tags { insert_if_not_empty("ua.name", ua_result.name, &mut tags); insert_if_not_empty("ua.os.ver", &ua_result.os_version.to_owned(), &mut tags); insert_if_not_empty("ua.browser.ver", ua_result.version, &mut tags); + extra.insert("ua".to_owned(), uas.to_string()); } } - // `uri.path` causes too much cardinality for influx. tags.insert("uri.method".to_owned(), req_head.method.to_string()); - Tags { - tags, - extra: HashMap::new(), - } + // `uri.path` causes too much cardinality for influx but keep it in + // extra for sentry + extra.insert("uri.path".to_owned(), req_head.uri.to_string()); + Tags { tags, extra } } pub fn with_tags(tags: HashMap) -> Tags { From c11b3af3548415b1831fa9ef9508b89ebb507a6f Mon Sep 17 00:00:00 2001 From: JR Conlin Date: Wed, 19 Aug 2020 09:24:19 -0700 Subject: [PATCH 14/17] Feat/772 (#784) * feat: Better "Archive" header error, don't report some HAWK errors Closes: #771, #772 --- src/error.rs | 7 ++++++- src/web/error.rs | 6 ++++++ src/web/extractors.rs | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/error.rs b/src/error.rs index 85588b97ba..e2d96dd610 100644 --- a/src/error.rs +++ b/src/error.rs @@ -24,7 +24,7 @@ use serde::{ use crate::db::error::{DbError, DbErrorKind}; use crate::server::metrics::Metrics; use crate::server::ServerState; -use crate::web::error::{HawkError, ValidationError, ValidationErrorKind}; +use crate::web::error::{HawkError, HawkErrorKind, ValidationError, ValidationErrorKind}; use crate::web::extractors::RequestErrorLocation; /// Legacy Sync 1.1 error codes, which Sync 1.5 also returns by replacing the descriptive JSON @@ -127,6 +127,11 @@ impl ApiError { DbErrorKind::Conflict => return false, _ => (), }, + ApiErrorKind::Hawk(hawke) => match hawke.kind() { + HawkErrorKind::MissingHeader => return false, + HawkErrorKind::InvalidHeader => return false, + _ => (), + }, _ => (), } true diff --git a/src/web/error.rs b/src/web/error.rs index 7bb21f0c32..a675211b28 100644 --- a/src/web/error.rs +++ b/src/web/error.rs @@ -24,6 +24,12 @@ pub struct HawkError { inner: Context, } +impl HawkError { + pub fn kind(&self) -> &HawkErrorKind { + self.inner.get_context() + } +} + /// Causes of HAWK errors. #[derive(Debug, Fail)] pub enum HawkErrorKind { diff --git a/src/web/extractors.rs b/src/web/extractors.rs index e366411eb7..ba1a52d833 100644 --- a/src/web/extractors.rs +++ b/src/web/extractors.rs @@ -788,7 +788,7 @@ impl FromRequest for CollectionRequest { "application/json" | "" => ReplyFormat::Json, _ => { return Err(ValidationErrorKind::FromDetails( - "Invalid accept".to_string(), + format!("Invalid Accept header specified: {:?}", accept), RequestErrorLocation::Header, Some("accept".to_string()), Some(tags), From 7e526cb831dfacce65415822841c8881b0ce771e Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Wed, 19 Aug 2020 16:10:47 -0700 Subject: [PATCH 15/17] refactor: cleanup/rearrange --- src/db/spanner/models.rs | 39 +++++++++++++++++++-------------------- src/db/spanner/pool.rs | 4 +++- src/db/spanner/support.rs | 10 ++++++---- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/db/spanner/models.rs b/src/db/spanner/models.rs index b45d1b9e31..17fd169c6c 100644 --- a/src/db/spanner/models.rs +++ b/src/db/spanner/models.rs @@ -7,47 +7,46 @@ use std::{ sync::Arc, }; -use bb8::PooledConnection; use futures::future::TryFutureExt; -use googleapis_raw::spanner::v1::transaction::{ - self, TransactionOptions, TransactionOptions_ReadOnly, TransactionOptions_ReadWrite, -}; use googleapis_raw::spanner::v1::{ mutation::{Mutation, Mutation_Write}, spanner::{BeginTransactionRequest, CommitRequest, ExecuteSqlRequest, RollbackRequest}, + transaction::{ + TransactionOptions, TransactionOptions_ReadOnly, TransactionOptions_ReadWrite, + TransactionSelector, + }, type_pb::TypeCode, }; #[allow(unused_imports)] use protobuf::{well_known_types::ListValue, Message, RepeatedField}; -use super::manager::{SpannerConnectionManager, SpannerSession}; -use super::pool::CollectionCache; - -use crate::db::{ - error::{DbError, DbErrorKind}, - params, results, - spanner::support::{as_type, StreamedResultSetAsync}, - util::SyncTimestamp, - Db, DbFuture, Sorting, FIRST_CUSTOM_COLLECTION_ID, +use crate::{ + db::{ + error::{DbError, DbErrorKind}, + params, results, + util::SyncTimestamp, + Db, DbFuture, Sorting, FIRST_CUSTOM_COLLECTION_ID, + }, + server::metrics::Metrics, + web::extractors::{BsoQueryParams, HawkIdentifier, Offset}, }; -use crate::server::metrics::Metrics; -use crate::web::extractors::{BsoQueryParams, HawkIdentifier, Offset}; -use super::support::{bso_to_insert_row, bso_to_update_row}; use super::{ batch, - support::{as_list_value, as_value, bso_from_row, ExecuteSqlRequestBuilder}, + pool::{CollectionCache, Conn}, + support::{ + as_list_value, as_type, as_value, bso_from_row, ExecuteSqlRequestBuilder, + StreamedResultSetAsync, + }, + support::{bso_to_insert_row, bso_to_update_row}, }; -pub type TransactionSelector = transaction::TransactionSelector; - #[derive(Debug, Eq, PartialEq)] pub enum CollectionLock { Read, Write, } -pub(super) type Conn<'a> = PooledConnection<'a, SpannerConnectionManager>; pub type Result = std::result::Result; /// The ttl to use for rows that are never supposed to expire (in seconds) diff --git a/src/db/spanner/pool.rs b/src/db/spanner/pool.rs index 610335c7be..43304456bd 100644 --- a/src/db/spanner/pool.rs +++ b/src/db/spanner/pool.rs @@ -1,5 +1,5 @@ use async_trait::async_trait; -use bb8::{ErrorSink, Pool}; +use bb8::{ErrorSink, Pool, PooledConnection}; use std::{ collections::HashMap, @@ -16,6 +16,8 @@ use super::manager::{SpannerConnectionManager, SpannerSession}; use super::models::SpannerDb; use crate::error::ApiResult; +pub(super) type Conn<'a> = PooledConnection<'a, SpannerConnectionManager>; + embed_migrations!(); /// Run the diesel embedded migrations diff --git a/src/db/spanner/support.rs b/src/db/spanner/support.rs index 2a9b1715b5..5766cb4748 100644 --- a/src/db/spanner/support.rs +++ b/src/db/spanner/support.rs @@ -16,14 +16,16 @@ use protobuf::{ RepeatedField, }; -use super::models::{Conn, Result}; -use crate::db::{results, util::SyncTimestamp, DbError, DbErrorKind}; - use crate::{ - db::{params, spanner::models::DEFAULT_BSO_TTL, util::to_rfc3339}, + db::{ + params, results, spanner::models::DEFAULT_BSO_TTL, util::to_rfc3339, util::SyncTimestamp, + DbError, DbErrorKind, + }, web::extractors::HawkIdentifier, }; +use super::{models::Result, pool::Conn}; + pub fn as_value(string_value: String) -> Value { let mut value = Value::new(); value.set_string_value(string_value); From 077bf091ecaededfa3c937ce5ac5a5f6f95015f3 Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Wed, 19 Aug 2020 16:20:59 -0700 Subject: [PATCH 16/17] feat: switch spanner's db pool to deadpool this is a quick integration of deadpool to be revisited with some more cleanup Issue #794 --- Cargo.lock | 15 +++ Cargo.toml | 1 + src/db/results.rs | 9 ++ src/db/spanner/batch.rs | 16 +-- src/db/spanner/{manager.rs => manager/bb8.rs} | 9 +- src/db/spanner/manager/deadpool.rs | 119 ++++++++++++++++++ src/db/spanner/manager/mod.rs | 2 + src/db/spanner/models.rs | 20 +-- src/db/spanner/pool.rs | 31 +++-- src/db/spanner/support.rs | 6 +- 10 files changed, 194 insertions(+), 34 deletions(-) rename src/db/spanner/{manager.rs => manager/bb8.rs} (95%) create mode 100644 src/db/spanner/manager/deadpool.rs create mode 100644 src/db/spanner/manager/mod.rs diff --git a/Cargo.lock b/Cargo.lock index add81b9a3c..7549eeea6c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -777,6 +777,20 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "deadpool" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4aaff9a7a1de9893f4004fa08527b31cb2ae4121c44e053cf53f29203c73bd23" +dependencies = [ + "async-trait", + "config", + "crossbeam-queue", + "num_cpus", + "serde 1.0.114", + "tokio", +] + [[package]] name = "debugid" version = "0.7.2" @@ -2719,6 +2733,7 @@ dependencies = [ "cadence", "chrono", "config", + "deadpool", "diesel", "diesel_logger", "diesel_migrations", diff --git a/Cargo.toml b/Cargo.toml index ea17f1a7da..102cb80688 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ bytes = "0.5" cadence = "0.20.0" chrono = "0.4" config = "0.10" +deadpool = "0.5.2" diesel = { version = "1.4.4", features = ["mysql", "r2d2"] } diesel_logger = "0.1.1" diesel_migrations = { version = "1.4.0", features = ["mysql"] } diff --git a/src/db/results.rs b/src/db/results.rs index 3b063f44cf..93f49f706c 100644 --- a/src/db/results.rs +++ b/src/db/results.rs @@ -94,6 +94,15 @@ impl From for PoolState { } } +impl From for PoolState { + fn from(status: deadpool::Status) -> PoolState { + PoolState { + connections: status.size as u32, + idle_connections: status.available as u32, + } + } +} + #[cfg(test)] pub type GetCollectionId = i32; diff --git a/src/db/spanner/batch.rs b/src/db/spanner/batch.rs index 6869e4805d..66ba26b41f 100644 --- a/src/db/spanner/batch.rs +++ b/src/db/spanner/batch.rs @@ -18,7 +18,7 @@ use crate::{ }; pub async fn create_async( - db: &SpannerDb<'_>, + db: &SpannerDb, params: params::CreateBatch, ) -> Result { let batch_id = Uuid::new_v4().to_simple().to_string(); @@ -57,7 +57,7 @@ pub async fn create_async( Ok(batch_id) } -pub async fn validate_async(db: &SpannerDb<'_>, params: params::ValidateBatch) -> Result { +pub async fn validate_async(db: &SpannerDb, params: params::ValidateBatch) -> Result { let collection_id = db.get_collection_id_async(¶ms.collection).await?; let exists = db .sql( @@ -81,7 +81,7 @@ pub async fn validate_async(db: &SpannerDb<'_>, params: params::ValidateBatch) - Ok(exists.is_some()) } -pub async fn append_async(db: &SpannerDb<'_>, params: params::AppendToBatch) -> Result<()> { +pub async fn append_async(db: &SpannerDb, params: params::AppendToBatch) -> Result<()> { let mut metrics = db.metrics.clone(); metrics.start_timer("storage.spanner.append_items_to_batch", None); @@ -106,7 +106,7 @@ pub async fn append_async(db: &SpannerDb<'_>, params: params::AppendToBatch) -> } pub async fn get_async( - db: &SpannerDb<'_>, + db: &SpannerDb, params: params::GetBatch, ) -> Result> { let collection_id = db.get_collection_id_async(¶ms.collection).await?; @@ -142,7 +142,7 @@ pub async fn get_async( Ok(batch) } -pub async fn delete_async(db: &SpannerDb<'_>, params: params::DeleteBatch) -> Result<()> { +pub async fn delete_async(db: &SpannerDb, params: params::DeleteBatch) -> Result<()> { let collection_id = db.get_collection_id_async(¶ms.collection).await?; // Also deletes child batch_bsos rows (INTERLEAVE IN PARENT batches ON // DELETE CASCADE) @@ -165,7 +165,7 @@ pub async fn delete_async(db: &SpannerDb<'_>, params: params::DeleteBatch) -> Re } pub async fn commit_async( - db: &SpannerDb<'_>, + db: &SpannerDb, params: params::CommitBatch, ) -> Result { let mut metrics = db.metrics.clone(); @@ -239,7 +239,7 @@ pub async fn commit_async( } pub async fn do_append_async( - db: &SpannerDb<'_>, + db: &SpannerDb, user_id: HawkIdentifier, collection_id: i32, batch_id: String, @@ -335,7 +335,7 @@ pub async fn do_append_async( /// For the special case of a user creating a batch for a collection with no /// prior data. async fn pretouch_collection_async( - db: &SpannerDb<'_>, + db: &SpannerDb, user_id: &HawkIdentifier, collection_id: i32, ) -> Result<()> { diff --git a/src/db/spanner/manager.rs b/src/db/spanner/manager/bb8.rs similarity index 95% rename from src/db/spanner/manager.rs rename to src/db/spanner/manager/bb8.rs index 344625a968..181e01548a 100644 --- a/src/db/spanner/manager.rs +++ b/src/db/spanner/manager/bb8.rs @@ -18,7 +18,8 @@ use crate::{ settings::Settings, }; -const SPANNER_ADDRESS: &str = "spanner.googleapis.com:443"; +// XXX: +pub const SPANNER_ADDRESS: &str = "spanner.googleapis.com:443"; pub struct SpannerConnectionManager { database_name: String, @@ -33,6 +34,7 @@ impl<_T> fmt::Debug for SpannerConnectionManager<_T> { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt.debug_struct("SpannerConnectionManager") .field("database_name", &self.database_name) + .field("test_transactions", &self.test_transactions) .finish() } } @@ -65,7 +67,7 @@ pub struct SpannerSession { pub client: SpannerClient, pub session: Session, - pub(super) use_test_transactions: bool, + pub(in crate::db::spanner) use_test_transactions: bool, } #[async_trait] @@ -130,7 +132,8 @@ impl ManageConnection } } -async fn create_session( +// XXX: +pub async fn create_session( client: &SpannerClient, database_name: &str, ) -> Result { diff --git a/src/db/spanner/manager/deadpool.rs b/src/db/spanner/manager/deadpool.rs new file mode 100644 index 0000000000..0077b6705c --- /dev/null +++ b/src/db/spanner/manager/deadpool.rs @@ -0,0 +1,119 @@ +use std::{fmt, sync::Arc}; + +use actix_web::web::block; +use async_trait::async_trait; +use deadpool::managed::{RecycleError, RecycleResult}; +use googleapis_raw::spanner::v1::{spanner::GetSessionRequest, spanner_grpc::SpannerClient}; +use grpcio::{ChannelBuilder, ChannelCredentials, EnvBuilder, Environment}; + +use crate::{ + db::error::{DbError, DbErrorKind}, + server::metrics::Metrics, + settings::Settings, +}; + +// XXX: +use super::bb8::{create_session, SpannerSession, SPANNER_ADDRESS}; + +// - -> SpannerSessionManager (and bb8 too) +// - bb8s doesn't need the PhantomData +// - kill the lifetimes for now or PhantomData one +pub struct Manager { + database_name: String, + /// The gRPC environment + env: Arc, + metrics: Metrics, + test_transactions: bool, +} + +impl fmt::Debug for Manager { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_struct("Manager") + .field("database_name", &self.database_name) + .field("test_transactions", &self.test_transactions) + .finish() + } +} + +impl Manager { + pub fn new(settings: &Settings, metrics: &Metrics) -> Result { + let url = &settings.database_url; + if !url.starts_with("spanner://") { + Err(DbErrorKind::InvalidUrl(url.to_owned()))?; + } + let database_name = url["spanner://".len()..].to_owned(); + let env = Arc::new(EnvBuilder::new().build()); + + #[cfg(not(test))] + let test_transactions = false; + #[cfg(test)] + let test_transactions = settings.database_use_test_transactions; + + Ok(Manager { + database_name, + env, + metrics: metrics.clone(), + test_transactions, + }) + } +} + +#[async_trait] +impl deadpool::managed::Manager for Manager { + async fn create(&self) -> Result { + let env = self.env.clone(); + let mut metrics = self.metrics.clone(); + // XXX: issue732: Could google_default_credentials (or + // ChannelBuilder::secure_connect) block?! + let chan = block(move || -> Result { + metrics.start_timer("storage.pool.grpc_auth", None); + // Requires + // GOOGLE_APPLICATION_CREDENTIALS=/path/to/service-account.json + let creds = ChannelCredentials::google_default_credentials()?; + Ok(ChannelBuilder::new(env) + .max_send_message_len(100 << 20) + .max_receive_message_len(100 << 20) + .secure_connect(SPANNER_ADDRESS, creds)) + }) + .await + .map_err(|e| match e { + actix_web::error::BlockingError::Error(e) => e.into(), + actix_web::error::BlockingError::Canceled => { + DbError::internal("web::block Manager operation canceled") + } + })?; + let client = SpannerClient::new(chan); + + // Connect to the instance and create a Spanner session. + let session = create_session(&client, &self.database_name).await?; + + Ok(SpannerSession { + client, + session, + use_test_transactions: self.test_transactions, + }) + } + + async fn recycle(&self, conn: &mut SpannerSession) -> RecycleResult { + let mut req = GetSessionRequest::new(); + req.set_name(conn.session.get_name().to_owned()); + if let Err(e) = conn + .client + .get_session_async(&req) + .map_err(|e| RecycleError::Backend(e.into()))? + .await + { + match e { + grpcio::Error::RpcFailure(ref status) + if status.status == grpcio::RpcStatusCode::NOT_FOUND => + { + conn.session = create_session(&conn.client, &self.database_name) + .await + .map_err(|e| RecycleError::Backend(e.into()))?; + } + _ => return Err(RecycleError::Backend(e.into())), + } + } + Ok(()) + } +} diff --git a/src/db/spanner/manager/mod.rs b/src/db/spanner/manager/mod.rs new file mode 100644 index 0000000000..d29dca6f4b --- /dev/null +++ b/src/db/spanner/manager/mod.rs @@ -0,0 +1,2 @@ +pub mod bb8; +pub mod deadpool; diff --git a/src/db/spanner/models.rs b/src/db/spanner/models.rs index 17fd169c6c..cba4bbb8ee 100644 --- a/src/db/spanner/models.rs +++ b/src/db/spanner/models.rs @@ -80,8 +80,8 @@ struct SpannerDbSession { } #[derive(Clone, Debug)] -pub struct SpannerDb<'a> { - pub(super) inner: Arc>, +pub struct SpannerDb { + pub(super) inner: Arc, /// Pool level cache of collection_ids and their names coll_cache: Arc, @@ -89,28 +89,28 @@ pub struct SpannerDb<'a> { pub metrics: Metrics, } -pub struct SpannerDbInner<'a> { - pub(super) conn: Conn<'a>, +pub struct SpannerDbInner { + pub(super) conn: Conn, session: RefCell, } -impl fmt::Debug for SpannerDbInner<'_> { +impl fmt::Debug for SpannerDbInner { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "SpannerDbInner") } } -impl<'a> Deref for SpannerDb<'a> { - type Target = SpannerDbInner<'a>; +impl Deref for SpannerDb { + type Target = SpannerDbInner; fn deref(&self) -> &Self::Target { &self.inner } } -impl<'a> SpannerDb<'a> { - pub fn new(conn: Conn<'a>, coll_cache: Arc, metrics: &Metrics) -> Self { +impl SpannerDb { + pub fn new(conn: Conn, coll_cache: Arc, metrics: &Metrics) -> Self { let inner = SpannerDbInner { conn, session: RefCell::new(Default::default()), @@ -1604,7 +1604,7 @@ impl<'a> SpannerDb<'a> { } } -impl<'a> Db<'a> for SpannerDb<'a> { +impl<'a> Db<'a> for SpannerDb { fn commit(&self) -> DbFuture<'_, ()> { let db = self.clone(); Box::pin(async move { db.commit_async().map_err(Into::into).await }) diff --git a/src/db/spanner/pool.rs b/src/db/spanner/pool.rs index 43304456bd..94bcd5009f 100644 --- a/src/db/spanner/pool.rs +++ b/src/db/spanner/pool.rs @@ -1,5 +1,5 @@ use async_trait::async_trait; -use bb8::{ErrorSink, Pool, PooledConnection}; +use bb8::ErrorSink; use std::{ collections::HashMap, @@ -12,11 +12,12 @@ use crate::db::{error::DbError, results, Db, DbPool, STD_COLLS}; use crate::server::metrics::Metrics; use crate::settings::Settings; -use super::manager::{SpannerConnectionManager, SpannerSession}; +use super::manager::bb8::SpannerSession; use super::models::SpannerDb; use crate::error::ApiResult; -pub(super) type Conn<'a> = PooledConnection<'a, SpannerConnectionManager>; +//pub(super) type Conn<'a> = PooledConnection<'a, SpannerConnectionManager>; +pub(super) type Conn = deadpool::managed::Object; embed_migrations!(); @@ -32,7 +33,8 @@ embed_migrations!(); #[derive(Clone)] pub struct SpannerDbPool { /// Pool of db connections - pool: Pool>, + //pool: Pool>, + pool: deadpool::managed::Pool, /// In-memory cache of collection_ids and their names coll_cache: Arc, @@ -47,24 +49,33 @@ impl SpannerDbPool { } pub async fn new_without_migrations(settings: &Settings, metrics: &Metrics) -> Result { - let manager = SpannerConnectionManager::::new(settings, metrics)?; + //let manager = SpannerConnectionManager::::new(settings, metrics)?; let max_size = settings.database_pool_max_size.unwrap_or(10); + /* let builder = bb8::Pool::builder() .max_size(max_size) .min_idle(settings.database_pool_min_idle) .error_sink(Box::new(LoggingErrorSink)); + */ + let manager = super::manager::deadpool::Manager::new(settings, metrics)?; + let config = deadpool::managed::PoolConfig::new(max_size as usize); + let pool = deadpool::managed::Pool::from_config(manager, config); Ok(Self { - pool: builder.build(manager).await?, + pool, coll_cache: Default::default(), metrics: metrics.clone(), }) } - pub async fn get_async(&self) -> Result> { + pub async fn get_async(&self) -> Result { let conn = self.pool.get().await.map_err(|e| match e { - bb8::RunError::User(dbe) => dbe, - bb8::RunError::TimedOut => DbError::internal("bb8:TimedOut"), + //bb8::RunError::User(dbe) => dbe, + //bb8::RunError::TimedOut => DbError::internal("bb8:TimedOut"), + deadpool::managed::PoolError::Backend(dbe) => dbe, + deadpool::managed::PoolError::Timeout(timeout_type) => { + DbError::internal(&format!("deadpool Timeout: {:?}", timeout_type)) + } })?; Ok(SpannerDb::new( conn, @@ -84,7 +95,7 @@ impl DbPool for SpannerDbPool { } fn state(&self) -> results::PoolState { - self.pool.state().into() + self.pool.status().into() } fn validate_batch_id(&self, id: String) -> Result<()> { diff --git a/src/db/spanner/support.rs b/src/db/spanner/support.rs index 5766cb4748..6c6ca0ccfd 100644 --- a/src/db/spanner/support.rs +++ b/src/db/spanner/support.rs @@ -88,7 +88,7 @@ impl ExecuteSqlRequestBuilder { self } - fn prepare_request(self, conn: &Conn<'_>) -> ExecuteSqlRequest { + fn prepare_request(self, conn: &Conn) -> ExecuteSqlRequest { let mut request = self.execute_sql; request.set_session(conn.session.get_name().to_owned()); if let Some(params) = self.params { @@ -103,7 +103,7 @@ impl ExecuteSqlRequestBuilder { } /// Execute a SQL read statement but return a non-blocking streaming result - pub fn execute_async(self, conn: &Conn<'_>) -> Result { + pub fn execute_async(self, conn: &Conn) -> Result { let stream = conn .client .execute_streaming_sql(&self.prepare_request(conn))?; @@ -111,7 +111,7 @@ impl ExecuteSqlRequestBuilder { } /// Execute a DML statement, returning the exact count of modified rows - pub async fn execute_dml_async(self, conn: &Conn<'_>) -> Result { + pub async fn execute_dml_async(self, conn: &Conn) -> Result { let rs = conn .client .execute_sql_async(&self.prepare_request(conn))? From a9da1541dd8aa0a7bc964ae8116edb1765d7d4b5 Mon Sep 17 00:00:00 2001 From: Philip Jenvey Date: Fri, 21 Aug 2020 17:14:31 -0700 Subject: [PATCH 17/17] f kill dev comments --- src/db/spanner/manager/bb8.rs | 2 -- src/db/spanner/manager/deadpool.rs | 4 ---- src/db/spanner/pool.rs | 10 ---------- 3 files changed, 16 deletions(-) diff --git a/src/db/spanner/manager/bb8.rs b/src/db/spanner/manager/bb8.rs index 181e01548a..b154889059 100644 --- a/src/db/spanner/manager/bb8.rs +++ b/src/db/spanner/manager/bb8.rs @@ -18,7 +18,6 @@ use crate::{ settings::Settings, }; -// XXX: pub const SPANNER_ADDRESS: &str = "spanner.googleapis.com:443"; pub struct SpannerConnectionManager { @@ -132,7 +131,6 @@ impl ManageConnection } } -// XXX: pub async fn create_session( client: &SpannerClient, database_name: &str, diff --git a/src/db/spanner/manager/deadpool.rs b/src/db/spanner/manager/deadpool.rs index 0077b6705c..e75e7dd3ad 100644 --- a/src/db/spanner/manager/deadpool.rs +++ b/src/db/spanner/manager/deadpool.rs @@ -12,12 +12,8 @@ use crate::{ settings::Settings, }; -// XXX: use super::bb8::{create_session, SpannerSession, SPANNER_ADDRESS}; -// - -> SpannerSessionManager (and bb8 too) -// - bb8s doesn't need the PhantomData -// - kill the lifetimes for now or PhantomData one pub struct Manager { database_name: String, /// The gRPC environment diff --git a/src/db/spanner/pool.rs b/src/db/spanner/pool.rs index 94bcd5009f..74150e09dd 100644 --- a/src/db/spanner/pool.rs +++ b/src/db/spanner/pool.rs @@ -16,7 +16,6 @@ use super::manager::bb8::SpannerSession; use super::models::SpannerDb; use crate::error::ApiResult; -//pub(super) type Conn<'a> = PooledConnection<'a, SpannerConnectionManager>; pub(super) type Conn = deadpool::managed::Object; embed_migrations!(); @@ -49,14 +48,7 @@ impl SpannerDbPool { } pub async fn new_without_migrations(settings: &Settings, metrics: &Metrics) -> Result { - //let manager = SpannerConnectionManager::::new(settings, metrics)?; let max_size = settings.database_pool_max_size.unwrap_or(10); - /* - let builder = bb8::Pool::builder() - .max_size(max_size) - .min_idle(settings.database_pool_min_idle) - .error_sink(Box::new(LoggingErrorSink)); - */ let manager = super::manager::deadpool::Manager::new(settings, metrics)?; let config = deadpool::managed::PoolConfig::new(max_size as usize); let pool = deadpool::managed::Pool::from_config(manager, config); @@ -70,8 +62,6 @@ impl SpannerDbPool { pub async fn get_async(&self) -> Result { let conn = self.pool.get().await.map_err(|e| match e { - //bb8::RunError::User(dbe) => dbe, - //bb8::RunError::TimedOut => DbError::internal("bb8:TimedOut"), deadpool::managed::PoolError::Backend(dbe) => dbe, deadpool::managed::PoolError::Timeout(timeout_type) => { DbError::internal(&format!("deadpool Timeout: {:?}", timeout_type))