[Owncloud] Fwd: Re: Files_Sharing Update

[Arthur Schiwon]

On Tuesday, September 25, 2012 12:39:27 PM Michael Gapczynski wrote:
> On Tuesday, September 25, 2012 06:32:22 PM Arthur Schiwon wrote:
> > On Tuesday, September 25, 2012 12:02:06 PM Michael Gapczynski wrote:
> > > On Tuesday, September 25, 2012 05:51:06 PM Arthur Schiwon wrote:
> > [..]
> > 
> > > > > What do you mean that there will be collisions with the old table
> > > > > still
> > > > > existing? Nothing should be using the oc_sharing table.
> > > > 
> > > > I was a bit unprecise. When the update is not successful, it will be
> > > > tried
> > > > all over again on the next page request. The already transfered shares
> > > > will
> > > > flood then the screen with error messages like "already exists" and
> > > > fail
> > > > again.
> > > 
> > > Well that doesn't sound like a good way to do app upgrades. I say bump
> > > the
> > > version up no matter what for any app upgrade. If an upgrade scripts
> > > fails
> > > the first time, there's a good chance it will fail again.
> > > 
> > > About the exceptions, I think Bart wrapped the app upgrade in a try
> > > catch
> > > so there shouldn't be any errors shown to the user. No need for logging
> > > stuff in the actual upgrade because the Share API does that.
> > 
> > The exception thrown on line 59 in update.php will never be catched.
> > 
> > There are catches before, but only printed back on the screen.. Here it's
> > also better to write them to the log file instead, so things can be
> > checked
> > in an emergency case.
> > 
> > Using the logfile in both cases will let the update succeed (so first
> > effect will not happen and we can leave the table there for now)  and
> > problems can be seen in the log.
> > OK?
> 
> Nothing should be echoed. So, that should be removed. There should still be
> a try/catch inside the update so we still go through each row. 
Yes, that's what already is going on. So we agree on this :)

> Exceptions
> are caught here:
> 
> https://github.com/owncloud/core/blob/master/lib/app.php#L642
> 
> The exception on line 59 will be caught there.

And few lines later we die...

The handling with failing updates is double-plus-user-unfriendly in newspeak, 
but that's something for the future. That's why we should avoid ending up in 
throwing exceptions at the end of the update script, if possible.

> 
> > [..]
> > 
> > > > However, the new link looks like
> > > > https://owncloud.server/public.php?service=files&file=/path/to/shared/
> > > > fo
> > > > ld
> > > > er
> > > > 
> > > > the old link was:
> > > > https://owncloud.server/public.php?service=files&token=20288a20dd02750
> > > > f8
> > > > 1d
> > > > 23 07e32bd367dcda331b6&file=/path/to/shared/folder
> > > 
> > > Now I think I understand, you can't use the old link because we aren't
> > > using tokens anymore. The share_type for links is 3 in the database.
> > 
> > Mh, that would mean that all old links are being killed. Personally,
> > renotifying all people about this is nasty, but doable. In larger setups
> > however or with heavy use of this feature it's really nasty and brings bad
> > user experience.
> > 
> > I suggest to catch those links, forward them to the new URL and display a
> > notification that the URL changed. And eventually drop the support in OC
> > 5.
> 
> Unfortunately, I don't have time to do that.

K, mine is also limited. I file a bug report and add it to the spreadsheet, 
let's see.



> 
> > Cheers
> > Arthur
> > 
> > > > Does it help you?
> > > > 
> > > > Cheers
> > > > Arthur
> > 
> > _______________________________________________
> > Owncloud mailing list
> > Owncloud at kde.org
> > https://mail.kde.org/mailman/listinfo/owncloud