Repository navigation
Manage templates, categories and comments over AtomPub - #209
Conversation
Add three AtomPub collections to each weblog's workspace:
- Templates (/{handle}/templates): create, read, update and delete a
custom theme's templates, with the standard rendition as atom:content
and the mobile rendition as roller:rendition. Weblog administrators
only, and only while the weblog uses a custom theme and the site
allows custom themes. An empty mobile rendition removes it.
- Categories (/{handle}/categories): create, read, update and delete.
A category in use is removed only with ?moveTo={id}. The entries
collection lists categories inline and also references a new RFC 5023
category document at /{handle}/categories.atomcat.
- Comments (/{handle}/comments): moderation only. List (filter by status
or entry), change roller:status, delete. thr:in-reply-to (RFC 4685)
links to the weblog entry.
Move the template rules from the Templates and TemplateEdit pages into
WeblogTemplateEditor so the UI and AtomPub enforce the same rules, and
add WeblogManager.removeTemplateRendition() for mobile renditions.
snoopdave
left a comment
snoopdave
left a comment
There was a problem hiding this comment.
PR-Review: 3 inline comments posted.
| if (target == null || target.getId().equals(category.getId())) { | ||
| throw CollectionSupport.badRequest("moveTo must name another category of this weblog"); | ||
| } | ||
| mgr.moveWeblogCategoryContents(category, target); |
There was a problem hiding this comment.
🐞Claude Issue: Blocking: Deleting the weblog's default (Blogger API) category with moveTo breaks the categories collection. removeWeblogCategory() sets weblog.bloggerCategory to null, and JPAWeblogEntryManagerImpl.isWeblogCategoryInUse() then calls cat.getWeblog().getBloggerCategory().equals(cat), which throws a NullPointerException. Every later GET of /categories or /category/{id} (createAtomEntry calls isWeblogCategoryInUse) then returns 500. Fix: when category is the weblog's blogger category, set weblog.setBloggerCategory(target) and save the weblog before removing it. A test that deletes the default category with moveTo and then lists the collection would catch this.
There was a problem hiding this comment.
🤖Claude: not fixed as described — the finding was wrong. moveWeblogCategoryContents() already makes the target the default (Blogger API) category when the source was the default, so the default is never null after a moveTo delete. Added defaultCategoryRemovedWithMoveToHandsOverTheDefault, which deletes the default category with moveTo, checks the new default, and lists the collection. The NPE can still happen in a weblog that has no default category, so isWeblogCategoryInUse() now compares null-safely (cat.equals(weblog.getBloggerCategory())).
| "Category is in use; name a category to move its entries to with moveTo"); | ||
| } | ||
| CacheManager.invalidate(category); | ||
| mgr.removeWeblogCategory(category); |
There was a problem hiding this comment.
🐞Claude Issue: Important: Nothing stops a client from deleting the weblog's last category. The Categories page hides the delete link when only one category is left (AllCategories.size() > 1 in Categories.jsp), but AtomPub does not. With no categories, saveWeblogEntry() falls back to getWeblogCategories().iterator().next(), which throws NoSuchElementException, so new entries fail from both the UI and AtomPub. Refuse the delete with 409 when it is the only category.
There was a problem hiding this comment.
🤖Claude: fixed by refusing DELETE with 409 "A weblog must keep at least one category" when the category is the weblog's only one, matching the Categories page. Test: lastCategoryCannotBeRemoved.
| response.setContentType(AtomConstants.SERVICE_MEDIA_TYPE); | ||
| new AtomWriter().writeServiceDoc(response.getOutputStream(), service); | ||
|
|
||
| } else if (handler.isCategoriesDocURI(areq)) { |
There was a problem hiding this comment.
🐞Claude Issue: Important: The new category-document branch has no servlet test. RollerAtomServletTest does not cover GET of /{handle}/categories.atomcat, so nothing checks that it is routed before the collection branch and served as application/atomcat+xml. Add a case with the mocked handler returning an AtomCategories, and verify the content type and the app:categories root element.
There was a problem hiding this comment.
🤖Claude: fixed by adding categoryDocumentIsServedAsAtomcat to RollerAtomServletTest: GET /blog/categories.atomcat is served with application/atomcat+xml;charset=utf-8, writes an app:categories document with the handler's categories, and never reaches getCollection.
- Refuse to delete a weblog's last category (409), as the Categories page does. - Make isWeblogCategoryInUse() null-safe for weblogs without a default category. - Test that deleting the default category with moveTo hands the default over, and that the servlet serves the category document.
- roller:image must be an http or https URL, as on the Categories page, which now uses the same check. The category wrapper gives templates only such URLs. - Category names and descriptions, and template names, links and descriptions, are limited to 255 characters, as in the editors. - Test that AtomPub tags use the shared tag character rules.
* Add an AtomPub reference guide docs/roller-atompub-guide.adoc documents Roller's whole AtomPub interface: enabling and limits, BASIC and OAuth authentication, the service document, and the entries, media, category, template and comment resources (URIs, permissions, elements, status codes), with a curl session and an error table. It is built to PDF and shipped with the other guides. - docs/atompub/examples: XML examples included by the guide. Response examples are generated from AtomWriter by AtomPubDocExamplesTest, which fails when they no longer match (-Datompub.docs.update=true rewrites them). Request examples are hand-written and must parse. - docs/atompub/roller-atompub.rnc: RELAX NG grammar for the roller:* and thr:in-reply-to elements; the test validates every example against it and against the RFC 4287/5023 grammars. - Move the Jing helper into RelaxNgValidator for both schema tests. * Document the category image and length rules in the AtomPub guide
#209 (templates, categories and comments over AtomPub) and #211 (AtomPub reference guide) were merged into their stacked base branches after #210 was squash-merged into master, so their changes never reached master. This commit applies them: the tree is that of replace-propono-atompub at 95d2419, which is master plus #209 and #211.
1 participant
Stacked on #161 (base:
replace-propono-atompub). Review only the commit on top of it.Adds three collections to each weblog's AtomPub workspace. The service document lists a collection only when the user may use it.
/{handle}/templates,/template/{id}/{handle}/categories,/category/{id}?moveTo={id}for a category in use)/{handle}/categories.atomcat/{handle}/comments,/comment/{id}?status=,?entry=), PUT (status only), DELETEatom:contentis the standard rendition androller:rendition type="mobile"the mobile one. An empty mobile rendition removes it (newWeblogManager.removeTemplateRendition()). The template rules move from the Templates/TemplateEdit pages intoWeblogTemplateEditor, so the UI and AtomPub share them.thr:in-reply-to(RFC 4685) links to the entry.http://roller.apache.org/ns/atompub/1.0. The user guide documents the URIs and elements.Tests: integration tests on Derby for all three collections (permissions, custom-theme rule, cross-weblog ids, conflicts),
WeblogTemplateEditorTest, schema validation of the new formats against the RFC grammars, and handler routing.mvn -pl app test: 524 tests, 0 failures. Not yet tested over the wire against a running server.