search

ponzu-cms / ponzu

Headless CMS with automatic JSON API. Featuring auto-HTTPS from Let's Encrypt, HTTP/2 Server Push, and flexible server framework written in Go.
https://docs.ponzu-cms.org
BSD 3-Clause "New" or "Revised" License
5.68k stars 387 forks source link

Database changes - BoltDB is archived; allow db.Save(); return ID on hooks #270

Closed nanohard closed 4 years ago

nanohard commented 6 years ago

BoltDB is now archived and unmaintained. The author suggests looking at bbolt. It seems to be a drop-in replacement:

bbolt includes bug fixes, performance enhancements, and features not found in Bolt while preserving backwards compatibility with the Bolt API.

Now, onto moar. Storm is a wrapper for bbolt (they switched from BoltDB when it got archived), which would allow for db.Save() and some other neat wrappers, which is something that is sorely needed. I had to implement some hackery to get around not being able to update a db entry on a hook; we need to allow for this.

This leads me to hooks in Ponzu; for all afterXyzCreate() interfaces we should be returning the ID of the entry.

Please leave your thoughts and candy... or just candy.

nilslice commented 6 years ago

🍬

💭

BoltDB is now archived and unmaintained.

True. Even though it has been this way for quite some time, the bolt API and codebase remains stable as far as I can tell. So, I'm not worried yet about swapping these dependencies, but would like to further investigate how bbolt could bring additional value to Ponzu. Glad you have looked into this, and I appreciate the call-out.

There should definitely be more emphasis put on the db package for Ponzu, and adding some tests around the boltdb usage is probably something that should be done soon.

[...] This leads me to hooks in Ponzu; for all afterXyzCreate() interfaces we should be returning the ID of the entry.

That's a great idea. I think it would be a useful addition to these hooks: 

AfterAPICreate(http.ResponseWriter, *http.Request) error
AfterAdminCreate(http.ResponseWriter, *http.Request) error
AfterApprove(http.ResponseWriter, *http.Request) error
AfterEnable(http.ResponseWriter, *http.Request) error // I'll have to think about this one a bit more, since the addon API is likely to change a bit

Any other hooks I miss? Let me know if you think so.

Re: moar. I'd like to avoid introducing the dependency on Storm. Not that it isn't a useful tool, but keeping the core database code as low-level and close to the stdlib as possible is a goal. I'm sure there would be things that could be done easier using a wrapper for bolt's API, but I think it is still important to have as few dependencies between the Go stdlib and Ponzu's database layer. I could be convinced otherwise though, as I have not tried Storm or other wrappers for Ponzu. If you feel compelled to, feel free to try it out though and share your code/thoughts.

nanohard commented 6 years ago

Hooks system/item/item.go These hooks should be returning the ID, because the ID is available. I can think of several situations where you'd want to manipulate something based on what gets returned. If the user wants the whole object, they would then have the ID to query. I am already returning the ID for AfterAdminCreate in my customized Ponzu:

AfterAPICreate()

AfterAdminCreate()

BeforeAPIUpdate()
AfterAPIUpdate()

BeforeAdminUpdate()
AfterAdminUpdate()

BeforeSave()  // if applicable
AfterSave()

BeforeApprove()  // if applicable
AfterApprove()

BeforeReject()  // if applicable
AfterReject()  // if applicable

BeforeDelete()

Storm My thought process for switching to Storm. TL;DR - less work, lower barrier of entry:

I don't mind creating an alternate version of Ponzu that implements Storm to try out and report on. I would keep it private so as to not cause any confusion. But this is a big alteration and would take some time, especially the testing/timing comparison as I'm unfamiliar with test driven development (this would force me to use tests).

nilslice commented 6 years ago

I'll take a closer look at Storm, but re: the hooks -- I believe that all, aside for the ones I mentioned, have the ID available on the pointer receiver already. Being lifecycle hooks, they are called on objects mid-request, whom have already been hydrated by data from the DB.

I could be wrong about some, but we could easily double-check.. Unless I'm being a jerk and you have already performed these tests, and determined that they do not have the ID available 😆

For example, in the case of the following:

type MyContent struct {
    item.Item

    Name string `json:"string"`
    // ...
}

func (m *MyContent) BeforeAPIUpdate(http.ResponseWriter, *http.Request) error {
    fmt.Println(m.ID) // would print the actual ID, since it was set
}

The setting of the fields is done above the assertion to the item.Hookable interface and use of the hook method here: https://github.com/ponzu-cms/ponzu/blob/e43cda203b2b3eadab6bea1891e83fe232555fb5/system/api/update.go#L61-L171

nanohard commented 6 years ago

I'll have to set up some tests. I may have only tried that on AfterAdminCreate, but I'm also using AfterAdminUpdate and I can't remember if the latter had an accessible ID. I'll post the tests here when I have them, might be a few weeks as I'd like to finish the HTTPS conversion first.

ghost commented 5 years ago

what about https://github.com/timshannon/bolthold ?

Its wraps bbolt but still gives full access to bolt.

One thing we really need is a way for Bolt to be clustered for HA purposes. There are a few Bolt Raft implementations out there.

olliephillips commented 4 years ago

Going to close this issue. Reading through, my own thoughts are that BoltDB is sufficient for the Ponzu CMS project at this time. Please re-open if needs be.