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

Hooks v2 #72

Closed
jiangz222 opened this issue Aug 31, 2020 · 9 comments
Closed

Hooks v2 #72

jiangz222 opened this issue Aug 31, 2020 · 9 comments
Labels
proposal question Further information is requested

Comments

@jiangz222
Copy link
Collaborator

jiangz222 commented Aug 31, 2020

#71 implement the Hooks v1 in qmgo: More about hooks | Hooks详情介绍

But more jobs need to do:

@jiangz222 jiangz222 added question Further information is requested proposal labels Aug 31, 2020
@mirusky
Copy link

mirusky commented Sep 1, 2020

I will try to help, if you can explain to me what you did in hooks v1 will be helpfull

@jiangz222
Copy link
Collaborator Author

I will try to help, if you can explain to me what you did in hooks v1 will be helpfull

Thanks!
You can check pr #71 first, and follow the manual and examples in collection_hook_test.go to dig the codes.
Basically, when we are doing a certain database operation, we create a struct which implements the hook interface, then pass in it by option. Qmgo will call back these struct methods.

@mirusky
Copy link

mirusky commented Sep 3, 2020

Hi thanks for explain the beginning!

Now i'm currently trying to implement a better way to hooks... And I liked this one:

package hook

type Operation int

const (
	Insert Operation = iota
	Update
	Delete
)

type Moment int

const (
	After Moment = iota
	Before
)

// Hook ...
type Hook struct {
	FuncName string // FuncName represent default name to call
	Prior    int    // Prior represent the priority of this hook
}

// Processor ...
type Processor struct {
	Hooks map[Moment]map[Operation]Hook
}

So I'm thinking to create a generic caller to call FuncName in struct ... And then we could use it somethink like:

// InsertOne insert one document into the collection
// Reference: https://docs.mongodb.com/manual/reference/command/insert/
func (c *Collection) InsertOne(ctx context.Context, doc interface{}, opts ...qOpts.InsertOneOptions) (result *InsertOneResult, err error) {
	hook.Call(hook.Processor[After][Insert], &doc)
	res, err := c.collection.InsertOne(ctx, doc)
	if res != nil {
		result = &InsertOneResult{InsertedID: res.InsertedID}
	}
	hook.Call(hook.Processor[Before][Insert], &doc)
	return
}

Here is an example of what i'm calling generic caller, note when the method does not exist we could just ignore or log it

@jiangz222
Copy link
Collaborator Author

Hi thanks for explain the beginning!

Now i'm currently trying to implement a better way to hooks... And I liked this one:

package hook

type Operation int

const (
	Insert Operation = iota
	Update
	Delete
)

type Moment int

const (
	After Moment = iota
	Before
)

// Hook ...
type Hook struct {
	FuncName string // FuncName represent default name to call
	Prior    int    // Prior represent the priority of this hook
}

// Processor ...
type Processor struct {
	Hooks map[Moment]map[Operation]Hook
}

So I'm thinking to create a generic caller to call FuncName in struct ... And then we could use it somethink like:

// InsertOne insert one document into the collection
// Reference: https://docs.mongodb.com/manual/reference/command/insert/
func (c *Collection) InsertOne(ctx context.Context, doc interface{}, opts ...qOpts.InsertOneOptions) (result *InsertOneResult, err error) {
	hook.Call(hook.Processor[After][Insert], &doc)
	res, err := c.collection.InsertOne(ctx, doc)
	if res != nil {
		result = &InsertOneResult{InsertedID: res.InsertedID}
	}
	hook.Call(hook.Processor[Before][Insert], &doc)
	return
}

Here is an example of what i'm calling generic caller, note when the method does not exist we could just ignore or log it

Thanks!
To user, what's the difference in your solution?
please note that: no document struct is passed in in Update operation.

@mirusky
Copy link

mirusky commented Sep 6, 2020

@jiangz222 I believe the way I'm coding, it is more advantageous for the user because he does not need to pass the option in each operation, he could just register hooks before client operations and that's it. All the hard work is for processor and hook call method.

I haven't tested it yet, but I believe we could use a struct in second parameter in Update operation. Or we can refactor to add more flexibility...

@jiangz222
Copy link
Collaborator Author

jiangz222 commented Sep 6, 2020

@mirusky Actually, the current implement of hook is flexible. the real problem is #73.
For example, because insert operation supports struct of document as parameter, we can use insertOne like below after make little changes in InsertOne (works like gorm):

type UserHook struct {
}
func (u *UserHook) BeforeInsert() error {
}
func (u *UserHook) AfterInsert() error {
}


func (c *Collection) InsertOne(ctx context.Context, doc interface{}) (result *InsertOneResult, err error) {
      // use doc directly and hook callback method of struct of doc.(UserType)
	if err = hook.Do(doc, hook.BeforeInsert); err != nil {
		return
	}
	res, err := c.collection.InsertOne(ctx, doc)
	if res != nil {
		result = &InsertOneResult{InsertedID: res.InsertedID}
	}
	if err = hook.Do(doc, hook.AfterInsert); err != nil {
		return
	}

	return
}

Above solution is supported and is the first choice to hook, and I already test it, you can check more here.
But the update operation doesn't support the document struct as parameter yet, so options become the solution in hook v1.

@mirusky
Copy link

mirusky commented Sep 7, 2020

So how to resolve a multiple "BeforeInsert" hook? Like user has custom Methods in struct, so he needs use "BeforeInsert" Method to call all others?

Like:

type UserHook struct {
}
func (u *UserHook) BeforeInsert() error {
  u.MyCustomHook()
  u.MyCustomHook2()
}
func (u *UserHook) AfterInsert() error {
}
func (u *UserHook) MyCustomHook() error {
}
func (u *UserHook) MyCustomHook2() error {
}

@jiangz222
Copy link
Collaborator Author

@mirusky
Yes,I think interface(like BeforeInsert) should be simple, you can do whatever you want in it.

@jiangz222
Copy link
Collaborator Author

Because Qmgo is not a typical ORM, so After #78 supported(hooks of InsertOneInsertMany and UpdateWithDocument can work without options), close this issue at this stage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants