Based on my review, this is still a work in progress. I do not guarantee
that this is a complete list of spec issues. These are simply the issues
that seem to be the most evident.
Missing from spec:
Messaging
Albums
MediaItem
Line 588: "Highlights" is really a section. Please change from <t> to
<section title="Highlights">
Line 600: Please strike. This is a sentence that occurs in the
discussion of why you included the examples early. It is not needed for
the spec.
Line 601: "A Person request, the most basic example" should be moved
into the <figure> tag as:
<figure>
<preamble>A basic Person
Request</preamble>
Line 609 & 618: Move content inside /figure/preamble tag
Line 631 Move this to Highlights list: Batch calls can be cascaded.
Maybe I'm old school, but it seems that the correct term is
"chained|chaining"? Suggested edit when moving to Highlights (near line
588)
"Batch calls are designed for call chaining."
The examples demonstrate this concept adequately.
Line 635. Please strike "This is a short namespace, so it's easier to
type, and it is different from the existing apis so that they can
coexist until the old apis are removed." I don't believe that this
statement enhances the spec. Line 586 already addresses deprecation.
Line 648: "When the Gadget server renders the osapi library, it calls
the SystemHandler's list Methods endpoint, and adds the available
services."
'it' is ambiguous.
SystemHandler is a new term. I believe that this may be a Shindig type,
not something that is actually part of the system.
Line 648 contains a TODO: "The js client then uses this to generate
(TODO Not implemented yet)". The document here MUST NOT refer to TODOs
in any given implementation. The document MUST NOT contain TODOs for
feature descriptions. I'm not sure which case the TODO refers to, but
the TODO needs to disappear and be rectified.
Line 661: Define behavior of <Require feature="osapi" />
Line 662 | 682 | 698 | 715 | 735 | 750 | 775
Why are we defining subsets of the feature? Osapi should
be a single feature, introducing the basic APIs around Activities,
People, AppData. The extra granularity seems to be new and unnecessary.
I fail to see the benefit of the extra knobs.
Line 662: "that map to the activities endpoint" should be "that map to
the people endpoint". Need precise definition of what the endpoint is.
Probably need a cross reference to the spec where the endpoint for each
item is defined
Line 666 | 667 | 668 | 669: You repeat "convenience" 2x. Delete the
'Note'
Line 682 | 698: Need precise definition of what the endpoint is.
Probably need a cross reference to the spec where the endpoint for each
item is defined
From: [email protected]
[mailto:[email protected]] On Behalf Of Lane
LiaBraaten
Sent: Thursday, April 02, 2009 9:45 PM
To: [email protected]
Cc: [email protected]
Subject: [opensocial-and-gadgets-spec] Re: OS Lite (OSAPI) spec doc
oops - here's the patch: http://codereview.appspot.com/33058
On Thu, Apr 2, 2009 at 9:42 PM, Lane LiaBraaten <[email protected]>
wrote:
I created a spec patch for the OSAPI stuff, but I didn't realize Bob
hadn't incorporated Adam's comments yet. I made a few changes, but some
of these we still need to finalize. My comments inline.
-Lane
On Tue, Mar 31, 2009 at 10:10 AM, Adam Winer <[email protected]> wrote:
Comments:
- At this stage, we really need spec updates in the form of
diffs to
http://opensocial-resources.googlecode.com/svn/spec/draft/Opensocial-Spe
cification.xml.
I kinda wish we'd split this huuuge doc into multiple smaller
docs,
as the XSLT transform hangs Firefox for awhile, but that should
be its
own spec diff.
- osapi.base should largely be undocumented, and I'm not sure it
should exist as a feature name, though I could be convinced
otherwise.
no change ... I'm not clear on this so I don't have an opinion right now
- if (osapi.people.getViewer) {
should be
if (osapi.people && osapi.people.getViewer) {
since you have to test for both the service and the method.
fixed
- "When no parameters are specified, the server defaults will be
used:
{ user : @me, group : @self}. This is built on
osapi.newJsonRequest()."
Two notes. It's userId: @me, groupId: @self. And what it's
built
on is an implementation detail that the spec should leave out.
fixed
- osapi.people.get.execute(callback)
isn't a great description, since there is no such function on
osapi.people.get. Need some central documentation describing
the
execute() method on all request and batch objects
no change - not sure what the change should be here
- osapi.activities.create(params) - maps to the activities
create
endpoint. This is a non-ui intermediated version of the
osapi.ui.requestCreateActivity call.
I think the distinction between UI-intermediated calls and
non-UI-intermediated calls is artificial. A container may make
*any*
JS call ui-mediated - permission dialogs being the obvious
example.
Strongly prefer if we:
(1) Kill requestCreateActivity(), and for that matter
everything in osapi.ui
(2) Leave UI mediation up to the container, in particular use
activities.create() instead of requestCreateActivity()
(3) If we have time, build the message service API for the
requestSendMessage()/requestShareApp(), if not leave that for
v.Next
(4) Don't bother with requestPermission() in this API, though
that
opinion is 99% based on experience with a few containers, and
other
container developers may have very different feedback.
no change, but I'm +1 on these comments
- osapi.appdata.deleteData
Since JS prevents us from just calling this "delete", I'd
rather
call it "remove()", which is at least a predictable naming
pattern.
Or "deleteAppdata()".
no change, but I'm +1
- " osapi.messages TODO"
I think this means it's not in 0.9.
removed
* osapi.newBatch() - returns a new osapi.base.batch request.
* osapi.base.batch.add(key, request) - Adds a service request
to
the batch associated with the specified key. These may be
jsonrpc
calls, such as osapi.people.get(), or osapi.makeRequest calls to
third
parties, or a mixture of the two.
* osapi.base.batch.execute(callback) - executes all of the
requests in the batch. Callback is passed a JSON object mapping
each
request key to a JSON response object.
Don't doc that batches are in "osapi.base.batch". All we need
to doc
is that batch objects have add() and execute() methods.
no change, I think this makes sense though, esp if we take out the
osapi.base docs
- Batch functions:"
One thing I would recommend adding is tweaking osapi.newBatch()
to be:
osapi.newBatch(opt_json)
... where opt_json is a JSON array describing a full request.
This
lets developers that are truly fluent with JSON-RPC write:
osapi.newBatch([{method: "people.get"} ...
etc...]).execute(callback);
... and done.
no change, seems reasonable
- osapi.makeRequest
This should be consistent with everything else in osapi, not
with
gadgets.io. I think the right API is to follow Data Pipelining:
osapi.http.get(params)
osapi.http.post(params)
(and any other HTTP methods we care to specify at this point).
"params" should exactly match the attributes on os:HttpRequest,
and
must use the same data format returned by os:HttpRequest (which
I
haven't specified yet, I will send out that today). It also
shouldn't
mention that it uses gadgets.io.makeRequest().
no change, but I like it
- osapi.ui:
As said above, I think this needs to be killed, as ui-mediated
is up
to the container, not the spec. Batching semantics get a little
tricky, but we're far better off supporting batching for
ui-mediated
requests than not, because it gives the container a lot more
flexibility for good UIs - instead of showing 5 dialogs in
succession,
the container can show a single dialog saying the gadget's
trying to
do 5 things. For now, we can leave the batch semantics up to
the
container, as I can come up with a variety of plausible
semantics.
Also, everything else is method(params).execute(callback), and
there's
no reason to break down and use positional parameters here.
no change - the ui namespace does seem strange since there's nothing
that says the container *has* to make these calls user-mediated
- osapi.base:
I don't think any of these need to be documented. If
containers
offer additional service methods, it's up to containers to
support
getting those methods into osapi. The SystemHandler method you
describe above is reasonable.
no change. I don't fully understand this, but if containers don't need
to be consistent on the osapi.base namespace, then we can leave this out
- Error Handling:
I like the pattern of using and propagating "error" through the
structures. A big +1, as it's simple and clear. That said, the
doc
needs to be explicit that:
- "error" may not be used as a key to batch.add().
- "error" is reserved and may not be used as a property of a
JSON
RPC batch response item.
fixed
Also, when the doc says "The error handling works exactly as it
does
for the JSONRPC interface, since this api is based on that
protocol."
that's not true. In JSON-RPC, a network error means that you
get a
JSON structure with just an error:
{error: {message: "", code: ""}}
... without any per-request-item content. What you're doing -
which I
like - is creating dummy response items at each key and copying
the
network error into each response item.
no change - not sure how to update
In "Example: Processing any sub requests in the batch that
succeeded:
", the example is incorrect. We don't have an error property
within
each returned activity, since all activities are in a single
request.
We need an example like:
var batch = osapi.newBatch().
add('v', osapi.people.getViewer()).
add('vf', osapi.people.getViewerFriends());
batch.execute(function(result) {
for (var name in result) {
if (result.hasOwnProperty(name)) {
if (result[name].error) {
gadgets.log("Found an error: ",
result[name].error.message);
} else {
gadgets.log("Got some data: ", result[name]);
}
}
}
});
fixed
On Mon, Mar 30, 2009 at 12:15 PM, Bob Evans
<[email protected]> wrote:
>
> The URL:
http://wiki.opensocial.org/index.php?title=OSAPI_Specification
>
> On Mon, Mar 30, 2009 at 12:01 PM, Bob Evans
<[email protected]> wrote:
>> Hi,
>>
>> I have updated the OS Lite spec doc to match the current
>> implementation (there is a follow on patch coming today with
the
>> metadata-based discovery of service methods).
>>
>> Known issues:
>>
>> * requestCreateActivity and activities.create. It might be
possible to
>> make all calls ui-interruptible, but this prevents batching,
well,
>> it makes batching a lot stranger and more involved. For
example, what
>> happens if I have a batch where 2 of 4 calls require UI
interaction
>> with the user. It's do-able, but would require a lot more
thought than
>> .9 allows, I think. My proposed solution keep requestX
methods, and
>> discuss making all service calls ui-intermediable in the next
version
>> spec discussion.
>>
>> * send vs execute vs call naming. I prefer execute, but if
people want
>> a shorter name, then send seems good. Some have suggested
call as a
>> name, but this is used in the language, so seems like a bad
idea.
>>
>> * Rename osapi.makeRequest to osapi.http.get/post. To me,
makeRequest
>> reflects the existing, underlying gadgets calls, but I could
see
>> making it http.
>>
>> * osapi.ui namespace is weird, better suggestions?
>>
>> * Should we have a bucket of params for the requestX calls,
or
>> positional params as it is now? Maybe all of this is obviated
if we
>> make the requestX calls be jsonrpc calls.
>>
>>
>> Thanks,
>> Bob
>>
>
> >
>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google
Groups "OpenSocial and Gadgets Specification Discussion" group.
To post to this group, send email to
[email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/opensocial-and-gadgets-spec?hl=en
-~----------~----~----~----~------~----~------~--~---
