Improved reflection based type conversion for collection types This avoids some panics when the types are not compatible I added tests for On*Collection* with all collection types

Changes

9 changed files (+231/-21)

• collection.go
  • @@ -277,8 +277,10 @@ func ToCollection(it Item) (*Collection, error) {
    default: // NOTE(marius): this is an ugly way of dealing with the interface conversion error: types from different scopes typ := reflect.TypeOf(new(Collection)) - if reflect.TypeOf(it).ConvertibleTo(typ) { - if i, ok := reflect.ValueOf(it).Convert(typ).Interface().(*Collection); ok { + val := reflect.ValueOf(it) + if val.IsValid() && typ.Elem().Name() == val.Type().Elem().Name() { + conv := val.Convert(typ) + if i, ok := conv.Interface().(*Collection); ok { return i, nil } }
• collection_page.go
  • @@ -285,8 +285,10 @@ func ToCollectionPage(it Item) (*CollectionPage, error) {
    default: // NOTE(marius): this is an ugly way of dealing with the interface conversion error: types from different scopes typ := reflect.TypeOf(new(CollectionPage)) - if reflect.TypeOf(it).ConvertibleTo(typ) { - if i, ok := reflect.ValueOf(it).Convert(typ).Interface().(*CollectionPage); ok { + val := reflect.ValueOf(it) + if val.IsValid() && typ.Elem().Name() == val.Type().Elem().Name() { + conv := val.Convert(typ) + if i, ok := conv.Interface().(*CollectionPage); ok { return i, nil } }
• collection_page_test.go
  • @@ -1,6 +1,7 @@
    package activitypub import ( + "fmt" "reflect" "testing" )
  • @@ -137,7 +138,56 @@ func TestCollectionPage_Count(t *testing.T) {
    } func TestToCollectionPage(t *testing.T) { - t.Skipf("TODO") + err := fmt.Errorf("unable to convert to collection page") + tests := map[string]struct { + it Item + want *CollectionPage + wantErr error + }{ + "CollectionPage": { + it: new(CollectionPage), + want: new(CollectionPage), + wantErr: nil, + }, + "OrderedCollectionPage": { + it: new(OrderedCollectionPage), + want: new(CollectionPage), + wantErr: err, + }, + "OrderedCollection": { + it: new(OrderedCollection), + want: new(CollectionPage), + wantErr: err, + }, + "Collection": { + it: new(Collection), + want: new(CollectionPage), + wantErr: err, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got, err := ToCollectionPage(tt.it) + if tt.wantErr != nil && err == nil { + t.Errorf("ToCollectionPage() no error returned, wanted error = [%T]%s", tt.wantErr, tt.wantErr) + return + } + if err != nil { + if tt.wantErr == nil { + t.Errorf("ToCollectionPage() returned unexpected error[%T]%s", err, err) + return + } + if !reflect.DeepEqual(err, tt.wantErr) { + t.Errorf("ToCollectionPage() received error %v, wanted error %v", err, tt.wantErr) + return + } + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ToCollectionPage() got %v, want %v", got, tt.want) + } + }) + } } func TestCollectionPage_Contains(t *testing.T) {
• collection_test.go
  • @@ -1,6 +1,7 @@
    package activitypub import ( + "fmt" "reflect" "testing" )
  • @@ -162,10 +163,6 @@ func TestCollection_Count(t *testing.T) {
    } } - func TestToCollection(t *testing.T) { - t.Skipf("TODO") - } - func TestCollection_Contains(t *testing.T) { t.Skipf("TODO") }
  • @@ -189,3 +186,56 @@ func TestCollection_MarshalJSON(t *testing.T) {
    func TestCollection_ItemMatches(t *testing.T) { t.Skipf("TODO") } + + func TestToCollection(t *testing.T) { + err := fmt.Errorf("unable to convert to collection") + tests := map[string]struct { + it Item + want *Collection + wantErr error + }{ + "Collection": { + it: new(Collection), + want: new(Collection), + wantErr: nil, + }, + "CollectionPage": { + it: new(CollectionPage), + want: new(Collection), + wantErr: nil, + }, + "OrderedCollectionPage": { + it: new(OrderedCollectionPage), + want: new(Collection), + wantErr: err, + }, + "OrderedCollection": { + it: new(OrderedCollection), + want: new(Collection), + wantErr: err, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got, err := ToCollection(tt.it) + if tt.wantErr != nil && err == nil { + t.Errorf("ToCollection() no error returned, wanted error = [%T]%s", tt.wantErr, tt.wantErr) + return + } + if err != nil { + if tt.wantErr == nil { + t.Errorf("ToCollection() returned unexpected error[%T]%s", err, err) + return + } + if !reflect.DeepEqual(err, tt.wantErr) { + t.Errorf("ToCollection() received error %v, wanted error %v", err, tt.wantErr) + return + } + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ToCollection() got = %v, want %v", got, tt.want) + } + }) + } + }
• helpers_test.go
  • @@ -137,10 +137,6 @@ func TestOnCollectionPage(t *testing.T) {
    t.Skipf("TODO") } - func TestOnOrderedCollection(t *testing.T) { - t.Skipf("TODO") - } - func TestOnOrderedCollectionPage(t *testing.T) { t.Skipf("TODO") }
• ordered_collection.go
  • @@ -285,8 +285,10 @@ func ToOrderedCollection(it Item) (*OrderedCollection, error) {
    default: // NOTE(marius): this is an ugly way of dealing with the interface conversion error: types from different scopes typ := reflect.TypeOf(new(OrderedCollection)) - if reflect.TypeOf(it).ConvertibleTo(typ) { - if i, ok := reflect.ValueOf(it).Convert(typ).Interface().(*OrderedCollection); ok { + val := reflect.ValueOf(it) + if val.IsValid() && typ.Elem().Name() == val.Type().Elem().Name() { + conv := val.Convert(typ) + if i, ok := conv.Interface().(*OrderedCollection); ok { return i, nil } }
• ordered_collection_page.go
  • @@ -240,8 +240,12 @@ func ToOrderedCollectionPage(it Item) (*OrderedCollectionPage, error) {
    default: // NOTE(marius): this is an ugly way of dealing with the interface conversion error: types from different scopes typ := reflect.TypeOf(new(OrderedCollectionPage)) - if i, ok := reflect.ValueOf(it).Convert(typ).Interface().(*OrderedCollectionPage); ok { - return i, nil + val := reflect.ValueOf(it) + if val.IsValid() && typ.Elem().Name() == val.Type().Elem().Name() { + conv := val.Convert(typ) + if i, ok := conv.Interface().(*OrderedCollectionPage); ok { + return i, nil + } } } return nil, errors.New("unable to convert to ordered collection page")
• ordered_collection_page_test.go
  • @@ -1,6 +1,7 @@
    package activitypub import ( + "fmt" "reflect" "testing" )
  • @@ -111,10 +112,59 @@ func TestOrderedCollectionPage_Collection(t *testing.T) {
    } } - func TestToOrderedCollectionPage(t *testing.T) { + func TestOrderedCollectionPage_Contains(t *testing.T) { t.Skipf("TODO") } - func TestOrderedCollectionPage_Contains(t *testing.T) { - t.Skipf("TODO") + func TestToOrderedCollectionPage(t *testing.T) { + err := fmt.Errorf("unable to convert to ordered collection page") + tests := map[string]struct { + it Item + want *OrderedCollectionPage + wantErr error + }{ + "OrderedCollectionPage": { + it: new(OrderedCollectionPage), + want: new(OrderedCollectionPage), + wantErr: nil, + }, + "OrderedCollection": { + it: new(OrderedCollection), + want: new(OrderedCollectionPage), + wantErr: err, + }, + "Collection": { + it: new(Collection), + want: new(OrderedCollectionPage), + wantErr: err, + }, + "CollectionPage": { + it: new(CollectionPage), + want: new(OrderedCollectionPage), + wantErr: err, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got, err := ToOrderedCollectionPage(tt.it) + if tt.wantErr != nil && err == nil { + t.Errorf("ToOrderedCollectionPage() no error returned, wanted error = [%T]%s", tt.wantErr, tt.wantErr) + return + } + if err != nil { + if tt.wantErr == nil { + t.Errorf("ToOrderedCollectionPage() returned unexpected error[%T]%s", err, err) + return + } + if !reflect.DeepEqual(err, tt.wantErr) { + t.Errorf("ToOrderedCollectionPage() received error %v, wanted error %v", err, tt.wantErr) + return + } + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ToOrderedCollectionPage() got = %v, want %v", got, tt.want) + } + }) + } }
• ordered_collection_test.go
  • @@ -1,6 +1,7 @@
    package activitypub import ( + "fmt" "reflect" "testing" )
  • @@ -208,10 +209,63 @@ func TestOrderedCollectionPage_Count(t *testing.T) {
    } } - func TestToOrderedCollection(t *testing.T) { + func TestOnOrderedCollection(t *testing.T) { t.Skipf("TODO") } + func TestToOrderedCollection(t *testing.T) { + err := fmt.Errorf("unable to convert to ordered collection") + tests := map[string]struct { + it Item + want *OrderedCollection + wantErr error + }{ + "OrderedCollection": { + it: new(OrderedCollection), + want: new(OrderedCollection), + wantErr: nil, + }, + "OrderedCollectionPage": { + it: new(OrderedCollectionPage), + want: new(OrderedCollection), + wantErr: nil, + }, + "Collection": { + it: new(Collection), + want: new(OrderedCollection), + wantErr: err, + }, + "CollectionPage": { + it: new(CollectionPage), + want: new(OrderedCollection), + wantErr: err, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got, err := ToOrderedCollection(tt.it) + if tt.wantErr != nil && err == nil { + t.Errorf("ToOrderedCollection() no error returned, wanted error = [%T]%s", tt.wantErr, tt.wantErr) + return + } + if err != nil { + if tt.wantErr == nil { + t.Errorf("ToOrderedCollection() returned unexpected error[%T]%s", err, err) + return + } + if !reflect.DeepEqual(err, tt.wantErr) { + t.Errorf("ToOrderedCollection() received error %v, wanted error %v", err, tt.wantErr) + return + } + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ToOrderedCollection() got = %v, want %v", got, tt.want) + } + }) + } + } + func TestOrderedCollection_Contains(t *testing.T) { t.Skipf("TODO") }