From 5c8fe0dc0d18ea2ae210047b9c556149d27399d3 Mon Sep 17 00:00:00 2001 From: Suyash Kumar Date: Sun, 9 Jun 2024 22:25:51 -0400 Subject: [PATCH] Add inference tests using Write API --- parse_internal_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++ write.go | 46 ++++++++++++++++++++------------ 2 files changed, 89 insertions(+), 16 deletions(-) diff --git a/parse_internal_test.go b/parse_internal_test.go index b239855..874d59c 100644 --- a/parse_internal_test.go +++ b/parse_internal_test.go @@ -1,11 +1,18 @@ package dicom import ( + "bytes" + "errors" "os" "strings" "testing" + + "github.com/suyashkumar/dicom/pkg/tag" ) +// parse_internal_test.go holds tests that must exist in the dicom package (as +// opposed to dicom_test), in order to access internal entities. + // TestParseUntilEOFConformsToParse runs both the dicom.ParseUntilEOF and the dicom.Parse APIs against each // testdata file and ensures the outputs are the same. // This test lives in parse_internal_test.go because this test cannot live in the dicom_test package, due @@ -48,6 +55,58 @@ func TestParseUntilEOFConformsToParse(t *testing.T) { } } +func TestParse_InfersMissingTransferSyntax(t *testing.T) { + dsWithMissingTS := Dataset{Elements: []*Element{ + mustNewElement(tag.MediaStorageSOPClassUID, []string{"1.2.840.10008.5.1.4.1.1.1.2"}), + mustNewElement(tag.MediaStorageSOPInstanceUID, []string{"1.2.3.4.5.6.7"}), + mustNewElement(tag.PatientName, []string{"Bob", "Jones"}), + mustNewElement(tag.Rows, []int{128}), + mustNewElement(tag.FloatingPointValue, []float64{128.10}), + mustNewElement(tag.DimensionIndexPointer, []int{32, 36950}), + mustNewElement(tag.RedPaletteColorLookupTableData, make([]byte, 200)), + }} + + cases := []struct { + name string + overrideTransferSyntax string + }{ + { + name: "Little Endian Implicit", + overrideTransferSyntax: "1.2.840.10008.1.2", + }, + { + name: "Little Endian Explicit", + overrideTransferSyntax: "1.2.840.10008.1.2.1", + }, + { + name: "Big Endian Explicit", + overrideTransferSyntax: "1.2.840.10008.1.2.2", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // Write out Dataset with OverrideMissingTransferSyntax option _and_ + // the testSkipWritingTransferSyntax to ensure no Transfer Syntax + // element is written to the test dicom. The test later verifies + // that no Transfer Syntax element was written to the metadata. + writtenDICOM := &bytes.Buffer{} + if err := Write(writtenDICOM, dsWithMissingTS, OverrideMissingTransferSyntax(tc.overrideTransferSyntax), testSkipWritingTransferSyntax()); err != nil { + t.Errorf("Write(OverrideMissingTransferSyntax(%v)) returned unexpected error: %v", tc.overrideTransferSyntax, err) + } + + parsedDS, err := ParseUntilEOF(writtenDICOM, nil) + if err != nil { + t.Fatalf("ParseUntilEOF returned unexpected error when reading written dataset back in: %v", err) + } + _, err = parsedDS.FindElementByTag(tag.TransferSyntaxUID) + if !errors.Is(err, ErrorElementNotFound) { + t.Fatalf("expected test dicom dataset to be missing explicit TransferSyntaxUID tag, but found one. got: %v, want: ErrorElementNotFound", err) + } + }) + } +} + func readTestdataFile(t *testing.T, name string) *os.File { dcm, err := os.Open("./testdata/" + name) if err != nil { diff --git a/write.go b/write.go index 3b9be99..0db5f88 100644 --- a/write.go +++ b/write.go @@ -156,12 +156,24 @@ func OverrideMissingTransferSyntax(transferSyntaxUID string) WriteOption { } } +// testSkipWritingTransferSyntax is a test WriteOption that cause Write to skip +// writing the transfer syntax uid element in the DICOM metadata. When used in +// combination with OverrideMissingTransferSyntax, this can be used to set the +// TransferSyntax for the written dataset without writing the actual transfer +// syntax element to the metadata. +func testSkipWritingTransferSyntax() WriteOption { + return func(set *writeOptSet) { + set.skipWritingTransferSyntaxForTests = true + } +} + // writeOptSet represents the flattened option set after all WriteOptions have been applied. type writeOptSet struct { - skipVRVerification bool - skipValueTypeVerification bool - defaultMissingTransferSyntax bool - overrideMissingTransferSyntaxUID string + skipVRVerification bool + skipValueTypeVerification bool + defaultMissingTransferSyntax bool + overrideMissingTransferSyntaxUID string + skipWritingTransferSyntaxForTests bool } func (w *writeOptSet) validate() error { @@ -203,21 +215,23 @@ func writeFileHeader(w *dicomio.Writer, ds *Dataset, metaElems []*Element, opts return err } - err = writeMetaElem(subWriter, tag.TransferSyntaxUID, ds, &tagsUsed, opts) + if !opts.skipWritingTransferSyntaxForTests { + err = writeMetaElem(subWriter, tag.TransferSyntaxUID, ds, &tagsUsed, opts) - if errors.Is(err, ErrorElementNotFound) && opts.defaultMissingTransferSyntax { - // Write the default transfer syntax - if err = writeElement(subWriter, mustNewElement(tag.TransferSyntaxUID, []string{uid.ImplicitVRLittleEndian}), opts); err != nil { - return err - } - } else if errors.Is(err, ErrorElementNotFound) && opts.overrideMissingTransferSyntaxUID != "" { - // Write the override transfer syntax - if err = writeElement(subWriter, mustNewElement(tag.TransferSyntaxUID, []string{opts.overrideMissingTransferSyntaxUID}), opts); err != nil { + if errors.Is(err, ErrorElementNotFound) && opts.defaultMissingTransferSyntax { + // Write the default transfer syntax + if err = writeElement(subWriter, mustNewElement(tag.TransferSyntaxUID, []string{uid.ImplicitVRLittleEndian}), opts); err != nil { + return err + } + } else if errors.Is(err, ErrorElementNotFound) && opts.overrideMissingTransferSyntaxUID != "" { + // Write the override transfer syntax + if err = writeElement(subWriter, mustNewElement(tag.TransferSyntaxUID, []string{opts.overrideMissingTransferSyntaxUID}), opts); err != nil { + return err + } + } else if err != nil { + // Return the error if none of the above conditions/overrides apply. return err } - } else if err != nil { - // Return the error if none of the above conditions/overrides apply. - return err } for _, elem := range metaElems {