Skip to content

Various improvements to link handling #47

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

Merged
merged 2 commits into from
Sep 30, 2025
Merged

Various improvements to link handling #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
605f461
mock: Improve handling of `_link` entries
refi64 Sep 25, 2025
e4c08cc
Avoid failing to deserialize broken links
refi64 Sep 25, 2025
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
14 changes: 8 additions & 6 deletions open-build-service-api/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,16 @@
pub project: String,
#[serde(rename = "@package")]
pub package: String,
#[serde(rename = "@srcmd5")]
pub srcmd5: String,
#[serde(rename = "@xsrcmd5")]
pub xsrcmd5: String,
#[serde(rename = "@lsrcmd5")]
pub lsrcmd5: String,
#[serde(default, rename = "@srcmd5")]
pub srcmd5: Option<String>,
#[serde(default, rename = "@xsrcmd5")]
pub xsrcmd5: Option<String>,
#[serde(default, rename = "@lsrcmd5")]
pub lsrcmd5: Option<String>,
#[serde(default, rename = "@missingok")]
pub missingok: bool,
#[serde(default, rename = "@error")]
pub error: Option<String>,
}

#[derive(Deserialize, Debug)]
Expand Down Expand Up @@ -1297,7 +1299,7 @@
&self.base
}

pub fn project(&self, project: String) -> ProjectBuilder {

Check warning on line 1302 in open-build-service-api/src/lib.rs

View workflow job for this annotation

GitHub Actions / cargo clippy latest

hiding a lifetime that's elided elsewhere is confusing 

warning: hiding a lifetime that's elided elsewhere is confusing
    --> open-build-service-api/src/lib.rs:1302:20
     |
1302 |     pub fn project(&self, project: String) -> ProjectBuilder {
     |                    ^^^^^                      -------------- the same lifetime is hidden here
     |                    |
     |                    the lifetime is elided here
     |
     = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
     = note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
     |
1302 |     pub fn project(&self, project: String) -> ProjectBuilder<'_> {
     |                                                             ++++

Check warning on line 1302 in open-build-service-api/src/lib.rs

View workflow job for this annotation

GitHub Actions / cargo clippy latest

hiding a lifetime that's elided elsewhere is confusing 

warning: hiding a lifetime that's elided elsewhere is confusing
    --> open-build-service-api/src/lib.rs:1302:20
     |
1302 |     pub fn project(&self, project: String) -> ProjectBuilder {
     |                    ^^^^^                      -------------- the same lifetime is hidden here
     |                    |
     |                    the lifetime is elided here
     |
     = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
     = note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
     |
1302 |     pub fn project(&self, project: String) -> ProjectBuilder<'_> {
     |                                                             ++++

Check warning on line 1302 in open-build-service-api/src/lib.rs

View workflow job for this annotation

GitHub Actions / cargo clippy latest

hiding a lifetime that's elided elsewhere is confusing 

warning: hiding a lifetime that's elided elsewhere is confusing
    --> open-build-service-api/src/lib.rs:1302:20
     |
1302 |     pub fn project(&self, project: String) -> ProjectBuilder {
     |                    ^^^^^                      -------------- the same lifetime is hidden here
     |                    |
     |                    the lifetime is elided here
     |
     = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
     = note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
     |
1302 |     pub fn project(&self, project: String) -> ProjectBuilder<'_> {
     |                                                             ++++

Check warning on line 1302 in open-build-service-api/src/lib.rs

View workflow job for this annotation

GitHub Actions / cargo clippy latest

hiding a lifetime that's elided elsewhere is confusing 

warning: hiding a lifetime that's elided elsewhere is confusing
    --> open-build-service-api/src/lib.rs:1302:20
     |
1302 |     pub fn project(&self, project: String) -> ProjectBuilder {
     |                    ^^^^^                      -------------- the same lifetime is hidden here
     |                    |
     |                    the lifetime is elided here
     |
     = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
     = note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
     |
1302 |     pub fn project(&self, project: String) -> ProjectBuilder<'_> {
     |                                                             ++++
ProjectBuilder {
client: self,
project,
Expand Down
48 changes: 43 additions & 5 deletions open-build-service-api/tests/integration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,9 @@ async fn test_source_list() {
TEST_PACKAGE_2.to_owned(),
MockBranchOptions {
srcmd5: branch_srcmd5.clone(),
xsrcmd5: branch_xsrcmd5.clone(),
link_resolution: MockLinkResolution::Available {
xsrcmd5: branch_xsrcmd5.clone(),
},
..Default::default()
},
);
Expand All @@ -297,15 +299,51 @@ async fn test_source_list() {
assert_eq!(dir.rev.unwrap(), "1");
assert_eq!(dir.vrev.unwrap(), "1");
assert_eq!(dir.srcmd5, branch_srcmd5);
assert_eq!(dir.entries.len(), 1);
assert_eq!(dir.entries.len(), 2);
assert_eq!(dir.linkinfo.len(), 1);

assert!(dir.entries.iter().any(|e| e.name == "_link"));
assert!(dir.entries.iter().any(|e| e.name == "test"));

let linkinfo = &dir.linkinfo[0];
assert_eq!(linkinfo.project, TEST_PROJECT);
assert_eq!(linkinfo.package, TEST_PACKAGE_1);
assert_eq!(linkinfo.srcmd5.as_ref().unwrap(), &srcmd5);
assert_eq!(linkinfo.lsrcmd5.as_ref().unwrap(), &branch_srcmd5);
assert_eq!(linkinfo.xsrcmd5.as_ref().unwrap(), &branch_xsrcmd5);

mock.branch(
TEST_PROJECT.to_owned(),
TEST_PACKAGE_1.to_owned(),
TEST_PROJECT,
TEST_PACKAGE_2.to_owned(),
MockBranchOptions {
srcmd5: branch_srcmd5.clone(),
link_resolution: MockLinkResolution::Error {
error: "an error".to_owned(),
},
..Default::default()
},
);

let dir = package_2.list(None).await.unwrap();

assert_eq!(dir.rev.unwrap(), "1");
assert_eq!(dir.vrev.unwrap(), "1");
assert_eq!(dir.srcmd5, branch_srcmd5);
assert_eq!(dir.entries.len(), 2);
assert_eq!(dir.linkinfo.len(), 1);

assert!(dir.entries.iter().any(|e| e.name == "_link"));
assert!(dir.entries.iter().any(|e| e.name == "test"));

let linkinfo = &dir.linkinfo[0];
assert_eq!(linkinfo.project, TEST_PROJECT);
assert_eq!(linkinfo.package, TEST_PACKAGE_1);
assert_eq!(linkinfo.srcmd5, srcmd5);
assert_eq!(linkinfo.lsrcmd5, branch_srcmd5);
assert_eq!(linkinfo.xsrcmd5, branch_xsrcmd5);
assert_eq!(linkinfo.srcmd5, None);
assert_eq!(linkinfo.lsrcmd5, None);
assert_eq!(linkinfo.xsrcmd5, None);
assert_eq!(linkinfo.error.as_ref().unwrap(), "an error");
}

#[tokio::test]
Expand Down
25 changes: 19 additions & 6 deletions open-build-service-mock/src/api/source.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ use wiremock::ResponseTemplate;
use wiremock::{Request, Respond};

use crate::{
MockBranchOptions, MockEntry, MockPackage, MockPackageOptions, MockProject, MockRevision,
MockRevisionOptions, MockSourceFile, MockSourceFileKey, ObsMock, ZERO_REV_SRCMD5, random_md5,
MockBranchOptions, MockEntry, MockLinkResolution, MockPackage, MockPackageOptions, MockProject,
MockRevision, MockRevisionOptions, MockSourceFile, MockSourceFileKey, ObsMock, ZERO_REV_SRCMD5,
random_md5,
};

use super::*;
Expand Down Expand Up @@ -48,11 +49,21 @@ fn source_listing_xml(
("project", linkinfo.project.as_str()),
("package", &linkinfo.package),
("baserev", &linkinfo.baserev),
("srcmd5", &linkinfo.srcmd5),
("xsrcmd5", &linkinfo.xsrcmd5),
("lsrcmd5", &linkinfo.lsrcmd5),
]);

match &linkinfo.link_resolution {
MockLinkResolution::Available { xsrcmd5 } => {
linkinfo_xml = linkinfo_xml.with_attributes([
("srcmd5", linkinfo.srcmd5.as_str()),
("lsrcmd5", &linkinfo.lsrcmd5),
("xsrcmd5", xsrcmd5),
]);
}
MockLinkResolution::Error { error } => {
linkinfo_xml = linkinfo_xml.with_attribute(("error", error.as_str()));
}
}

if linkinfo.missingok {
linkinfo_xml = linkinfo_xml.with_attribute(("missingok", "1"));
}
Expand Down Expand Up @@ -752,7 +763,9 @@ fn do_branch(
&target_package_name,
MockBranchOptions {
srcmd5: random_md5(),
xsrcmd5: random_md5(),
link_resolution: MockLinkResolution::Available {
xsrcmd5: random_md5(),
},
user: mock.auth().username().to_owned(),
time: SystemTime::now(),
comment: comment.map(Cow::into_owned),
Expand Down
62 changes: 53 additions & 9 deletions open-build-service-mock/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
}

impl MockSourceFile {
const LINK_PATH: &'static str = "_link";
const META_PATH: &'static str = "_meta";

pub fn new_metadata(
Expand Down Expand Up @@ -111,6 +112,31 @@
}
}

pub fn new_link(project: &str, baserev: &str, missingok: bool) -> MockSourceFile {
let mut xml = XMLWriter::new_with_indent(Default::default(), b' ', 2);
xml.create_element("link")
.with_attributes([
("project", project),
("baserev", baserev),
("missingok", &missingok.to_string()),
])
.write_inner_content(|writer| {
writer
.create_element("patches")
.write_inner_content(|writer| {
writer.create_element("branch").write_empty()?;
Ok(())
})?;
Ok(())
})
.unwrap();

MockSourceFile {
path: MockSourceFile::LINK_PATH.to_owned(),
contents: xml.into_inner().into_inner(),
}
}

fn md5(&self) -> String {
base16ct::lower::encode_string(&Md5::digest(&self.contents))
}
Expand All @@ -133,7 +159,7 @@
baserev: String,
srcmd5: String,
lsrcmd5: String,
xsrcmd5: String,
link_resolution: MockLinkResolution,
missingok: bool,
}

Expand Down Expand Up @@ -287,7 +313,7 @@
branched_package_name: &str,
options: MockBranchOptions,
) -> MockPackage {
let (mut files, entries, origin_srcmd5, disabled) = if let Some((origin, origin_rev)) =
let (mut files, mut entries, origin_srcmd5, disabled) = if let Some((origin, origin_rev)) =
origin.and_then(|origin| origin.revisions.last().map(|rev| (origin, rev)))
{
(
Expand All @@ -311,12 +337,19 @@
let meta_entry = MockEntry::from_key(&meta_key, options.time);
files.insert(meta_key, meta_contents);

let (link_key, link_contents) =
MockSourceFile::new_link(branched_project_name, &origin_srcmd5, options.missingok)
.into_key_and_contents();
let link_entry = MockEntry::from_key(&link_key, options.time);
files.insert(link_key, link_contents);
entries.insert(MockSourceFile::LINK_PATH.to_owned(), link_entry);

let linkinfo = MockLinkInfo {
project: origin_project_name,
package: origin_package_name,
baserev: origin_srcmd5.clone(),
srcmd5: origin_srcmd5,
xsrcmd5: options.xsrcmd5,
link_resolution: options.link_resolution,
lsrcmd5: options.srcmd5.clone(),
missingok: options.missingok,
};
Expand Down Expand Up @@ -372,18 +405,27 @@
self.revisions.push(MockRevision {
vrev: Some(*vrev),
options,
linkinfo: if entries.contains_key(MockSourceFile::LINK_PATH) {
self.revisions
.last()
.map_or_else(Vec::new, |rev| rev.linkinfo.clone())
} else {
vec![]
Comment on lines +408 to +413
Copy link

Copilot AI Sep 30, 2025

Choose a reason for hiding this comment

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

[nitpick] The linkinfo assignment logic could be simplified by extracting the condition into a local variable or using a more readable pattern. Consider: let has_link = entries.contains_key(MockSourceFile::LINK_PATH); let linkinfo = if has_link { ... } else { vec![] };

Copilot uses AI. Check for mistakes.

},
entries,
linkinfo: self
.revisions
.last()
.map_or_else(Vec::new, |rev| rev.linkinfo.clone()),
});
}
}

#[derive(Debug, Clone)]
pub enum MockLinkResolution {
Available { xsrcmd5: String },
Error { error: String },
}

pub struct MockBranchOptions {
pub srcmd5: String,
pub xsrcmd5: String,
pub link_resolution: MockLinkResolution,
pub user: String,
pub time: SystemTime,
pub comment: Option<String>,
Expand All @@ -394,7 +436,9 @@
fn default() -> Self {
Self {
srcmd5: random_md5(),
xsrcmd5: random_md5(),
link_resolution: MockLinkResolution::Available {
xsrcmd5: random_md5(),
},
time: SystemTime::now(),
user: ADMIN_USER.to_owned(),
comment: None,
Expand Down Expand Up @@ -809,7 +853,7 @@
}

pub fn add_package_files(
&self,

Check warning on line 856 in open-build-service-mock/src/lib.rs

View workflow job for this annotation

GitHub Actions / cargo clippy latest

hiding a lifetime that's elided elsewhere is confusing 

warning: hiding a lifetime that's elided elsewhere is confusing
   --> open-build-service-mock/src/lib.rs:856:9
    |
856 |         &self,
    |         ^^^^^ the lifetime is elided here
...
860 |     ) -> MockSourceFileKey {
    |          -----------------
    |          |
    |          the same lifetime is hidden here
    |          the same lifetime is hidden here
    |
    = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
    = note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
    |
860 |     ) -> MockSourceFileKey<'_> {
    |                           ++++

Check warning on line 856 in open-build-service-mock/src/lib.rs

View workflow job for this annotation

GitHub Actions / cargo clippy latest

hiding a lifetime that's elided elsewhere is confusing 

warning: hiding a lifetime that's elided elsewhere is confusing
   --> open-build-service-mock/src/lib.rs:856:9
    |
856 |         &self,
    |         ^^^^^ the lifetime is elided here
...
860 |     ) -> MockSourceFileKey {
    |          -----------------
    |          |
    |          the same lifetime is hidden here
    |          the same lifetime is hidden here
    |
    = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
    = note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
    |
860 |     ) -> MockSourceFileKey<'_> {
    |                           ++++

Check warning on line 856 in open-build-service-mock/src/lib.rs

View workflow job for this annotation

GitHub Actions / cargo clippy latest

hiding a lifetime that's elided elsewhere is confusing 

warning: hiding a lifetime that's elided elsewhere is confusing
   --> open-build-service-mock/src/lib.rs:856:9
    |
856 |         &self,
    |         ^^^^^ the lifetime is elided here
...
860 |     ) -> MockSourceFileKey {
    |          -----------------
    |          |
    |          the same lifetime is hidden here
    |          the same lifetime is hidden here
    |
    = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
    = note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
    |
860 |     ) -> MockSourceFileKey<'_> {
    |                           ++++

Check warning on line 856 in open-build-service-mock/src/lib.rs

View workflow job for this annotation

GitHub Actions / cargo clippy latest

hiding a lifetime that's elided elsewhere is confusing 

warning: hiding a lifetime that's elided elsewhere is confusing
   --> open-build-service-mock/src/lib.rs:856:9
    |
856 |         &self,
    |         ^^^^^ the lifetime is elided here
...
860 |     ) -> MockSourceFileKey {
    |          -----------------
    |          |
    |          the same lifetime is hidden here
    |          the same lifetime is hidden here
    |
    = help: the same lifetime is referred to in inconsistent ways, making the signature confusing
    = note: `#[warn(mismatched_lifetime_syntaxes)]` on by default
help: use `'_` for type paths
    |
860 |     ) -> MockSourceFileKey<'_> {
    |                           ++++
project_name: &str,
package_name: &str,
file: MockSourceFile,
Expand Down
Loading