Skip to content
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.

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

feat: Deserializer/Denormalizer #86

Open
davidkarlsen opened this issue Jun 21, 2022 · 10 comments
Open

feat: Deserializer/Denormalizer #86

davidkarlsen opened this issue Jun 21, 2022 · 10 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@davidkarlsen
Copy link

It would be great to provide a factory/deserializer, in order to provide a cdx.Models.Bom from an inputstream of json.

  return JSON.parse(fs.readFileSync(sbomFile, 'utf8')) as cdx.Models.Bom

returns a "halfbaked" model, where calls to

  const packages: Package[] = bom.components.sorted().map(component => {
    const packageUrl: PackageURL = component.purl as PackageURL
    return new Package(packageUrl)
  })

will raise

TypeError: bom.components.sorted is not a function
@jkowalleck jkowalleck added the enhancement New feature or request label Jun 22, 2022
@jkowalleck
Copy link
Member

jkowalleck commented Jun 22, 2022

Pull requests are welcome.
Do you plan to work on this feature, or should it be marked as "help wanted"?

Having a Denormalizer would be the first step, don't you think?
This would take JSON-parsed structures/interfaces and build complete CycloneDX data models from it.
The data types the Denormalizer could use as an input are already defined: https://github.com/CycloneDX/cyclonedx-javascript-library/blob/main/src/serialize/json/types.ts#L63

A denormalizer should be implemented in a file /src/serialize/json/denormalize.ts


PS: do not use the types from normalizer for the job of denormalization, as long as you cannot assert them.

@davidkarlsen
Copy link
Author

Pull requests are welcome. Do you plan to work on this feature, or should it be marked as "help wanted"?

We could, but I can't promise how fast it will be done, as we're quite busy and approaching holidays - but it makes sense to put it into this lib.

Having a Denormalizer would be the first step, don't you think? This would take JSON-parsed structures/interfaces and build complete CycloneDX data models from it. The data types the Denormalizer would use as an input are already defined: https://github.com/CycloneDX/cyclonedx-javascript-library/blob/main/src/serialize/json/types.ts#L63

Maybe :) I didn't study it to close. What is the concept behind Denormalizer/Normalizer?

@jkowalleck
Copy link
Member

jkowalleck commented Jun 22, 2022

What is the concept behind Denormalizer/Normalizer?

Data models do not provide logic for de/normalization, de/serialization , conversion etc. These tasks are done by others.

A JSON-normalizer, for example, converts CycloneDX data models into these structures that a JSON-serializer could use. And that JSON-serializer will transform these structures into a string, does beautification, etc ...
This way every "step" in the chain to serialize a CycloneDX data model to JSON-string is encapsulated and exchangeable.

A Normalizer transforms data models to data structures that conform to the JSON-/XML-spec of CycloneDX.
A Denormalizer transforms data structures that conform to the JSON-/XML-spec of CycloneDX into data models .

JSON is easy, it is a first-class-citizen of JavaScript.
XML on the other hand has no native suppot with JavaScript. So the actual serializer might be a custom implementation tailored for a special environment, but every XML-serializer should be able to handle the output of the XML-normalizer.

The thing with the data models is, that they are near the JSON-/XML-data-structures, but they differ for obvious reasons.

@jkowalleck jkowalleck changed the title Provide deserializer feat: Deserializer/Denormalizer Jun 22, 2022
@jkowalleck jkowalleck added the help wanted Extra attention is needed label Sep 17, 2022
@jkowalleck
Copy link
Member

Regarding implementation

First, don't rely the denormalizer on the data structures defined for normalizing.
These data structures are ONE opinionated subset of the allowed data structures - according to the JSON schemas

preamble: the deserializers should be as forgiving as possible. dont abort deserializations, if optional values are unexpected.

An idea i had:
0. have tests not only for happy paths, but also for a lot of invalid or broken inputs.

  1. deserialize JSON to native JavaScript data structures. This would be of type Any.
  2. check if this result is an object,
    with the property bomFormat being "CycloneDX"
    and the property specVersion being one of the known values.
    Else: throw error
  3. Fetch the spec according to the value of specVersion.
    If none found: throw error
  4. Start denormalization by injecting the deserialized structure into a BOM-denormalizer.
    That BOM-denormalizer should accept any, and check for needed types and structures on.
    Only mandatory values cause throw on mismatch of type or range.
    Optional properties cause omitting of them on mismatch of type or range.

example test data:

{
  "bomFormat": "CycloneDX",
  "specVersion": "1.4",
  "externalReferences": null,
  "metadata": { 
    "authors": [ 
      "me", 
      {
        "email": "[email protected]", 
        "foobar": "bazz"
      }
    ] 
  }
}

results in:

  • instance of Bom, because all mandatory values are there. Yes, mandatory property version is missing, but this property has a default value of "1" in the schema, so we are fine to create a Bom object.
  • The Bom.externalReferences is an empty set, because of type mismatch. The schema demands externalReferences to be a list.
  • the Bom.metadata is preset, because the data type is as expected (object) with all mandatory properties.
  • the Bom.metadata.authors is a set with one element.
    • The item "me" is of the wrong type and is omitted.
    • The item type object is taken into account, as it has every mandatory property.
      The property foobar is omitted, as it is unknown to the schema.

@jkowalleck
Copy link
Member

jkowalleck commented Mar 7, 2023

📣 please read before implementing

! Do not simply start implementing, or drop a PullRequest out of nowhere.
This topic is a complex one (whilst the implementation is really trivial wile time-consuming).
Not all concepts and facts are yet written down here, so please:
before you start, ask for architectural constraints, guidance and answers to all your questions.

Most importantly: embrace the change. No reason to keep this library as it is.
If you want to introduce breaking changes on the way of implementing this feature, just announce/document them and you are good.


here are critical remarks from previous implementation attempts, that should be followed:

@jkowalleck
Copy link
Member

JSON- and XML- validators are in place now. (#620 via #652, #691)
THis allowes input validation, so implementing this feature should be much easier, now.

@hakandilek
Copy link

Any progress on this one?

Last attempt with #506 seems to be hanging after code review. Do you know if anyone is working on it?

@jkowalleck
Copy link
Member

This is a community project. I am not aware of anybody championing or working this feature.
If you intend to do so, please let us know. :-)

@hakandilek
Copy link

This is a community project. I am not aware of anybody championing or working this feature.

Sure it is, and it is generating invaluable tools for the community.

! Do not simply start implementing, or drop a PullRequest out of nowhere.
This topic is a complex one (whilst the implementation is really trivial wile time-consuming).
Not all concepts and facts are yet written down here, so please:
before you start, ask for architectural constraints, guidance and answers to all your questions.

Since, you requested people to show up and ask for some guidance, I was hoping that someone may have done it and already started working on this and did not want to step on anyone's feet but it seems like the previous PR has been sitting there about a year.

I'd be happy to chip in if you want to give some guidance (if you've much more to add on top of the comments and notes from #506).
@jkowalleck considering your profile location (Nuremberg) is up to date (and if you prefer to), I'd also be happy to meet face to face and discuss it over a beer.

@jkowalleck
Copy link
Member

considering your profile location (Nuremberg) is up to date (and if you prefer to), I'd also be happy to meet face to face and discuss it over a beer.

@hakandilek sure. I will ping you in the CycloneDX community slack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants