Skip to content

Commit

Permalink
Include only built-in compression algorithms as enum variants (#2121)
Browse files Browse the repository at this point in the history 
* Include only built-in compression algorithms as enum variants

This enables compile-time errors when a compression algorithm is requested which
is not actually enabled for the current Cargo project. The cost is that indexes
using other compression algorithms cannot even be loaded (even though they
are not fully accessible in any case).

As a drive-by, this also fixes `--no-default-features` on `cfg(unix)`.

* Provide more instructive error messages for unsupported, but not unknown compression variants.
  • Loading branch information
@adamreichold
adamreichold authored Jul 14, 2023
1 parent 5fafe4b commit 7e6c4a1
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 114 deletions.
25 changes: 24 additions & 1 deletion src/core/index_meta.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,9 @@ mod tests {
use super::IndexMeta;
use crate::core::index_meta::UntrackedIndexMeta;
use crate::schema::{Schema, TEXT};
use crate::store::{Compressor, ZstdCompressor};
use crate::store::Compressor;
#[cfg(feature = "zstd-compression")]
use crate::store::ZstdCompressor;
use crate::{IndexSettings, IndexSortByField, Order};

#[test]
Expand Down Expand Up @@ -446,6 +448,7 @@ mod tests {
}

#[test]
#[cfg(feature = "zstd-compression")]
fn test_serialize_metas_zstd_compressor() {
let schema = {
let mut schema_builder = Schema::builder();
Expand Down Expand Up @@ -482,6 +485,12 @@ mod tests {
}

#[test]
#[cfg(all(
feature = "lz4-compression",
feature = "brotli-compression",
feature = "snappy-compression",
feature = "zstd-compression"
))]
fn test_serialize_metas_invalid_comp() {
let json = r#"{"index_settings":{"sort_by_field":{"field":"text","order":"Asc"},"docstore_compression":"zsstd","docstore_blocksize":1000000},"segments":[],"schema":[{"name":"text","type":"text","options":{"indexing":{"record":"position","fieldnorms":true,"tokenizer":"default"},"stored":false,"fast":false}}],"opstamp":0}"#;

Expand All @@ -502,6 +511,20 @@ mod tests {
);
}

#[test]
#[cfg(not(feature = "zstd-compression"))]
fn test_serialize_metas_unsupported_comp() {
let json = r#"{"index_settings":{"sort_by_field":{"field":"text","order":"Asc"},"docstore_compression":"zstd","docstore_blocksize":1000000},"segments":[],"schema":[{"name":"text","type":"text","options":{"indexing":{"record":"position","fieldnorms":true,"tokenizer":"default"},"stored":false,"fast":false}}],"opstamp":0}"#;

let err = serde_json::from_str::<UntrackedIndexMeta>(json).unwrap_err();
assert_eq!(
err.to_string(),
"unsupported variant `zstd`, please enable Tantivy's `zstd-compression` feature at \
line 1 column 95"
.to_string()
);
}

#[test]
#[cfg(feature = "lz4-compression")]
fn test_index_settings_default() {
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ pub use crate::schema::{DateOptions, DateTimePrecision, Document, Term};
/// Index format version.
const INDEX_FORMAT_VERSION: u32 = 5;

#[cfg(unix)]
#[cfg(all(feature = "mmap", unix))]
pub use memmap2::Advice;

/// Structure version for the index.
Expand Down
156 changes: 84 additions & 72 deletions src/store/compressors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@ pub enum Compressor {
/// No compression
None,
/// Use the lz4 compressor (block format)
#[cfg(feature = "lz4-compression")]
Lz4,
/// Use the brotli compressor
#[cfg(feature = "brotli-compression")]
Brotli,
/// Use the snap compressor
#[cfg(feature = "snappy-compression")]
Snappy,
/// Use the zstd compressor
#[cfg(feature = "zstd-compression")]
Zstd(ZstdCompressor),
}

Expand All @@ -31,9 +35,13 @@ impl Serialize for Compressor {
where S: serde::Serializer {
match *self {
Compressor::None => serializer.serialize_str("none"),
#[cfg(feature = "lz4-compression")]
Compressor::Lz4 => serializer.serialize_str("lz4"),
#[cfg(feature = "brotli-compression")]
Compressor::Brotli => serializer.serialize_str("brotli"),
#[cfg(feature = "snappy-compression")]
Compressor::Snappy => serializer.serialize_str("snappy"),
#[cfg(feature = "zstd-compression")]
Compressor::Zstd(zstd) => serializer.serialize_str(&zstd.ser_to_string()),
}
}
Expand All @@ -45,27 +53,60 @@ impl<'de> Deserialize<'de> for Compressor {
let buf = String::deserialize(deserializer)?;
let compressor = match buf.as_str() {
"none" => Compressor::None,
#[cfg(feature = "lz4-compression")]
"lz4" => Compressor::Lz4,
#[cfg(not(feature = "lz4-compression"))]
"lz4" => {
return Err(serde::de::Error::custom(
"unsupported variant `lz4`, please enable Tantivy's `lz4-compression` feature",
))
}
#[cfg(feature = "brotli-compression")]
"brotli" => Compressor::Brotli,
#[cfg(not(feature = "brotli-compression"))]
"brotli" => {
return Err(serde::de::Error::custom(
"unsupported variant `brotli`, please enable Tantivy's `brotli-compression` \
feature",
))
}
#[cfg(feature = "snappy-compression")]
"snappy" => Compressor::Snappy,
#[cfg(not(feature = "snappy-compression"))]
"snappy" => {
return Err(serde::de::Error::custom(
"unsupported variant `snappy`, please enable Tantivy's `snappy-compression` \
feature",
))
}
#[cfg(feature = "zstd-compression")]
_ if buf.starts_with("zstd") => Compressor::Zstd(
ZstdCompressor::deser_from_str(&buf).map_err(serde::de::Error::custom)?,
),
#[cfg(not(feature = "zstd-compression"))]
_ if buf.starts_with("zstd") => {
return Err(serde::de::Error::custom(
"unsupported variant `zstd`, please enable Tantivy's `zstd-compression` \
feature",
))
}
_ => {
if buf.starts_with("zstd") {
Compressor::Zstd(
ZstdCompressor::deser_from_str(&buf).map_err(serde::de::Error::custom)?,
)
} else {
return Err(serde::de::Error::unknown_variant(
&buf,
&[
"none",
"lz4",
"brotli",
"snappy",
"zstd",
"zstd(compression_level=5)",
],
));
}
return Err(serde::de::Error::unknown_variant(
&buf,
&[
"none",
#[cfg(feature = "lz4-compression")]
"lz4",
#[cfg(feature = "brotli-compression")]
"brotli",
#[cfg(feature = "snappy-compression")]
"snappy",
#[cfg(feature = "zstd-compression")]
"zstd",
#[cfg(feature = "zstd-compression")]
"zstd(compression_level=5)",
],
));
}
};

Expand Down Expand Up @@ -127,18 +168,21 @@ impl ZstdCompressor {
}

impl Default for Compressor {
#[allow(unreachable_code)]
fn default() -> Self {
if cfg!(feature = "lz4-compression") {
Compressor::Lz4
} else if cfg!(feature = "brotli-compression") {
Compressor::Brotli
} else if cfg!(feature = "snappy-compression") {
Compressor::Snappy
} else if cfg!(feature = "zstd-compression") {
Compressor::Zstd(ZstdCompressor::default())
} else {
Compressor::None
}
#[cfg(feature = "lz4-compression")]
return Compressor::Lz4;

#[cfg(feature = "brotli-compression")]
return Compressor::Brotli;

#[cfg(feature = "snappy-compression")]
return Compressor::Snappy;

#[cfg(feature = "zstd-compression")]
return Compressor::Zstd(ZstdCompressor::default());

Compressor::None
}
}

Expand All @@ -155,50 +199,18 @@ impl Compressor {
compressed.extend_from_slice(uncompressed);
Ok(())
}
Self::Lz4 => {
#[cfg(feature = "lz4-compression")]
{
super::compression_lz4_block::compress(uncompressed, compressed)
}
#[cfg(not(feature = "lz4-compression"))]
{
panic!("lz4-compression feature flag not activated");
}
}
Self::Brotli => {
#[cfg(feature = "brotli-compression")]
{
super::compression_brotli::compress(uncompressed, compressed)
}
#[cfg(not(feature = "brotli-compression"))]
{
panic!("brotli-compression-compression feature flag not activated");
}
}
Self::Snappy => {
#[cfg(feature = "snappy-compression")]
{
super::compression_snap::compress(uncompressed, compressed)
}
#[cfg(not(feature = "snappy-compression"))]
{
panic!("snappy-compression feature flag not activated");
}
}
Self::Zstd(_zstd_compressor) => {
#[cfg(feature = "zstd-compression")]
{
super::compression_zstd_block::compress(
uncompressed,
compressed,
_zstd_compressor.compression_level,
)
}
#[cfg(not(feature = "zstd-compression"))]
{
panic!("zstd-compression feature flag not activated");
}
}
#[cfg(feature = "lz4-compression")]
Self::Lz4 => super::compression_lz4_block::compress(uncompressed, compressed),
#[cfg(feature = "brotli-compression")]
Self::Brotli => super::compression_brotli::compress(uncompressed, compressed),
#[cfg(feature = "snappy-compression")]
Self::Snappy => super::compression_snap::compress(uncompressed, compressed),
#[cfg(feature = "zstd-compression")]
Self::Zstd(_zstd_compressor) => super::compression_zstd_block::compress(
uncompressed,
compressed,
_zstd_compressor.compression_level,
),
}
}
}
Expand Down
68 changes: 28 additions & 40 deletions src/store/decompressors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,30 @@ pub enum Decompressor {
/// No compression
None,
/// Use the lz4 decompressor (block format)
#[cfg(feature = "lz4-compression")]
Lz4,
/// Use the brotli decompressor
#[cfg(feature = "brotli-compression")]
Brotli,
/// Use the snap decompressor
#[cfg(feature = "snappy-compression")]
Snappy,
/// Use the zstd decompressor
#[cfg(feature = "zstd-compression")]
Zstd,
}

impl From<Compressor> for Decompressor {
fn from(compressor: Compressor) -> Self {
match compressor {
Compressor::None => Decompressor::None,
#[cfg(feature = "lz4-compression")]
Compressor::Lz4 => Decompressor::Lz4,
#[cfg(feature = "brotli-compression")]
Compressor::Brotli => Decompressor::Brotli,
#[cfg(feature = "snappy-compression")]
Compressor::Snappy => Decompressor::Snappy,
#[cfg(feature = "zstd-compression")]
Compressor::Zstd(_) => Decompressor::Zstd,
}
}
Expand All @@ -41,9 +49,13 @@ impl Decompressor {
pub(crate) fn from_id(id: u8) -> Decompressor {
match id {
0 => Decompressor::None,
#[cfg(feature = "lz4-compression")]
1 => Decompressor::Lz4,
#[cfg(feature = "brotli-compression")]
2 => Decompressor::Brotli,
#[cfg(feature = "snappy-compression")]
3 => Decompressor::Snappy,
#[cfg(feature = "zstd-compression")]
4 => Decompressor::Zstd,
_ => panic!("unknown compressor id {id:?}"),
}
Expand All @@ -52,9 +64,13 @@ impl Decompressor {
pub(crate) fn get_id(&self) -> u8 {
match self {
Self::None => 0,
#[cfg(feature = "lz4-compression")]
Self::Lz4 => 1,
#[cfg(feature = "brotli-compression")]
Self::Brotli => 2,
#[cfg(feature = "snappy-compression")]
Self::Snappy => 3,
#[cfg(feature = "zstd-compression")]
Self::Zstd => 4,
}
}
Expand All @@ -77,46 +93,14 @@ impl Decompressor {
decompressed.extend_from_slice(compressed);
Ok(())
}
Self::Lz4 => {
#[cfg(feature = "lz4-compression")]
{
super::compression_lz4_block::decompress(compressed, decompressed)
}
#[cfg(not(feature = "lz4-compression"))]
{
panic!("lz4-compression feature flag not activated");
}
}
Self::Brotli => {
#[cfg(feature = "brotli-compression")]
{
super::compression_brotli::decompress(compressed, decompressed)
}
#[cfg(not(feature = "brotli-compression"))]
{
panic!("brotli-compression feature flag not activated");
}
}
Self::Snappy => {
#[cfg(feature = "snappy-compression")]
{
super::compression_snap::decompress(compressed, decompressed)
}
#[cfg(not(feature = "snappy-compression"))]
{
panic!("snappy-compression feature flag not activated");
}
}
Self::Zstd => {
#[cfg(feature = "zstd-compression")]
{
super::compression_zstd_block::decompress(compressed, decompressed)
}
#[cfg(not(feature = "zstd-compression"))]
{
panic!("zstd-compression feature flag not activated");
}
}
#[cfg(feature = "lz4-compression")]
Self::Lz4 => super::compression_lz4_block::decompress(compressed, decompressed),
#[cfg(feature = "brotli-compression")]
Self::Brotli => super::compression_brotli::decompress(compressed, decompressed),
#[cfg(feature = "snappy-compression")]
Self::Snappy => super::compression_snap::decompress(compressed, decompressed),
#[cfg(feature = "zstd-compression")]
Self::Zstd => super::compression_zstd_block::decompress(compressed, decompressed),
}
}
}
Expand All @@ -129,9 +113,13 @@ mod tests {
#[test]
fn compressor_decompressor_id_test() {
assert_eq!(Decompressor::from(Compressor::None), Decompressor::None);
#[cfg(feature = "lz4-compression")]
assert_eq!(Decompressor::from(Compressor::Lz4), Decompressor::Lz4);
#[cfg(feature = "brotli-compression")]
assert_eq!(Decompressor::from(Compressor::Brotli), Decompressor::Brotli);
#[cfg(feature = "snappy-compression")]
assert_eq!(Decompressor::from(Compressor::Snappy), Decompressor::Snappy);
#[cfg(feature = "zstd-compression")]
assert_eq!(
Decompressor::from(Compressor::Zstd(Default::default())),
Decompressor::Zstd
Expand Down

0 comments on commit 7e6c4a1

Please sign in to comment.