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

MVC: get the incorrect services in controller #1342

Closed
zheeeng opened this issue Aug 21, 2019 · 12 comments
Closed

MVC: get the incorrect services in controller #1342

zheeeng opened this issue Aug 21, 2019 · 12 comments

Comments

@zheeeng
Copy link

zheeeng commented Aug 21, 2019

package checkpoint

import (
	"fmt"

	"github.com/kataras/iris/mvc"
)

type FortyTwo interface {
}

type fortyTwo struct {
	ft string
}

func InitFortyTwo() FortyTwo {
	return &fortyTwo{
		ft: "42",
	}
}

type TwentyFour interface {
}

type twentyFour struct {
	tf int
}

func InitTwentyFour() TwentyFour {
	return &twentyFour{
		tf: 24,
	}
}

type controller struct {
	TwentyFour TwentyFour
	FortyTwo   FortyTwo
}

func (c *controller) Get() string {
	return fmt.Sprint(c.FortyTwo) + "|" + fmt.Sprint(c.TwentyFour)

}

func App(app *mvc.Application) {
	fortyTwo := InitFortyTwo()
	twentyFour := InitTwentyFour()
	app.Register(fortyTwo, twentyFour)

	app.Handle(new(controller))
}

Try this code and test the output.

The register sequence is: fortyTwo, twentyFour
In the controller, we declare the injected services as the sequence: TwentyFour, FortyTwo
I try to output as the sequence: FortyTwo, TwentyFour --> '&{42}|&{24}'
Actually, the output keeps the same sequence of registering sequence: '&{24}|&{42}'

If change the controller to:

type controller struct {
	FortyTwo   FortyTwo
	TwentyFour TwentyFour
}

We get the right output!

It's very unsafe and brings some unexpected bugs

@kataras
Copy link
Owner

kataras commented Aug 21, 2019

Alright, I will check that when I am back at my office @zheeeng, stay tuned.

@zheeeng
Copy link
Author

zheeeng commented Aug 21, 2019

Thx, if it is confirmed, is it possible to release a quick fix cos my project is just upgraded to the latest version but these undesired behaviors stunned me.

@kataras
Copy link
Owner

kataras commented Aug 22, 2019

Actually there is no framework-error here, it just binds one type to one value, both fields you want to set are actually empty interface{} so they bind to whatever value, so the order matters.

@kataras
Copy link
Owner

kataras commented Aug 22, 2019

There is no way that framework knows what you want to bind if all fields types accept any value. except those two:

  1. Currently it does it by order as it's expected by developer as you already found out by yourself.
  2. By field name (that is missing on and I am implementing an API for that as we speak).

@zheeeng these all are not real-world scenarios though, it looks more like "find an issue on an extreme scenario -- possible user's miss use of the feature" to me.

@zheeeng
Copy link
Author

zheeeng commented Aug 22, 2019

Sadly, this behavior seems to be a burden to developers. Fields are matching by the structure instead of its nominal type.

@zheeeng these all are not real-world scenarios though, it looks more like "find an issue on an extreme scenario -- possible user's miss use of the feature" to me.

This happens on feature refactoring. I deleted all the implementation and kept the empty service shell for future filling.

@kataras
Copy link
Owner

kataras commented Aug 22, 2019

But binding is not wrong, it matches one value per one type by order, how a program can detect what you want to bind if all fields are bindable(compatible types) with the binding value, the only way is order or naming. Do you want to add an API for naming match too?

@kataras
Copy link
Owner

kataras commented Aug 22, 2019

You may name your interface as FortyXXX but it's an empty interface{} in terms of reflection.

@zheeeng
Copy link
Author

zheeeng commented Aug 22, 2019

Sorry, I'm not pro at Golang, I'm just using the experience from Angular annotation DI and TypeScript decorator DI. Cos in JavaScript the instanceof operator provides us the chance to pair instance and class

@kataras
Copy link
Owner

kataras commented Aug 22, 2019

in Go, take for example that you have that interface:

type  Something interface {  }
type  Anything interface{  }

^ They are exactly the same as empty interface{}, they can be anything.

And you have two values that are Something and Anything type:

  • Field1 Something
  • Field2 Anything

And you have a value that you want to bind to, remember those fields can accept a string, int, custom struct anything!

So even if you do something like:

func newService() Something { return &customStruct{} }
func newOtherService() Anything { return &customStruct{} or anything }

Those both can be bind-ed to both fields because they both (Field1 and Field2) expect an interface{}, so any value, so the only way that a framework or library can detect the entry point is by order or field - to bind value naming match.

Did I helped or..?

@zheeeng
Copy link
Author

zheeeng commented Aug 22, 2019

Thx, I figured out your points. You mean that if two services have the same interface methods but with different implementations, the order matters to runtime, am I right?

@kataras
Copy link
Owner

kataras commented Aug 22, 2019

Yes, in Iris only one value can be bind-ed to one field (see ./hero/di/struct.go#MakeStructInjector), so the first match is bind-ed and the value cannot be bind-ed to other field.

In these cases like this issue and the next one you posted, the order is only way that a go program can understand, it's logical if you think of it. However, we can add a register by field name if you wish so, this can fix your issue - (still I recommend not to use empty interfaces without reason)

@zheeeng
Copy link
Author

zheeeng commented Aug 23, 2019

Generally, I don't expect extra APIs are added for hacking or used for instructing the program going. Ideal APIs are all about business instead of the language. Thx for your replies.

@zheeeng zheeeng closed this as completed Aug 23, 2019
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

No branches or pull requests

2 participants