Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 11277188 authored by Marybeth Fair's avatar Marybeth Fair Committed by Gerrit Code Review
Browse files

Merge "Revert "Add fingerprint to packages.map."" into main

parents 8478830c a80e298d
Loading
Loading
Loading
Loading
+0 −8
Original line number Diff line number Diff line
@@ -68,14 +68,6 @@ aconfig_values {
    ],
}

aconfig_values {
    name: "aconfig.test.flag.second_values",
    package: "com.android.aconfig.test",
    srcs: [
        "tests/third.values",
    ],
}

aconfig_value_set {
    name: "aconfig.test.flag.value_set",
    values: [
+18 −56
Original line number Diff line number Diff line
@@ -17,7 +17,7 @@
use anyhow::{bail, ensure, Context, Result};
use itertools::Itertools;
use protobuf::Message;
use std::collections::HashMap;
use std::collections::{BTreeMap, HashMap};
use std::hash::Hasher;
use std::io::Read;
use std::path::PathBuf;
@@ -422,30 +422,23 @@ where
    Ok(flag_ids)
}

// Creates a fingerprint of the flag names. Sorts the vector.
pub fn compute_flags_fingerprint(flag_names: &mut Vec<String>) -> Result<u64> {
    flag_names.sort();

    let mut hasher = SipHasher13::new();
    for flag in flag_names {
        hasher.write(flag.as_bytes());
    }
    Ok(hasher.finish())
}

#[allow(dead_code)] // TODO: b/316357686 - Use fingerprint in codegen to
                    // protect hardcoded offset reads.
fn compute_fingerprint_from_parsed_flags(flags: ProtoParsedFlags) -> Result<u64> {
    let separated_flags: Vec<ProtoParsedFlag> = flags.parsed_flag.into_iter().collect::<Vec<_>>();
pub fn compute_flag_offsets_fingerprint(flags_map: &HashMap<String, u16>) -> Result<u64> {
    let mut hasher = SipHasher13::new();

    // All flags must belong to the same package as the fingerprint is per-package.
    let Some(_package) = find_unique_package(&separated_flags) else {
        bail!("No parsed flags, or the parsed flags use different packages.");
    };
    // Need to sort to ensure the data is added to the hasher in the same order
    // each run.
    let sorted_map: BTreeMap<&String, &u16> = flags_map.iter().collect();

    let mut flag_names =
        separated_flags.into_iter().map(|flag| flag.name.unwrap()).collect::<Vec<_>>();
    compute_flags_fingerprint(&mut flag_names)
    for (flag, offset) in sorted_map {
        // See https://docs.rs/siphasher/latest/siphasher/#note for use of write
        // over write_i16. Similarly, use to_be_bytes rather than to_ne_bytes to
        // ensure consistency.
        hasher.write(flag.as_bytes());
        hasher.write(&offset.to_be_bytes());
    }
    Ok(hasher.finish())
}

#[cfg(test)]
@@ -456,46 +449,15 @@ mod tests {
    #[test]
    fn test_offset_fingerprint() {
        let parsed_flags = crate::test::parse_test_flags();
        let expected_fingerprint: u64 = 5801144784618221668;
        let package = find_unique_package(&parsed_flags.parsed_flag).unwrap().to_string();
        let flag_ids = assign_flag_ids(&package, parsed_flags.parsed_flag.iter()).unwrap();
        let expected_fingerprint = 10709892481002252132u64;

        let hash_result = compute_fingerprint_from_parsed_flags(parsed_flags);
        let hash_result = compute_flag_offsets_fingerprint(&flag_ids);

        assert_eq!(hash_result.unwrap(), expected_fingerprint);
    }

    #[test]
    fn test_offset_fingerprint_matches_from_package() {
        let parsed_flags: ProtoParsedFlags = crate::test::parse_test_flags();

        // All test flags are in the same package, so fingerprint from all of them.
        let result_from_parsed_flags = compute_fingerprint_from_parsed_flags(parsed_flags.clone());

        let mut flag_names_vec = parsed_flags
            .parsed_flag
            .clone()
            .into_iter()
            .map(|flag| flag.name.unwrap())
            .map(String::from)
            .collect::<Vec<_>>();
        let result_from_names = compute_flags_fingerprint(&mut flag_names_vec);

        // Assert the same hash is generated for each case.
        assert_eq!(result_from_parsed_flags.unwrap(), result_from_names.unwrap());
    }

    #[test]
    fn test_offset_fingerprint_different_packages_does_not_match() {
        // Parse flags from two packages.
        let parsed_flags: ProtoParsedFlags = crate::test::parse_test_flags();
        let second_parsed_flags = crate::test::parse_second_package_flags();

        let result_from_parsed_flags = compute_fingerprint_from_parsed_flags(parsed_flags).unwrap();
        let second_result = compute_fingerprint_from_parsed_flags(second_parsed_flags).unwrap();

        // Different flags should have a different fingerprint.
        assert_ne!(result_from_parsed_flags, second_result);
    }

    #[test]
    fn test_parse_flags() {
        let parsed_flags = crate::test::parse_test_flags(); // calls parse_flags
+1 −8
Original line number Diff line number Diff line
@@ -25,14 +25,12 @@ use crate::storage::{
    flag_table::create_flag_table, flag_value::create_flag_value,
    package_table::create_package_table,
};
use aconfig_protos::ProtoParsedFlag;
use aconfig_protos::ProtoParsedFlags;
use aconfig_protos::{ProtoParsedFlag, ProtoParsedFlags};
use aconfig_storage_file::StorageFileType;

pub struct FlagPackage<'a> {
    pub package_name: &'a str,
    pub package_id: u32,
    pub fingerprint: u64,
    pub flag_names: HashSet<&'a str>,
    pub boolean_flags: Vec<&'a ProtoParsedFlag>,
    // The index of the first boolean flag in this aconfig package among all boolean
@@ -45,7 +43,6 @@ impl<'a> FlagPackage<'a> {
        FlagPackage {
            package_name,
            package_id,
            fingerprint: 0,
            flag_names: HashSet::new(),
            boolean_flags: vec![],
            boolean_start_index: 0,
@@ -81,8 +78,6 @@ where
    for p in packages.iter_mut() {
        p.boolean_start_index = boolean_start_index;
        boolean_start_index += p.boolean_flags.len() as u32;

        // TODO: b/316357686 - Calculate fingerprint and add to package.
    }

    packages
@@ -120,8 +115,6 @@ mod tests {
    use super::*;
    use crate::Input;

    use aconfig_protos::ProtoParsedFlags;

    pub fn parse_all_test_flags() -> Vec<ProtoParsedFlags> {
        let aconfig_files = [
            (
+0 −1
Original line number Diff line number Diff line
@@ -48,7 +48,6 @@ impl PackageTableNodeWrapper {
        let node = PackageTableNode {
            package_name: String::from(package.package_name),
            package_id: package.package_id,
            fingerprint: package.fingerprint,
            boolean_start_index: package.boolean_start_index,
            next_offset: None,
        };
+0 −18
Original line number Diff line number Diff line
@@ -295,24 +295,6 @@ parsed_flag {
        aconfig_protos::parsed_flags::try_from_binary_proto(&bytes).unwrap()
    }

    pub fn parse_second_package_flags() -> ProtoParsedFlags {
        let bytes = crate::commands::parse_flags(
            "com.android.aconfig.second_test",
            Some("system"),
            vec![Input {
                source: "tests/test_second_package.aconfig".to_string(),
                reader: Box::new(include_bytes!("../tests/test_second_package.aconfig").as_slice()),
            }],
            vec![Input {
                source: "tests/third.values".to_string(),
                reader: Box::new(include_bytes!("../tests/third.values").as_slice()),
            }],
            crate::commands::DEFAULT_FLAG_PERMISSION,
        )
        .unwrap();
        aconfig_protos::parsed_flags::try_from_binary_proto(&bytes).unwrap()
    }

    pub fn first_significant_code_diff(a: &str, b: &str) -> Option<String> {
        let a = a.lines().map(|line| line.trim_start()).filter(|line| !line.is_empty());
        let b = b.lines().map(|line| line.trim_start()).filter(|line| !line.is_empty());
Loading