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.

Sign up for GitHub

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

Jump to bottom

REST APIs that allows saving/edit/delete/search/share devfiles #17843

Merged
merged 15 commits into from
Oct 6, 2020
Merged

REST APIs that allows saving/edit/delete/search/share devfiles #17843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
e872904
REST APIs that allows saving/edit/delete/search/share devfiles
skabashnyuk Sep 17, 2020
2278504
Devfile integration tests
skabashnyuk Sep 17, 2020
e89ff75
Devfile permission implementation
skabashnyuk Sep 17, 2020
079e192
Devfile permission integration testing
skabashnyuk Sep 17, 2020
0d1fbe6
Devfile service assembly
skabashnyuk Sep 17, 2020
aa42eae
Merge remote-tracking branch 'upstream/master' into che16981_model2
skabashnyuk Sep 29, 2020
d790930
Applying reviewers suggestions
skabashnyuk Sep 29, 2020
50e9f09
Applying reviewers suggestions
skabashnyuk Sep 29, 2020
ca02dfb
Merge remote-tracking branch 'upstream/master' into che16981_model2
skabashnyuk Sep 30, 2020
f8cb092
Added explanation of what UserDevfile.namespace is
skabashnyuk Sep 30, 2020
a7f9eb3
Cover posibility of using invalid search and order filed names
skabashnyuk Oct 1, 2020
8546948
Merge remote-tracking branch 'upstream/master' into che16981_model2
skabashnyuk Oct 1, 2020
bf7faa7
Merge remote-tracking branch 'upstream/master' into che16981_model2
skabashnyuk Oct 5, 2020
b2580d0
Fixup
skabashnyuk Oct 5, 2020
9fca600
Merge remote-tracking branch 'upstream/master' into che16981_model2
skabashnyuk Oct 5, 2020
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
4 changes: 4 additions & 0 deletions assembly/assembly-wsmaster-war/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@
<groupId>org.eclipse.che.core</groupId>
<artifactId>che-core-api-core</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.che.core</groupId>
<artifactId>che-core-api-devfile</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.che.core</groupId>
<artifactId>che-core-api-factory</artifactId>
Expand Down
9 changes: 8 additions & 1 deletion assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ protected void configure() {
bind(WorkspaceEntityProvider.class);
bind(org.eclipse.che.api.workspace.server.TemporaryWorkspaceRemover.class);
bind(org.eclipse.che.api.workspace.server.WorkspaceService.class);
bind(org.eclipse.che.api.devfile.server.DevfileService.class);
bind(org.eclipse.che.api.devfile.server.UserDevfileEntityProvider.class);
install(new FactoryModuleBuilder().build(ServersCheckerFactory.class));

Multibinder<InternalEnvironmentProvisioner> internalEnvironmentProvisioners =
Expand Down Expand Up @@ -300,6 +302,7 @@ private void configureSingleUserMode(

install(new org.eclipse.che.api.user.server.jpa.UserJpaModule());
install(new org.eclipse.che.api.workspace.server.jpa.WorkspaceJpaModule());
install(new org.eclipse.che.api.devfile.server.jpa.UserDevfileJpaModule());

bind(org.eclipse.che.api.user.server.CheUserCreator.class);

Expand Down Expand Up @@ -361,6 +364,11 @@ private void configureMultiUserMode(
new org.eclipse.che.multiuser.permission.workspace.server.jpa
.MultiuserWorkspaceJpaModule());
install(new MultiUserWorkspaceActivityModule());
install(
new org.eclipse.che.multiuser.permission.devfile.server.jpa
.MultiuserUserDevfileJpaModule());
install(
new org.eclipse.che.multiuser.permission.devfile.server.UserDevfileApiPermissionsModule());

// Permission filters
bind(org.eclipse.che.multiuser.permission.system.SystemServicePermissionsFilter.class);
Expand All @@ -375,7 +383,6 @@ private void configureMultiUserMode(
bind(org.eclipse.che.multiuser.permission.user.UserServicePermissionsFilter.class);
bind(org.eclipse.che.multiuser.permission.logger.LoggerServicePermissionsFilter.class);

bind(org.eclipse.che.multiuser.permission.devfile.DevfilePermissionsFilter.class);
bind(org.eclipse.che.multiuser.permission.workspace.activity.ActivityPermissionsFilter.class);
bind(AdminPermissionInitializer.class).asEagerSingleton();
bind(
Expand Down
2 changes: 2 additions & 0 deletions assembly/assembly-wsmaster-war/src/main/resources/META-INF/persistence.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,14 @@
<class>org.eclipse.che.api.workspace.server.model.impl.devfile.CommandImpl</class>
<class>org.eclipse.che.api.workspace.server.model.impl.devfile.EndpointImpl</class>
<class>org.eclipse.che.api.workspace.server.devfile.SerializableConverter</class>
<class>org.eclipse.che.api.devfile.server.model.impl.UserDevfileImpl</class>

<class>org.eclipse.che.api.ssh.server.model.impl.SshPairImpl</class>

<class>org.eclipse.che.multiuser.api.permission.server.model.impl.SystemPermissionsImpl</class>
<class>org.eclipse.che.multiuser.api.permission.server.model.impl.AbstractPermissions</class>
<class>org.eclipse.che.multiuser.permission.workspace.server.model.impl.WorkerImpl</class>
<class>org.eclipse.che.multiuser.permission.devfile.server.model.impl.UserDevfilePermissionImpl</class>

<class>org.eclipse.che.multiuser.resource.spi.impl.FreeResourcesLimitImpl</class>
<class>org.eclipse.che.multiuser.resource.spi.impl.ResourceImpl</class>
Expand Down
34 changes: 34 additions & 0 deletions ...api-model/src/main/java/org/eclipse/che/api/core/model/workspace/devfile/UserDevfile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright (c) 2012-2018 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
*/
package org.eclipse.che.api.core.model.workspace.devfile;

/** Devfile that is persisted in permanent storage. */
public interface UserDevfile {
sparkoo marked this conversation as resolved.
Show resolved Hide resolved
/** Returns the identifier of this persisted devfile instance. It is mandatory and unique. */
String getId();

/** Returns the name of devfile. It is mandatory. */
String getName();

/**
* Returns the namespace also known as the account name. This name can be the name of the
* organization or the name of the user to which this devfile belong to. Namespace and name
* uniquely identify devfile.
*/
String getNamespace();
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 here namespace? I assumed that UserDevfile is really only template, so it's not a workspace instance that lives in the namespace. If I share my devfile with someone else, will I share the namespace as well? It does not make much sense to me, unless I'm missing something here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know the "namespace" here means "user or organization" but I am not sure we should stick to this Che6 nomenclature. It is highly confusing to use the same name for completely different things in the model and in the infrastructures.

What about calling this ownerName or accountName? It would IMHO slightly better express where this is coming from as well as imply the same semantics (at least to an extent) - the same owner cannot have two devfiles with the same name, yet devfiles with the same name can have different owners.

Copy link
Member

Choose a reason for hiding this comment

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

aha, that's what baffled me (#17843 (comment)) :D


/** Returns description of devfile */
String getDescription();

/** Returns devfile content */
Devfile getDevfile();
}
10 changes: 10 additions & 0 deletions multiuser/integration-tests/che-multiuser-cascade-removal/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@
<artifactId>che-core-api-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.che.core</groupId>
<artifactId>che-core-api-devfile</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.che.core</groupId>
<artifactId>che-core-api-factory</artifactId>
Expand Down Expand Up @@ -178,6 +183,11 @@
<artifactId>che-multiuser-machine-authentication</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.che.multiuser</groupId>
<artifactId>che-multiuser-permission-devfile</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.eclipse.che.multiuser</groupId>
<artifactId>che-multiuser-permission-workspace</artifactId>
Expand Down
48 changes: 47 additions & 1 deletion ...g/eclipse/che/multiuser/integration/jpa/cascaderemoval/JpaEntitiesCascadeRemovalTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
import static org.eclipse.che.multiuser.integration.jpa.cascaderemoval.TestObjectsFactory.createUser;
import static org.eclipse.che.multiuser.integration.jpa.cascaderemoval.TestObjectsFactory.createWorker;
import static org.eclipse.che.multiuser.integration.jpa.cascaderemoval.TestObjectsFactory.createWorkspace;
import static org.eclipse.che.multiuser.permission.devfile.server.UserDevfileDomain.DELETE;
import static org.eclipse.che.multiuser.permission.devfile.server.UserDevfileDomain.READ;
import static org.eclipse.che.multiuser.permission.devfile.server.UserDevfileDomain.UPDATE;
import static org.eclipse.che.multiuser.resource.spi.jpa.JpaFreeResourcesLimitDao.RemoveFreeResourcesLimitSubscriber;
import static org.mockito.Mockito.mock;
import static org.testng.Assert.assertEquals;
Expand All @@ -33,6 +36,7 @@
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;

import com.google.common.collect.ImmutableList;
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Injector;
Expand All @@ -54,6 +58,8 @@
import org.eclipse.che.api.core.NotFoundException;
import org.eclipse.che.api.core.ServerException;
import org.eclipse.che.api.core.notification.EventService;
import org.eclipse.che.api.devfile.server.model.impl.UserDevfileImpl;
import org.eclipse.che.api.devfile.server.spi.UserDevfileDao;
import org.eclipse.che.api.factory.server.jpa.FactoryJpaModule;
import org.eclipse.che.api.factory.server.model.impl.FactoryImpl;
import org.eclipse.che.api.factory.server.spi.FactoryDao;
Expand Down Expand Up @@ -106,6 +112,10 @@
import org.eclipse.che.multiuser.organization.spi.MemberDao;
import org.eclipse.che.multiuser.organization.spi.impl.MemberImpl;
import org.eclipse.che.multiuser.organization.spi.impl.OrganizationImpl;
import org.eclipse.che.multiuser.permission.devfile.server.jpa.MultiuserUserDevfileJpaModule;
import org.eclipse.che.multiuser.permission.devfile.server.model.impl.UserDevfilePermissionImpl;
import org.eclipse.che.multiuser.permission.devfile.server.spi.UserDevfilePermissionDao;
import org.eclipse.che.multiuser.permission.devfile.server.spi.jpa.JpaUserDevfilePermissionDao.RemoveUserDevfilePermissionsBeforeUserRemovedEventSubscriber;
import org.eclipse.che.multiuser.permission.workspace.server.jpa.MultiuserWorkspaceJpaModule;
import org.eclipse.che.multiuser.permission.workspace.server.spi.WorkerDao;
import org.eclipse.che.multiuser.resource.api.AvailableResourcesProvider;
Expand Down Expand Up @@ -143,6 +153,8 @@ public class JpaEntitiesCascadeRemovalTest {
private SshDao sshDao;
private FactoryDao factoryDao;
private WorkerDao workerDao;
private UserDevfilePermissionDao userDevfilePermissionDao;
private UserDevfileDao userDevfileDao;
private SignatureKeyDao signatureKeyDao;
private FreeResourcesLimitDao freeResourcesLimitDao;
private OrganizationManager organizationManager;
Expand Down Expand Up @@ -195,6 +207,9 @@ public class JpaEntitiesCascadeRemovalTest {

private FreeResourcesLimitImpl freeResourcesLimit2;

private UserDevfileImpl devfile;
private UserDevfilePermissionImpl devfilePermission;

private H2JpaCleaner h2JpaCleaner;

@BeforeMethod
Expand All @@ -221,6 +236,8 @@ protected void configure() {
install(new MultiuserWorkspaceJpaModule());
install(new MachineAuthModule());
install(new DevfileModule());
install(new MultiuserUserDevfileJpaModule());

bind(ExecutorServiceWrapper.class).to(NoopExecutorServiceWrapper.class);

bind(FreeResourcesLimitDao.class).to(JpaFreeResourcesLimitDao.class);
Expand Down Expand Up @@ -305,6 +322,8 @@ protected void configure() {
workspaceDao = injector.getInstance(WorkspaceDao.class);
factoryDao = injector.getInstance(FactoryDao.class);
workerDao = injector.getInstance(WorkerDao.class);
userDevfileDao = injector.getInstance(UserDevfileDao.class);
userDevfilePermissionDao = injector.getInstance(UserDevfilePermissionDao.class);
signatureKeyDao = injector.getInstance(SignatureKeyDao.class);
freeResourcesLimitDao = injector.getInstance(FreeResourcesLimitDao.class);
organizationManager = injector.getInstance(OrganizationManager.class);
Expand Down Expand Up @@ -334,10 +353,18 @@ public void shouldDeleteAllTheEntitiesWhenUserIsDeleted() throws Exception {
assertTrue(preferenceDao.getPreferences(user.getId()).isEmpty());
assertTrue(sshDao.get(user.getId()).isEmpty());
assertTrue(workspaceDao.getByNamespace(account.getName(), 30, 0).isEmpty());
assertTrue(userDevfileDao.getByNamespace(account.getName(), 30, 0).isEmpty());
assertTrue(factoryDao.getByUser(user.getId(), 30, 0).isEmpty());
// Check workers and parent entity is removed
assertTrue(workspaceDao.getByNamespace(user2.getId(), 30, 0).isEmpty());
assertTrue(userDevfileDao.getByNamespace(user2.getId(), 30, 0).isEmpty());
assertEquals(workerDao.getWorkers(workspace3.getId(), 1, 0).getTotalItemsCount(), 0);
assertNull(
notFoundToNull(
() ->
userDevfilePermissionDao.getUserDevfilePermission(devfile.getId(), user2.getId())));
assertFalse(userDevfileDao.getById(devfile.getId()).isPresent());

// Permissions are removed
// Non-removed user permissions and stack are present
// Check existence of organizations
Expand Down Expand Up @@ -391,6 +418,11 @@ public void shouldRollbackTransactionWhenFailedToRemoveAnyOfEntries(
assertNotNull(notFoundToNull(() -> organizationManager.getById(childOrganization.getId())));
assertNotNull(notFoundToNull(() -> organizationManager.getById(organization2.getId())));
assertNotNull(notFoundToNull(() -> signatureKeyDao.get(workspace2.getId())));
assertTrue(userDevfileDao.getById(devfile.getId()).isPresent());
assertNotNull(
notFoundToNull(
() ->
userDevfilePermissionDao.getUserDevfilePermission(devfile.getId(), user2.getId())));
assertFalse(
organizationResourcesDistributor.getResourcesCaps(childOrganization.getId()).isEmpty());
wipeTestData();
Expand All @@ -399,7 +431,11 @@ public void shouldRollbackTransactionWhenFailedToRemoveAnyOfEntries(
@DataProvider(name = "beforeRemoveRollbackActions")
public Object[][] beforeRemoveActions() {
return new Class[][] {
{RemoveOrganizationOnLastUserRemovedEventSubscriber.class, BeforeUserRemovedEvent.class}
{RemoveOrganizationOnLastUserRemovedEventSubscriber.class, BeforeUserRemovedEvent.class},
{
RemoveUserDevfilePermissionsBeforeUserRemovedEventSubscriber.class,
BeforeUserRemovedEvent.class
}
};
}

Expand Down Expand Up @@ -453,6 +489,13 @@ private void createTestData()
organizationResourcesDistributor.capResources(
childOrganization.getId(),
singletonList(new ResourceImpl(RamResourceType.ID, 1024, RamResourceType.UNIT)));

userDevfileDao.create(
devfile = TestObjectsFactory.createUserDevfile("id-dev1", "devfile1", account));
userDevfilePermissionDao.store(
devfilePermission =
new UserDevfilePermissionImpl(
devfile.getId(), user2.getId(), ImmutableList.of(READ, DELETE, UPDATE)));
}

private void prepareCreator(String userId) {
Expand All @@ -477,6 +520,9 @@ private void wipeTestData() throws ConflictException, ServerException, NotFoundE

workerDao.removeWorker(workspace3.getId(), user2.getId());

userDevfilePermissionDao.removeUserDevfilePermission(devfile.getId(), user2.getId());
userDevfileDao.remove(devfile.getId());

factoryDao.remove(factory1.getId());
factoryDao.remove(factory2.getId());

Expand Down
Loading