[PATCH] 767 Fixed VLV usage in TPS token and activity services.
The TPS token and activity services have been modified to use VLV only when the search filter matches the VLV, which is the default filter when there is no search keyword/attributes specified by the client. In other cases the services will use a normal search. https://fedorahosted.org/pki/ticket/2342 -- Endi S. Dewata
On Fri, Jun 10, 2016 at 10:29:51AM -0500, Endi Sukma Dewata wrote:
The TPS token and activity services have been modified to use VLV only when the search filter matches the VLV, which is the default filter when there is no search keyword/attributes specified by the client. In other cases the services will use a normal search.
https://fedorahosted.org/pki/ticket/2342
-- Endi S. Dewata
Code changes look good. Might be nice to construct the (Token|Activity)Collection objects inside the find* methods rather than constructing it at call side and passing it in to be populated. Just a nit, though. Searching/filtering continues to work as expected. ACK.
On 6/15/2016 7:22 PM, Fraser Tweedale wrote:
Code changes look good. Might be nice to construct the (Token|Activity)Collection objects inside the find* methods rather than constructing it at call side and passing it in to be populated. Just a nit, though.
Thanks for the suggestion. I also considered that, but the reason I decided to create the response (i.e. collection) object outside the internal find*() methods is because the internal methods are just helper methods to populate certain aspects of the response object only (i.e. the entries and the total). Suppose they do create the response object, we probably would expect that the response object would be complete. However, notice that after the internal find*() invocations we're still adding links to the response object. Also they do not perform any parameter validation either, so these methods are not meant to be called directly by anything else. To clarify the distinction I've changed them to protected retrieve*WithVLV() and retrieve*WithoutVLV(). BTW, in the future we might want to change the latter method with simple paged results.
Searching/filtering continues to work as expected.
ACK.
Thanks! Pushed to master. -- Endi S. Dewata
On Wed, Jun 15, 2016 at 09:39:46PM -0500, Endi Sukma Dewata wrote:
On 6/15/2016 7:22 PM, Fraser Tweedale wrote:
Code changes look good. Might be nice to construct the (Token|Activity)Collection objects inside the find* methods rather than constructing it at call side and passing it in to be populated. Just a nit, though.
Thanks for the suggestion. I also considered that, but the reason I decided to create the response (i.e. collection) object outside the internal find*() methods is because the internal methods are just helper methods to populate certain aspects of the response object only (i.e. the entries and the total). Suppose they do create the response object, we probably would expect that the response object would be complete. However, notice that after the internal find*() invocations we're still adding links to the response object. Also they do not perform any parameter validation either, so these methods are not meant to be called directly by anything else. To clarify the distinction I've changed them to protected retrieve*WithVLV() and retrieve*WithoutVLV(). BTW, in the future we might want to change the latter method with simple paged results.
No worries; thanks for explaining.
Searching/filtering continues to work as expected.
ACK.
Thanks! Pushed to master.
-- Endi S. Dewata
participants (2)
-
Endi Sukma Dewata -
Fraser Tweedale