Various improvements to link handling #47
Conversation
In OBS proper, links are accompanied by a `_link` entry in the source, and deleting that will break the link. Implementing full support for *editing* that is a bit more complex, but ensuring that it is at least present and removable makes sense.
It's possible for a branch link to be invalid, in which case one or more
of `{srcmd5, xsrcmd5, lsrcmd5}` may be missing, so this adds support
for capturing and makes the `*srcmd5` parameters optional. (As-is, on
link error, OBS seems to only set `xsrcmd5`, if possible, but simply
making them all optional is a bit less brittle than trying to encode all
this logic in enums.) This also adds some basic artificial error
injection in the mocks for testing this out.
refi64
changed the title
There was a problem hiding this comment.
Pull Request Overview
Improves link handling in the OpenBuildService mock and API by adding support for _link entries in file listings, treating link removal as a link break scenario, and fixing deserialization errors when links are broken.
- Adds
_linkfile generation and inclusion in directory listings - Introduces
MockLinkResolutionenum to handle both successful and error states for link resolution - Updates serialization to make link fields optional to prevent deserialization failures on broken links
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| open-build-service-mock/src/lib.rs | Adds link file creation, updates data structures with link resolution enum, and modifies branching logic |
| open-build-service-mock/src/api/source.rs | Updates XML generation to handle different link resolution states and includes new enum import |
| open-build-service-api/tests/integration.rs | Adds comprehensive tests for both successful and error link resolution scenarios |
| open-build-service-api/src/lib.rs | Makes LinkInfo fields optional to handle broken link deserialization and adds error field |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| linkinfo: if entries.contains_key(MockSourceFile::LINK_PATH) { | ||
| self.revisions | ||
| .last() | ||
| .map_or_else(Vec::new, |rev| rev.linkinfo.clone()) | ||
| } else { | ||
| vec![] |
There was a problem hiding this comment.
[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.
Adds the
_linkentry to file listings, treats removal of it as a link break, and fixes deserialization errors on broken links.