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.

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
72 changes: 40 additions & 32 deletions .github/workflows/sqlx.yml
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
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
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

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