Request only hash from dev-server

[caalador]

In dev mode when parsing the stats file for
template sources only request the latest
stats hash from dev-server.

This instead of requesting the whole
stats file for checking the hash before choosing
if the cache should be updated.

Fixes #6191

---

This change is 

[ujoni]

Reviewed 1 of 3 files at r1, 2 of 2 files at r2.
Reviewable status: 5 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @caalador)

---

flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/NpmTemplateParser.java, line 33 at r2 (raw file):

import com.vaadin.flow.component.dependency.JsModule;
import com.vaadin.flow.function.DeploymentConfiguration;

Unrelated to the actual PR, but why in the world is DeploymentConfiguration in flow.function? 😕

---

flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/NpmTemplateParser.java, line 175 at r2 (raw file):

                .equals(hash)) {
            // Only load for null jsonStats if we use bundle files.
            if(jsonStats == null || !usesBundleFile(config)) {

The comment and if clause do not match. The comment states that "Only load for null jsonStats if.." but the if clause starts as "Always load for null jsonStats" (since it is followed by or operator). What is the correct approach here?

If we follow the comment, the if would be if (jsonStats != null || usesBundleFile(config))

---

flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/NpmTemplateParser.java, line 201 at r2 (raw file):

    }

    private void updateCache(String url, String fileContents) {

This method is now in a weird place, since half of its code is duplicated getSourcesFromStats and it is called from one place only, getSourcesFromStats.

Should thing be changed into

  private String getCachedStatsSource(String url) {
     if (jsonStats != null) {
        return cache.computeIfAbsent(url, u -> BundleParser.getSourceFromStatistics(u, jsonStats);
     } else {
        return null;
     }
  }

The only current call to this method would be replaced by setting the jsonStats and getSourcesFromStats would return value from this method. I guess the setting of jsonStats would need to reset the cache.

Idk. Thoughts?

---

flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java, line 397 at r2 (raw file):

        }

        return "";

Why empty string? Is this some magical effective java trick I have not encountered?

[caalador]

Reviewable status: 5 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @caalador and @ujoni)

---

flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/NpmTemplateParser.java, line 33 at r2 (raw file):

Previously, ujoni (Joni) wrote…

Unrelated to the actual PR, but why in the world is DeploymentConfiguration in flow.function? 😕

#2435
No extra memory of it.

---

flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/NpmTemplateParser.java, line 175 at r2 (raw file):

Previously, ujoni (Joni) wrote…

The comment and if clause do not match. The comment states that "Only load for null jsonStats if.." but the if clause starts as "Always load for null jsonStats" (since it is followed by or operator). What is the correct approach here?

If we follow the comment, the if would be if (jsonStats != null || usesBundleFile(config))

Well the hard part is that we should

• always load if jsonStats is null.
• else never load when we have a bundle as it never changes.
• always load a new stats if the hash has changed, but we do not have a bundle.

---

flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/NpmTemplateParser.java, line 201 at r2 (raw file):

Previously, ujoni (Joni) wrote…

This method is now in a weird place, since half of its code is duplicated getSourcesFromStats and it is called from one place only, getSourcesFromStats.

Should thing be changed into

  private String getCachedStatsSource(String url) {
     if (jsonStats != null) {
        return cache.computeIfAbsent(url, u -> BundleParser.getSourceFromStatistics(u, jsonStats);
     } else {
        return null;
     }
  }

The only current call to this method would be replaced by setting the jsonStats and getSourcesFromStats would return value from this method. I guess the setting of jsonStats would need to reset the cache.

Idk. Thoughts?

Update chache is refresh cache eg. clear and parse the new stats. renamed and moved the lock so we lock at the correct position.

---

flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java, line 397 at r2 (raw file):

Previously, ujoni (Joni) wrote…

Why empty string? Is this some magical effective java trick I have not encountered?

The magic is that we always want to do a stats equals hash check and for production mode we do not care what the hash is as we load the stats once and do not want to load it another time for getting the hash as we know that it doesn't change. as such it's fine to return anything at this point if we are not in dev mode.

[ujoni]

Reviewable status: 2 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @caalador and @ujoni)

---

flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/NpmTemplateParser.java, line 175 at r2 (raw file):

Previously, caalador wrote…

Well the hard part is that we should

• always load if jsonStats is null.
• else never load when we have a bundle as it never changes.
• always load a new stats if the hash has changed, but we do not have a bundle.

Alright, the if matches that. Maybe just slap that bullet point as the comment :D

---

flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendUtils.java, line 397 at r2 (raw file):

Previously, caalador wrote…

The magic is that we always want to do a stats equals hash check and for production mode we do not care what the hash is as we load the stats once and do not want to load it another time for getting the hash as we know that it doesn't change. as such it's fine to return anything at this point if we are not in dev mode.

Fair enough

[caalador]

Reviewable status: 2 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @caalador and @ujoni)

---

flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/NpmTemplateParser.java, line 175 at r2 (raw file):

Previously, ujoni (Joni) wrote…

Alright, the if matches that. Maybe just slap that bullet point as the comment :D

added

[ujoni]

Reviewed 1 of 1 files at r3.
Reviewable status: 3 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @caalador and @ujoni)

[ujoni]

Reviewable status: 3 unresolved discussions, 1 of 1 LGTMs obtained (waiting on @caalador and @ujoni)

[caalador]

Reviewable status: 3 unresolved discussions, 0 of 1 LGTMs obtained, and 1 stale (waiting on @caalador and @ujoni)

---

flow-server/src/main/java/com/vaadin/flow/component/polymertemplate/NpmTemplateParser.java, line 175 at r2 (raw file):

Previously, caalador wrote…

added

simplified.

[ujoni]

Reviewed 2 of 2 files at r4.
Reviewable status: 2 unresolved discussions, 1 of 1 LGTMs obtained (waiting on @caalador)