Fix issues in SitemapService #243
Conversation
| import org.springframework.scheduling.annotation.Scheduled | ||
|
|
||
| import java.text.SimpleDateFormat | ||
|
|
||
| class SitemapService { | ||
|
|
||
| static lazyInit = !Holders.getConfig().getProperty("sitemap.enabled", Boolean.TYPE, false) |
There was a problem hiding this comment.
If sitemap is enabled the SitemapService should be loaded on application startup thereby enabling scheduled runs. Otherwise scheduled runs are enabled only after a manual run from admin UI.
| @@ -40,6 +44,8 @@ class SitemapService { | |||
| // run daily, initial delay 1hr | |||
| @Scheduled(fixedDelay = 86400000L, initialDelay = 3600000L) | |||
| def build() throws Exception { | |||
| fileCount = 0; | |||
There was a problem hiding this comment.
fileCount needs to be reset each time the method is run to prevent mismatch between entries in the sitemap index file and the actual sitemap files created
| @@ -63,7 +69,7 @@ class SitemapService { | |||
| new File(grailsApplication.config.sitemap.dir + "/sitemap" + i + ".xml.tmp").renameTo(newFile) | |||
|
|
|||
| // add an entry for this new file | |||
| fw.write("<sitemap><url>" + grailsApplication.config.grails.serverURL + "/sitemap" + i + ".xml" + "</url>") | |||
| fw.write("<sitemap><loc>" + grailsApplication.config.grails.serverURL + "/sitemap" + i + ".xml" + "</loc>") | |||
There was a problem hiding this comment.
The correct syntax is <loc> and not <url>
| @@ -106,22 +112,23 @@ class SitemapService { | |||
| countUrls++ | |||
| } | |||
|
|
|||
| @Transactional | |||
There was a problem hiding this comment.
Scheduled runs crash with org.springframework.dao.DataAccessResourceFailureException: Could not obtain current Hibernate Session; nested exception is org.hibernate.HibernateException: No Session found for current thread. Manual runs from admin UI are ok. Adding @Transactional fixes the crash. But there might be other/better solutions?
| def buildSitemap() throws Exception { | ||
|
|
||
| Collection.findAll().each {Collection it -> | ||
| writeUrl(it.lastUpdated, "weekly", grailsApplication.config.grails.serverURL + "/public/show/co" + it.id) | ||
| writeUrl(it.lastUpdated, "weekly", grailsApplication.config.grails.serverURL + "/public/show/" + it.uid) |
There was a problem hiding this comment.
Should be uid and not id. SELECT id, uid FROM collection order by 1; will show that they are not the same (at least not in a database where items have been deleted).
This PR fixes the following issues in SitemapService (#244):
<url>tags that should be<loc>idis used instead ofuidwhen creating url:s which is incorrectSee code comments below for details.