Skip to content

Commit

Permalink
Merge pull request #207 from brave/optimize-raw-line-field
Browse files Browse the repository at this point in the history
Optimize raw_line field
  • Loading branch information
antonok-edm authored Apr 4, 2022
2 parents 2a778d4 + fd9d533 commit 6a0487d
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 17 deletions.
6 changes: 3 additions & 3 deletions src/data_format/legacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ struct NetworkFilterLegacySerializeFmt<'a> {
csp: &'a Option<String>,
bug: &'a Option<u32>,
tag: &'a Option<String>,
raw_line: &'a Option<String>,
raw_line: Option<String>,
id: &'a crate::utils::Hash,
_fuzzy_signature: Option<Vec<crate::utils::Hash>>,
opt_domains_union: &'a Option<crate::utils::Hash>,
Expand All @@ -59,7 +59,7 @@ impl<'a, T> From<&'a T> for NetworkFilterLegacySerializeFmt<'a> where T: std::bo
csp: &v.csp,
bug: &v.bug,
tag: &v.tag,
raw_line: &v.raw_line,
raw_line: v.raw_line.as_ref().map(|raw| *raw.clone()),
id: &v.id,
_fuzzy_signature: None,
opt_domains_union: &v.opt_domains_union,
Expand Down Expand Up @@ -191,7 +191,7 @@ impl From<NetworkFilterLegacyDeserializeFmt> for NetworkFilter {
csp: v.csp,
bug: v.bug,
tag: v.tag,
raw_line: v.raw_line,
raw_line: v.raw_line.map(|raw| Box::new(raw)),
id: v.id,
opt_domains_union: v.opt_domains_union,
opt_not_domains_union: v.opt_not_domains_union,
Expand Down
4 changes: 2 additions & 2 deletions src/filters/cosmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub struct CosmeticFilter {
pub mask: CosmeticFilterMask,
pub not_entities: Option<Vec<Hash>>,
pub not_hostnames: Option<Vec<Hash>>,
pub raw_line: Option<String>,
pub raw_line: Option<Box<String>>,
pub selector: String,
pub key: Option<String>,
pub style: Option<String>,
Expand Down Expand Up @@ -303,7 +303,7 @@ impl CosmeticFilter {
not_entities,
not_hostnames,
raw_line: if debug {
Some(String::from(line))
Some(Box::new(String::from(line)))
} else {
None
},
Expand Down
13 changes: 8 additions & 5 deletions src/filters/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ pub struct NetworkFilter {
pub bug: Option<u32>,
pub tag: Option<String>,

pub raw_line: Option<String>,
pub raw_line: Option<Box<String>>,

pub id: Hash,

Expand Down Expand Up @@ -862,7 +862,7 @@ impl NetworkFilter {
opt_not_domains,
tag,
raw_line: if debug {
Some(String::from(line))
Some(Box::new(String::from(line)))
} else {
None
},
Expand Down Expand Up @@ -2571,7 +2571,8 @@ mod parse_tests {
fn parses_hosts_style() {
{
let filter = NetworkFilter::parse_hosts_style("example.com", true).unwrap();
assert_eq!(filter.raw_line, Some("||example.com^".to_string()));
assert!(filter.raw_line.is_some());
assert_eq!(*filter.raw_line.clone().unwrap(), "||example.com^".to_string());
let mut defaults = default_network_filter_breakdown();
defaults.hostname = Some("example.com".to_string());
defaults.is_plain = true;
Expand All @@ -2582,7 +2583,8 @@ mod parse_tests {
}
{
let filter = NetworkFilter::parse_hosts_style("www.example.com", true).unwrap();
assert_eq!(filter.raw_line, Some("||example.com^".to_string()));
assert!(filter.raw_line.is_some());
assert_eq!(*filter.raw_line.clone().unwrap(), "||example.com^".to_string());
let mut defaults = default_network_filter_breakdown();
defaults.hostname = Some("example.com".to_string());
defaults.is_plain = true;
Expand All @@ -2593,7 +2595,8 @@ mod parse_tests {
}
{
let filter = NetworkFilter::parse_hosts_style("malware.example.com", true).unwrap();
assert_eq!(filter.raw_line, Some("||malware.example.com^".to_string()));
assert!(filter.raw_line.is_some());
assert_eq!(*filter.raw_line.clone().unwrap(), "||malware.example.com^".to_string());
let mut defaults = default_network_filter_breakdown();
defaults.hostname = Some("malware.example.com".to_string());
defaults.is_plain = true;
Expand Down
4 changes: 2 additions & 2 deletions src/lists.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ impl FilterSet {
let mut filters_used = vec![];

self.network_filters.into_iter().for_each(|filter| {
let original_rule = filter.raw_line.clone().expect("All rules should be in debug mode");
let original_rule = *filter.raw_line.clone().expect("All rules should be in debug mode");
if let Ok(equivalent) = TryInto::<content_blocking::CbRuleEquivalent>::try_into(filter) {
filters_used.push(original_rule);
equivalent.into_iter().for_each(|cb_rule| {
Expand All @@ -179,7 +179,7 @@ impl FilterSet {
let add_fp_document_exception = !filters_used.is_empty();

self.cosmetic_filters.into_iter().for_each(|filter| {
let original_rule = filter.raw_line.clone().expect("All rules should be in debug mode");
let original_rule = *filter.raw_line.clone().expect("All rules should be in debug mode");
if let Ok(cb_rule) = TryInto::<content_blocking::CbRule>::try_into(filter) {
filters_used.push(original_rule);
match &cb_rule.action.typ {
Expand Down
8 changes: 4 additions & 4 deletions src/optimizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ impl Optimization for SimplePatternGroup {
filter.mask.set(NetworkFilterMask::IS_COMPLETE_REGEX, is_complete_regex);

if base_filter.raw_line.is_some() {
filter.raw_line = Some(
filter.raw_line = Some(Box::new(
filters
.iter()
.flat_map(|f| f.raw_line.clone())
.join(" <+> "),
.join(" <+> ")),
)
}

Expand Down Expand Up @@ -170,11 +170,11 @@ impl Optimization for UnionDomainGroup {


if base_filter.raw_line.is_some() {
filter.raw_line = Some(
filter.raw_line = Some(Box::new(
filters
.iter()
.flat_map(|f| f.raw_line.clone())
.join(" <+> "),
.join(" <+> ")),
)
}

Expand Down
2 changes: 1 addition & 1 deletion tests/hashing_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fn check_rule_ids_no_collisions() {

for filter in network_filters {
let id = filter.get_id();
let rule = filter.raw_line.unwrap_or_default();
let rule = *filter.raw_line.unwrap_or_default();
let existing_rule = filter_ids.get(&id);
assert!(existing_rule.is_none() || existing_rule.unwrap() == &rule, "ID {} for {} already present from {}", id, rule, existing_rule.unwrap());
filter_ids.insert(id, rule);
Expand Down

0 comments on commit 6a0487d

Please sign in to comment.