Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CELDEV-1006 - refactor AttachmentURLCommand to ResourceUrlService #297

Draft
wants to merge 32 commits into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
80c1f47
refactor AttachmentURLCommand to RessourceUrlService
fpichler Feb 2, 2022
1280d39
rename ressource_uri package to com.celements.filebase.uri
fpichler Feb 3, 2022
80b0d1a
rename RessourceUrlXXX to FileUriXXX
fpichler Feb 3, 2022
47750ab
update components.txt
fpichler Feb 3, 2022
588f034
inline getAction
fpichler Feb 3, 2022
150a7fc
inline getAction
fpichler Feb 3, 2022
3b0ec74
reset refactorings before deprecation
fpichler Feb 7, 2022
9e3352c
overwrite hashCode, equals and toString
fpichler Feb 7, 2022
b1c77ba
correct javadoc links
fpichler Feb 7, 2022
37b6138
remove not used modelAccess mock
fpichler Feb 7, 2022
ab8fd7c
add FileReference
fpichler Feb 7, 2022
504b79c
refactor FileUriService to use FileReference
fpichler Feb 7, 2022
b9feaa7
change return types to UriBuilder
fpichler Feb 7, 2022
8933716
add FileUriScriptService
fpichler Feb 7, 2022
69aed61
update scheduler plugin dependency
fpichler Feb 7, 2022
0f4570c
refactorings
fpichler Feb 8, 2022
ee3a2bd
change createFileUrl to return
fpichler Feb 8, 2022
a74d3f9
remove getExternalFileURL and instead use createAbsoluteFileUri
fpichler Feb 8, 2022
c62b5f6
mark deprecated
fpichler Feb 8, 2022
09a4c02
add configsource
fpichler Feb 8, 2022
c507cc8
fix testing
fpichler Feb 8, 2022
6f4b916
set FileReference to final.
fpichler Feb 8, 2022
229455d
adding Singleton annotations
fpichler Feb 8, 2022
f2fa74c
Update src/main/java/com/celements/filebase/references/FileReference.…
fpichler Feb 9, 2022
334d07e
inline FileReferenceType
fpichler Feb 9, 2022
eb05687
Merge branch 'CELDEV-1006' of [email protected]:celements/celements-core…
fpichler Feb 9, 2022
094d2c0
Merge branch 'CELDEV-1006' of
fpichler Feb 9, 2022
38fdb38
add NotNull/NotEmpty/Nullable annotations
fpichler Feb 9, 2022
4c02d4f
remove ImmutableDocumentReference
msladek Feb 11, 2022
4c2a217
Merge branch 'CELDEV-1006' of github.com:celements/celements-core int…
msladek Feb 11, 2022
2ccb773
Merge branch 'dev' into CELDEV-1006
fpichler Apr 12, 2022
d05cae7
redefine FileUriServiceRole as FileUrlServiceRole
fpichler Apr 20, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.xpn.xwiki.platform.plugins</groupId>
<groupId>com.celements</groupId>
<artifactId>xwiki-plugin-scheduler</artifactId>
<scope>provided</scope>
</dependency>
Expand Down
202 changes: 202 additions & 0 deletions src/main/java/com/celements/filebase/references/FileReference.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
package com.celements.filebase.references;

import static com.google.common.base.Preconditions.*;

import java.io.Serializable;
import java.util.Objects;
import java.util.function.Supplier;
import java.util.regex.Pattern;

import javax.annotation.Nullable;
import javax.annotation.concurrent.NotThreadSafe;
import javax.validation.constraints.NotEmpty;
import javax.validation.constraints.NotNull;
import javax.ws.rs.core.UriBuilder;

import org.xwiki.model.reference.DocumentReference;

import com.celements.model.util.ModelUtils;
import com.google.common.base.Strings;
import com.google.common.base.Suppliers;
import com.google.errorprone.annotations.Immutable;
import com.xpn.xwiki.web.Utils;

@Immutable
public final class FileReference implements Serializable {

private static final long serialVersionUID = 1L;

@NotThreadSafe
public static final class Builder {

private static final String ATTACHMENT_LINK_REGEX = "([\\w\\-]*:)?([\\w\\-]*\\.[\\w\\-]*){1};.*";
private static final Supplier<Pattern> ATTACHMENT_LINK_PATTERN = Suppliers
.memoize(() -> Pattern.compile(ATTACHMENT_LINK_REGEX));
private static final String ON_DISK_LINK_REGEX = "^:[/\\w\\-\\.]*";
private static final Supplier<Pattern> ON_DISK_LINK_PATTERN = Suppliers
.memoize(() -> Pattern.compile(ON_DISK_LINK_REGEX));
fpichler marked this conversation as resolved.
Show resolved Hide resolved

private String name;
private FileReferenceType type;
private DocumentReference docRef;
private String fullPath;
private String queryString;

@NotNull
private static String getAttachmentName(@NotEmpty String link) {
return link.split(";")[1];
}

@NotNull
private static String getPathFileName(@NotEmpty String link) {
String[] linkParts = link.split("/");
return linkParts[linkParts.length - 1];
}

private static boolean isAttachmentLink(@Nullable String link) {
if (link != null) {
return ATTACHMENT_LINK_PATTERN.get().matcher(link.trim()).matches();
}
return false;
}

private static boolean isOnDiskLink(@Nullable String link) {
if (link != null) {
return ON_DISK_LINK_PATTERN.get().matcher(link.trim()).matches();
}
return false;
}

@NotNull
private static DocumentReference getPageDocRef(@NotNull String link) {
return Utils.getComponent(ModelUtils.class).resolveRef(link.split(";")[0],
DocumentReference.class);
}

private static FileReferenceType getTypeOfLink(@NotEmpty String link) {
if (isOnDiskLink(link)) {
return FileReferenceType.ON_DISK;
} else if (isAttachmentLink(link)) {
return FileReferenceType.ATTACHMENT;
}
return FileReferenceType.EXTERNAL;
}

public Builder setFileName(@NotNull String fileName) {
checkNotNull(fileName);
this.name = fileName;
return this;
}

public Builder setType(@NotNull FileReferenceType type) {
checkNotNull(type);
this.type = type;
return this;
}

public void setDocRef(@NotNull DocumentReference docRef) {
checkNotNull(docRef);
this.docRef = docRef;
}

public void setFullPath(@NotNull String fullPath) {
checkNotNull(fullPath);
this.fullPath = fullPath;
}

public void setQueryString(@NotNull String queryString) {
checkNotNull(queryString);
this.queryString = queryString;
}

public FileReference build() {
return new FileReference(this);
}

}

private final String name;
private final FileReferenceType type;
private final DocumentReference docRef;
private final String fullPath;
private final String queryString;

public FileReference(Builder builder) {
fpichler marked this conversation as resolved.
Show resolved Hide resolved
this.name = builder.name;
this.type = builder.type;
this.fullPath = builder.fullPath;
this.docRef = builder.docRef;
this.queryString = builder.queryString;
}

public String getName() {
fpichler marked this conversation as resolved.
Show resolved Hide resolved
return name;
}

public FileReferenceType getType() {
return type;
}

public DocumentReference getDocRef() {
return docRef;
}

public String getFullPath() {
return fullPath;
}

public String getQueryString() {
return queryString;
}

public UriBuilder getUri() {
return UriBuilder.fromPath(fullPath).replaceQuery(queryString);
}

public boolean isAttachmentReference() {
return type == FileReferenceType.ATTACHMENT;
}

public boolean isOnDiskReference() {
return type == FileReferenceType.ON_DISK;
}

@Override
public int hashCode() {
return Objects.hash(name, type, docRef, fullPath);
}

@Override
public boolean equals(@Nullable Object obj) {
return (obj instanceof FileReference)
&& Objects.equals(((FileReference) obj).name, this.name)
&& Objects.equals(((FileReference) obj).type, this.type)
&& Objects.equals(((FileReference) obj).docRef, this.docRef)
&& Objects.equals(((FileReference) obj).fullPath, this.fullPath);
}

@Override
public String toString() {
return "FileReference [name=" + name + ", type=" + type + ", docRef=" + docRef + ", fullPath="
+ fullPath + "]";
}

public static Builder of(@NotEmpty String link) {
fpichler marked this conversation as resolved.
Show resolved Hide resolved
checkArgument(!Strings.isNullOrEmpty(link), "link may not be empty");
final String[] linkParts = link.split("\\?");
Builder builder = new Builder();
builder.setType(Builder.getTypeOfLink(linkParts[0]));
if (builder.type == FileReferenceType.ATTACHMENT) {
builder.setFileName(Builder.getAttachmentName(linkParts[0]));
builder.setDocRef(Builder.getPageDocRef(linkParts[0]));
} else {
builder.setFileName(Builder.getPathFileName(linkParts[0]));
builder.setFullPath(linkParts[0]);
}
if (linkParts.length > 1) {
builder.setQueryString(linkParts[1]);
}
return builder;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.celements.filebase.references;

public enum FileReferenceType {
fpichler marked this conversation as resolved.
Show resolved Hide resolved

ON_DISK, ATTACHMENT, EXTERNAL;

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.celements.filebase.uri;

import javax.validation.constraints.NotNull;

import com.celements.filebase.references.FileReference;

public class FileNotExistException extends Exception {

private static final long serialVersionUID = 1L;

private final FileReference fileReference;

public FileNotExistException(@NotNull FileReference fileRef) {
super("Url fileRef [" + fileRef + "] does not exist.");
this.fileReference = fileRef;
}

public FileNotExistException(@NotNull String message, @NotNull FileReference fileRef) {
super(message);
this.fileReference = fileRef;
}

public FileNotExistException(@NotNull String message, @NotNull FileReference fileRef,
Exception wrapExp) {
super(message, wrapExp);
this.fileReference = fileRef;
}

public FileReference getFileReference() {
return fileReference;
}
}
105 changes: 105 additions & 0 deletions src/main/java/com/celements/filebase/uri/FileUriScriptService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package com.celements.filebase.uri;

import java.util.Optional;

import javax.annotation.Nullable;
import javax.validation.constraints.NotEmpty;
import javax.validation.constraints.NotNull;
import javax.ws.rs.core.UriBuilder;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.xwiki.component.annotation.Component;
import org.xwiki.component.annotation.Requirement;
import org.xwiki.script.service.ScriptService;

import com.celements.filebase.references.FileReference;
import com.google.common.base.Strings;

import groovy.lang.Singleton;
fpichler marked this conversation as resolved.
Show resolved Hide resolved

@Component(FileUriScriptService.NAME)
@Singleton
public class FileUriScriptService implements ScriptService {

public static final String NAME = "fileUri";

private static final Logger LOGGER = LoggerFactory.getLogger(FileUriScriptService.class);

@Requirement
private FileUriServiceRole fileUriService;

@NotNull
public FileReference createFileReference(@NotEmpty String link) {
return FileReference.of(link).build();
}

@NotNull
public UriBuilder createFileUrl(@Nullable String fileRef) {
return createFileUrl(fileRef, null);
}

@NotNull
public UriBuilder createFileUrl(@Nullable String fileRef, @Nullable String action) {
return createFileUrl(fileRef, action, null);
}

@NotNull
public UriBuilder createFileUrl(@Nullable String fileRef, @Nullable String action,
@Nullable String queryString) {
if (!Strings.isNullOrEmpty(fileRef)) {
return createFileUrl(createFileReference(fileRef), action, queryString);
}
return UriBuilder.fromPath("");
}

@NotNull
public UriBuilder createFileUrl(@Nullable FileReference fileRef, @Nullable String action,
@Nullable String queryString) {
if (fileRef != null) {
try {
return fileUriService.createFileUri(fileRef, Optional.ofNullable(action),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is the service method called ...Uri, the SS name ...Url but both return a UriBuilder? a few thoughts:

  1. velocity cannot handle the exceptions thrown by UriBuilder#build and URI#toURL. So if we actually handle URLs here, the java logic should capture any MalformedURLException.
  2. Afaik the FileUriService actually builds URLs, so why not return URL objects? This would encapsulate the handling of URL malformation inside the service and make it's usage more robust. If a client wants to change the URL, it can always create a UriBuilder from it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to 1. There is a #try statement for velocity
to 2. Nope URL must be fully qualified. UriBuilder can handle e.g. "/folder/to/file.js" which is not fully quallified and returns a correct toString()

Copy link
Member

@msladek msladek Feb 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I was referring to calling UriBuilder#build and URI#toURL within velocity. the try/catch here won't prevent MalformedURLException thrown outside of the try/catch.
  2. it can be misleading calling the SS method names ...Url... if they don't return at minimum Url's but incomplete Uri's. The service methods are named ...Uri.... Why this inconsistency?

Optional.ofNullable(queryString));
} catch (FileNotExistException exp) {
LOGGER.info("createFileUrl for [{}] with action [{}] an queryString [{}] failed.", fileRef,
action, queryString, exp);
}
}
return UriBuilder.fromPath("");
}

@NotNull
public UriBuilder getFileURLPrefix() {
return fileUriService.getFileUriPrefix(Optional.empty());
}

@NotNull
public UriBuilder getFileURLPrefix(@Nullable String action) {
return fileUriService.getFileUriPrefix(Optional.ofNullable(action));
}

@NotNull
public UriBuilder createAbsoluteFileUri(@Nullable String fileRef, @Nullable String action) {
return createAbsoluteFileUri(fileRef, action, null);
}

@NotNull
public UriBuilder createAbsoluteFileUri(@Nullable String fileRef, String action,
@Nullable String queryString) {
if (!Strings.isNullOrEmpty(fileRef)) {
return createAbsoluteFileUri(createFileReference(fileRef), action, queryString);
}
return UriBuilder.fromPath("");
}

@NotNull
public UriBuilder createAbsoluteFileUri(@Nullable FileReference fileRef, String action,
@Nullable String queryString) {
if (fileRef != null) {
return fileUriService.createAbsoluteFileUri(fileRef, Optional.ofNullable(action),
Optional.ofNullable(queryString));
}
return UriBuilder.fromPath("");
}

}
Loading