Append random chars to downloaded file

[nheingit]

fixes #329

[anbsky]

@nheingit please note the tests are failing.

[nheingit]

Sorry I forgot to squash the commits. This should fix both the TestUploadHandler, as well as Test_fetchFile.

There were some other tests failing on my machine, but I think that's due to the previously discussed problems I'm having with the test suite.

[anbsky]

Thank you @nheingit, the tests are passing now. While reviewing it, I realised the solution originally proposed is not ideal, as instead of xxx_1Mb.dat users will have files named 1Mb.dat_xxx. How about instead of simply adding random strings to the file name we create a randomly named folder and save the file under its original name in it before publishing? This would result in downloaded files being saved under the name uploader intended.

Please let me know if you would like to go down further that path.

[nheingit]

Yeah I can do that, but given the troubles I've had getting everything to play nicely on my machine it won't be until later this week.

[nheingit]

Hey, I should be able to start on this tomorrow, I just wanted to make sure this is still what we want done?

Just create a Tempdir with the random bits and place the untouched file inside?

Do you want there to be any naming structure to the directory, or just the randomly generated string?

[nheingit]

I'm sure the tests will be failing for this, but I can't yet run the tests on everything. Can you provide me the output of what this generates?

[anbsky]

Github seems to be backed up at this time and the test run is queued. But I suspect you're right and it will indeed fail as test code hasn't been modified to accomodate the new logic. Can you run tests just for the publish package on your machine? From our conversation, I remember the tests you had failing were not related to this.

Just create a Tempdir with the random bits and place the untouched file inside?

Yes, this was the idea.

Another issue stands out to me, there's no code to cleanup those temporary folders so they will stay forever on the filesystem, even though the file itself will be deleted after upload has been processed.

[nheingit]

Pushed this for you to take a look at. All of the tests are passing, but couldn't think of a way to cleanup the tempfolder.

I don't know if that should be the responsibility of createFile? Open to ideas.

[anbsky]

There's already a cleanup procedure for the file itself in the Handle function, no reason why folder removal can't be done there as well.

[nheingit]

After some poking around, and asking some of the gophers over in the golang discord I couldn't really find a way to obtain the path of f.Name() in the handler function. It seems that I'm breaking convention by not deleting randomDir in the function that I call it in. Obviously I can't do that if we're planning on doing anything after the createFile method is called.

Thoughts on how you might do this, or how you would proceed?

[nheingit]

After further reflection it came to me. I'm not sure if there are any other files or directories that are stored under the /upload_path/{user_id} path.

If there isn't I believe my solution works.

I would also add that I'm not sure what I should be logging if anything, so I copied the log that was being done previously into what I believe would be the pertinent spot.

[anbsky]

We cannot assume our user is only ever going to be uploading a single file simultaneously so recursively deleting all files and folders in his directory doesn't seem like the way to go.

filepath.Dir(f.Name()) should get you the directory part of the path.

[nheingit]

that was the first thing I tried. f.Name() only returns the name, and not the path.

Showcased here: https://play.golang.org/p/LQ_zF6X_BBr

[anbsky]

In your showcase f.Name() returns ./filename, relative to your current dir, and filepath.Dir correctly returns . because current dir is where you created it.

If you provide os.Create with a full path, f.Name() will give it back to you.

[nheingit]

Sorry I suppose I didn’t give context for that.

im having trouble getting access to the full path as in this example you have the Finalpath because you initialize the TempDir in that function.

my question would be how do you gain access to the path without that?

Or am I incorrectly assuming that fetchfile doesn’t return the full path?

[anbsky]

I don't understand what exactly is not working. What difference does it make that we do it in a different function? Could you perhaps write a test to illustrate where things go wrong?

[nheingit]

If you provide os.Create with a full path, f.Name() will give it back to you.

I was under the impression that in this context f.Name() was only returning the /file.ext portion of the path and not the entire thing. I must have not implemented it properly the first time I tried this, I apologize!

Thanks for your patience. All tests are passing and I made the changes as requested.

[anbsky]

I think it's ready for merging after the extraneous comments and files are removed!

[anbsky]

Sorry, I would appreciate it if you squashed git commit history, 16 commits seems a bit much! I'll merge it right away. Please send us your LBC wallet address so we can compensate you for the contribution.

[nheingit]

YIKES! sorry about that.

just one moment.

[nheingit]

as for the tokens, it looks like LBC is supported on coinomi. I haven't used it before, but I got the app and I think this would be what you're looking for?

lbc1q80auf6fxu9qg9ruhlv7wt42ank2ajgt7jmvv9j