uniqush / uniqush-push

Uniqush is a free and open source software system which provides a unified push service for server side notification to apps on mobile devices.
http://uniqush.org
Apache License 2.0
1.53k stars 201 forks source link

Duplicate service in same pushservicetype should not be allowed, results in problem in subscribe and push #197

Closed victorlang closed 5 years ago

victorlang commented 6 years ago

Currently, service is allowed to add multiple psp no matter in same push service type or not. I notice that it tries to allow multiple psp in different type for the same service, unfortunately, sometimes, the user will add a same service in same type with different setting twice. It will result in multiple redundant psp2srv. And during subscription and push, we have no idea which psp will work for pushing. That's a big problem.
As a fix, I tried to validate whether a service already has a psp in same push service type before add a new psp to a existing service. Waiting for code review and commit right to commit this bug fix.

pushdb.go:

... func (f pushDatabaseOpts) AddPushServiceProviderToService(service string, push_service_provider push.PushServiceProvider) error { if push_service_provider == nil { return nil } name := push_service_provider.Name() if len(name) == 0 { return errors.New("InvalidPushServiceProvider") } f.dblock.Lock() defer f.dblock.Unlock()

//UNS EXT
//Before add a new psp to service, try to verify there is no redundant psp in same type exists under the same service.
//Currently, redundant psp to service will result in problem in subscribe and push later.

expsps, err := f.db.GetPushServiceProvidersByService(service)
if err != nil {
    return fmt.Errorf("Error querying psp with name: %v", err)
}
if len(expsps) > 0 {
    for index := range expsps {
        pushpsp, perr := f.db.GetPushServiceProvider(expsps[index])

        if perr != nil {
            return fmt.Errorf("Error to retrieve psp with name: %v", err)
        }

        if pushpsp.PushServiceName() == push_service_provider.PushServiceName() {
            //The service has a psp in same push service type
            return fmt.Errorf("Provider in same type exists under the service %s. new psp detail: %v", service, push_service_provider)
        }
    }

}

e := f.db.SetPushServiceProvider(push_service_provider)
if e != nil {
    return fmt.Errorf("Error associating psp with name: %v", e)
}
return f.db.AddPushServiceProviderToService(service, push_service_provider.Name())

} ......

victorlang commented 6 years ago

Anyone help to review my pull request against this issue #197? Thanks!