owncloud / pyocclient

ownCloud client library for Python
MIT License
295 stars 115 forks source link

subfiles=True : is the value according to the spec? #154

Open moscicki opened 8 years ago

moscicki commented 8 years ago

This spec https://doc.owncloud.org/server/8.0/developer_manual/core/ocs-share-api.html is quite unspecific about what boolean argument is and how it should be specified.

The owncloud clients seem to use true/false strings in their requests to the server.

However this python module would use: True

Is this intended and is this compatible with owncloud server?

See: https://github.com/owncloud/pyocclient/blob/master/owncloud/owncloud.py#L893

BTW. A suggestion:

AFAIK this isinstance(subfile,bool) and **kwds handling is not the python way of doing things when you have a known list of keyword arguments which you expect to check in a boolean style. What about:

def get_shares(self,path="",subfiles=False,...) .... if subfiles: args['subfiles]='true'

PVince81 commented 8 years ago

What a coincidence... we also found this today, fix is here: https://github.com/owncloud/pyocclient/pull/153

moscicki commented 8 years ago

@PVince81: What about making this pyocclient module the reference implementation for the OCS SHARE API spec? Together with desktop sync client devs we attempt something like that for the sync protocol part here: https://github.com/cernbox/smashbox/tree/master/protocol

The idea is that the set of tests validates the requests and responses in given, known conditions. If written correctly then most if not all spec changes or silent implementation changes or regressions server-side may be easily detected.

The purpose is that if someone comes up with version v2 of OCS SHARE API they have to fully specify it first in your reference implementation which then together with the spec can be used as complete and correct reference for sync client and server developers.

@dragotin: would this be helpful for you since the desktop sync client now involves OCS SHARE API calls?

@DeepDiver1975: would this be helpful for regression control for your brand new sharing reimplementation in 9.0?

@cmonteroluque: do you have already some way to control the consistence of API specs across various software components in your process? we have seen various discrepancies in the past between mobile and desktop clients.

Note: we also test sharing use-cases but the protocol testing is complementary to it. For reference: https://github.com/cernbox/smashbox/tree/master/lib/oc-tests

PVince81 commented 8 years ago

Not sure if suitable, but we already have some integration tests in core that test the API's consistency: https://github.com/owncloud/core/blob/master/build/integration/features/sharing-v1.feature

Having a reference / example implementation does sound nice.