Merge pull request #13 from robsdedude/ci/api-checks

CI: add automated API checks

.github/workflows/tests.yaml

@@ -7,8 +7,48 @@ on:
     branches:
       - master
 jobs:
+  lints:
+    name: Linting
+    runs-on: "ubuntu-latest"
+    steps:
+      - uses: actions/checkout@v4
+      - name: install nightly toolchain for cargo-check-external-types
+        uses: dtolnay/rust-toolchain@v1
+        with:
+          toolchain: nightly-2024-02-07
+      - name: install latest nightly toolchain for rustfmt
+        uses: dtolnay/rust-toolchain@nightly
+        with:
+          components: rustfmt
+      - name: install stable toolchain
+        uses: dtolnay/rust-toolchain@stable
+        with:
+          components: clippy
+      - name: versions
+        run: |
+          set -x
+          rustup show
+          cargo --version 
+          cargo +nightly --version 
+          cargo clippy --version
+      - name: cargo fmt
+        run: cargo +nightly fmt --all -- --check
+      - name: clippy all
+        run: cargo clippy --all --all-features -- -D warnings
+      - name: clippy driver
+        run: cargo clippy -p neo4j -- -D warnings
+      - name:  check exported dependencies
+        run: |
+          set -x
+          pushd neo4j
+          cargo install --locked cargo-check-external-types
+          cargo +nightly-2024-02-07 check-external-types
+          popd
+      - name: API stability check
+        run: cargo test --workspace --features _internal_public_api public_api
+
   tests:
-    name: Linting and Testing
+    name: Testing
     runs-on: "ubuntu-latest"
     strategy:
       matrix:
@@ -25,27 +65,20 @@ jobs:
           -e NEO4J_dbms_security_auth__minimum__password__length=1
           neo4j:5.6-enterprise &
       - uses: actions/checkout@v4
-      - uses: dtolnay/rust-toolchain@nightly
-        with:
-          components: rustfmt
-      - uses: dtolnay/rust-toolchain@v1
+      - name: install ${{ matrix.rust-version }} toolchain to test against
+        uses: dtolnay/rust-toolchain@v1
         with:
           toolchain: ${{ matrix.rust-version }}
           components: clippy
       - name: versions
-        run: cargo --version && cargo clippy --version
-      - name: versions nightly
-        run: cargo +nightly --version && cargo +nightly fmt --version
+        run: | 
+          rustup show
+          cargo --version 
+          cargo clippy --version
       - name: cargo check
         env:
           RUSTFLAGS: -D warnings
         run: cargo check
-      - name: cargo fmt
-        run: cargo +nightly fmt --all -- --check
-      - name: clippy all
-        run: cargo clippy --all --all-features -- -D warnings
-      - name: clippy driver
-        run: cargo clippy -p neo4j -- -D warnings
       - name: docs build
         run: RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps
       - name: tests
@@ -68,9 +101,10 @@ jobs:
           TEST_NEO4J_EDITION: enterprise
           TEST_NEO4J_VERSION: 5.6
         run: cargo test --workspace --doc
+
   testkit:
     name: TestKit
-    needs: [ tests ]
+    needs: [ tests, lints ]
     runs-on: ubuntu-latest
     strategy:
       matrix:
@@ -117,3 +151,11 @@ jobs:
           cd testkit
           python3 main.py --tests ${{ matrix.tests }} \
             ${{ matrix.config && format('--config {0}', matrix.config) || ''}}
+
+  ci-end:
+    name: CI End
+    needs: [ tests, lints, testkit ]
+    runs-on: ubuntu-latest
+    steps:
+      - name: All Done!
+        run: echo This is a single job to check all the other jobs have passed.

.pre-commit-config.yaml

@@ -27,10 +27,16 @@ repos:
         pass_filenames: false
       - id: unit
         name: unit tests
-        entry: cargo test --tests --benches
+        entry: cargo test --workspace --tests --benches --features _internal_public_api
         language: system
         types: [rust]
         pass_filenames: false
+      - id: dependency type exports
+        name: check exported dependencies
+        entry: bash -ec "cd neo4j; cargo +nightly check-external-types"
+        language: system
+        types_or: [rust, toml]
+        pass_filenames: false
       - id: docs
         name: cargo doc
         entry: env RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps

CHANGELOG.md

@@ -5,47 +5,48 @@
 ## NEXT
 ***
 **⭐ New Features**
-- Add support for Bolt 5.2, which adds notification filtering.
-- Add support for Bolt 5.3 (bolt agent).
-- Add support for Bolt 5.4 (telemetry).
-- Add `Driver::is_encrypted()`.
-- Introduce `neo4j::driver::Conifg::with_keep_alive()` and `without_keep_alive()`.
+ - Add support for Bolt 5.2, which adds notification filtering.
+ - Add support for Bolt 5.3 (bolt agent).
+ - Add support for Bolt 5.4 (telemetry).
+ - Add `Driver::is_encrypted()`.
+ - Introduce `neo4j::driver::Conifg::with_keep_alive()` and `without_keep_alive()`.
 
 **🔧 Fixes**
-- Fix `Transaction::rolblack()` failing if a result stream failed before.
-- Fix errors during transaction `BEGIN` not being properly propagated.
-- Fix propagation of `is_retryable()` of errors within transactions.
-- Fix connection hint `connection.recv_timeout_seconds` not always being respected leading to connections timeing out too late.
+ - Fix `Transaction::rolblack()` failing if a result stream failed before.
+ - Fix errors during transaction `BEGIN` not being properly propagated.
+ - Fix propagation of `is_retryable()` of errors within transactions.
+ - Fix connection hint `connection.recv_timeout_seconds` not always being respected leading to connections timeing out too late.
 
 **🧹Clean-up**
-- ⚠️ Removed useless lifetime parameter from `SessionConfig::with_database()`.
-- ⚠️ Changed return type of `ConnectionConfig::with_encryption_trust_any_certificate() ` from `Result<Self, Error>` to `Self`.
-- ⚠️ Reduce the number of lifetime generic parameters in `TransactionQueryBuilder` and `TransactionRecordStream`.
+ - ⚠️ Remove useless lifetime parameter from `SessionConfig::with_database()`.
+ - ⚠️ Change return type of `ConnectionConfig::with_encryption_trust_any_certificate() ` from `Result<Self, Error>` to `Self`.
+ - ⚠️ Reduce the number of lifetime generic parameters in `TransactionQueryBuilder` and `TransactionRecordStream`.
+ - ⚠️ Remove `impl From<URIError> for ConnectionConfigParseError`.
 
 
 ## 0.0.2
 ***
 **👏 Improvements**
-- Impl `FromStr` for `neo4j::driver::ConnectionConfig` (besides `TryFrom<&str>`).
+ - Impl `FromStr` for `neo4j::driver::ConnectionConfig` (besides `TryFrom<&str>`).
 
-- **🧹Clean-up**
-- ⚠️ Update dependencies.  
+ - **🧹Clean-up**
+ - ⚠️ Update dependencies.  
   Among others `rustls`.
   To accommodate this change, the `rustls_dangerous_configuration` feature was removed.
   This update also affects `ConnectionConfig::with_encryption_custom_tls_config()`, which accepts a
   custom `rustls::ClientConfig`.
-- ⚠️ Make `Record{entries}` private and offer many helper methods instead.
-- Add `EagerResult::into_scalar()`.
-- ⚠️ Renamed `RetryableError` to `RetryError`
-- Fix `Driver::execute_query()::run()` not committing the transaction.
-- ⚠️ Removed `AutoCommitBuilder::without_transaction_timeout` and `AutoCommitBuilder::with_default_transaction_timeout`
+ - ⚠️ Make `Record{entries}` private and offer many helper methods instead.
+ - Add `EagerResult::into_scalar()`.
+ - ⚠️ Renamed `RetryableError` to `RetryError`
+ - Fix `Driver::execute_query()::run()` not committing the transaction.
+ - ⚠️ Removed `AutoCommitBuilder::without_transaction_timeout` and `AutoCommitBuilder::with_default_transaction_timeout`
   in favor of `AutoCommitBuilder::with_transaction_timeout` in combination with `TransactionTimeout::none`,
   `TransactionTimeout::from_millis` and `TransactionTimeout::default`.  
   Same for `TransactionBuilder`.
-- ⚠️ Move `neo4j::Address` to `neo4j::address::Address`
+ - ⚠️ Move `neo4j::Address` to `neo4j::address::Address`
 
 **📚 Docs**
-- Much more documentation.
+ - Much more documentation.
 
 ## 0.0.1
 

CONTRIBUTING.md

@@ -0,0 +1,28 @@
+# Contributing
+
+## Used Tools
+### Pre-Commit
+https://pre-commit.com/
+Automatically runs some checks before committing, so you don't have to wait for the CI to complain.
+```bash
+pip install pre-commit
+pre-commit install
+```
+
+To skip the checks, use `git commit --no-verify`.
+
+
+### Cargo Public API
+https://github.com/enselic/cargo-public-api
+If you intentionally change the public API, you need to run:
+```bash
+UPDATE_EXPECT=1 cargo test --workspace --features _internal_public_api public_api
+```
+**IMPORTANT:** make sure to run this with the latest stable toolchain have all the latest blanket impls included. 
+
+
+### Cargo Check External Types
+https://github.com/awslabs/cargo-check-external-types
+```bash
+cargo install --locked cargo-check-external-types
+```

README.md

@@ -105,10 +105,12 @@ A bump in MSRV is considered a minor breaking change.
    * [x] rustfmt
    * [x] clippy
    * [x] Unit Tests
-     * [ ] Public API SemVer Compatibility Tests ([cargo-public-api](https://github.com/enselic/cargo-public-api))
+     * [x] Public API SemVer Compatibility Tests ([cargo-public-api](https://github.com/enselic/cargo-public-api))
+   * [x] Exposed Dependency Types Check ([cargo-check-external-types](https://github.com/awslabs/cargo-check-external-types))
    * [x] TestKit
 
-## Note on async  
+
+## Note on async
 Currently, there are no plans to add async support until a proper abstraction over multiple runtimes is available so that users can choose their preferred runtime.
 As it stands, the async ecosystem would either force this crate to dictate a runtime or do an ugly dance to support multiple runtimes.
 Even then, the supported runtimes would be limited to the ones chosen by this crate.

neo4j/Cargo.toml

@@ -17,8 +17,14 @@ exclude = ["/testkit"]
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [features]
-# internal feature, do not use, not supported
+# internal features: DO NOT USE! No support, no sem-ver guarantees, no nothing.
 _internal_testkit_backend = []
+_internal_public_api = [
+    "public-api",
+    "expect-test",
+    "rustdoc-json",
+    "rustup-toolchain",
+]
 
 [dependencies]
 atomic_refcell = "0.1.13"
@@ -40,9 +46,29 @@ thiserror = "1.0.58"
 uriparse = "0.6.4"
 usize_cast = "1.1.0"
 
+rustup-toolchain = { version = "0.1.6", optional = true }
+rustdoc-json = { version = "0.9.0", optional = true }
+public-api = { version = "0.33.1", optional = true }
+expect-test = { version = "1.5.0", optional = true }
+
 [dev-dependencies]
 doc_test_utils = { path = "../doc_test_utils" }
 env_logger = "0.10.2"
 lazy_static = "1.4.0"
 mockall = "0.12.1"
 rstest = "0.18.2"
+
+[package.metadata.cargo_check_external_types]
+allowed_external_types = [
+    "chrono::date::Date",
+    "chrono::datetime::DateTime",
+    "chrono::naive::date::NaiveDate",
+    "chrono::naive::datetime::NaiveDateTime",
+    "chrono::naive::time::NaiveTime",
+    "chrono::offset::fixed::FixedOffset",
+    "chrono::time::Time",
+
+    "chrono_tz::timezones::Tz",
+
+    "rustls::client::client_conn::ClientConfig",
+]

neo4j/src/driver/config.rs

@@ -26,7 +26,7 @@ use std::time::Duration;
 use mockall_double::double;
 use rustls::ClientConfig;
 use thiserror::Error;
-use uriparse::{Query, URIError, URI};
+use uriparse::{Query, URI};
 
 use crate::address_::resolution::AddressResolver;
 use crate::address_::Address;
@@ -722,7 +722,7 @@ impl ConnectionConfig {
     }
 
     fn parse_uri(uri: &str) -> StdResult<ConnectionConfig, ConnectionConfigParseError> {
-        let uri = URI::try_from(uri)?;
+        let uri = URI::try_from(uri).map_err(URIError)?;
 
         let (routing, tls_config) = match uri.scheme().as_str() {
             "neo4j" => (true, None),
@@ -868,9 +868,12 @@ pub struct ConnectionConfigParseError(String);
 
 impl ConnectionConfigParseError {}
 
+#[derive(Debug)]
+struct URIError(uriparse::URIError);
+
 impl From<URIError> for ConnectionConfigParseError {
     fn from(e: URIError) -> Self {
-        ConnectionConfigParseError(format!("couldn't parse URI {e}"))
+        ConnectionConfigParseError(format!("couldn't parse URI {}", e.0))
     }
 }
 

neo4j/src/driver/io/bolt/bolt5x4/protocol.rs

@@ -45,24 +45,18 @@ use super::super::{
 use crate::error_::Result;
 use crate::value::ValueReceive;
 
-const SERVER_AGENT_KEY: &str = "server";
 const HINTS_KEY: &str = "hints";
-const RECV_TIMEOUT_KEY: &str = "connection.recv_timeout_seconds";
 const TELEMETRY_ENABLED_KEY: &str = "telemetry.enabled";
 
 #[derive(Debug)]
 pub(crate) struct Bolt5x4<T: BoltStructTranslator> {
-    translator: T,
     pub(in super::super) bolt5x3: Bolt5x3<T>,
-    protocol_version: ServerAwareBoltVersion,
 }
 
 impl<T: BoltStructTranslator> Bolt5x4<T> {
     pub(in super::super) fn new(protocol_version: ServerAwareBoltVersion) -> Self {
         Self {
-            translator: T::default(),
             bolt5x3: Bolt5x3::new(protocol_version),
-            protocol_version,
         }
     }
 

neo4j/src/lib.rs

@@ -235,3 +235,28 @@ mod private {
     // Trait to prevent traits from being implemented outside of this crate.
     pub trait Sealed {}
 }
+
+#[cfg(test)]
+mod test {
+    #[cfg(feature = "public-api")]
+    #[test]
+    fn public_api() {
+        // Install a compatible nightly toolchain if it is missing
+        rustup_toolchain::install(public_api::MINIMUM_NIGHTLY_RUST_VERSION).unwrap();
+
+        // Build rustdoc JSON
+        let rustdoc_json = rustdoc_json::Builder::default()
+            .toolchain(public_api::MINIMUM_NIGHTLY_RUST_VERSION)
+            .build()
+            .unwrap();
+
+        // Derive the public API from the rustdoc JSON
+        let public_api = public_api::Builder::from_rustdoc_json(rustdoc_json)
+            .omit_blanket_impls(true)
+            .build()
+            .unwrap();
+
+        // Assert that the public API looks correct
+        expect_test::expect_file!["test_data/public-api.txt"].assert_eq(&public_api.to_string());
+    }
+}