From c78702db737aae733a126968c49e5ae9a7b3edd0 Mon Sep 17 00:00:00 2001 From: Varsha Prasad Narsing Date: Wed, 22 Mar 2023 12:40:01 -0400 Subject: [PATCH] [entity] Not error when optional properties are not present in the entity This PR introduces the optional field while loading entities from bundles. If the property is optional and is not present, we do not error. Signed-off-by: Varsha Prasad Narsing --- .../variable_sources/entity/bundle_entity.go | 33 +++++++++++-------- .../entity/bundle_entity_test.go | 16 ++++----- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/internal/resolution/variable_sources/entity/bundle_entity.go b/internal/resolution/variable_sources/entity/bundle_entity.go index 350566a1d..cadb79d65 100644 --- a/internal/resolution/variable_sources/entity/bundle_entity.go +++ b/internal/resolution/variable_sources/entity/bundle_entity.go @@ -19,6 +19,13 @@ type ChannelProperties struct { SkipRange string `json:"skipRange,omitempty"` } +type propertyRequirement bool + +const ( + required propertyRequirement = true + optional propertyRequirement = false +) + type PackageRequired struct { property.PackageRequired SemverRange *semver.Range `json:"-"` @@ -121,7 +128,7 @@ func (b *BundleEntity) loadPackage() error { b.mu.Lock() defer b.mu.Unlock() if b.bundlePackage == nil { - bundlePackage, err := loadFromEntity[property.Package](b.Entity, property.TypePackage) + bundlePackage, err := loadFromEntity[property.Package](b.Entity, property.TypePackage, required) if err != nil { return fmt.Errorf("error determining package for entity '%s': %w", b.ID, err) } @@ -141,7 +148,7 @@ func (b *BundleEntity) loadProvidedGVKs() error { b.mu.Lock() defer b.mu.Unlock() if b.providedGVKs == nil { - providedGVKs, err := loadFromEntity[[]GVK](b.Entity, property.TypeGVK) + providedGVKs, err := loadFromEntity[[]GVK](b.Entity, property.TypeGVK, optional) if err != nil { return fmt.Errorf("error determining bundle provided gvks for entity '%s': %w", b.ID, err) } @@ -154,7 +161,7 @@ func (b *BundleEntity) loadRequiredGVKs() error { b.mu.Lock() defer b.mu.Unlock() if b.requiredGVKs == nil { - requiredGVKs, err := loadFromEntity[[]GVKRequired](b.Entity, property.TypeGVKRequired) + requiredGVKs, err := loadFromEntity[[]GVKRequired](b.Entity, property.TypeGVKRequired, optional) if err != nil { return fmt.Errorf("error determining bundle required gvks for entity '%s': %w", b.ID, err) } @@ -167,7 +174,7 @@ func (b *BundleEntity) loadRequiredPackages() error { b.mu.Lock() defer b.mu.Unlock() if b.requiredPackages == nil { - requiredPackages, err := loadFromEntity[[]PackageRequired](b.Entity, property.TypePackageRequired) + requiredPackages, err := loadFromEntity[[]PackageRequired](b.Entity, property.TypePackageRequired, optional) if err != nil { return fmt.Errorf("error determining bundle required packages for entity '%s': %w", b.ID, err) } @@ -187,7 +194,7 @@ func (b *BundleEntity) loadChannelProperties() error { b.mu.Lock() defer b.mu.Unlock() if b.channelProperties == nil { - channel, err := loadFromEntity[ChannelProperties](b.Entity, property.TypeChannel) + channel, err := loadFromEntity[ChannelProperties](b.Entity, property.TypeChannel, required) if err != nil { return fmt.Errorf("error determining bundle channel properties for entity '%s': %w", b.ID, err) } @@ -200,7 +207,7 @@ func (b *BundleEntity) loadBundlePath() error { b.mu.Lock() defer b.mu.Unlock() if b.bundlePath == "" { - bundlePath, err := loadFromEntity[string](b.Entity, PropertyBundlePath) + bundlePath, err := loadFromEntity[string](b.Entity, PropertyBundlePath, required) if err != nil { return fmt.Errorf("error determining bundle path for entity '%s': %w", b.ID, err) } @@ -209,15 +216,15 @@ func (b *BundleEntity) loadBundlePath() error { return nil } -func loadFromEntity[T interface{}](entity *input.Entity, propertyName string) (T, error) { +func loadFromEntity[T interface{}](entity *input.Entity, propertyName string, required propertyRequirement) (T, error) { deserializedProperty := *new(T) propertyValue, ok := entity.Properties[propertyName] - if !ok { - return deserializedProperty, fmt.Errorf("property '%s' not found", propertyName) - } - - if err := json.Unmarshal([]byte(propertyValue), &deserializedProperty); err != nil { - return deserializedProperty, fmt.Errorf("property '%s' ('%s') could not be parsed: %w", propertyName, propertyValue, err) + if ok { + if err := json.Unmarshal([]byte(propertyValue), &deserializedProperty); err != nil { + return deserializedProperty, fmt.Errorf("property '%s' ('%s') could not be parsed: %w", propertyName, propertyValue, err) + } + } else if required { + return deserializedProperty, fmt.Errorf("required property '%s' not found", propertyName) } return deserializedProperty, nil } diff --git a/internal/resolution/variable_sources/entity/bundle_entity_test.go b/internal/resolution/variable_sources/entity/bundle_entity_test.go index 328f5bb7c..16aa5e87c 100644 --- a/internal/resolution/variable_sources/entity/bundle_entity_test.go +++ b/internal/resolution/variable_sources/entity/bundle_entity_test.go @@ -33,7 +33,7 @@ var _ = Describe("BundleEntity", func() { bundleEntity := olmentity.NewBundleEntity(entity) packageName, err := bundleEntity.PackageName() Expect(packageName).To(Equal("")) - Expect(err.Error()).To(Equal("error determining package for entity 'operatorhub/prometheus/0.14.0': property 'olm.package' not found")) + Expect(err.Error()).To(Equal("error determining package for entity 'operatorhub/prometheus/0.14.0': required property 'olm.package' not found")) }) It("should return error if the property is malformed", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ @@ -61,7 +61,7 @@ var _ = Describe("BundleEntity", func() { bundleEntity := olmentity.NewBundleEntity(entity) version, err := bundleEntity.Version() Expect(version).To(BeNil()) - Expect(err.Error()).To(Equal("error determining package for entity 'operatorhub/prometheus/0.14.0': property 'olm.package' not found")) + Expect(err.Error()).To(Equal("error determining package for entity 'operatorhub/prometheus/0.14.0': required property 'olm.package' not found")) }) It("should return error if the property is malformed", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ @@ -101,7 +101,7 @@ var _ = Describe("BundleEntity", func() { bundleEntity := olmentity.NewBundleEntity(entity) providedGvks, err := bundleEntity.ProvidedGVKs() Expect(providedGvks).To(BeNil()) - Expect(err.Error()).To(Equal("error determining bundle provided gvks for entity 'operatorhub/prometheus/0.14.0': property 'olm.gvk' not found")) + Expect(err).To(BeNil()) }) It("should return error if the property is malformed", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ @@ -132,7 +132,7 @@ var _ = Describe("BundleEntity", func() { bundleEntity := olmentity.NewBundleEntity(entity) requiredGvks, err := bundleEntity.RequiredGVKs() Expect(requiredGvks).To(BeNil()) - Expect(err.Error()).To(Equal("error determining bundle required gvks for entity 'operatorhub/prometheus/0.14.0': property 'olm.gvk.required' not found")) + Expect(err).To(BeNil()) }) It("should return error if the property is malformed", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ @@ -163,7 +163,7 @@ var _ = Describe("BundleEntity", func() { bundleEntity := olmentity.NewBundleEntity(entity) requiredPackages, err := bundleEntity.RequiredPackages() Expect(requiredPackages).To(BeNil()) - Expect(err.Error()).To(Equal("error determining bundle required packages for entity 'operatorhub/prometheus/0.14.0': property 'olm.package.required' not found")) + Expect(err).To(BeNil()) }) It("should return error if the property is malformed", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ @@ -191,7 +191,7 @@ var _ = Describe("BundleEntity", func() { bundleEntity := olmentity.NewBundleEntity(entity) channelName, err := bundleEntity.ChannelName() Expect(channelName).To(BeEmpty()) - Expect(err.Error()).To(Equal("error determining bundle channel properties for entity 'operatorhub/prometheus/0.14.0': property 'olm.channel' not found")) + Expect(err.Error()).To(Equal("error determining bundle channel properties for entity 'operatorhub/prometheus/0.14.0': required property 'olm.channel' not found")) }) It("should return error if the property is malformed", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ @@ -227,7 +227,7 @@ var _ = Describe("BundleEntity", func() { bundleEntity := olmentity.NewBundleEntity(entity) channelProperties, err := bundleEntity.ChannelProperties() Expect(channelProperties).To(BeNil()) - Expect(err.Error()).To(Equal("error determining bundle channel properties for entity 'operatorhub/prometheus/0.14.0': property 'olm.channel' not found")) + Expect(err.Error()).To(Equal("error determining bundle channel properties for entity 'operatorhub/prometheus/0.14.0': required property 'olm.channel' not found")) }) It("should return error if the property is malformed", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ @@ -255,7 +255,7 @@ var _ = Describe("BundleEntity", func() { bundleEntity := olmentity.NewBundleEntity(entity) bundlePath, err := bundleEntity.BundlePath() Expect(bundlePath).To(BeEmpty()) - Expect(err.Error()).To(Equal("error determining bundle path for entity 'operatorhub/prometheus/0.14.0': property 'olm.bundle.path' not found")) + Expect(err.Error()).To(Equal("error determining bundle path for entity 'operatorhub/prometheus/0.14.0': required property 'olm.bundle.path' not found")) }) It("should return error if the property is malformed", func() { entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{