Address reviewer comments

Signed-off-by: Predrag Knezevic <predrag.knezevic@upbound.io>
This commit is contained in:
Predrag Knezevic 2024-04-19 15:47:18 +02:00
parent b5462b512d
commit d049fcc2e9
7 changed files with 76 additions and 64 deletions

View File

@ -85,7 +85,7 @@ type Condition struct {
}
// Equal returns true if the condition is identical to the supplied condition,
// ignoring the LastTransitionTime.
// ignoring the LastTransitionTime and ObservedGeneration.
func (c Condition) Equal(other Condition) bool {
return c.Type == other.Type &&
c.Status == other.Status &&

View File

@ -223,7 +223,7 @@ func (c *Unstructured) SetPublishConnectionDetailsTo(ref *xpv1.PublishConnection
// GetCondition of this composite resource claim.
func (c *Unstructured) GetCondition(ct xpv1.ConditionType) xpv1.Condition {
conditioned := xpv1.ResourceStatus{}
conditioned := xpv1.ConditionedStatus{}
// The path is directly `status` because conditions are inline.
if err := fieldpath.Pave(c.Object).GetValueInto("status", &conditioned); err != nil {
return xpv1.Condition{}
@ -233,7 +233,7 @@ func (c *Unstructured) GetCondition(ct xpv1.ConditionType) xpv1.Condition {
// SetConditions of this composite resource claim.
func (c *Unstructured) SetConditions(conditions ...xpv1.Condition) {
conditioned := xpv1.ResourceStatus{}
conditioned := xpv1.ConditionedStatus{}
// The path is directly `status` because conditions are inline.
_ = fieldpath.Pave(c.Object).GetValueInto("status", &conditioned)
conditioned.SetConditions(conditions...)
@ -256,15 +256,15 @@ func (c *Unstructured) SetConnectionDetailsLastPublishedTime(t *metav1.Time) {
// SetObservedGeneration of this composite resource claim.
func (c *Unstructured) SetObservedGeneration(generation int64) {
status := &xpv1.ResourceStatus{}
status := &xpv1.ObservedStatus{}
_ = fieldpath.Pave(c.Object).GetValueInto("status", status)
status.SetObservedGeneration(generation)
_ = fieldpath.Pave(c.Object).SetValue("status", status)
_ = fieldpath.Pave(c.Object).SetValue("status.observedGeneration", status.ObservedGeneration)
}
// GetObservedGeneration of this composite resource claim.
func (c *Unstructured) GetObservedGeneration() int64 {
status := &xpv1.ResourceStatus{}
status := &xpv1.ObservedStatus{}
_ = fieldpath.Pave(c.Object).GetValueInto("status", status)
return status.GetObservedGeneration()
}

View File

@ -370,22 +370,26 @@ func TestConnectionDetailsLastPublishedTime(t *testing.T) {
}
func TestObservedGeneration(t *testing.T) {
u := &Unstructured{unstructured.Unstructured{Object: map[string]any{}}}
want := int64(123)
u.SetObservedGeneration(want)
if got := u.GetObservedGeneration(); got != want {
t.Errorf("u.GetObservedGeneration() got: %v, want %v", got, want)
cases := map[string]struct {
u *Unstructured
want int64
}{
"Set": {
u: New(func(u *Unstructured) {
u.SetObservedGeneration(123)
}),
want: 123,
},
"NotFound": {
u: New(),
},
}
if g := u.GetUnstructured().Object["status"].(map[string]any)["observedGeneration"]; g != want {
t.Errorf("Generations do not match! got: %v (%T)", g, g)
}
}
func TestObservedGenerationNotFound(t *testing.T) {
u := &Unstructured{unstructured.Unstructured{Object: map[string]any{}}}
if g := u.GetObservedGeneration(); g != 0 {
t.Errorf("u.GetObservedGeneration(): expected nil, but got %v", g)
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
got := tc.u.GetObservedGeneration()
if diff := cmp.Diff(tc.want, got); diff != "" {
t.Errorf("\nu.GetObservedGeneration(): -want, +got:\n%s", diff)
}
})
}
}

View File

@ -72,7 +72,7 @@ func (cr *Unstructured) GetUnstructured() *unstructured.Unstructured {
// GetCondition of this Composed resource.
func (cr *Unstructured) GetCondition(ct xpv1.ConditionType) xpv1.Condition {
conditioned := xpv1.ResourceStatus{}
conditioned := xpv1.ConditionedStatus{}
// The path is directly `status` because conditions are inline.
if err := fieldpath.Pave(cr.Object).GetValueInto("status", &conditioned); err != nil {
return xpv1.Condition{}
@ -82,7 +82,7 @@ func (cr *Unstructured) GetCondition(ct xpv1.ConditionType) xpv1.Condition {
// SetConditions of this Composed resource.
func (cr *Unstructured) SetConditions(c ...xpv1.Condition) {
conditioned := xpv1.ResourceStatus{}
conditioned := xpv1.ConditionedStatus{}
// The path is directly `status` because conditions are inline.
_ = fieldpath.Pave(cr.Object).GetValueInto("status", &conditioned)
conditioned.SetConditions(c...)
@ -171,15 +171,15 @@ func (cr *UnstructuredList) GetUnstructuredList() *unstructured.UnstructuredList
// SetObservedGeneration of this composite resource claim.
func (cr *Unstructured) SetObservedGeneration(generation int64) {
status := &xpv1.ResourceStatus{}
status := &xpv1.ObservedStatus{}
_ = fieldpath.Pave(cr.Object).GetValueInto("status", status)
status.SetObservedGeneration(generation)
_ = fieldpath.Pave(cr.Object).SetValue("status", status)
_ = fieldpath.Pave(cr.Object).SetValue("status.observedGeneration", status.ObservedGeneration)
}
// GetObservedGeneration of this composite resource claim.
func (cr *Unstructured) GetObservedGeneration() int64 {
status := &xpv1.ResourceStatus{}
status := &xpv1.ObservedStatus{}
_ = fieldpath.Pave(cr.Object).GetValueInto("status", status)
return status.GetObservedGeneration()
}

View File

@ -148,22 +148,26 @@ func TestWriteConnectionSecretToReference(t *testing.T) {
}
func TestObservedGeneration(t *testing.T) {
u := &Unstructured{unstructured.Unstructured{Object: map[string]any{}}}
want := int64(123)
u.SetObservedGeneration(want)
if got := u.GetObservedGeneration(); got != want {
t.Errorf("u.GetObservedGeneration() got: %v, want %v", got, want)
cases := map[string]struct {
u *Unstructured
want int64
}{
"Set": {
u: New(func(u *Unstructured) {
u.SetObservedGeneration(123)
}),
want: 123,
},
"NotFound": {
u: New(),
},
}
if g := u.GetUnstructured().Object["status"].(map[string]any)["observedGeneration"]; g != want {
t.Errorf("Generations do not match! got: %v (%T)", g, g)
}
}
func TestObservedGenerationNotFound(t *testing.T) {
u := &Unstructured{unstructured.Unstructured{Object: map[string]any{}}}
if g := u.GetObservedGeneration(); g != 0 {
t.Errorf("u.GetObservedGeneration(): expected nil, but got %v", g)
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
got := tc.u.GetObservedGeneration()
if diff := cmp.Diff(tc.want, got); diff != "" {
t.Errorf("\nu.GetObservedGeneration(): -want, +got:\n%s", diff)
}
})
}
}

View File

@ -206,7 +206,7 @@ func (c *Unstructured) SetPublishConnectionDetailsTo(ref *xpv1.PublishConnection
// GetCondition of this Composite resource.
func (c *Unstructured) GetCondition(ct xpv1.ConditionType) xpv1.Condition {
conditioned := xpv1.ResourceStatus{}
conditioned := xpv1.ConditionedStatus{}
// The path is directly `status` because conditions are inline.
if err := fieldpath.Pave(c.Object).GetValueInto("status", &conditioned); err != nil {
return xpv1.Condition{}
@ -216,7 +216,7 @@ func (c *Unstructured) GetCondition(ct xpv1.ConditionType) xpv1.Condition {
// SetConditions of this Composite resource.
func (c *Unstructured) SetConditions(conditions ...xpv1.Condition) {
conditioned := xpv1.ResourceStatus{}
conditioned := xpv1.ConditionedStatus{}
// The path is directly `status` because conditions are inline.
_ = fieldpath.Pave(c.Object).GetValueInto("status", &conditioned)
conditioned.SetConditions(conditions...)
@ -261,15 +261,15 @@ func (c *Unstructured) SetEnvironmentConfigReferences(refs []corev1.ObjectRefere
// SetObservedGeneration of this composite resource claim.
func (c *Unstructured) SetObservedGeneration(generation int64) {
status := &xpv1.ResourceStatus{}
status := &xpv1.ObservedStatus{}
_ = fieldpath.Pave(c.Object).GetValueInto("status", status)
status.SetObservedGeneration(generation)
_ = fieldpath.Pave(c.Object).SetValue("status", status)
_ = fieldpath.Pave(c.Object).SetValue("status.observedGeneration", status.ObservedGeneration)
}
// GetObservedGeneration of this composite resource claim.
func (c *Unstructured) GetObservedGeneration() int64 {
status := &xpv1.ResourceStatus{}
status := &xpv1.ObservedStatus{}
_ = fieldpath.Pave(c.Object).GetValueInto("status", status)
return status.GetObservedGeneration()
}

View File

@ -358,22 +358,26 @@ func TestConnectionDetailsLastPublishedTime(t *testing.T) {
}
func TestObservedGeneration(t *testing.T) {
u := &Unstructured{unstructured.Unstructured{Object: map[string]any{}}}
want := int64(123)
u.SetObservedGeneration(want)
if got := u.GetObservedGeneration(); got != want {
t.Errorf("u.GetObservedGeneration() got: %v, want %v", got, want)
cases := map[string]struct {
u *Unstructured
want int64
}{
"Set": {
u: New(func(u *Unstructured) {
u.SetObservedGeneration(123)
}),
want: 123,
},
"NotFound": {
u: New(),
},
}
if g := u.GetUnstructured().Object["status"].(map[string]any)["observedGeneration"]; g != want {
t.Errorf("Generations do not match! got: %v (%T)", g, g)
}
}
func TestObservedGenerationNotFound(t *testing.T) {
u := &Unstructured{unstructured.Unstructured{Object: map[string]any{}}}
if g := u.GetObservedGeneration(); g != 0 {
t.Errorf("u.GetObservedGeneration(): expected nil, but got %v", g)
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
got := tc.u.GetObservedGeneration()
if diff := cmp.Diff(tc.want, got); diff != "" {
t.Errorf("\nu.GetObservedGeneration(): -want, +got:\n%s", diff)
}
})
}
}