Add support for Jottacloud #2433

buengese · 2018-08-01T23:45:06Z

Summary

This is my work adding support for Jottacloud as requested in #307. The basic Interfaces are implemented and working but there are still some things to do.

Some open stuff.

The Jottacloud API situation:

Jottacloud does not publish any documentation on there API but they do seem to allow API usage. Therefore I mostly used the implementations in havardgulldahl/jottalib and albertony/jafs as reference.
I'm only using the https://www.jottacloud.com/jfs/ API (this one is somewhat documented in there forum) even tough there seems to be newer API on https://api.jottacloud.com/files/v1 but so far only their own jotta-cli tool uses it and it's not documented anywhere.
In fact there seems to be a third api https://www.jottacloud.com/rest/webrest.
None of these API's seem to implement all the functions.

What's left to do

Add config opions for device and mountpoint
Implement the optional Purger, Copier, Mover, DirMover interfaces
Maybe look into the newer upload API
Cleanup

On a final note this is pretty much the first time I have written anything in go so there may be some mistakes / bad practices in here.

jkaberg · 2018-08-02T06:22:50Z

Wonderfull @buengese. If you wan't I could try and reverse engineer the jotta-cli tool API, however I see they're missing some functions to make it viable.

Looking forward to see what you come up with, let me know if you need beta testing done.

ncw · 2018-08-02T07:02:48Z

That is looking really good :-)

The tests are failing because errcheck is failing due to Sprintf parameters.

You can use

 make build_dep

To install the checkers locally

Then

 make check

To run them.

What is the status of the integration tests? Are they passing?

If you can get the tests passing then I'll give it a review and try it locally.

Well done getting this far, it is a lot of work.

jkaberg · 2018-08-02T15:41:30Z

backend/jottacloud/jottacloud.go

+	maxSleep          = 2 * time.Second
+	decayConstant     = 2       // bigger for slower decay, exponential
+	defaultDevice     = "Jotta" // TODO ? : Make this a config Option?
+	defaultMountpoint = "Sync"  // TODO ? : Make this a config Option?


defaultDevice usually never changes so IDK, but defaultMountpoint could so it makes sense to make it configureable. It makes even more sense to change defaultMountpoint to something other than Sync as thats the default folder for the official client (things placed in this folder will get synced to all official clients).

I agree on device. It's actually possible to create you own mountpoints but I wasn't sure if and how i should implement that.

I guess if you were to implement that, it could be treated the same way as folders? Eg an mountpoint "is just another folder", if that makes sense.

jkaberg · 2018-08-02T15:46:47Z

backend/jottacloud/jottacloud.go

+	defaultMountpoint = "Sync"  // TODO ? : Make this a config Option?
+	rootURL           = "https://www.jottacloud.com/jfs/"
+	//newApiRootUrl   = "https://api.jottacloud.com"
+	//newUploadUrl	  = "https://up-no-001.jottacloud.com"


When I reversed the corresponding upload url for the JFS api (https://www.jottacloud.com/jfs/) one needed to use https://up.jottacloud.com/ to upload (ref https://github.com/havardgulldahl/jottalib/blob/master/src/jottalib/JFS.py#L1128).

However I can't seem to find it here, did this change later on?

both seem to work fine. I'm pretty sure I've seen both being used but I can change it.

No need to change it if it works I guess? Was just curious 😃

But it would be preferable to mimic the official client, might be a reason behind it using the up.jottacloud.com endpoint

buengese · 2018-08-03T22:14:52Z

I implemented the Purger, Copier, Mover, DirMover interfaces. Current implementation is kind a redundant will still be simplified. All unit tests pass however the TestCopyFile integration test fails if run together with the other tests (It does pass if run on it's own). This seems to be a problem with how Jottacloud handles deleted files. If I find some free time I'll look into it tomorrow. Everything else should be in a reviewable state and some manual testing could also be done.

jkaberg · 2018-08-04T08:39:25Z

@buengese I made a local build and tried it, however it fails on,

2018/08/04 10:35:39 Failed to create file system for "jcloud:/test": couldn't get account info: strconv.ParseUint: parsing "-1": invalid syntax

Seem's to be related to https://github.com/buengese/rclone/blob/jottacloud/backend/jottacloud/api/types.go#L102 which is -1 in my case (unlimited)

GET https://www.jottacloud.com/jfs/get-12345678/

<user time="2018-08-04-T08:37:48Z" host="dn-111">
<script/>
<username>get-12345678</username>
<account-type>unlimited</account-type>
<locked>false</locked>
<capacity>-1</capacity>
<max-devices>-1</max-devices>
<max-mobile-devices>-1</max-mobile-devices>
<usage>1231231231234</usage>
<read-locked>false</read-locked>
<write-locked>false</write-locked>
<quota-write-locked>false</quota-write-locked>
<enable-sync>false</enable-sync>
<devices>
<device>
<name xml:space="preserve">Jotta</name>
<display_name xml:space="preserve">Jotta</display_name>
<type>JOTTA</type>
<sid>XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX</sid>
<size>1231231231234</size>
<modified>2018-08-03-T23:47:50Z</modified>
</device>
</devices>
</user>

buengese · 2018-08-04T20:12:18Z

@jkaberg It's fixed. Didn't expect negative values there.

jkaberg · 2018-08-05T00:07:58Z

Seem's to be working as expected (copy, move etc), haven't tried mount or anything more advanced but I will soon enough 😄

Well done @buengese!

ncw

That is a really excellent piece of work ⭐

I've put a few comments, queries and things to fix up inline which don't detract at all from a first rate pull request :-)

I'm going to write more things which need to be done on the PR in a moment as this box is very small for typing in!

ncw · 2018-08-06T17:41:59Z

backend/jottacloud/api/types.go

+	if err != nil {
+		return err
+	}
+	// Parse the time format in multiple possible ways


This comment looks like a leftover

ncw · 2018-08-06T17:44:54Z

backend/jottacloud/jottacloud.go

+	minSleep          = 10 * time.Millisecond
+	maxSleep          = 2 * time.Second
+	decayConstant     = 2       // bigger for slower decay, exponential
+	defaultDevice     = "Jotta" // TODO ? : Make this a config Option?


What does it do? Might the user need to change it?

My information on this is not exactly clear either but it seems it was supposed to identify different computers for their own sync functionality. (The Jotta Device is used by the WebUI program implementing the Jottacloud API)

I've been using Jotta and the api for many years, this has never changed over the years. Neither with Desktop/WebUI usage or mobile devices.

I guess this is just some legacy stuff that never changed

ncw · 2018-08-06T17:45:22Z

backend/jottacloud/api/types.go

+	if err != nil {
+		return err
+	}
+	// Parse the time format in multiple possible ways


This comment looks obsolete

ncw · 2018-08-06T17:52:05Z

backend/jottacloud/jottacloud.go

+		switch apiErr.StatusCode {
+		case http.StatusNotFound:
+			return nil, fs.ErrorObjectNotFound
+		case http.StatusMovedPermanently, http.StatusFound, http.StatusSeeOther:


Do you need these other statuses? I would have thought http.StatusNotFound should cover it?

That's a leftover.

ncw · 2018-08-06T17:54:45Z

backend/jottacloud/jottacloud.go

+		CanHaveEmptyDirectories: true,
+	}).Fill(f)
+
+	if user != "" || pass != "" {


Is anonymous access possible to Jottacloud? If not then this should probably error if user == "" || pass == ""

It doesn't. This appears to be a copy-paste leftover.

ncw · 2018-08-06T18:06:09Z

backend/jottacloud/jottacloud.go

+
+	var resp *http.Response
+	err = f.pacer.Call(func() (bool, error) {
+		resp, err = f.srv.Call(&opts)


If you are using Call you need to explicitly close the resp.Body otherwise you'll leak a file descriptor - see its docs.

You can do this by using by using CallXML(&opts, nil, nil) or by setting NoResponse: true in opts.

ncw · 2018-08-06T18:10:14Z

backend/jottacloud/jottacloud.go

+		Method:        "POST",
+		Path:          o.filePath(),
+		Body:          in,
+		ContentType:   "application/octet-stream",


I think you probably want fs.MimeType(src) here if that is setting the content type of the uploaded object.

ncw · 2018-08-06T18:11:34Z

backend/jottacloud/jottacloud.go

+		opts.ExtraHeaders["JMd5"] = md5
+		opts.Parameters.Set("cphash", md5)
+
+		// TODO ? : If MD5 unsupported calculate MD5 ourself


Calculating the MD5 involves streaming the file into a temporary file and we usually don't bother unless the upload absolutely requires it.

Yeah I expected that this. The MD5 seems to be required only if you want the uploads to be resumable.

ncw · 2018-08-06T18:14:25Z

backend/jottacloud/replace.go

@@ -0,0 +1,84 @@
+/*
+Translate file names for JottaCloud adapted from OneDrive


Can you copy and then adapt the tests from backent/onedrive/replace_test.go too please.

ncw · 2018-08-06T18:16:01Z

fs/options.go

@@ -147,6 +147,10 @@ func FixRangeOption(options []OpenOption, size int64) {
 				x = &RangeOption{Start: size - x.End, End: -1}
 				options[i] = x
 			}
+			if x.End > size {


Can you stick this in a separate commit please (doesn't need to be a new PR), update the comment for this function and justify why you need to make the change in the commit message - thanks!

This one might actually need a bit of discussion. I added this as quick fix for passing the TestObjectOpenRange unit test. Specificly this one:
{fs.RangeOption{Start: 81, End: 100000}, 81, 100}
As far as I understand RFC7233 this request should actually fail with Range not Satisfiable and on Jottacloud it does. However all the other backends implemented so far seem to be fine with it.

Interesting... It is a bit vague the RFC isn't it. Especially this bit:

Thus, clients cannot depend on receiving a 416 (Range Not Satisfiable) response even when it is most appropriate.

I'm happy for this to go in a separate commit and it will get run through all the integration tests for the other backends so we can see if it breaks anything else (I don't think it will).

ncw · 2018-08-06T18:27:41Z

An impressive piece of work - thank you! And most importantly it works! I ran some tests by hand copying things about and it worked very well.

The unit tests pass. The operations test pass. The sync tests give one test failure which seems repeatable

~/go/src/github.com/ncw/rclone/fs/sync$ go test -v -run TestServerSideCopy -remote TestJottacloud:
=== RUN   TestServerSideCopy
2018/08/06 19:22:11 ERROR : sub dir/hello world: Failed to copy: copy failed: Error 404: no.jotta.backup.errors.NoSuchPathException: Destination parent path /8vopdox6wjnjmz3l0mfiitae/Jotta/Sync/rclone-test-lofokod4weqesez0yuyaduv1/sub dir does not exist (Not Found)
2018/08/06 19:22:11 purge failed: directory not found
--- FAIL: TestServerSideCopy (7.17s)
	run.go:173: Remote "jottacloud root 'rclone-test-wuxaqog6sazecah5fitalav2'", Local "Local file system at /tmp/rclone454131250", Modify Window "1s"
	sync_test.go:118: Server side copy (if possible) jottacloud root 'rclone-test-wuxaqog6sazecah5fitalav2' -> jottacloud root 'rclone-test-lofokod4weqesez0yuyaduv1'
	require.go:794: 
			Error Trace:	sync_test.go:121
			Error:      	Received unexpected error:
			            	Error 404: no.jotta.backup.errors.NoSuchPathException: Destination parent path /8vopdox6wjnjmz3l0mfiitae/Jotta/Sync/rclone-test-lofokod4weqesez0yuyaduv1/sub dir does not exist (Not Found)
			            	copy failed
			            	github.com/ncw/rclone/backend/jottacloud.(*Fs).Copy
			            		/home/ncw/go/src/github.com/ncw/rclone/backend/jottacloud/jottacloud.go:538
			            	github.com/ncw/rclone/fs.(Copier).Copy-fm
			            		/home/ncw/go/src/github.com/ncw/rclone/fs/fs.go:527
			            	github.com/ncw/rclone/fs/operations.Copy
			            		/home/ncw/go/src/github.com/ncw/rclone/fs/operations/operations.go:269
			            	github.com/ncw/rclone/fs/sync.(*syncCopyMove).pairCopyOrMove
			            		/home/ncw/go/src/github.com/ncw/rclone/fs/sync/sync.go:306
			            	runtime.goexit
			            		/opt/go/go1.10/src/runtime/asm_amd64.s:2361
			Test:       	TestServerSideCopy
FAIL
exit status 1
FAIL	github.com/ncw/rclone/fs/sync	8.898s

So I would say, bar a few little cleanups and fixes the code is done :-)

So looking at the writing a new backend checklist I'd say you've got down to the part beginning "Add your fs to the docs..." so nearly done!

Let me know if you want any help with anything.

I'm hoping we can land this in time for the 1.43 release at the end of the month.

jkaberg

Found 2 typo's you might want to change 😄

jkaberg · 2018-08-08T11:25:07Z

docs/content/jottacloud.md

+
+### Limitations ###
+
+Note that Box is case insensitive so you can't have a file called


Box? Jottacloud? 😄

jkaberg · 2018-08-08T11:25:17Z

docs/content/jottacloud.md

+Note that Box is case insensitive so you can't have a file called
+"Hello.doc" and one called "hello.doc".
+
+There are quite a few characters that can't be in OneDrive file names. Rclone will map these names to and from an identical looking unicode equivalent. For example if a file has a ? in it will be mapped to ？ instead.


ncw · 2018-08-10T10:14:29Z

I've looked through all that - and it looks really good - well done!

The failing test passed when I tried it. I don't know whether you fixed it or whether it is to do with this (from the docs).

Jottacloud exhibits some inconsistent behaviours regarding deleted files and folders which may cause Copy, Move and DirMove operations to previously deleted paths to fail. Emptying the trash should help in such cases

I'm going to merge this now.

You can see the daily integration tests run here: https://pub.rclone.org/integration-tests/ - so we can keep an eye on what happens there

If you'd like to be added to the project as a maintainer then drop me an email to nick@craig-wood.com to discuss.

Thank you very much for a great contribution :-)

ncw · 2018-08-10T10:20:36Z

I stuck a post on the forum encouraging people to test it: https://forum.rclone.org/t/jottacloud-now-in-beta/6449

buengese force-pushed the jottacloud branch from f2171c1 to 18eda79 Compare August 2, 2018 14:00

jkaberg reviewed Aug 2, 2018

View reviewed changes

buengese force-pushed the jottacloud branch from 18eda79 to 4ba3437 Compare August 3, 2018 22:08

buengese force-pushed the jottacloud branch 2 times, most recently from 5b5f0e9 to 01c6d27 Compare August 4, 2018 19:32

ncw reviewed Aug 6, 2018

View reviewed changes

buengese added 2 commits August 7, 2018 18:35

New backend: Jottacloud

7891465

fs/OpenOptions: Make FixRangeOption clamp range to filesize.

a520368

buengese force-pushed the jottacloud branch 2 times, most recently from 82050fe to aef2d33 Compare August 7, 2018 18:53

buengese changed the title ~~[WIP] Add support for Jottacloud~~ Add support for Jottacloud Aug 7, 2018

jkaberg reviewed Aug 8, 2018

View reviewed changes

buengese force-pushed the jottacloud branch from aef2d33 to d5210e1 Compare August 8, 2018 13:07

Add docs for Jottacloud

157ca1c

buengese force-pushed the jottacloud branch from d5210e1 to 157ca1c Compare August 8, 2018 13:08

ncw merged commit 007c775 into rclone:master Aug 10, 2018

buengese deleted the jottacloud branch August 10, 2018 21:55

		@@ -0,0 +1,84 @@
		/*
		Translate file names for JottaCloud adapted from OneDrive


		### Limitations ###

		Note that Box is case insensitive so you can't have a file called

Add support for Jottacloud #2433

Add support for Jottacloud #2433

Conversation

buengese commented Aug 1, 2018 • edited

Summary

The Jottacloud API situation:

What's left to do

jkaberg commented Aug 2, 2018

ncw commented Aug 2, 2018

jkaberg Aug 2, 2018 • edited

Choose a reason for hiding this comment

buengese Aug 2, 2018 • edited

Choose a reason for hiding this comment

jkaberg Aug 2, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkaberg Aug 2, 2018 • edited

Choose a reason for hiding this comment

buengese commented Aug 3, 2018

jkaberg commented Aug 4, 2018 • edited

buengese commented Aug 4, 2018

jkaberg commented Aug 5, 2018

ncw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

buengese Aug 6, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ncw commented Aug 6, 2018

jkaberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ncw commented Aug 10, 2018

ncw commented Aug 10, 2018

buengese commented Aug 1, 2018 •

edited

jkaberg Aug 2, 2018 •

edited

buengese Aug 2, 2018 •

edited

jkaberg Aug 2, 2018 •

edited

jkaberg Aug 2, 2018 •

edited

jkaberg commented Aug 4, 2018 •

edited

buengese Aug 6, 2018 •

edited