rave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Erin Noe-Payne <erin.noe.pa...@gmail.com>
Subject Re: [Proposal] REST API interface
Date Tue, 23 Jul 2013 03:33:13 GMT
On Mon, Jul 22, 2013 at 11:24 PM, Matt Franklin
<m.ben.franklin@gmail.com> wrote:
>
>
>> On Jul 22, 2013, at 15:39, Erin Noe-Payne <erin.noe.payne@gmail.com> wrote:
>>
>> The cxf interceptors model is very powerful though, because it lets us
>> reduce the weight of each controller significantly. Rather than
>> returning a Response, each controller can return an instance of the
>> object that it acts upon (Page or List<Page>, User or List<User> and
>> so on).
>>
>> We can have a series of interceptors that do any common tasks...
>> Incoming:
>> - On Post / Put responses check for correctly formed attached data,
>> and return a 400 with meaningful error message on bad format
>>
>> Outgoing:
>> - If response is null return 404 with meaningful error
>> - Field selector support
>> - Pagination for List<T> responses
>> - Wrapping of objects in the JsonResponseWrapper
>
> +1.  This seems like the cleanest approach and makes injecting new Apis much simpler.
 Ie provider specific extension Apis

Quick question regarding performance, since I'm not up on how the Java
persistence layer / caching works. Which of these makes more sense:
- For paginated list results, run 1 query against the db to get all
results. I now have a list of all results, and the pagination
interceptor can operate that list to get the count, subset, and move
on.
- Run two queries, one to get only the paginated data set and one to
retrieve the count.

>
>>
>> On Mon, Jul 22, 2013 at 4:15 PM, Erin Noe-Payne
>> <erin.noe.payne@gmail.com> wrote:
>>> I would also point out that we don't have a pressing need to actually
>>> implement fields support. Our first use case - the angular client -
>>> does not particularly need it since we will be using a
>>> client-optimized "pagesForRender" endpoint anyway.
>>>
>>> As long as we ensure that our approach can be extended to support
>>> fields selection in the future without refactor, that should be good
>>> enough.
>>>
>>> On Mon, Jul 22, 2013 at 3:58 PM, Erin Noe-Payne
>>> <erin.noe.payne@gmail.com> wrote:
>>>> - Simple solution: All rest response models are flat. We ignore any
>>>> nested data, and just have separate endpoints to deliver that data.
>>>> I.E. Every model in the org.apache.rave.rest.model package has only
>>>> properties of "primitive" types, with no lists, no other classes. That
>>>> is NOT currently the case. Then the fields interceptor checks for the
>>>> presence of a fields argument. If not present, the object is delivered
>>>> as is. If present the argument (a string) is split by comma and only
>>>> the matched properties are delivered. The fields qs argument only has
>>>> to support comma-delimited list of property names.
>>>> Ex: ?fields=name,pageType
>>>> //returns a page or pages with only name and pageType properties
>>>>
>>>> - Complicated solution: All rest response models include references to
>>>> their nested data. This is the currently the case, and can be seen in
>>>> org.apache.rave.rest.model.Page. The fields interceptor checks for
>>>> presence of fields qs argument. If not present it strips all nested
>>>> data from the models and only returns properties. If it is present, it
>>>> parses the argument and updates the data. The fields argument needs to
>>>> support a more complicated syntax that allows the requesting of nested
>>>> data. I would copy the syntax of facebook's graph search api, which
>>>> has a pretty readable solution. You allow for .fields and .limit on
>>>> fields, which can be nested.
>>>> Ex: ?fields=name,pageType,regions.limit(2).fields(regionWidgets.fields(widgetId,locked))
>>>> //returns a page or pages with name and pageType properties, nested
>>>> list of regions (max of 2) with nested list of regionWidgets with only
>>>> properties of widgetId and locked
>>>>
>>>> In all cases, id should always be returned.
>>>> I think the algorithm in the simple solution is easy.
>>>> In a sense the algorithm in the second should be simple, because the
>>>> service layer is already getting all the nested data, and you are just
>>>> stripping it off. Not sure what the performance implications of that
>>>> are though.
>>>>
>>>>> On Mon, Jul 22, 2013 at 3:40 PM, Chris Geer <chris@cxtsoftware.com>
wrote:
>>>>> On Mon, Jul 22, 2013 at 12:07 PM, Erin Noe-Payne
>>>>> <erin.noe.payne@gmail.com>wrote:
>>>>>
>>>>>> Going back to the discussion on field selection - I am currently
going
>>>>>> through the exercise of writing out the Resource interfaces to define
>>>>>> our endpoints.  There is a set of generic query string parameters
that
>>>>>> we wish to support on all or many of the endpoints - fields (any
get
>>>>>> request), limit / offset (any get request that returns a list).
>>>>>>
>>>>>> Rather than writing each endpoint to accept QueryParam()'s and repeat
>>>>>> the appropriate logic, I assume we would want to take advantage of
cxf
>>>>>> interceptors [1] to intelligently and generically handle those qs
>>>>>> arguments?
>>>>>
>>>>> I like the concept but I'm not sure how we generically filter, especially
>>>>> with nested data. I'd love to see it work that way though. Interceptors
are
>>>>> pretty easy to use, it's the filter algorithm I haven't figured out yet.
>>>>> Thoughts?
>>>>>
>>>>>>
>>>>>> [1] http://cxf.apache.org/docs/interceptors.html
>>>>>>
>>>>>> On Mon, Jul 22, 2013 at 11:40 AM, Erin Noe-Payne
>>>>>> <erin.noe.payne@gmail.com> wrote:
>>>>>>> Ok, so the endpoint is now working. Any thoughts about the
>>>>>>> JsonResponseWrapper approach? Does that seem like the best way
to get
>>>>>>> wrapped responses?
>>>>>>>
>>>>>>> For the next step I would like to start writing out all of the
>>>>>>> resource interfaces so that we can begin writing angular $resource
>>>>>>> services against them.
>>>>>>>
>>>>>>> On Sun, Jul 21, 2013 at 8:54 PM, Erin Noe-Payne
>>>>>>> <erin.noe.payne@gmail.com> wrote:
>>>>>>>> Awesome, thanks Chris. Not sure I would have ever figured
that one
>>>>>> out...
>>>>>>>>
>>>>>>>> On Sun, Jul 21, 2013 at 3:59 PM, Chris Geer <chris@cxtsoftware.com>
>>>>>> wrote:
>>>>>>>>> Erin,
>>>>>>>>>
>>>>>>>>> I got it working, at least the CXF part. Couple things:
>>>>>>>>>
>>>>>>>>> 1) In the interface, make sure to annotate the @GET methods
>>>>>>>>> 2) In your DefaultRegionWidgetsResource class, remove
the @ParamPath
>>>>>>>>> attributes from variable signatures. I know Intellij
puts those in
>>>>>> there
>>>>>>>>> but they cause problems. Only the interface should be
annotated.
>>>>>>>>>
>>>>>>>>> Chris
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Sat, Jul 20, 2013 at 2:00 PM, Erin Noe-Payne <
>>>>>> erin.noe.payne@gmail.com>wrote:
>>>>>>>>>
>>>>>>>>>> Review board is not accepting my patch and is not
accepting the valid
>>>>>>>>>> file paths. I have attached the patch as a file to
the review.
>>>>>>>>>>
>>>>>>>>>> On Sat, Jul 20, 2013 at 4:40 PM, Chris Geer <chris@cxtsoftware.com>
>>>>>> wrote:
>>>>>>>>>>> Erin, I'm not seeing a patch posted up there.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Fri, Jul 19, 2013 at 2:27 PM, Erin Noe-Payne
<
>>>>>>>>>> erin.noe.payne@gmail.com>wrote:
>>>>>>>>>>>
>>>>>>>>>>>> I was never able to hit the endpoint as expected.
I've posted the
>>>>>>>>>>>> patch on the review board if anyone can take
a look and offer
>>>>>> advice -
>>>>>>>>>>>> https://reviews.apache.org/r/12777/.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks
>>>>>>>>>>>>
>>>>>>>>>>>> On Fri, Jul 19, 2013 at 2:41 PM, Chris Geer
<chris@cxtsoftware.com
>>>>>>>
>>>>>>>>>> wrote:
>>>>>>>>>>>>>> On Friday, July 19, 2013, Erin Noe-Payne
wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Fri, Jul 19, 2013 at 1:24 PM,
Chris Geer <
>>>>>> chris@cxtsoftware.com
>>>>>>>>>>>> <javascript:;>>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>> In the xml file you need to create
the bean, then reference
>>>>>> it in
>>>>>>>>>> the
>>>>>>>>>>>>>>> server element near the top.
Other than that...no, that
>>>>>> should be
>>>>>>>>>>>> all. I
>>>>>>>>>>>>>>> assume you set the Path attribute
on the resource.
>>>>>>>>>>>>>> I did. I'm also messing around with
the service injection,
>>>>>> which may
>>>>>>>>>>>>>> be the issue. Haven't gotten it to
work yet though.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I thought we were going to do
>>>>>>>>>>>> pages/<id>/regions/<id>/regionwidgets/<id>
>>>>>>>>>>>>>>> since it makes no sense to manage
a region widget outside a
>>>>>> region
>>>>>>>>>>>>>> outside
>>>>>>>>>>>>>>> a page?
>>>>>>>>>>>>>> Possibly. Right now I'm just trying
to do a proof of concept
>>>>>> with
>>>>>>>>>> the
>>>>>>>>>>>>>> wrapped json object so I picked something
simple with the
>>>>>> service and
>>>>>>>>>>>>>> rest models already in place.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> In general though I don't see any
value to dealing with region
>>>>>>>>>> widgets
>>>>>>>>>>>>>> as a nested resource (pages/:id/regions/:id...)
over just
>>>>>> dealing
>>>>>>>>>> with
>>>>>>>>>>>>>> them directly. It's just adding weight
to the pages controller,
>>>>>>>>>> rather
>>>>>>>>>>>>>> than breaking them up and dealing
with resource concerns
>>>>>> separately.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I get what you're saying about regions
and regionwidgets only
>>>>>> making
>>>>>>>>>>>>>> sense in the context of a page, but
you could say the same
>>>>>> thing for
>>>>>>>>>>>>>> any 1-many associated resource. Both
entities are always
>>>>>> uniquely
>>>>>>>>>>>>>> identified, so why not deal with
them individually? I see an
>>>>>> upside
>>>>>>>>>> of
>>>>>>>>>>>>>> simpler code, consistent api endpoints,
and I see no downside.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Honestly, my hope is that someday they
aren't uniquely
>>>>>> identified and
>>>>>>>>>> are
>>>>>>>>>>>>> really sun objects unlike JPA today.
But that is a longer
>>>>>>>>>> conversation.
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Fri, Jul 19, 2013 at 10:16
AM, Erin Noe-Payne
>>>>>>>>>>>>>>> <erin.noe.payne@gmail.com>wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I'm trying to register a
new endpoint for regionWidgets. I've
>>>>>>>>>> added
>>>>>>>>>>>>>>>> the interface and default
implementation, and created /
>>>>>> registered
>>>>>>>>>>>> the
>>>>>>>>>>>>>>>> bean in cxf-applicationContext.xml.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> However, when I hit the endpoint
I get an error:
>>>>>>>>>>>>>>>> [INFO] [talledLocalContainer]
WARN :
>>>>>>>>>>>>>>>> org.apache.cxf.jaxrs.utils.JAXRSUtils
- No operation matching
>>>>>>>>>> request
>>>>>>>>>>>>>>>> path "/portal/api/rest/regionWidgets/1"
is found, Relative
>>>>>> Path:
>>>>>>>>>> /1,
>>>>>>>>>>>>>>>> HTTP Method: GET, ContentType:
*/*, Accept:
>>>>>> text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8,.
>>>>>>>>>>>>>>>> Please enable FINE/TRACE
log level for more details.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Is there anything else I
need to do in order to create and
>>>>>>>>>> register a
>>>>>>>>>>>>>>>> new endpoint?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Tue, Jul 16, 2013 at 11:53
PM, Erin Noe-Payne
>>>>>>>>>>>>>>>> <erin.noe.payne@gmail.com>
wrote:
>>>>>>>>>>>>>>>>> On Tue, Jul 16, 2013
at 10:24 PM, Chris Geer <
>>>>>>>>>>>> chris@cxtsoftware.com>
>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>> On Tue, Jul 16, 2013
at 7:04 PM, Erin Noe-Payne <
>>>>>>>>>>>>>>>> erin.noe.payne@gmail.com>wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On Tue, Jul 16,
2013 at 9:20 PM, Matt Franklin <
>>>>>>>>>>>>>>>> m.ben.franklin@gmail.com>
>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>> On Tue, Jul
16, 2013 at 12:53 PM, Chris Geer <
>>>>>>>>>>>>>> chris@cxtsoftware.com>
>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> On Tue,
Jul 16, 2013 at 10:32 AM, Erin Noe-Payne
>>>>>>>>>>>>>>>>>>>>> <erin.noe.payne@gmail.com>wrote:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Any
further discussion here? I would like to start
>>>>>>>>>>>> implementing
>>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>>>>>>>>> of
the REST APIs, as it is foundational for the
>>>>>> entire
>>>>>>>>>>>> angular
>>>>>>>>>>>>>>>>>>>>>> architecture.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> My
understanding from Matt is that the current apis
>>>>>> in
>>>>>>>>>> trunk
>>>>>>>>>>>>>> are
>>>>>>>>>>>>>>>>>>>>>> mostly
proof of concept - they are not tested and
>>>>>> much of
>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>> functionality
is just stubbed. Are any of the rest
>>>>>> api
>>>>>>>>>>>>>>>> implementations
>>>>>>>>>>>>>>>>>>>>>> in
the code base a good working example? Is there
>>>>>> other
>>>>>>>>>>>>>>>> documentation
>>>>>>>>>>>>>>>>>>>>>> we
can reference?
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> I've
been working on the People resource as a
>>>>>> "reference"
>>>>>>>>>> of
>>>>>>>>>>>> how
>>>>>>>>>>>>>> I'd
>>>>>>>>>>>>>>>>>>> like
>>>>>>>>>>>>>>>>>>>>> to see
them done but it's still a work in progress. I
>>>>>> need
>>>>>>>>>> to
>>>>>>>>>>>> go
>>>>>>>>>>>>>>>> back
>>>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>>>> pull
out the JSONView stuff and reimplement the
>>>>>> "fields"
>>>>>>>>>>>> concept.
>>>>>>>>>>>>>>>>>>> Couple of
>>>>>>>>>>>>>>>>>>>>> notes:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> - Object
representations should be as flat as
>>>>>> possible
>>>>>>>>>>>>>>>>>>>>> and separate
requests should be made to nested
>>>>>> resources to
>>>>>>>>>>>> get
>>>>>>>>>>>>>>>> nested
>>>>>>>>>>>>>>>>>>>>> details
(i.e. if you have regions and
>>>>>>>>>> regions/1/regionwidgets,
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>> regions
>>>>>>>>>>>>>>>>>>>>> representation
should not contain an array of
>>>>>>>>>> regionwidgets)
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> I am concerned
about the round trips to support this
>>>>>> when
>>>>>>>>>>>>>> rendering
>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>> page.  With
any page that has a sufficient number of
>>>>>>>>>> gadgets,
>>>>>>>>>>>>>> adding
>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>> number of
requests becomes problematic.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I see that rule
applying to the "standard" rest
>>>>>> endpoints for
>>>>>>>>>>>> crud
>>>>>>>>>>>>>>>>>>> operations on
resources. We
>>>>>>

Mime
View raw message