From dbd63eaf33ee90f85f7db67052dbfcad2447b5b3 Mon Sep 17 00:00:00 2001 From: Ujjwal Agarwal Date: Tue, 10 Aug 2021 23:32:03 +0530 Subject: [PATCH 1/2] Jsonsaver : maintain order while converting maps to json arrays - bug fix : sort maos keys and then range over keys instead of maps - to maintain order while savings properties as maps Signed-off-by: Ujjwal Agarwal --- jsonsaver/saver2v2/save_document_test.go | 49 ++++++++++++------------ jsonsaver/saver2v2/save_files.go | 25 ++++++++++-- jsonsaver/saver2v2/save_files_test.go | 16 ++++---- jsonsaver/saver2v2/save_package.go | 24 ++++++++++-- 4 files changed, 76 insertions(+), 38 deletions(-) diff --git a/jsonsaver/saver2v2/save_document_test.go b/jsonsaver/saver2v2/save_document_test.go index 90d01f13..9651367a 100644 --- a/jsonsaver/saver2v2/save_document_test.go +++ b/jsonsaver/saver2v2/save_document_test.go @@ -177,14 +177,14 @@ func TestRenderDocument2_2(t *testing.T) { "File": { FileSPDXIdentifier: "File", FileChecksums: map[spdx.ChecksumAlgorithm]spdx.Checksum{ + "MD5": { + Algorithm: "MD5", + Value: "624c1abb3664f4b35547e7c73864ad24", + }, "SHA1": { Algorithm: "SHA1", Value: "d6a770ba38583ed4bb4525bd96e50461655d2758", }, - // "MD5": { - // Algorithm: "MD5", - // Value: "624c1abb3664f4b35547e7c73864ad24", - // }, }, FileComment: "The concluded license was taken from the package level that the file was .\nThis information was found in the COPYING.txt file in the xyz directory.", FileCopyrightText: "Copyright 2008-2010 John Smith", @@ -317,6 +317,22 @@ func TestRenderDocument2_2(t *testing.T) { }, }, "files": []interface{}{ + map[string]interface{}{ + "SPDXID": "SPDXRef-DoapSource", + "checksums": []interface{}{ + map[string]interface{}{ + "algorithm": "SHA1", + "checksumValue": "2fd4e1c67a2d28fced849ee1bb76e7391b93eb12", + }, + }, + "copyrightText": "Copyright 2010, 2011 Source Auditor Inc.", + "fileContributors": []string{"Protecode Inc.", "SPDX Technical Team Members", "Open Logic Inc.", "Source Auditor Inc.", "Black Duck Software In.c"}, + "fileDependencies": []string{"SPDXRef-JenaLib", "SPDXRef-CommonsLangSrc"}, + "fileName": "./src/org/spdx/parser/DOAPProject.java", + "fileTypes": []string{"SOURCE"}, + "licenseConcluded": "Apache-2.0", + "licenseInfoInFiles": []string{"Apache-2.0"}, + }, map[string]interface{}{ "SPDXID": "SPDXRef-File", "annotations": []interface{}{ @@ -328,14 +344,15 @@ func TestRenderDocument2_2(t *testing.T) { }, }, "checksums": []interface{}{ + map[string]interface{}{ + "algorithm": "MD5", + "checksumValue": "624c1abb3664f4b35547e7c73864ad24", + }, + map[string]interface{}{ "algorithm": "SHA1", "checksumValue": "d6a770ba38583ed4bb4525bd96e50461655d2758", }, - // map[string]interface{}{ - // "algorithm": "MD5", - // "checksumValue": "624c1abb3664f4b35547e7c73864ad24", - // }, }, "comment": "The concluded license was taken from the package level that the file was .\nThis information was found in the COPYING.txt file in the xyz directory.", "copyrightText": "Copyright 2008-2010 John Smith", @@ -347,22 +364,6 @@ func TestRenderDocument2_2(t *testing.T) { "licenseInfoInFiles": []string{"GPL-2.0-only", "LicenseRef-2"}, "noticeText": "Copyright (c) 2001 Aaron Lehmann aaroni@vitelus.", }, - map[string]interface{}{ - "SPDXID": "SPDXRef-DoapSource", - "checksums": []interface{}{ - map[string]interface{}{ - "algorithm": "SHA1", - "checksumValue": "2fd4e1c67a2d28fced849ee1bb76e7391b93eb12", - }, - }, - "copyrightText": "Copyright 2010, 2011 Source Auditor Inc.", - "fileContributors": []string{"Protecode Inc.", "SPDX Technical Team Members", "Open Logic Inc.", "Source Auditor Inc.", "Black Duck Software In.c"}, - "fileDependencies": []string{"SPDXRef-JenaLib", "SPDXRef-CommonsLangSrc"}, - "fileName": "./src/org/spdx/parser/DOAPProject.java", - "fileTypes": []string{"SOURCE"}, - "licenseConcluded": "Apache-2.0", - "licenseInfoInFiles": []string{"Apache-2.0"}, - }, }, "snippets": []interface{}{ map[string]interface{}{ diff --git a/jsonsaver/saver2v2/save_files.go b/jsonsaver/saver2v2/save_files.go index ca6d6b35..d866fd31 100644 --- a/jsonsaver/saver2v2/save_files.go +++ b/jsonsaver/saver2v2/save_files.go @@ -3,15 +3,25 @@ package saver2v2 import ( + "sort" + "github.com/spdx/tools-golang/spdx" ) func renderFiles2_2(doc *spdx.Document2_2, jsondocument map[string]interface{}) ([]interface{}, error) { + var keys []string + for ke := range doc.UnpackagedFiles { + keys = append(keys, string(ke)) + } + sort.Strings(keys) + var files []interface{} - for k, v := range doc.UnpackagedFiles { + // for k, v := range doc.UnpackagedFiles { + for _, k := range keys { + v := doc.UnpackagedFiles[spdx.ElementID(k)] file := make(map[string]interface{}) - file["SPDXID"] = spdx.RenderElementID(k) + file["SPDXID"] = spdx.RenderElementID(spdx.ElementID(k)) ann, _ := renderAnnotations2_2(doc.Annotations, spdx.MakeDocElementID("", string(v.FileSPDXIdentifier))) if ann != nil { file["annotations"] = ann @@ -26,7 +36,16 @@ func renderFiles2_2(doc *spdx.Document2_2, jsondocument map[string]interface{}) // save package checksums if v.FileChecksums != nil { var checksums []interface{} - for _, value := range v.FileChecksums { + + var algos []string + for alg := range v.FileChecksums { + algos = append(algos, string(alg)) + } + sort.Strings(algos) + + // for _, value := range v.FileChecksums { + for _, algo := range algos { + value := v.FileChecksums[spdx.ChecksumAlgorithm(algo)] checksum := make(map[string]interface{}) checksum["algorithm"] = string(value.Algorithm) checksum["checksumValue"] = value.Value diff --git a/jsonsaver/saver2v2/save_files_test.go b/jsonsaver/saver2v2/save_files_test.go index c2e61d8d..fbc6ba1c 100644 --- a/jsonsaver/saver2v2/save_files_test.go +++ b/jsonsaver/saver2v2/save_files_test.go @@ -75,10 +75,10 @@ func Test_renderFiles2_2(t *testing.T) { Algorithm: "SHA1", Value: "d6a770ba38583ed4bb4525bd96e50461655d2758", }, - // "MD5": { - // Algorithm: "MD5", - // Value: "624c1abb3664f4b35547e7c73864ad24", - // }, + "MD5": { + Algorithm: "MD5", + Value: "624c1abb3664f4b35547e7c73864ad24", + }, }, FileComment: "The concluded license was taken from the package level that the file was .", FileCopyrightText: "Copyright 2008-2010 John Smith", @@ -107,14 +107,14 @@ func Test_renderFiles2_2(t *testing.T) { }, }, "checksums": []interface{}{ + map[string]interface{}{ + "algorithm": "MD5", + "checksumValue": "624c1abb3664f4b35547e7c73864ad24", + }, map[string]interface{}{ "algorithm": "SHA1", "checksumValue": "d6a770ba38583ed4bb4525bd96e50461655d2758", }, - // map[string]interface{}{ - // "algorithm": "MD5", - // "checksumValue": "624c1abb3664f4b35547e7c73864ad24", - // }, }, "comment": "The concluded license was taken from the package level that the file was .", "copyrightText": "Copyright 2008-2010 John Smith", diff --git a/jsonsaver/saver2v2/save_package.go b/jsonsaver/saver2v2/save_package.go index a402945d..ab96045b 100644 --- a/jsonsaver/saver2v2/save_package.go +++ b/jsonsaver/saver2v2/save_package.go @@ -4,6 +4,7 @@ package saver2v2 import ( "fmt" + "sort" "github.com/spdx/tools-golang/spdx" ) @@ -11,9 +12,18 @@ import ( func renderPackage2_2(doc *spdx.Document2_2, jsondocument map[string]interface{}) ([]interface{}, error) { var packages []interface{} - for k, v := range doc.Packages { + + var keys []string + for ke := range doc.Packages { + keys = append(keys, string(ke)) + } + sort.Strings(keys) + + // for k, v := range doc.Packages { + for _, k := range keys { + v := doc.Packages[spdx.ElementID(k)] pkg := make(map[string]interface{}) - pkg["SPDXID"] = spdx.RenderElementID(k) + pkg["SPDXID"] = spdx.RenderElementID(spdx.ElementID(k)) ann, _ := renderAnnotations2_2(doc.Annotations, spdx.MakeDocElementID("", string(v.PackageSPDXIdentifier))) if ann != nil { pkg["annotations"] = ann @@ -24,7 +34,15 @@ func renderPackage2_2(doc *spdx.Document2_2, jsondocument map[string]interface{} // save package checksums if v.PackageChecksums != nil { var checksums []interface{} - for _, value := range v.PackageChecksums { + + var algos []string + for alg := range v.PackageChecksums { + algos = append(algos, string(alg)) + } + sort.Strings(algos) + + for _, algo := range algos { + value := v.PackageChecksums[spdx.ChecksumAlgorithm(algo)] checksum := make(map[string]interface{}) checksum["algorithm"] = string(value.Algorithm) checksum["checksumValue"] = value.Value From f679856ddf17e0335474cf309d940a6517dda0aa Mon Sep 17 00:00:00 2001 From: Ujjwal Agarwal Date: Fri, 13 Aug 2021 20:18:54 +0530 Subject: [PATCH 2/2] JsonParser : JsonParser test coverage increased Signed-off-by: Ujjwal Agarwal --- jsonloader/parser2v2/parse_files_test.go | 12 +++---- jsonloader/parser2v2/parse_other_license.go | 2 +- .../parser2v2/parse_other_license_test.go | 31 ++++++++++++++++--- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/jsonloader/parser2v2/parse_files_test.go b/jsonloader/parser2v2/parse_files_test.go index 30f1bdc4..4d54fad5 100644 --- a/jsonloader/parser2v2/parse_files_test.go +++ b/jsonloader/parser2v2/parse_files_test.go @@ -197,13 +197,11 @@ func TestJSONSpdxDocument_parseJsonFiles2_2(t *testing.T) { t.Errorf("JSONSpdxDocument.parseJsonFiles2_2() error = %v, wantErr %v", err, tt.wantErr) } - // if !reflect.DeepEqual(tt.args.doc.UnpackagedFiles, tt.want) { - // t.Errorf("Load2_2() = %v, want %v", tt.args.doc.UnpackagedFiles, tt.want) - // } - - for k, v := range tt.want { - if !reflect.DeepEqual(tt.args.doc.UnpackagedFiles[k], v) { - t.Errorf("Load2_2() = %v, want %v", tt.args.doc.UnpackagedFiles[k], v) + if !tt.wantErr { + for k, v := range tt.want { + if !reflect.DeepEqual(tt.args.doc.UnpackagedFiles[k], v) { + t.Errorf("Load2_2() = %v, want %v", tt.args.doc.UnpackagedFiles[k], v) + } } } diff --git a/jsonloader/parser2v2/parse_other_license.go b/jsonloader/parser2v2/parse_other_license.go index de9e8f24..997ad025 100644 --- a/jsonloader/parser2v2/parse_other_license.go +++ b/jsonloader/parser2v2/parse_other_license.go @@ -34,7 +34,7 @@ func (spec JSONSpdxDocument) parseJsonOtherLicenses2_2(key string, value interfa } } default: - return fmt.Errorf("received unknown tag %v in Annotation section", k) + return fmt.Errorf("received unknown tag %v in Licenses section", k) } } doc.OtherLicenses = append(doc.OtherLicenses, &license) diff --git a/jsonloader/parser2v2/parse_other_license_test.go b/jsonloader/parser2v2/parse_other_license_test.go index 0ccee1cc..41d8ede9 100644 --- a/jsonloader/parser2v2/parse_other_license_test.go +++ b/jsonloader/parser2v2/parse_other_license_test.go @@ -45,9 +45,20 @@ func TestJSONSpdxDocument_parseJsonOtherLicenses2_2(t *testing.T) { }, } + otherLicensestest2 := []byte(`{ + "hasExtractedLicensingInfos":[{ + "extractedText" : "\"THE BEER-WARE LICENSE\" (Revision 42):\nphk@FreeBSD.ORG wrote this file. As long as you retain this notice you\ncan do whatever you want with this stuff. If we meet some day, and you think this stuff is worth it, you can buy me a beer in return Poul-Henning Kamp unknown tag", + spec: specs2, + args: args{ + key: "hasExtractedLicensingInfos", + value: specs2["hasExtractedLicensingInfos"], + doc: &spdxDocument2_2{}, + }, + want: nil, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -79,12 +101,13 @@ func TestJSONSpdxDocument_parseJsonOtherLicenses2_2(t *testing.T) { t.Errorf("JSONSpdxDocument.parseJsonOtherLicenses2_2() error = %v, wantErr %v", err, tt.wantErr) } - for i := 0; i < len(tt.want); i++ { - if !reflect.DeepEqual(tt.args.doc.OtherLicenses[i], tt.want[i]) { - t.Errorf("Load2_2() = %v, want %v", tt.args.doc.OtherLicenses[i], tt.want[i]) + if !tt.wantErr { + for i := 0; i < len(tt.want); i++ { + if !reflect.DeepEqual(tt.args.doc.OtherLicenses[i], tt.want[i]) { + t.Errorf("Load2_2() = %v, want %v", tt.args.doc.OtherLicenses[i], tt.want[i]) + } } } - }) } }