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

add generateName to devfile metadata #14157

Merged
merged 5 commits into from
Aug 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,7 @@ public interface Metadata {

/** @return the name of the devfile */
String getName();

sparkoo marked this conversation as resolved.
Show resolved Hide resolved
/** 'generateName' is used as a base string for generated name, when 'name' is not defined. */
String getGenerateName();
}
Original file line number Diff line number Diff line change
Expand Up @@ -195,5 +195,8 @@ public final class Constants {
/** Attribute of {@link Server} that specifies exposure of which port created the server */
public static final String SERVER_PORT_ATTRIBUTE = "port";

/** When generating workspace name from generateName, append this many characters. */
public static final int WORKSPACE_GENERATE_NAME_CHARS_APPEND = 5;

private Constants() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,6 @@ public interface MetadataDto extends Metadata {
void setName(String name);

MetadataDto withName(String name);

MetadataDto withGenerateName(String generateName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static org.eclipse.che.api.workspace.shared.Constants.STOPPED_ABNORMALLY_ATTRIBUTE_NAME;
import static org.eclipse.che.api.workspace.shared.Constants.STOPPED_ATTRIBUTE_NAME;
import static org.eclipse.che.api.workspace.shared.Constants.UPDATED_ATTRIBUTE_NAME;
import static org.eclipse.che.api.workspace.shared.Constants.WORKSPACE_GENERATE_NAME_CHARS_APPEND;

import com.google.inject.Inject;
import java.util.Collections;
Expand All @@ -48,11 +49,14 @@
import org.eclipse.che.api.workspace.server.model.impl.WorkspaceConfigImpl;
import org.eclipse.che.api.workspace.server.model.impl.WorkspaceImpl;
import org.eclipse.che.api.workspace.server.model.impl.devfile.DevfileImpl;
import org.eclipse.che.api.workspace.server.model.impl.devfile.MetadataImpl;
import org.eclipse.che.api.workspace.server.spi.WorkspaceDao;
import org.eclipse.che.api.workspace.shared.Constants;
import org.eclipse.che.api.workspace.shared.event.WorkspaceCreatedEvent;
import org.eclipse.che.commons.annotation.Nullable;
import org.eclipse.che.commons.annotation.Traced;
import org.eclipse.che.commons.env.EnvironmentContext;
import org.eclipse.che.commons.lang.NameGenerator;
import org.eclipse.che.commons.subject.Subject;
import org.eclipse.che.commons.tracing.TracingTags;
import org.slf4j.Logger;
Expand Down Expand Up @@ -159,6 +163,8 @@ public WorkspaceImpl createWorkspace(
requireNonNull(namespace, "Required non-null namespace");
validator.validateAttributes(attributes);

devfile = generateNameIfNeeded(devfile);

try {
devfileIntegrityValidator.validateContentReferences(devfile, contentProvider);
} catch (DevfileFormatException e) {
Expand Down Expand Up @@ -569,6 +575,26 @@ private WorkspaceImpl doCreateWorkspace(
return workspace;
}

/**
* If 'generateName' is defined and 'name' is not, we generate name using 'generateName' as a
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use generateName all the time if it is defined ?

Copy link
Member Author

Choose a reason for hiding this comment

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

kubernetes takes name in that case and we follow kubernetes approach here.

* prefix following {@link Constants#WORKSPACE_GENERATE_NAME_CHARS_APPEND} random characters and
* set it to 'name'.
*/
private Devfile generateNameIfNeeded(Devfile origDevfile) {
if (origDevfile.getMetadata() != null) {
MetadataImpl metadata = new MetadataImpl(origDevfile.getMetadata());
if (metadata.getName() == null && metadata.getGenerateName() != null) {
metadata.setName(
NameGenerator.generate(
metadata.getGenerateName(), WORKSPACE_GENERATE_NAME_CHARS_APPEND));
DevfileImpl devfileWithGeneratedName = new DevfileImpl(origDevfile);
devfileWithGeneratedName.setMetadata(metadata);
return devfileWithGeneratedName;
}
}
return origDevfile;
sparkoo marked this conversation as resolved.
Show resolved Hide resolved
}

private WorkspaceImpl getByKey(String key) throws NotFoundException, ServerException {

int lastColonIndex = key.indexOf(":");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.Objects;
import javax.persistence.Column;
import javax.persistence.Embeddable;
import javax.persistence.Transient;
import org.eclipse.che.api.core.model.workspace.devfile.Metadata;

@Embeddable
Expand All @@ -22,6 +23,12 @@ public class MetadataImpl implements Metadata {
@Column(name = "meta_name")
private String name;

/**
* generateName is used just at workspace create time, when name is generated from it and stored
* into {@link MetadataImpl#name}, thus it's not needed to persist
*/
@Transient private String generateName;

public MetadataImpl() {}

public MetadataImpl(String name) {
Expand All @@ -30,6 +37,7 @@ public MetadataImpl(String name) {

public MetadataImpl(Metadata metadata) {
this.name = metadata.getName();
this.generateName = metadata.getGenerateName();
}

@Override
Expand All @@ -41,6 +49,15 @@ public void setName(String name) {
this.name = name;
}

@Override
public String getGenerateName() {
return generateName;
}

public void setGenerateName(String generateName) {
this.generateName = generateName;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand All @@ -50,16 +67,24 @@ public boolean equals(Object o) {
return false;
}
MetadataImpl metadata = (MetadataImpl) o;
return name.equals(metadata.name);
return Objects.equals(name, metadata.name)
&& Objects.equals(generateName, metadata.generateName);
}

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

@Override
public String toString() {
return "MetadataImpl{'name='" + name + '\'' + '}';
return "MetadataImpl{"
+ "name='"
+ name
+ '\''
+ ", generateName='"
+ generateName
+ '\''
+ '}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,26 @@
"examples": [
"petclinic-dev-environment"
]
},
"generateName": {
"type": "string",
"minLength": 1,
"title": "Devfile Generate Name",
"description": "Workspaces created from devfile, will use it as base and append random suffix. It's used when name is not defined.",
"examples": [
"petclinic-"
]
}
},
"additionalProperties": false,
"required": ["name"]
"anyOf": [
{
"required": ["name"]
},
{
"required": ["generateName"]
}
]
},
"projects": {
"type": "array",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@
import static org.eclipse.che.api.core.model.workspace.WorkspaceStatus.STARTING;
import static org.eclipse.che.api.core.model.workspace.WorkspaceStatus.STOPPED;
import static org.eclipse.che.api.core.model.workspace.config.MachineConfig.MEMORY_LIMIT_ATTRIBUTE;
import static org.eclipse.che.api.workspace.server.devfile.Constants.CURRENT_API_VERSION;
import static org.eclipse.che.api.workspace.shared.Constants.CREATED_ATTRIBUTE_NAME;
import static org.eclipse.che.api.workspace.shared.Constants.ERROR_MESSAGE_ATTRIBUTE_NAME;
import static org.eclipse.che.api.workspace.shared.Constants.STOPPED_ABNORMALLY_ATTRIBUTE_NAME;
import static org.eclipse.che.api.workspace.shared.Constants.STOPPED_ATTRIBUTE_NAME;
import static org.eclipse.che.api.workspace.shared.Constants.UPDATED_ATTRIBUTE_NAME;
import static org.eclipse.che.api.workspace.shared.Constants.WORKSPACE_GENERATE_NAME_CHARS_APPEND;
import static org.eclipse.che.dto.server.DtoFactory.newDto;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
Expand Down Expand Up @@ -177,6 +179,44 @@ public void createsWorkspace() throws Exception {
verify(workspaceDao).create(workspace);
}

@Test
public void createsWorkspaceFromDevfile()
throws ValidationException, ConflictException, NotFoundException, ServerException {
final DevfileImpl devfile = new DevfileImpl();
devfile.setApiVersion(CURRENT_API_VERSION);
devfile.setName("ws");
Workspace workspace = workspaceManager.createWorkspace(devfile, NAMESPACE_1, null, null);
assertEquals(workspace.getDevfile(), devfile);
}

@Test
public void createsWorkspaceFromDevfileWithGenerateName()
throws ValidationException, ConflictException, NotFoundException, ServerException {
final String testDevfileGenerateName = "ws-";
final DevfileImpl devfile = new DevfileImpl();
devfile.setApiVersion(CURRENT_API_VERSION);
devfile.getMetadata().setGenerateName(testDevfileGenerateName);
Workspace workspace = workspaceManager.createWorkspace(devfile, NAMESPACE_1, null, null);

assertTrue(workspace.getDevfile().getName().startsWith(testDevfileGenerateName));
assertEquals(
workspace.getDevfile().getName().length(),
testDevfileGenerateName.length() + WORKSPACE_GENERATE_NAME_CHARS_APPEND);
}

@Test
public void nameIsUsedWhenNameAndGenerateNameSet()
throws ValidationException, ConflictException, NotFoundException, ServerException {
final String devfileName = "workspacename";
final DevfileImpl devfile = new DevfileImpl();
devfile.setApiVersion(CURRENT_API_VERSION);
devfile.getMetadata().setName(devfileName);
devfile.getMetadata().setGenerateName("this_will_not_be_set_as_a_name");
Workspace workspace = workspaceManager.createWorkspace(devfile, NAMESPACE_1, null, null);

assertEquals(workspace.getDevfile().getName(), devfileName);
}

@Test
public void getsWorkspaceByIdWithoutRuntime() throws Exception {
WorkspaceImpl workspace = createAndMockWorkspace();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ public Object[][] validDevfiles() {
{"dockerimage_component/devfile_dockerimage_component_without_entry_point.yaml"},
{"editor_plugin_component/devfile_editor_component_with_custom_registry.yaml"},
{"editor_plugin_component/devfile_editor_plugins_components_with_memory_limit.yaml"},
{"editor_plugin_component/devfile_plugin_component_with_reference.yaml"}
{"editor_plugin_component/devfile_plugin_component_with_reference.yaml"},
{"devfile/devfile_just_generatename.yaml"},
{"devfile/devfile_name_and_generatename.yaml"}
};
}

Expand All @@ -89,7 +91,7 @@ public Object[][] invalidDevfiles() {
// Devfile model testing
{
"devfile/devfile_empty_metadata.yaml",
"(/metadata):The object must have a property whose name is \"name\"."
"At least one of the following sets of problems must be resolved.: [(/metadata):The object must have a property whose name is \"name\".(/metadata):The object must have a property whose name is \"generateName\".]"
},
{
"devfile/devfile_null_metadata.yaml",
Expand All @@ -100,8 +102,8 @@ public Object[][] invalidDevfiles() {
"The object must have a property whose name is \"metadata\"."
},
{
"devfile/devfile_missing_name.yaml",
"(/metadata/something):The object must not have a property whose name is \"something\".(/metadata):The object must have a property whose name is \"name\"."
"devfile/devfile_missing_name_and_generatename.yaml",
"(/metadata/something):The object must not have a property whose name is \"something\".At least one of the following sets of problems must be resolved.: [(/metadata):The object must have a property whose name is \"name\".(/metadata):The object must have a property whose name is \"generateName\".]"
},
{
"devfile/devfile_missing_api_version.yaml",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
apiVersion: 1.0.0
metadata:
name: petclinic-dev-environment
Copy link
Contributor

Choose a reason for hiding this comment

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

is it valid to have both name and generateName ?

Copy link
Member Author

Choose a reason for hiding this comment

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

it is valid, but 'generateName' is basically ignored in that case.

generateName: petclinic-
projects:
- name: petclinic
source:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#
# 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
#

---
apiVersion: 1.0.0
metadata:
generateName: test-
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#
# 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
#

---
apiVersion: 1.0.0
metadata:
name: testName
generateName: testGenerateName-