stbischof commented on PR #587: URL: https://github.com/apache/felix-dev/pull/587#issuecomment-6610496793
Thanks for the review. All points are addressed in 6268090cfd: 1. **Error handling:** a malformed typed value now fails with a `BundleException` (`MANIFEST_ERROR`) that names the attribute, its type and the value, e.g. `Invalid value for attribute 'longAttr' of type Long : abc`. Unknown types (`x:Foo=1`, `List<Foo>`) and a `version`/`specification-version` mismatch are reported as `MANIFEST_ERROR` too. 2. **Running twice:** `m_types` is cleared after the conversion, so converting a clause again does nothing. 3. **Empty list:** an empty list in a requirement now becomes `(attr=*)`: the capability must have the attribute, and any value matches. Before, it matched every capability, also one without the attribute. 4. **List semantics:** documented in the `SimpleFilter.convert` Javadoc, including that the spec does not define list matching for these headers. The TCK in osgi/org.osgi.annotation.bundle#24 does not exercise list attributes in requirement headers (bnd generates `Import-Package` with version ranges only), and it passes 23/23 against this branch. For comparison, Equinox builds `(l=[a, b])` here, which never matches. 5. **`filters.size()` fix:** `SimpleFilterTest` now covers a single bounded `VersionRange` and an empty list next to another attribute. Both tests fail on the code before this PR. 6. **Version check:** the helpers `getPackageVersion` and `convertVersionRanges` replace the duplicated blocks. `version`, `specification-version` and `bundle-version` must be of type `String` or `Version`, otherwise there is a clear error instead of a parse error on `"[1.0.0, 2.0.0]"`. The 1.0 vs 1.0.0 change is described in the commit message. 7. **Test gaps:** added negative cases (typed values, list elements, `bundle-version`), malformed values, unknown types, a typed `bundle-version` for `Require-Bundle` and `Fragment-Host`, and moved the constants and helpers to the bottom of the class. A second review pass found one more issue, also fixed in this commit: typed attributes are now only converted for R4 bundles in `Import-Package`, `DynamicImport-Package` and `Export-Package`. R3 bundles keep dropping unknown attributes with a warning instead of failing on a malformed one. All 147 framework tests pass. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
