vaadin / framework

Vaadin 6, 7, 8 is a Java framework for modern Java web applications.
http://vaadin.com/
Other
1.78k stars 729 forks source link

Can't modify Version for theme caching #6984

Open vaadin-bot opened 9 years ago

vaadin-bot commented 9 years ago

Originally by micael.kirouac@telus.com


Some nice changes were made to better support caching of themes when upgrading vaadin version: https://dev.vaadin.com/changeset/65904ffbdebc861a45c1b504d244d02a12f4561b/vaadin.

This is fine. However, if we modify our theme without changing version, we still have the caching problem. It would be nice if we had a way to modify that version or append something to it like "customer-version".

Another alternative would be to have the setupMainDiv method protected so we could override it, or to have a getVersion() method inside BootstrapHandler.

By the way the comment of the setupMainDiv method states "Override this method if you want to add some custom html around around the div element into which the actual Vaadin application will be rendered" but it is private.


Imported from https://dev.vaadin.com/ issue #18495

vaadin-bot commented 9 years ago

Originally by mrotteveel


A solution that generates a version based on the last modification date of the style would be preferable.

vaadin-bot commented 9 years ago

Originally by @Legioth


I have a feeling there's already an older ticket for this, but I'm on a quite bad network connection right now so I'd better not try to find one right now.

Using the modification timestamp (or even better: a hash of the contents) would be good in most cases, but there might be some issues since there's no guarantee that the file is actually deployed on the server running the java. It might be acceptable to simply not support automatic detection in such cases, but it should also be explicitly tested that nothing breaks down if the theme file is not found from the expected location.

vaadin-bot commented 9 years ago

Originally by @Artur-


Some more ideas in the duplicate #19197

vaadin-bot commented 9 years ago

Originally by @Legioth


Replying to Signell:

Some more ideas in the duplicate #19197

Based on the offline discussion related to that ticket, my suggested approach would be to add an overrideable method to VaadinService that by default checks the modification timestamp of the styles.css file if it can be found, and otherwise it returns null to signal that no versioning parameter should be added to the url. Users with more complex setups (e.g. styles.css served from some external location or styles.css importing other css files) can instead create their own subclass of VaadinService where the method is overridden to use some other semantics, e.g. checking a system property.

Care should also be taken since the theme can be defined through two different paths, one going through the bootstrap page and the other through UIState.

vaadin-bot commented 9 years ago

Originally by nfoerderreuther


We had the same problem, our solution is a patch in vaadin-server.jar and vaadin-client.jar. This could be integrated easily into the vaadin source code.

  1. New optional file "version.info" in the root of the theme, containing a timestamp. We generate this file via build script (Maven, Gradle).
  2. Change line 212 in vaadinBootstrap.js to
    loadTheme(themeUri, versionInfo && (versionInfo['themeVersion'] || versionInfo['vaadinVersion']));
  3. Subclass of ServletBootstrapHandler, overriding getApplicationParameters(BoostrapContext). This should be integrated in com.vaadin.server.BootstrapHandler. Example implementation (has to be adapted if integrated into BootstrapHandler):
    private Map<String, Optional<String>> themeVersions = newHashMap();

    @Override
    protected JsonObject getApplicationParameters(BootstrapContext context)
    {
        JsonObject json = super.getApplicationParameters(context);

        Optional<String> themeVersion = getThemeVersion(context.getThemeName());
        JsonObject versionInfo = json.get("versionInfo");
        if (versionInfo != null && themeVersion.isPresent())
        {
            versionInfo.put("themeVersion", themeVersion.get());
        }

        return json;
    }

    private Optional<String> getThemeVersion(String theme)
    {
        if (theme == null)
        {
            return Optional.<String> absent();
        }

        if (!themeVersions.containsKey(theme))
        {
            try
            {
                synchronized (this)
                {
                    if (!themeVersions.containsKey(theme))
                    {
                        InputStream versionStream = getClass().getResourceAsStream(
                            "/VAADIN/themes/" + theme + "/version.info");
                        String version = IOUtils.toString(versionStream);
                        IOUtils.closeQuietly(versionStream);
                        if (StringUtils.isEmpty(version))
                        {
                            themeVersions.put(theme, Optional.of(version));
                        }
                        else
                        {
                            themeVersions.put(theme, Optional.<String> absent());
                        }
                    }
                }
            }
            catch (Exception e)
            {
                LoggerFactory.getLogger(RbfServletBootstrapHandler.class).warn(
                    "Cannot load version.info from theme, using default version. " + e);
            }
        }

        return themeVersions.containsKey(theme) ? themeVersions.get(theme) : Optional.<String> absent();
    }
  1. UIConnector.onThemeChange() has to be modified to use the new theme URL
vaadin-bot commented 8 years ago

Originally by mgrankvi


https://dev.vaadin.com/review/12557

vaadin-bot commented 8 years ago

Originally by imsandli


Will this change go into a 7.6.x?

stale[bot] commented 6 years ago

A lot of tickets have been left hanging in the issue tracker through the years. Some of them are still relevant, some of them have been fixed a long time ago and some are no longer valid. To get a better look on what is important and still relevant, we are closing old tickets which have not been touched in a long time. No further work will be done on this ticket. If the ticket seems to be still actual, please verify the problem existence over latest framework version and then open a new ticket in vaadin/framework with all the suitable information.

wolfey commented 5 years ago

Whatever happened to this? It looks like the code review was fine, but it was never merged?! We would love to have this functionality in our Vaadin 8 installation. Any thoughts?

stale[bot] commented 4 years ago

Hello there!

We are sorry that this issue hasn't progressed lately. We are prioritizing issues by severity and the number of customers we expect are experiencing this and haven't gotten around to fix this issue yet.

There are a couple of things you could help to get things rolling on this issue (this is an automated message, so expect that some of these are already in use):

Thanks again for your contributions! Even though we haven't been able to get this issue fixed, we hope you to report your findings and enhancement ideas in the future too!