rave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Matt Franklin <m.ben.frank...@gmail.com>
Subject Re: [Proposal] REST API interface
Date Tue, 23 Jul 2013 03:24:11 GMT


> 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

> 
> 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