Feature Request: Add by Type, GetByType and GetManyByType

[ghstahl]

First off, nice library. I was able to add these features and not have to rearchitect anything in the original library. My version is located here

If you accept these features, I can do a proper fork and PR so you can see how I did it.

New Features

The following are non breaking changes to sarulabs/di.

SafeGetByType

// SafeGetByType retrieves the last object added from the Container.
// The object has to belong to this scope or a more generic one.
// If the object does not already exist, it is created and saved in the Container.
// If the object can not be created, it returns an error.
SafeGetByType(rt reflect.Type) (interface{}, error)

GetByType

// GetByType is similar to SafeGetByType but it does not return the error.
// Instead it panics.
GetByType(rt reflect.Type) interface{}

SafeGetManyByType

// SafeGetManyByType retrieves an array of objects from the Container.
// The objects have to belong to this scope or a more generic one.
// If the objects do not already exist, it is created and saved in the Container.
// If the objects can not be created, it returns an error.
SafeGetManyByType(rt reflect.Type) ([]interface{}, error)

GetManyByType

// GetManyByType is similar to SafeGetManyByType but it does not return the error.
// Instead it panics.
GetManyByType(rt reflect.Type) []interface{}

Just like dotnetcore, I would like to register a bunch of services that all implement the same interface. I would like to get back an array of all registered objects, and to do that I need to ask for them by type

For efficiency, type validation is done during registration. After that it should just be lookups.
When you do a GetByType, don't keep calling reflect to get a type of what is a compliled type. do it once and put it in a map.

Registration

type DEF struct

// Def contains information to build and close an object inside a Container.
type Def struct {
	Build            func(ctn Container) (interface{}, error)
	Close            func(obj interface{}) error
	Name             string //[ignored] if Type is used this is overriden and hidden.
	Scope            string
	Tags             []Tag
	Type             reflect.Type //[optional] only if you want to claim that this object also implements these types.
	ImplementedTypes TypeSet
	Unshared         bool
}

Two new fields where added to the Def struct.

	Type             reflect.Type
	ImplementedTypes TypeSet

Type: is the type of the object being registered.
ImplementedTypes: is the types that this object either is or implements

Only the Type option is needed to register and it is automatically added as an implemented type. The ImplementedTypes option is primarily for claiming that the added type supports a given set of interfaces. If this option is added, the original Type is automatically added to the ImplementedTypes set.

The following code will return an error if the added type DOES NOT implemented the ImplementedTypes that were claimed.

type ISomething interface {
	GetName() string
	SetName(name string)
}

type Service struct {
	name            string
}
// SetName ...
func (s *Service) SetName(in string) {
	s.name = in
}

// SetName ...
func (s *Service) GetName() string {
	return s.name
}

func AddTransientService(builder *di.Builder) {
	log.Info().Msg("IoC: AddTransientService")

	types := di.NewTypeSet()
	inter := di.GetInterfaceReflectType((*exampleServices.ISomething)(nil))
	types.Add(inter)
	
	builder.Add(di.Def{
		Name:             "[overidden and hidded if added by Type, can be empty]",
		Scope:            di.App,
		ImplementedTypes: types, //[optional] only if you want to claim that this object also implements these types.
		Type:             reflect.TypeOf(&Service{}),
		Unshared: true,
		Build: func(ctn di.Container) (interface{}, error) {
			service := &Service{
			}
			return service, nil
		},
	})
}

Retrieval

// please put this into a lookup map
inter := di.GetInterfaceReflectType((*exampleServices.ISomething)(nil))

dd := ctn.GetManyByType(inter)
for _, d := range dd {
  ds := d.(exampleServices.ISomething)
  ds.SetName("rabbit")
  log.Info().Msg(ds.GetName())
}

[sarulabs]

Hello, I haven't fully checked your solution but here my first impressions.

You added Type and ImplementedTypes to the definition. I think they are misleading because they can let people think that the library will check if the created object matches these two fields. But internally no check if done and they are just here to identify the definitions (there is a check to see if ImplementedTypes matches Type, but if Type is wrong it is not helpful).

In the current version of the library, there are mainly two fields with that purpose: Name and Tags. Tags were removed in v2 but reintroduced because of issue #5. I removed them at the time because I've never used them in my projects. So their design is probably lacking but I think a solution could be achieved by using the tags and without changing the containers.

type TagFilterer struct{/* to be determined */}

// just after building the container
t := &TagFilterer{}
t.registerDefs(ctn.Definitions())

// later when you want to use the objects
names := t.GetDefNamesForType(instanceWithWantedType)

for _, name := range names {
    obj := ctn.Get(name).(instanceWithWantedType)
}

The GetMany functions would not be required. A GetDefNameForType (singular) would also be possible.

The difficulties of this solution would be to define the role of the TagFilterer. Should it be restricted to types or can it be something more generic? Also the current version of the tags is probably not good enough to implement this. So we would need to find a way to improve them without breaking compatibility.

Another thing with your solution is that the name is not required if you provide a type. I think that if the tag system is improved in such a way that it is easy to find a definition without knowing its name, changing the behavior of the library to generate a random name (like _di_internal_autogenerated_name_1) when the name is empty would not be a bad idea. I don't think the change would impact anyone.

Anyway, thank you for trying to improve the library.

[ghstahl]

I do upfront validation of Type and ImplementedTypes during the Add(def) using reflection. This will return an error if wrong.

I think no matter what the final library path takes an upfront validation like this is needed.

Absolutely your suggestions will work, and I would like to help get it there.

As for the GetManyByType and GetByType: The important thing here is that the LAST item of a type added is what gets returned by GetByType. So, something like GetDefNameForType needs to return the correct one (the last one added). I am mapping this on how aspdotnet core DI works.

In one of my examples, I add early in the flow an NilEncrypter that implements IEncrypter, so that a downstream object that depends on it has at least something. Later in the main() I add AesEncrypter that implements IEncrypter, so now I have 2, but the downstream objects are asking for one, which now gives them the AesEncrypter implementation of IEncrypter.

I am finding that now that I have type, I use it for everything when retrieving the objects. So, for me, everything is by type and hence TagFilterer would just support type.

BTW: I don't use reflection after the Build happens because the Build produces reflect.Type lookup maps, so it should be as fast as what you have with the name lookups.

[sarulabs]

I thought about it again. It still think tags are the way to go and I don't want to make a lot of changes to the library.

To make tag more powerful, a new field Data would be added.

type Tag struct {
	Name string
	Args map[string]string
	Data interface{}
}

In addition to that change definitions would:

• not require a name (if empty, a name would be generated)
• have a new private field containing the order in which they were added with the builder

DefMap would have a new method to return the definitions in the order they were added with the builder.

That would be the only changes to the library. All the changes you implemented would have to be in another repository working as an extension of the main library.

That would be used like this:

typeManager := &ditypes.Manager{}

builder, _ := di.NewBuilder()

builder.Add(di.Def{
    Build: func(ctn di.Container) (interface{}, error) {
        return &MyObject{}, nil
    },
    Tags: []Tag{
        {
            Data: ditypes.TagData{
                Type:       reflect.TypeOf(&MyObject{}),
                Implements: []reflect.Type{},
            },
        },
    },
})

builder.set("typeManager", typeManager)

ctn := builder.Build()

orderedDefs := di.DefMap(ctn.Definitions()).SortedDefs()

err := typeManager.Register(orderedDefs)

// then in the request for example

typeManager := di.Get(r, "typeManager").(*ditypes.Manager)

di.Get(r, typeManager.FirstName(ditypes.Opts{
	Is: reflect.TypeOf(&MyObject{})
}))

In the documentation there would be a new part about tags (they are not documented yet) linking to the type extension.

This is more or less what I suggested last time, but the typeManager would not be generic and would be in an other repository. Do you think this design is acceptable? And do you think your code could be refactored easily into this Manager (name up to suggestions)?

[chapterjason]

Just the support for tags and a func to get a slice of all services which has the tag would be enough for me.

[sarulabs]

Hello, I rewrote the entire library 4 months ago, but I have been too lazy to write the new README.
https://github.com/sarulabs/di/blob/speed_improvements/containerGetter.go#L14
On this branch (speed_improvements) it is possible to retrieve an object from its type (the type must be specified in the definition). You can also retrieve an object with its definition, which is faster (the new code is a lot faster).
If you have more than one definition for a given type, a method DefinitionsForType will let you retrieve the definitions. Then you can call Get for each definition.
I'll try to work on the README soon. If you need the feature quickly, you can use the speed_improvements in the meantime as I don't plan to change the code. But you will have to read the documentation in the code to understand what to do.