RWS / dxa-web-application-java

SDL Digital Experience Accelerator Java Spring MVC web application
25 stars 37 forks source link

isNoMediaCache in processBinaryComponent of GraphQLStaticContentResolver #127

Open EBroersen opened 4 years ago

EBroersen commented 4 years ago

We were wondering whether the code for checking isNoMediaCache might be incorrect.

        if (!requestDto.isNoMediaCache()) {
            log.debug("File cannot be cached: {}", file.getAbsolutePath());
            binaryComponent.setLastPublishDate(new DateTime().toString());
        }

We always get a 200 ok for an image request and never receive a 304 on an image. We found out the a new Date is always passed as lastModifiedDate in the browser which led us to this code. We were wondering if the ! should be presented here.

Regards Erwin Broersen

alebastrov commented 3 years ago

Look like this double negation matters.

koteswar-k commented 3 years ago

Hi @alebastrov

StaticContentRequestDto requestDto = StaticContentRequestDto.builder(path, localizationId).localizationPath(localizationPath).baseUrl(this.webRequestContext.getBaseUrl()).noMediaCache(!essentialConfiguration && this.webRequestContext.isSessionPreview()).build();

or

private boolean isNoMediaCache(String path, String localizationPath) { return !FileUtils.isEssentialConfiguration(path, localizationPath) && this.webRequestContext.isSessionPreview(); }

The above code always return false because of && this.webRequestContext.isSessionPreview() on live environment. so !requestDto.isNoMediaCache() become true and it always set last publish date for binary component.

As per our analysis, double negation is not required. or are we missing something?