Ticket #10738 (reopened enhancement)

Opened 5 months ago

Last modified 5 months ago

XmlRpcPlugin does not respect ITicketManipulators

Reported by: rje@bitstruct.com Assigned to: osimons
Priority: normal Component: XmlRpcPlugin
Severity: normal Keywords:
Cc: olemis Trac Release: 1.0

Description

I have an ITicketManipulator to restrict ticket creation for a specific component but the JSON RPC interface skips past the ITicketManipulator.

Attachments

xmlrpcplugin-mps1.diff (3.9 kB) - added by anonymous on 12/31/12 04:48:24.

Change History

12/30/12 11:13:20 changed by osimons

  • status changed from new to closed.
  • resolution set to worksforme.

No it shouldn't. Not if you use the 'new-style' update that supports workflow, complete with manipulators and listeners.

As the current ticket.update docs say:

Update a ticket, returning the new ticket in the same form as get(). 'New-style' call requires two additional items in attributes: (1) 'action' for workflow support (including any supporting fields as retrieved by getActions()), (2) '_ts' changetime token for detecting update collisions (as received from get() or update() calls). Calling update without 'action' and '_ts' changetime token is deprecated, and will raise errors in a future version.

You can check the TicketRPC.update() method in source:/xmlrpcplugin/trunk/tracrpc/ticket.py to see exactly what steps are taken by update. It calls to Trac to make all necessary validations and checks before saving, including manipulators. There should be tests that shows how this works in source:/xmlrpcplugin/trunk/tracrpc/tests/ticket.py

I'm not aware of breaking changes in Trac 1.0, but please reopen ticket if there is something I've missed.

12/30/12 21:32:14 changed by rje@bitstruct.com

That looks really interesting. Can I use update to create a new ticket? Can I pass update a None id and how would that happen over JSON RPC? Thanks a lot.

12/30/12 21:56:32 changed by osimons

  • status changed from closed to reopened.
  • resolution deleted.

Oh. No, update() cannot create tickets - there is a create() method for that. However, reviewing my logic for create() I see now that manipulators are not in fact part of the call chain. For update() it is handled by Trac, but for create() it is not - nor is it implemented explicitly in RPC code.

We really should call TicketSystem._validate_ticket() for new tickets too (create()). The code can likely be used as-is from the update() implementation.

However, this may break various 'dumb insert' implementations that make no such assumption. Do we need to make the same distinction between 'new-style' and 'old-style' that we do for update()? Like maybe adding a custom parameter/field like {'action': 'new'} or similar that triggers the more advanced logic? Or adding a new keyword argument for the method, like check=True so that checks are enabled by default?

Patch with tests welcome ;-)

12/31/12 04:47:43 changed by rje@bitstruct.com

  • type changed from defect to enhancement.

Something like the attached diff is what I'm working with right now.

12/31/12 04:48:24 changed by anonymous

  • attachment xmlrpcplugin-mps1.diff added.

Add/Change #10738 (XmlRpcPlugin does not respect ITicketManipulators)




Change Properties
Action