Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow embedding structs for a common columns #351

Closed
duckbrain opened this issue Feb 22, 2019 · 8 comments
Closed

Allow embedding structs for a common columns #351

duckbrain opened this issue Feb 22, 2019 · 8 comments

Comments

@duckbrain
Copy link
Contributor

Description

I've got a project where I'm going to need to have a set of 6 database columns that are on every type.

I thought I'd take care of this through embedding. There is some logic I can apply to them too that will save me, but Go's reflection, and thus pop, handle embedded structs as an anonymous field. It would be great if pop would handle the embedded structs's fields as if they were primary fields.

Pop: v4.9.9, using Buffalo v0.14.0-alpha.4

Please precise your OS, the Pop version and if you're using Pop through Buffalo.

@duckbrain duckbrain changed the title Allow embedding structs for a standard set of fields. Allow embedding structs for a common collumns Feb 22, 2019
@sunho
Copy link

sunho commented Mar 11, 2019

It would be nice to encode them using JSON and store encoded text to character type column.

@karlhaas
Copy link
Contributor

Is there some documentation available how to use model embedding at all?

@sunho
Copy link

sunho commented Mar 20, 2019

I think I read a doc that says I can use golang's sql scanner to support custom types. I am currently doing this:

type SQLStrings []string

func (s SQLStrings) Value() (driver.Value, error) {
	str, err := json.Marshal(s)
	if err != nil {
		return nil, err
	}
	return driver.Value(str), nil
}

func (s *SQLStrings) Scan(i interface{}) error {
	var src []byte
	switch i.(type) {
	case string:
		src = []byte(i.(string))
	case []byte:
		src = i.([]byte)
	default:
		return errors.New("Incompatible type")
	}
	return json.Unmarshal(src, s)
}

But I have to copy & paste the code everywhere since golang doesn't support generics.

@duckbrain duckbrain changed the title Allow embedding structs for a common collumns Allow embedding structs for a common columns Mar 20, 2019
@duckbrain
Copy link
Contributor Author

@sunho You're talking about something different than I am. You're talking about putting JSON into a column. You can do that using the Valuer and Scanner interfaces like you showed, but that only works on a single column. That would also make more sense as a field that has a struct type, rather than an embedded struct. You could easily wrap the logic in a function, so it's a 1-liner to implement on each type you want to be serialized into a column. I don't think that's a responsibility for Pop.

I'm talking about struct embedding. In the Go language, it causes a syntax sugar that makes the function and fields on the embedded structs are accessible on the outer type. Those functions can also be used to implement interfaces. The reason I refer to it as syntax sugar, is because the reflect package does not seem them that way. (Which I'm sure is why Pop dosen't.) It reveals the embedded struct as an anonymous field of the type of the embedded struct, so what Pop sees, is a field of a struct type that it wants to use with column of the inner struct's name.

type Common struct {
    ID uuid.UUID
    DeletedAt nulls.Time
    ....
   OwnerID uuid.UUID
}

type Widgit struct {
    Common
    Name string
}

This is very powerful because I can have the inner struct implement part or all of a common interface between all my models, so they can operate on some of those common fields that almost all tables have.

I actually started using GORM because of this, because it does support this, but I find Pop to be a bit cleaner/idiomatic. so I'd love to be able to switch back.

@sunho
Copy link

sunho commented Mar 20, 2019

How could I not consider wrapping them as functions? Thank you.

@sunho
Copy link

sunho commented Mar 20, 2019

I'm trying to implement the feature because I also think it is useful for my project. Gorm supports embedded structs by flattening the struct into a map with string keys and using it as the input to sql driver. But, in pop, this way would require a complete refactoring of columns package. I wonder if there's easier way to implement this.

@duckbrain
Copy link
Contributor Author

I looked into creating a package that would wrap the reflect.Type interface, but it has some unexported methods. You could still implement the exported methods and have an interface that is the exported methods. You could then have a type that wraps the reflected type and if it's a struct, flatten the fields recursively.

package reflectx

type Type interface {
    // copy/paste exported methods from reflect.Type
}

func FlattenEmbedding(t Type) Type {
    // initialize and return &flattenEmbedding{}
    ...
}

type flattenEmbedding struct {
    inner Type
    fields map[string] reflect.FieldInfo
}

// implement Type interface

It's not as nice as implementing reflect.Type, but it shouldn't be too hard to replace references with reflectx.Type.

Note: Such a package might be useful for adding embedded behavior in other packages, like Plush, which also doesn't allow you to use the short references on embedded structs.

@zepatrik
Copy link
Contributor

I fixed this in #691, sorry for not looking up whether an issue exists already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants