TryGhost / Ghost

Independent technology for modern publishing, memberships, subscriptions and newsletters.
https://ghost.org
MIT License
46.97k stars 10.22k forks source link

{{#prev_post}} Refers to the current post #5573

Closed frederikaalund closed 8 years ago

frederikaalund commented 9 years ago

I'm using Ghost 0.6.4 with a MySQL database. Everything runs on IBM BlueMix in UTC.

The problem is that {{#prev_post}} refers to the current post. {{#next_post}} correctly links to the next article. This happens in both the Casper theme and in my own theme.

This issue remains even in the latest commit as of this writing.

I've tracked down the issue to a single line in post.js. The problem can be fixed by changing

.andWhere('published_at', '<', publishedAt)

to

.andWhere('published_at', '<', new Date(publishedAt - 1))

That is, by subtracting a millisecond from the publishedAt date.

Arguably, this fix shouldn't be necessary since we are using strict < comparisons. In practice, however, the above change fixed the issue for me.

halfdan commented 9 years ago

Hi @frederikaalund and thanks for the detailed report. I tried reproducing your issue, but failed to do so. The prev_post seems to work just fine for me with 0.6.4 and MySQL.

frederikaalund commented 9 years ago

@halfdan Thanks for trying. I also found it so strange that nobody had reported it before and I couldn't find the error replicated on any other Ghost site. Still, it happens in my setup. I can't explain why.

The fix is pretty harmless to implement so please consider adding it anyhow. Maybe with a comment denoting that it is necessary for some server configurations.

If I ever find out why this only happens on my site, I'll let you know.

cobbspur commented 9 years ago

Hi @frederikaalund

I think it will interesting to see an export of your data. I can only see this issue occurring if there are 2 posts with exactly the same published_at date which is a real edge case given it is to the millisecond unless there is some other detail we are missing.

Other information that may be useful is:

For the solution, that seems like it could cause potentially more problems than it solves. You can still get two posts separated by 1 millisecond for instance and in that case, this problem would still occur and posts with matching published_at dates would be left in limbo I think

If you fancy sending me an export of your blog data (david[at]ghost.org) I would be happy to help validate it for you.

halfdan commented 9 years ago

I can only see this issue occurring if there are 2 posts with exactly the same published_at date which is a real edge case given it is to the millisecond unless there is some other detail we are missing.

That's not correct. In that case the prev post would simply skip one post (the one with the same timestamp) - but never yield the current post again.

halfdan commented 9 years ago

What I can see being the problem here is the deserialization of published_at not working correctly (maybe some precision issue). Where the current post's published_at gets incorrectly deserialized (giving a slightly later timestamp than it actually has) and is then used the fetch the one with published_at < brokenValue.

cobbspur commented 9 years ago

hmmm interesting... off ya go lol

halfdan commented 9 years ago

@frederikaalund Following my hunch could you try whether https://github.com/TryGhost/Ghost/pull/5591 fixes the issue for you?

frederikaalund commented 9 years ago

Thanks for the quick replies!

@halfdan Unfortunately, #5591 did not fix the issue for me.

I've tried to console.log(publishedAt) from #5591 and it returns Fri Jan 09 2015 09:22:00 GMT+0000 (UTC) which seemingly coincides with the value 2015-01-09 09:22:00 which is stored in the MySQL database.

@cobbspur

Were these posts imported from another platform/installation of Ghost of written using Ghost itself?

Written using Ghost itself. Though I did edit the published at date manually (through the GUI) to reflect the posting date as it was on our previous website.

How many authors are on your blog?

We are three authors but I've published all posts so far.

How is your blog hosted?

We use IBM's BlueMix PaaS. The database is a MySQL "service" as it is called in BlueMix. Static files (images) are stored in a Cloudant NoSQL database using a custom storage script.

If you still want a MySQL dump let me know and I'll create one for you. From manual inspection I can assure you that no two posts have the same published_at field. All posts are published many minutes apart.

@cobbspur @halfdan You can see the issue in the development version (using cobbspur's patch) and compare it to the production version (with my fix).

halfdan commented 9 years ago

@frederikaalund Could you try and set debug: true in the database config of your Ghost installation. This should make the query builder output all queries that are sent to the database (including their data bindings). Can you try and copy/paste those here?

Otherwise, is there a way you could give @cobbspur or me temporary access to your development install?

frederikaalund commented 9 years ago

I've set debug: true and when I load a post, I get the following in the log:

sql: 'select `posts`.* from `posts` where `status` = \'published\' and `page` = 0 and `published_at` > \'2015-01-09 09:22:00.000\' order by `published_at` asc limit 1' }

and later

sql: 'select `posts`.* from `posts` where `status` = \'published\' and `page` = 0 and `published_at` < \'2015-01-09 09:22:00.000\' order by `published_at` desc limit 1' }

The latter query is what generates {{#prev_post}}.

The strange thing is that when I submit the latter query directly with phpMyAdmin then I get the correct result. Furthermore, if I change < to <= in phpMyAdmin then I get the current page as expected. Do note that I did change it slightly as to not escape the strings with a bacsklash

select `posts`.* from `posts` where `status` = 'published' and `page` = 0 and `published_at` < '2015-01-09 09:22:00.000' order by `published_at` desc limit 1

Otherwise, phpMyAdmin complained that the syntax was invalid.

Maybe the off-by-one-millisecond error is due to the string escaping? ...Or is the string escaping just due to the way that the query is output to the console? If not, then it would be very strange if the query was further modified between being logged and being submitted to the database.

@halfdan @cobbspur I'm reluctant to give you full access to the BlueMix system as I can't restrict developers from creating/deleting services. Mind you that BlueMix also hosts our production version. I can give you a full dump of the node.js log and MySQL database if you want.

halfdan commented 9 years ago

@frederikaalund This is really really strange. The escaping should just be in the output to the command line.

I think a datadump would help. Can you send it to @cobbspur? (He posted his email address above)

frederikaalund commented 9 years ago

@halfdan I've sent @cobbspur an e-mail with the database dump. I'm currently testing an alternative MySQL database service called ClearDB. However, I do not think the issue is with the database (as the queries run fine there) but rather with Ghosts interface with the database.

halfdan commented 9 years ago

@frederikaalund :sob: This seems to be an environment issue. I can't reproduce the behaviour, neither with 0.6.4 nor current master.

If I find the time I might try and set up Ghost on IBM BlueMix.

@tgriesser @rhys-vdw Are you aware of any environment specific issues with knex where a < on a date field incorrectly behaves like a <=? Maybe related: https://github.com/tgriesser/knex/issues/524?

rhys-vdw commented 9 years ago

@halfdan Nothing I'm aware of. My knex knowledge is not good though.

frederikaalund commented 9 years ago

I tried to use a ClearDB's MySQL database service instead and now the issue is completely gone. Previously I used the stock BlueMix MySQL service. Thus the error must be due to the Ghost <-> BlueMix MySQL service connection.

Recall that the prev_post query worked fine even with the plain BlueMix' MySQL service when input directly through phpMyAdmin. That is, the phpMyAdmin <-> BlueMix' MySQL service connection worked fine.

I don't know what phpMyAdmin is doing differently compared to Ghost when connecting to the BlueMix' MySQL service. I guess that you would have to try it out yourselves by installing Ghost on BlueMix. That is, if you care about compatibility with BlueMix' MySQL service (it's not widely used).

Anyhow, I'm glad to have found a work-around that doesn't require me to patch Ghost. Thanks for all the support and interest in clearing up this issue.

ErisDS commented 9 years ago

I'm going to suggest closing this until it becomes a problem for more than one person, or MySQL flavour? There's plenty of information here to come back to if it does raise it's head again or if BlueMix gains popularity.