reverse listing of changes #163
Conversation
the new feature uses a dataset change iterator. The iterator pattern moves flow control out of the dataset and storage layer. The plan is to move more internals over to "service" implementations gradually.
| } | ||
| type Iterator interface { | ||
| io.Closer | ||
| Inverse() Iterator |
There was a problem hiding this comment.
Not sure of this should be a property on the Iterator instance instead of it's own method. Or even a ReverseIterator implementation. So by calling a factory method table.GetIterator(reverse:true/false) would then just return the correct Iterator (a Next on a ReverseIterator would then go backwards)
| return nil | ||
| } | ||
|
|
||
| func (b *BadgerDatasetIterator) Inverse() Iterator { |
There was a problem hiding this comment.
Oh, I understand it, you use the call to Inverse() to switch the order. Still not sure that's the best way.
| "github.com/mimiro-io/datahub/internal/server" | ||
| ) | ||
|
|
||
| func SeekChanges(datasetID uint32, since uint64) []byte { |
There was a problem hiding this comment.
Do these need to be public methods?Also seem to be more Builders than Seek methods, so not sure about naming.
| @@ -250,6 +252,10 @@ func (handler *datasetHandler) getEntitiesHandler(c echo.Context) error { | |||
| } | |||
|
|
|||
| f := c.QueryParam("from") | |||
| reverse := c.QueryParam("reverse") | |||
| if reverse != "" { | |||
| return echo.NewHTTPError(http.StatusBadRequest, fmt.Errorf("reverse parameter only supported for changes")) | |||
There was a problem hiding this comment.
For Echo, you want to return text, not the err, it has to do with the pathing of the return values in Echo, a return echo.NewHttpError(500, err.Error()) returns the Echo error object, while the echo.NewHttpError(500, err) does not.
| }) | ||
| if err != nil { | ||
| return echo.NewHTTPError(http.StatusInternalServerError, err.Error()) | ||
| if reverse { |
There was a problem hiding this comment.
This is not pretty, and should be changed in the future, but I understand why it has to be like this (for now).
This change introduces the ability to list dataset changes in reverse order.
closes issue #161. only partially (changes only, not entities)