search

ssbc / patchcore

A shared library of depject modules to build Secure Scuttlebutt social network apps
GNU Affero General Public License v3.0
74 stars 17 forks source link

Make filtered backstream observable abortable #67

Closed Happy0 closed 6 years ago

Happy0 commented 6 years ago

The background to this is that I realised the live pull-streams for chess games were not being aborted. This means that every time a chess game, or the miniboards for the chess games are viewed, there is a pull stream left dangling (which is a resource link.) This can add up to hundreds quite quickly as you flick between your games and game tabs.

I followed the example of patchcore/backlinks/obs.js to use a cache for the observables and abort the underlying pull stream if nobody subscribes to the observable for the backlinks for a given amount of time.

Note: I created obs-cache.js to avoid code duplication from obs.js in obs-filter.js, but I haven't refactored obs.js to use it yet.

I was kind of nervous about touching it as it's very core to patchwork / patchbay and I wanted to check people were happy for me to refactor it. I know that the obs.filter file is only used by ssb-chess on the other hand (since I added it to patchcore for ssb-chess.)

If people are happy for me to do so I can update this pull request with obs.js refactored to use the cache or we can do it as a follow up request once my changes have been seen to be stable in ssb-chess for a while.

arj03 commented 6 years ago

Well if you added it in the first place and no-one else is using it, then go for it I'd say. One thing that would be great is if you can help others track down similar bugs.

Happy0 commented 6 years ago

Cheers @arj03 =]. I'll give matt and mix a couple of days to raise objections and then just merge it if there are none.

Happy0 commented 6 years ago

While I was in the shower, I just remembered @mmckegg pointed out to me last year that my backlinks.obs.filter module might be used with different filters for the same thread ID, so we couldn't use the same caching strategy as in obs.js. I've just updated this PR to take this into account by making the cache optional and passed in by the caller. Not all use cases will necessarily want or need a cache.