Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GitHub Actions and integration test #1346

Merged
merged 11 commits into from
Jul 28, 2021
Merged

Fix GitHub Actions and integration test #1346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ef589ce
fix test suite
Jul 23, 2021
d2f990e
rustfmt
Jul 27, 2021
00cfc4a
need Row
Jul 27, 2021
a70decf
test: fix integration test scripts and update the upstream supported …
AtkinsChang Jul 27, 2021
0a72efa
ci(actions): update supported databases
AtkinsChang Jul 27, 2021
812d920
ci(actions): use `pg_isready` instead of `sleep` to avoid error cause…
AtkinsChang Jul 25, 2021
15215de
feat(core): add `trait PgConnectionInfo` for connection parameter sta…
AtkinsChang Jul 27, 2021
7934e0d
test(postgres): fix integration test for postgres
AtkinsChang Jul 25, 2021
f5c0c4a
test(mysql): fix integration tests
AtkinsChang Jul 27, 2021
002e996
ci(actions): test database against the oldest and newest supported ve…
AtkinsChang Jul 28, 2021
61899d3
docs(core): document `trait PgConnectionInfo`
AtkinsChang Jul 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 40 additions & 32 deletions .github/workflows/sqlx.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
runtime: [async-std-native-tls, tokio-native-tls, actix-native-tls, async-std-rustls, tokio-rustls, actix-rustls]
runtime: [async-std, tokio, actix]
tls: [native-tls, rustls]
steps:
- uses: actions/checkout@v2

Expand All @@ -48,29 +49,30 @@ jobs:
~/.cargo/registry
~/.cargo/git
target
key: ${{ runner.os }}-check-${{ matrix.runtime }}-${{ hashFiles('**/Cargo.lock') }}
key: ${{ runner.os }}-check-${{ matrix.runtime }}-${{ matrix.tls }}-${{ hashFiles('**/Cargo.lock') }}

- uses: actions-rs/cargo@v1
with:
command: check
args: >
--manifest-path sqlx-core/Cargo.toml
--no-default-features
--features offline,all-databases,all-types,migrate,runtime-${{ matrix.runtime }}
--features offline,all-databases,all-types,migrate,runtime-${{ matrix.runtime }}-${{ matrix.tls }}

- uses: actions-rs/cargo@v1
with:
command: check
args: >
--no-default-features
--features offline,all-databases,all-types,migrate,runtime-${{ matrix.runtime }},macros
--features offline,all-databases,all-types,migrate,runtime-${{ matrix.runtime }}-${{ matrix.tls }},macros

test:
name: Unit Test
runs-on: ubuntu-20.04
strategy:
matrix:
runtime: [async-std-native-tls, tokio-native-tls, actix-native-tls, async-std-rustls, tokio-rustls, actix-rustls]
runtime: [async-std, tokio, actix]
tls: [native-tls, rustls]
steps:
- uses: actions/checkout@v2

Expand All @@ -93,7 +95,7 @@ jobs:
command: test
args: >
--manifest-path sqlx-core/Cargo.toml
--features offline,all-databases,all-types,runtime-${{ matrix.runtime }}
--features offline,all-databases,all-types,runtime-${{ matrix.runtime }}-${{ matrix.tls }}

cli:
name: CLI Binaries
Expand Down Expand Up @@ -148,7 +150,8 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
runtime: [async-std-native-tls, tokio-native-tls, actix-native-tls, async-std-rustls, tokio-rustls, actix-rustls]
runtime: [async-std, tokio, actix]
tls: [native-tls, rustls]
needs: check
steps:
- uses: actions/checkout@v2
Expand All @@ -165,14 +168,14 @@ jobs:
~/.cargo/registry
~/.cargo/git
target
key: ${{ runner.os }}-sqlite-${{ matrix.runtime }}-${{ hashFiles('**/Cargo.lock') }}
key: ${{ runner.os }}-sqlite-${{ matrix.runtime }}-${{ matrix.tls }}-${{ hashFiles('**/Cargo.lock') }}

- uses: actions-rs/cargo@v1
with:
command: test
args: >
--no-default-features
--features any,macros,migrate,sqlite,all-types,runtime-${{ matrix.runtime }}
--features any,macros,migrate,sqlite,all-types,runtime-${{ matrix.runtime }}-${{ matrix.tls }}
--
--test-threads=1
env:
Expand All @@ -183,8 +186,9 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
postgres: [12, 10, 9_6, 9_5]
runtime: [async-std-native-tls, tokio-native-tls, actix-native-tls, async-std-rustls, tokio-rustls, actix-rustls]
postgres: [13, 9_6]
runtime: [async-std, tokio, actix]
tls: [native-tls, rustls]
needs: check
steps:
- uses: actions/checkout@v2
Expand All @@ -201,23 +205,24 @@ jobs:
~/.cargo/registry
~/.cargo/git
target
key: ${{ runner.os }}-postgres-${{ matrix.runtime }}-${{ hashFiles('**/Cargo.lock') }}
key: ${{ runner.os }}-postgres-${{ matrix.runtime }}-${{ matrix.tls }}-${{ hashFiles('**/Cargo.lock') }}

- uses: actions-rs/cargo@v1
with:
command: build
args: >
--features postgres,all-types,runtime-${{ matrix.runtime }}
--features postgres,all-types,runtime-${{ matrix.runtime }}-${{ matrix.tls }}

- run: docker-compose -f tests/docker-compose.yml run -d -p 5432:5432 postgres_${{ matrix.postgres }}
- run: sleep 10
- run: |
docker-compose -f tests/docker-compose.yml run -d -p 5432:5432 --name postgres_${{ matrix.postgres }} postgres_${{ matrix.postgres }}
docker exec postgres_${{ matrix.postgres }} bash -c "until pg_isready; do sleep 1; done"

- uses: actions-rs/cargo@v1
with:
command: test
args: >
--no-default-features
--features any,postgres,macros,all-types,runtime-${{ matrix.runtime }}
--features any,postgres,macros,all-types,runtime-${{ matrix.runtime }}-${{ matrix.tls }}
env:
DATABASE_URL: postgres://postgres:password@localhost:5432/sqlx

Expand All @@ -226,7 +231,7 @@ jobs:
command: test
args: >
--no-default-features
--features any,postgres,macros,migrate,all-types,runtime-${{ matrix.runtime }}
--features any,postgres,macros,migrate,all-types,runtime-${{ matrix.runtime }}-${{ matrix.tls }}
env:
DATABASE_URL: postgres://postgres:password@localhost:5432/sqlx?sslmode=verify-ca&sslrootcert=.%2Ftests%2Fcerts%2Fca.crt

Expand All @@ -235,8 +240,9 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
mysql: [8, 5_7, 5_6]
runtime: [async-std-native-tls, tokio-native-tls, actix-native-tls, async-std-rustls, tokio-rustls, actix-rustls]
mysql: [8, 5_6]
runtime: [async-std, tokio, actix]
tls: [native-tls, rustls]
needs: check
steps:
- uses: actions/checkout@v2
Expand All @@ -253,13 +259,13 @@ jobs:
~/.cargo/registry
~/.cargo/git
target
key: ${{ runner.os }}-mysql-${{ matrix.runtime }}-${{ hashFiles('**/Cargo.lock') }}
key: ${{ runner.os }}-mysql-${{ matrix.runtime }}-${{ matrix.tls }}-${{ hashFiles('**/Cargo.lock') }}

- uses: actions-rs/cargo@v1
with:
command: build
args: >
--features mysql,all-types,runtime-${{ matrix.runtime }}
--features mysql,all-types,runtime-${{ matrix.runtime }}-${{ matrix.tls }}

- run: docker-compose -f tests/docker-compose.yml run -d -p 3306:3306 mysql_${{ matrix.mysql }}
- run: sleep 60
Expand All @@ -269,7 +275,7 @@ jobs:
command: test
args: >
--no-default-features
--features any,mysql,macros,migrate,all-types,runtime-${{ matrix.runtime }}
--features any,mysql,macros,migrate,all-types,runtime-${{ matrix.runtime }}-${{ matrix.tls }}
env:
DATABASE_URL: mysql://root:password@localhost:3306/sqlx

Expand All @@ -278,8 +284,9 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
mariadb: [10_5, 10_4, 10_3, 10_2, 10_1]
runtime: [async-std-native-tls, tokio-native-tls, actix-native-tls, async-std-rustls, tokio-rustls, actix-rustls]
mariadb: [10_6, 10_2]
runtime: [async-std, tokio, actix]
tls: [native-tls, rustls]
needs: check
steps:
- uses: actions/checkout@v2
Expand All @@ -297,13 +304,13 @@ jobs:
~/.cargo/registry
~/.cargo/git
target
key: ${{ runner.os }}-mysql-${{ matrix.runtime }}-${{ hashFiles('**/Cargo.lock') }}
key: ${{ runner.os }}-mysql-${{ matrix.runtime }}-${{ matrix.tls }}-${{ hashFiles('**/Cargo.lock') }}

- uses: actions-rs/cargo@v1
with:
command: build
args: >
--features mysql,all-types,runtime-${{ matrix.runtime }}
--features mysql,all-types,runtime-${{ matrix.runtime }}-${{ matrix.tls }}

- run: docker-compose -f tests/docker-compose.yml run -d -p 3306:3306 mariadb_${{ matrix.mariadb }}
- run: sleep 30
Expand All @@ -313,7 +320,7 @@ jobs:
command: test
args: >
--no-default-features
--features any,mysql,macros,migrate,all-types,runtime-${{ matrix.runtime }}
--features any,mysql,macros,migrate,all-types,runtime-${{ matrix.runtime }}-${{ matrix.tls }}
env:
DATABASE_URL: mysql://root:password@localhost:3306/sqlx

Expand All @@ -322,8 +329,9 @@ jobs:
runs-on: ubuntu-20.04
strategy:
matrix:
mssql: [2019]
runtime: [async-std-native-tls, tokio-native-tls, actix-native-tls, async-std-rustls, tokio-rustls, actix-rustls]
mssql: [2019, 2017]
runtime: [async-std, tokio, actix]
tls: [native-tls, rustls]
needs: check
steps:
- uses: actions/checkout@v2
Expand All @@ -340,13 +348,13 @@ jobs:
~/.cargo/registry
~/.cargo/git
target
key: ${{ runner.os }}-mssql-${{ matrix.runtime }}-${{ hashFiles('**/Cargo.lock') }}
key: ${{ runner.os }}-mssql-${{ matrix.runtime }}-${{ matrix.tls }}-${{ hashFiles('**/Cargo.lock') }}

- uses: actions-rs/cargo@v1
with:
command: build
args: >
--features mssql,all-types,runtime-${{ matrix.runtime }}
--features mssql,all-types,runtime-${{ matrix.runtime }}-${{ matrix.tls }}

- run: docker-compose -f tests/docker-compose.yml run -d -p 1433:1433 mssql_${{ matrix.mssql }}
- run: sleep 80 # MSSQL takes a "bit" to startup
Expand All @@ -356,6 +364,6 @@ jobs:
command: test
args: >
--no-default-features
--features any,mssql,macros,migrate,all-types,runtime-${{ matrix.runtime }}
--features any,mssql,macros,migrate,all-types,runtime-${{ matrix.runtime }}-${{ matrix.tls }}
env:
DATABASE_URL: mssql://sa:Password123!@localhost/sqlx
17 changes: 17 additions & 0 deletions sqlx-core/src/postgres/connection/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,20 @@ impl Connection for PgConnection {
!self.stream.wbuf.is_empty()
}
}

pub trait PgConnectionInfo {
/// the version number of the server in `libpq` format
fn server_version_num(&self) -> Option<u32>;
}

impl PgConnectionInfo for PgConnection {
fn server_version_num(&self) -> Option<u32> {
self.stream.server_version_num
}
}

impl PgConnectionInfo for crate::pool::PoolConnection<Postgres> {
fn server_version_num(&self) -> Option<u32> {
self.stream.server_version_num
}
}
88 changes: 86 additions & 2 deletions sqlx-core/src/postgres/connection/stream.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use std::collections::BTreeMap;
use std::ops::{Deref, DerefMut};
use std::str::FromStr;

use bytes::{Buf, Bytes};
use futures_channel::mpsc::UnboundedSender;
Expand All @@ -8,7 +10,7 @@ use log::Level;
use crate::error::Error;
use crate::io::{BufStream, Decode, Encode};
use crate::net::{MaybeTlsStream, Socket};
use crate::postgres::message::{Message, MessageFormat, Notice, Notification};
use crate::postgres::message::{Message, MessageFormat, Notice, Notification, ParameterStatus};
use crate::postgres::{PgConnectOptions, PgDatabaseError, PgSeverity};

// the stream is a separate type from the connection to uphold the invariant where an instantiated
Expand All @@ -27,6 +29,10 @@ pub struct PgStream {
// this is set when creating a PgListener and only written to if that listener is
// re-used for query execution in-between receiving messages
pub(crate) notifications: Option<UnboundedSender<Notification>>,

pub(crate) parameter_statuses: BTreeMap<String, String>,

pub(crate) server_version_num: Option<u32>,
}

impl PgStream {
Expand All @@ -41,6 +47,8 @@ impl PgStream {
Ok(Self {
inner,
notifications: None,
parameter_statuses: BTreeMap::default(),
server_version_num: None,
Comment on lines +50 to +51
Copy link
Contributor Author

@AtkinsChang AtkinsChang Jul 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abonander I can only put these things in PgStream instead of PgConnection without refactoring the Postgres part.

There is no mechanism to mark the connection as broken and not be used in the future when receiving invalid server message (ex. client_encoding change to non UTF-8) . Any suggestion or plan?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering we were just ignoring ParameterStatus before, I don't think it matters that much. Maybe in the future with this data we can add support for non-UTF-8 text encodings but those are also rapidly falling out of use, so probably only if someone comes along and says "I really really need support for this text encoding and I'm willing to pay a bug bounty for it."

I'm also fine having this stuff on PgStream, it's easier than refactoring all calls of .recv() to handle ParameterStatus.

})
}

Expand Down Expand Up @@ -108,7 +116,18 @@ impl PgStream {
// informs the frontend about the current (initial)
// setting of backend parameters

// we currently have no use for that data so we promptly ignore this message
let ParameterStatus { name, value } = message.decode()?;
// TODO: handle `client_encoding`, `DateStyle` change

match name.as_str() {
"server_version" => {
self.server_version_num = parse_server_version(&value);
}
_ => {
self.parameter_statuses.insert(name, value);
}
}

continue;
}

Expand Down Expand Up @@ -165,3 +184,68 @@ impl DerefMut for PgStream {
&mut self.inner
}
}

// reference:
// https://github.com/postgres/postgres/blob/6feebcb6b44631c3dc435e971bd80c2dd218a5ab/src/interfaces/libpq/fe-exec.c#L1030-L1065
fn parse_server_version(s: &str) -> Option<u32> {
let mut parts = Vec::<u32>::with_capacity(3);

let mut from = 0;
let mut chs = s.char_indices().peekable();
while let Some((i, ch)) = chs.next() {
match ch {
'.' => {
if let Ok(num) = u32::from_str(&s[from..i]) {
parts.push(num);
from = i + 1;
} else {
break;
}
}
_ if ch.is_digit(10) => {
if chs.peek().is_none() {
if let Ok(num) = u32::from_str(&s[from..]) {
parts.push(num);
}
break;
}
}
_ => {
if let Ok(num) = u32::from_str(&s[from..i]) {
parts.push(num);
}
break;
}
};
}

let version_num = match parts.as_slice() {
[major, minor, rev] => (100 * major + minor) * 100 + rev,
[major, minor] if *major >= 10 => 100 * 100 * major + minor,
[major, minor] => (100 * major + minor) * 100,
[major] => 100 * 100 * major,
_ => return None,
};

Some(version_num)
}

#[cfg(test)]
mod tests {
use super::parse_server_version;

#[test]
fn test_parse_server_version_num() {
// old style
assert_eq!(parse_server_version("9.6.1"), Some(90601));
// new style
assert_eq!(parse_server_version("10.1"), Some(100001));
// old style without minor version
assert_eq!(parse_server_version("9.6devel"), Some(90600));
// new style without minor version, e.g. */
assert_eq!(parse_server_version("10devel"), Some(100000));
assert_eq!(parse_server_version("13devel87"), Some(130000));
// unknown
assert_eq!(parse_server_version("unknown"), None);
}
}
Loading