Gitlab Community Edition Instance

Commit 14769745 authored by mhellka's avatar mhellka
Browse files

Allow partial getArchiveInfo if sub-resources are unavailable.

Fix: Do not require list_files permission just to show the file count.
Change: Return a partial ArchiveInfo even if sub-resources requested with
'with' are not available due to access restrictions.

This change should help clients trying to get as much information as
possible with a single request and reduce complexity and latency when
viewing cdstar archives.
parent 7a22d7e4
Pipeline #118354 passed with stages
in 8 minutes and 11 seconds
...@@ -257,7 +257,10 @@ endpoints: ...@@ -257,7 +257,10 @@ endpoints:
with: with:
type: list(enum) type: list(enum)
help: | help: |
Include additional information in the response. Include additional information in the response. This can be used as a shortcut for individual
requests to <<getArchiveACL>>, <<getFileList>> or <<getArchiveMetadata>>. If access
restrictions do not allow reading a subresource, the flag is silently ignored.
acl:: Include `acl` field with an <<AclInfo>> in the response. acl:: Include `acl` field with an <<AclInfo>> in the response.
files:: Include `files` field with a list of <<FileInfo>> in the response. This is implicitly enabled if any of the file listing parameters are present. files:: Include `files` field with a list of <<FileInfo>> in the response. This is implicitly enabled if any of the file listing parameters are present.
meta:: Include `meta` field with a <<MetaAttributes>> in the response. If files are listed, their <<FileInfo>> will also contain an additional `meta` field. meta:: Include `meta` field with a <<MetaAttributes>> in the response. If files are listed, their <<FileInfo>> will also contain an additional `meta` field.
......
...@@ -120,9 +120,10 @@ public interface CDStarArchive extends CDStarAnnotateable { ...@@ -120,9 +120,10 @@ public interface CDStarArchive extends CDStarAnnotateable {
*/ */
List<CDStarFile> getFiles(); List<CDStarFile> getFiles();
default int getFileCount() { /**
return getFiles().size(); * Get file count without read_files permission.
} */
int getFileCount();
/** /**
* Return true if the file exists. * Return true if the file exists.
......
...@@ -46,6 +46,7 @@ import de.gwdg.cdstar.runtime.client.CDStarSession; ...@@ -46,6 +46,7 @@ import de.gwdg.cdstar.runtime.client.CDStarSession;
import de.gwdg.cdstar.runtime.client.auth.ArchivePermission; import de.gwdg.cdstar.runtime.client.auth.ArchivePermission;
import de.gwdg.cdstar.runtime.client.auth.ArchivePermissionSet; import de.gwdg.cdstar.runtime.client.auth.ArchivePermissionSet;
import de.gwdg.cdstar.runtime.client.auth.StringSubject; import de.gwdg.cdstar.runtime.client.auth.StringSubject;
import de.gwdg.cdstar.runtime.client.exc.AccessError;
import de.gwdg.cdstar.runtime.client.exc.ArchiveNotFound; import de.gwdg.cdstar.runtime.client.exc.ArchiveNotFound;
import de.gwdg.cdstar.runtime.client.exc.VaultNotFound; import de.gwdg.cdstar.runtime.client.exc.VaultNotFound;
import de.gwdg.cdstar.ta.exc.TARollbackException; import de.gwdg.cdstar.ta.exc.TARollbackException;
...@@ -140,21 +141,35 @@ public class ArchiveEndpoint implements RestBlueprint { ...@@ -140,21 +141,35 @@ public class ArchiveEndpoint implements RestBlueprint {
info.created = archive.getCreated(); info.created = archive.getCreated();
info.modified = archive.getContentModified(); info.modified = archive.getContentModified();
info.owner = archive.getOwner(); info.owner = archive.getOwner();
info.file_count = archive.getFiles().size(); // TODO: Speed this up info.file_count = archive.getFileCount();
info.profile = archive.getProfile().getName(); info.profile = archive.getProfile().getName();
info.state = ModelHelper.getArchiveState(archive); info.state = ModelHelper.getArchiveState(archive);
ctx.header("Last-Modified", archive.getModified()); ctx.header("Last-Modified", archive.getModified());
if (showACL) if (showACL) {
info.acl = getArchiveAcl(archive, true); try {
info.acl = getArchiveAcl(archive, true);
} catch (final AccessError e) {
// Just keep it blank
}
}
if (showMeta) if (showMeta) {
info.meta = getArchiveMeta(archive); try {
info.meta = getArchiveMeta(archive);
} catch (final AccessError e) {
// Just keep it blank
}
}
if (showFiles) { if (showFiles) {
info.files = getFilesFiltered(archive, qh); try {
info.files = getFilesFiltered(archive, qh);
} catch (final AccessError e) {
// Just keep it blank
}
} }
return info; return info;
......
...@@ -13,6 +13,7 @@ import javax.ws.rs.client.Entity; ...@@ -13,6 +13,7 @@ import javax.ws.rs.client.Entity;
import javax.ws.rs.client.WebTarget; import javax.ws.rs.client.WebTarget;
import javax.ws.rs.core.Response.Status; import javax.ws.rs.core.Response.Status;
import org.glassfish.jersey.media.multipart.FormDataMultiPart;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
...@@ -148,23 +149,59 @@ public class ArchiveInfoTest extends BaseRestTest { ...@@ -148,23 +149,59 @@ public class ArchiveInfoTest extends BaseRestTest {
@Test @Test
public void testGetEmbeddedSubresources() { public void testGetEmbeddedSubresources() {
addFile("foo", "text/plain", "text");
final WebTarget baseTarget = target("/v3/test/", archiveId); final WebTarget baseTarget = target("/v3/test/", archiveId);
baseTarget.queryParam("with", "acl").request().get(); baseTarget.queryParam("with", "acl").request().get();
assertEquals("OWNER", getJsonString("acl.$owner.0"));
assertTrue(findJson("meta").isMissingNode()); assertTrue(findJson("meta").isMissingNode());
assertTrue(findJson("files").isMissingNode());
assertEquals("OWNER", getJsonString("acl.$owner.0"));
baseTarget.queryParam("with", "meta").request().get(); baseTarget.queryParam("with", "meta").request().get();
assertTrue(findJson("acl").isMissingNode()); assertTrue(findJson("acl").isMissingNode());
assertTrue(findJson("files").isMissingNode());
assertTrue(findJson("meta").isObject()); assertTrue(findJson("meta").isObject());
baseTarget.queryParam("with", "acl", "meta").request().get(); baseTarget.queryParam("with", "files").request().get();
assertTrue(findJson("acl").isMissingNode());
assertTrue(findJson("meta").isMissingNode());
assertTrue(findJson("files").isArray());
baseTarget.queryParam("with", "acl", "meta", "files").request().get();
assertEquals("OWNER", getJsonString("acl.$owner.0")); assertEquals("OWNER", getJsonString("acl.$owner.0"));
assertTrue(findJson("meta").isObject()); assertTrue(findJson("meta").isObject());
assertTrue(findJson("files").isArray());
baseTarget.queryParam("with", "acl,meta").request().get(); baseTarget.queryParam("with", "acl,meta,files").request().get();
assertEquals("OWNER", getJsonString("acl.$owner.0")); assertEquals("OWNER", getJsonString("acl.$owner.0"));
assertTrue(findJson("meta").isObject()); assertTrue(findJson("meta").isObject());
assertTrue(findJson("files").isArray());
}
/**
* Requests for embedded sub-resources should not fail the entire request if
* only a sub-resource is unavailable.
*/
@Test
public void testProtectedEmbeddedSubresources() {
addFile("foo", "text/plain", "text");
// Give "test" only load access
final FormDataMultiPart formData = new FormDataMultiPart();
formData.field("acl:test2", "load");
target("/v3/test/", archiveId, "/").request().post(Entity.entity(formData, formData.getMediaType()));
// Create test2 user and login
getAuthRealm().account("test2").password("test2").withPermissions("vault:test:*");
authBasic("test2", "test2");
target("/v3/test/", archiveId)
.queryParam("with", "acl", "meta", "files").request().get();
assertStatus(200);
assertTrue(findJson("acl").isMissingNode());
assertTrue(findJson("meta").isMissingNode());
assertTrue(findJson("files").isMissingNode());
} }
@Test @Test
......
...@@ -346,6 +346,11 @@ class ArchiveImpl implements CDStarArchive { ...@@ -346,6 +346,11 @@ class ArchiveImpl implements CDStarArchive {
return Collections.unmodifiableList(getInternalFileList()); return Collections.unmodifiableList(getInternalFileList());
} }
@Override
public int getFileCount() {
return getInternalFileList().size();
}
@Override @Override
public boolean hasFile(String name) { public boolean hasFile(String name) {
checkPermission(ArchivePermission.LIST_FILES); checkPermission(ArchivePermission.LIST_FILES);
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment