Copilot commented on code in PR #763: URL: https://github.com/apache/unomi/pull/763#discussion_r3367082404
########## tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/tenants/TenantCrudCommand.java: ########## @@ -0,0 +1,278 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.shell.dev.commands.tenants; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.karaf.shell.support.table.ShellTable; + +import java.io.PrintStream; +import org.apache.unomi.api.PartialList; +import org.apache.unomi.api.query.Query; +import org.apache.unomi.api.tenants.*; +import org.apache.unomi.common.DataTable; +import org.apache.unomi.persistence.spi.CustomObjectMapper; +import org.apache.unomi.shell.dev.services.BaseCrudCommand; +import org.apache.unomi.shell.dev.services.CrudCommand; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Reference; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.*; +import java.util.stream.Collectors; + +/** + * A command to perform CRUD operations on tenants + */ +@Component(service = CrudCommand.class, immediate = true) +public class TenantCrudCommand extends BaseCrudCommand { + + private static final Logger LOGGER = LoggerFactory.getLogger(TenantCrudCommand.class.getName()); + private static final ObjectMapper OBJECT_MAPPER = new CustomObjectMapper(); + private static final List<String> PROPERTY_NAMES = List.of( + "itemId", "name", "description", "status", "creationDate", "lastModificationDate", "resourceQuota", "properties", "restrictedEventPermissions", "authorizedIPs" + ); + + @Reference + private TenantService tenantService; + + @Override + public String getObjectType() { + return "tenant"; + } + + @Override + protected String[] getHeadersWithoutTenant() { + return new String[]{"ID", "Name", "Description", "Status", "Created", "Modified"}; + } + + /** + * Override to skip the tenant column since we're listing tenants themselves. + * The tenant ID would be the same as the ID column, making it redundant. + */ + @Override + public String[] getHeaders() { + return getHeadersWithoutTenant(); + } + + /** + * Override to skip prepending tenant ID to rows since we're listing tenants themselves. + */ + @Override + protected DataTable buildDataTable() { + PrintStream console = getConsole(); + try { + Query query = buildQuery(maxEntries); + PartialList<?> items = getItems(query); + + printPaginationWarning(items, console); + + DataTable dataTable = new DataTable(); + for (Object item : items.getList()) { + Comparable[] rowData = buildRow(item); + dataTable.addRow(rowData); + } + + return dataTable; + } catch (Exception e) { + LOGGER.error("Error building data table", e); + console.println("Error: " + e.getMessage()); + return new DataTable(); + } + } + + /** + * Override to skip prepending tenant ID to rows since we're listing tenants themselves. + */ + @Override + public void buildRows(ShellTable table, int maxEntries) { + PrintStream console = getConsole(); + try { + Query query = buildQuery(maxEntries); + PartialList<?> items = getItems(query); + + printPaginationWarning(items, console); + + for (Object item : items.getList()) { + Comparable[] rowData = buildRow(item); + table.addRow().addContent(rowData); + } + } catch (Exception e) { + console.println("Error: " + e.getMessage()); + LOGGER.error("Error building rows", e); + } + } + + @Override + protected PartialList<?> getItems(Query query) { + List<Tenant> tenants = tenantService.getAllTenants(); + // Filter out system tenant + tenants = tenants.stream() + .filter(tenant -> !TenantService.SYSTEM_TENANT.equals(tenant.getItemId())) + .collect(Collectors.toList()); + return new PartialList<>(tenants, 0, tenants.size(), tenants.size(), PartialList.Relation.EQUAL); + } + + @Override + protected Comparable[] buildRow(Object item) { + Tenant tenant = (Tenant) item; + return new Comparable[]{ + tenant.getItemId(), + tenant.getName(), + tenant.getDescription(), + tenant.getStatus() != null ? tenant.getStatus().toString() : "", + tenant.getCreationDate() != null ? tenant.getCreationDate().toString() : "", + tenant.getLastModificationDate() != null ? tenant.getLastModificationDate().toString() : "" + }; + } + + /** + * Special case for tenants: the tenant ID is the same as the item ID for tenant objects. + */ + @Override + protected String getTenantIdFromItem(Object item) { + if (item instanceof Tenant) { + Tenant tenant = (Tenant) item; + return tenant.getItemId(); + } + return super.getTenantIdFromItem(item); + } + + @Override + public Map<String, Object> read(String id) { + Tenant tenant = tenantService.getTenant(id); + if (tenant == null || TenantService.SYSTEM_TENANT.equals(tenant.getItemId())) { + return null; + } + return OBJECT_MAPPER.convertValue(tenant, Map.class); + } + + @Override + public String create(Map<String, Object> properties) { + String id = (String) properties.remove("itemId"); + if (id == null) { + return null; + } + + try { + // Create the tenant + Tenant tenant = tenantService.createTenant(id, properties); + + // Generate API keys with no expiration + ApiKey publicKey = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null); + ApiKey privateKey = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null); + + // Save the tenant with the new API keys + tenantService.saveTenant(tenant); + Review Comment: TenantServiceImpl#createTenant already saves the tenant and generates both API keys. Generating API keys again here is redundant and, worse, calling saveTenant(tenant) afterwards persists the *stale* Tenant instance returned by createTenant, which can overwrite the newly generated keys in persistence with the older ones held by the stale object. ########## tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/scheduler/PurgeTasksCommand.java: ########## @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.shell.dev.commands.scheduler; + +import org.apache.karaf.shell.api.action.Command; +import org.apache.karaf.shell.api.action.Option; +import org.apache.karaf.shell.api.action.lifecycle.Service; +import org.apache.unomi.api.tasks.ScheduledTask; + +import java.util.Calendar; +import java.util.Date; + +@Command(scope = "unomi", name = "task-purge", description = "Purges old completed tasks") +@Service +public class PurgeTasksCommand extends BaseSchedulerCommand { + + @Option(name = "-d", aliases = "--days", description = "Number of days to keep completed tasks (default: 7)", required = false) + private int daysToKeep = 7; + + @Option(name = "-f", aliases = "--force", description = "Skip confirmation prompt", required = false) + private boolean force = false; + + @Override + public Object execute() throws Exception { + if (!force) { + String response = session.readLine( + "This will permanently delete all completed tasks older than " + daysToKeep + " days. Continue? (y/n): ", + null + ); + if (!"y".equalsIgnoreCase(response != null ? response.trim() : "n")) { + println("Operation cancelled."); + return null; + } + } + + // Calculate cutoff date + Calendar cal = Calendar.getInstance(); + cal.add(Calendar.DAY_OF_MONTH, -daysToKeep); + Date cutoffDate = cal.getTime(); + + // Get completed tasks + int offset = 0; + int batchSize = 100; + int purgedCount = 0; + + while (true) { + var tasks = schedulerService.getTasksByStatus(ScheduledTask.TaskStatus.COMPLETED, offset, batchSize, null); + if (tasks.getList().isEmpty()) { + break; + } + + // Cancel old completed tasks + for (ScheduledTask task : tasks.getList()) { + if (task.getLastExecutionDate() != null && task.getLastExecutionDate().before(cutoffDate)) { + schedulerService.cancelTask(task.getItemId()); + purgedCount++; Review Comment: This command says it “permanently delete[s]”/“purge[s]” completed tasks, but it actually calls schedulerService.cancelTask(...). cancelTask() only marks a task as CANCELLED and keeps it in storage (per SchedulerService Javadoc), so this won’t purge anything and may corrupt task history by changing COMPLETED tasks to CANCELLED. A real purge needs a delete/purge API on SchedulerService (or similar) rather than cancelTask. ########## tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/apikeys/ApiKeyCrudCommand.java: ########## @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.shell.dev.commands.apikeys; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.commons.lang3.StringUtils; +import org.apache.unomi.api.PartialList; +import org.apache.unomi.api.query.Query; +import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.ApiKey.ApiKeyType; +import org.apache.unomi.api.tenants.Tenant; +import org.apache.unomi.api.tenants.TenantService; +import org.apache.unomi.persistence.spi.CustomObjectMapper; +import org.apache.unomi.shell.dev.services.BaseCrudCommand; +import org.apache.unomi.shell.dev.services.CrudCommand; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Reference; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +@Component(service = CrudCommand.class, immediate = true) +public class ApiKeyCrudCommand extends BaseCrudCommand { + + private static final ObjectMapper OBJECT_MAPPER = new CustomObjectMapper(); + private static final List<String> PROPERTY_NAMES = List.of( + "itemId", "name", "description", "keyType", "key", "tenantId" + ); + + @Reference + private TenantService tenantService; + + @Override + public String getObjectType() { + return "apikey"; + } + + @Override + protected String[] getHeadersWithoutTenant() { + return new String[] { + "Identifier", + "Name", + "Description", + "Key Type", + "Key" + }; + } + + @Override + protected PartialList<?> getItems(Query query) { + List<ApiKey> allApiKeys = new ArrayList<>(); + for (Tenant tenant : tenantService.getAllTenants()) { + if (tenant.getApiKeys() != null) { + allApiKeys.addAll(tenant.getApiKeys()); + } + } Review Comment: BaseCrudCommand prepends a Tenant column using Item#getTenantId(), but TenantServiceImpl doesn’t set tenantId on ApiKey objects. Here the ApiKeys are collected without setting tenantId, so the Tenant column will be empty/"null" for API keys and the output won’t be tenant-aware. ########## tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/apikeys/ApiKeyCrudCommand.java: ########## @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.shell.dev.commands.apikeys; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.commons.lang3.StringUtils; +import org.apache.unomi.api.PartialList; +import org.apache.unomi.api.query.Query; +import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.ApiKey.ApiKeyType; +import org.apache.unomi.api.tenants.Tenant; +import org.apache.unomi.api.tenants.TenantService; +import org.apache.unomi.persistence.spi.CustomObjectMapper; +import org.apache.unomi.shell.dev.services.BaseCrudCommand; +import org.apache.unomi.shell.dev.services.CrudCommand; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Reference; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +@Component(service = CrudCommand.class, immediate = true) +public class ApiKeyCrudCommand extends BaseCrudCommand { + + private static final ObjectMapper OBJECT_MAPPER = new CustomObjectMapper(); + private static final List<String> PROPERTY_NAMES = List.of( + "itemId", "name", "description", "keyType", "key", "tenantId" + ); Review Comment: validityPeriod is documented/handled in create(), but it’s missing from PROPERTY_NAMES, so tab-completion won’t suggest it for `unomi:crud create apikey ...`. ########## tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/scheduler/ListTasksCommand.java: ########## @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.shell.dev.commands.scheduler; + +import org.apache.karaf.shell.api.action.Action; +import org.apache.karaf.shell.api.action.Command; +import org.apache.karaf.shell.api.action.Option; +import org.apache.karaf.shell.api.action.lifecycle.Reference; +import org.apache.karaf.shell.api.action.lifecycle.Service; +import org.apache.karaf.shell.api.console.Session; +import org.apache.karaf.shell.support.table.Col; +import org.apache.karaf.shell.support.table.ShellTable; + +import org.apache.unomi.api.PartialList; +import org.apache.unomi.api.services.SchedulerService; +import org.apache.unomi.api.tasks.ScheduledTask; +import org.apache.unomi.shell.dev.commands.CommandUtils; + +import java.io.PrintStream; +import java.util.List; + +@Command(scope = "unomi", name = "task-list", description = "Lists scheduled tasks") +@Service +public class ListTasksCommand extends BaseSchedulerCommand { + + @Option(name = "-s", aliases = "--status", description = "Filter by task status (SCHEDULED, RUNNING, COMPLETED, FAILED, CANCELLED, CRASHED)", required = false) + private String status; + + @Option(name = "-t", aliases = "--type", description = "Filter by task type", required = false) + private String type; + + @Option(name = "--limit", description = "Maximum number of tasks to display (default: 50)", required = false) + private int limit = 50; + + @Override + public Object execute() throws Exception { + PrintStream console = getConsole(); + ShellTable table = new ShellTable(); + + // Configure table columns + table.column(new Col("ID").maxSize(36)); + table.column(new Col("Type").maxSize(30)); + table.column(new Col("Status").maxSize(10)); + table.column(new Col("Next Run").maxSize(19)); + table.column(new Col("Last Run").maxSize(19)); + table.column(new Col("Failures").alignRight()); + table.column(new Col("Successes").alignRight()); + table.column(new Col("Total Exec").alignRight()); + table.column(new Col("Persistent").maxSize(10)); + + // Get tasks based on filters + List<ScheduledTask> tasks; + if (status != null) { + try { + ScheduledTask.TaskStatus taskStatus = ScheduledTask.TaskStatus.valueOf(status.toUpperCase()); + // Get persistent tasks + PartialList<ScheduledTask> filteredTasks = schedulerService.getTasksByStatus(taskStatus, 0, limit, null); + tasks = filteredTasks.getList(); + // Add memory tasks with matching status + List<ScheduledTask> memoryTasks = schedulerService.getMemoryTasks(); + for (ScheduledTask task : memoryTasks) { + if (task.getStatus() == taskStatus) { + tasks.add(task); + } + } Review Comment: SchedulerService#getTasksByStatus already returns both persistent and in-memory tasks (see SchedulerServiceImpl), so manually adding getMemoryTasks() here will duplicate rows and can exceed the requested --limit. ########## tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/scheduler/ListTasksCommand.java: ########## @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.shell.dev.commands.scheduler; + +import org.apache.karaf.shell.api.action.Action; +import org.apache.karaf.shell.api.action.Command; +import org.apache.karaf.shell.api.action.Option; +import org.apache.karaf.shell.api.action.lifecycle.Reference; +import org.apache.karaf.shell.api.action.lifecycle.Service; +import org.apache.karaf.shell.api.console.Session; +import org.apache.karaf.shell.support.table.Col; +import org.apache.karaf.shell.support.table.ShellTable; + +import org.apache.unomi.api.PartialList; +import org.apache.unomi.api.services.SchedulerService; +import org.apache.unomi.api.tasks.ScheduledTask; +import org.apache.unomi.shell.dev.commands.CommandUtils; + +import java.io.PrintStream; +import java.util.List; + +@Command(scope = "unomi", name = "task-list", description = "Lists scheduled tasks") +@Service +public class ListTasksCommand extends BaseSchedulerCommand { + + @Option(name = "-s", aliases = "--status", description = "Filter by task status (SCHEDULED, RUNNING, COMPLETED, FAILED, CANCELLED, CRASHED)", required = false) + private String status; + + @Option(name = "-t", aliases = "--type", description = "Filter by task type", required = false) + private String type; + + @Option(name = "--limit", description = "Maximum number of tasks to display (default: 50)", required = false) + private int limit = 50; + + @Override + public Object execute() throws Exception { + PrintStream console = getConsole(); + ShellTable table = new ShellTable(); + + // Configure table columns + table.column(new Col("ID").maxSize(36)); + table.column(new Col("Type").maxSize(30)); + table.column(new Col("Status").maxSize(10)); + table.column(new Col("Next Run").maxSize(19)); + table.column(new Col("Last Run").maxSize(19)); + table.column(new Col("Failures").alignRight()); + table.column(new Col("Successes").alignRight()); + table.column(new Col("Total Exec").alignRight()); + table.column(new Col("Persistent").maxSize(10)); + + // Get tasks based on filters + List<ScheduledTask> tasks; + if (status != null) { + try { + ScheduledTask.TaskStatus taskStatus = ScheduledTask.TaskStatus.valueOf(status.toUpperCase()); + // Get persistent tasks + PartialList<ScheduledTask> filteredTasks = schedulerService.getTasksByStatus(taskStatus, 0, limit, null); + tasks = filteredTasks.getList(); + // Add memory tasks with matching status + List<ScheduledTask> memoryTasks = schedulerService.getMemoryTasks(); + for (ScheduledTask task : memoryTasks) { + if (task.getStatus() == taskStatus) { + tasks.add(task); + } + } + } catch (IllegalArgumentException e) { + println("Invalid status: " + status); + return null; + } + } else if (type != null) { + // Get persistent tasks + PartialList<ScheduledTask> filteredTasks = schedulerService.getTasksByType(type, 0, limit, null); + tasks = filteredTasks.getList(); + // Add memory tasks with matching type + List<ScheduledTask> memoryTasks = schedulerService.getMemoryTasks(); + for (ScheduledTask task : memoryTasks) { + if (task.getTaskType().equals(type)) { + tasks.add(task); + } + } Review Comment: SchedulerService#getTasksByType already includes in-memory tasks, so adding getMemoryTasks() again here will produce duplicate rows and potentially exceed --limit. ########## tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/apikeys/ApiKeyCrudCommand.java: ########## @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.shell.dev.commands.apikeys; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.commons.lang3.StringUtils; +import org.apache.unomi.api.PartialList; +import org.apache.unomi.api.query.Query; +import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.ApiKey.ApiKeyType; +import org.apache.unomi.api.tenants.Tenant; +import org.apache.unomi.api.tenants.TenantService; +import org.apache.unomi.persistence.spi.CustomObjectMapper; +import org.apache.unomi.shell.dev.services.BaseCrudCommand; +import org.apache.unomi.shell.dev.services.CrudCommand; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Reference; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +@Component(service = CrudCommand.class, immediate = true) +public class ApiKeyCrudCommand extends BaseCrudCommand { + + private static final ObjectMapper OBJECT_MAPPER = new CustomObjectMapper(); + private static final List<String> PROPERTY_NAMES = List.of( + "itemId", "name", "description", "keyType", "key", "tenantId" + ); + + @Reference + private TenantService tenantService; + + @Override + public String getObjectType() { + return "apikey"; + } + + @Override + protected String[] getHeadersWithoutTenant() { + return new String[] { + "Identifier", + "Name", + "Description", + "Key Type", + "Key" + }; + } + + @Override + protected PartialList<?> getItems(Query query) { + List<ApiKey> allApiKeys = new ArrayList<>(); + for (Tenant tenant : tenantService.getAllTenants()) { + if (tenant.getApiKeys() != null) { + allApiKeys.addAll(tenant.getApiKeys()); + } + } + + // Apply query limit + Integer offset = query.getOffset(); + Integer limit = query.getLimit(); + int start = offset == null ? 0 : offset; + int size = limit == null ? allApiKeys.size() : limit; + int end = Math.min(start + size, allApiKeys.size()); + + List<ApiKey> pagedApiKeys = allApiKeys.subList(start, end); + return new PartialList<ApiKey>(pagedApiKeys, start, pagedApiKeys.size(), allApiKeys.size(), PartialList.Relation.EQUAL); + } + + @Override + protected String[] buildRow(Object item) { + ApiKey apiKey = (ApiKey) item; + return new String[] { + apiKey.getItemId(), + apiKey.getName(), + apiKey.getDescription(), + apiKey.getKeyType().toString(), + apiKey.getKey() + }; + } + + @Override + public String create(Map<String, Object> properties) { + String tenantId = (String) properties.get("tenantId"); + if (StringUtils.isBlank(tenantId)) { + throw new IllegalArgumentException("tenantId is required"); + } + + ApiKeyType keyType = ApiKeyType.valueOf((String) properties.get("keyType")); + Long validityPeriod = properties.containsKey("validityPeriod") ? + Long.valueOf((String) properties.get("validityPeriod")) : null; + Review Comment: validityPeriod is parsed as `(String)` and passed to Long.valueOf, but JSON numbers are deserialized as Number (Integer/Long) by Jackson in UnomiCrudCommand. This will throw ClassCastException for typical inputs like `{"validityPeriod": 60000}`. ########## tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/tenants/TenantCrudCommand.java: ########## @@ -0,0 +1,278 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.shell.dev.commands.tenants; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.karaf.shell.support.table.ShellTable; + +import java.io.PrintStream; +import org.apache.unomi.api.PartialList; +import org.apache.unomi.api.query.Query; +import org.apache.unomi.api.tenants.*; +import org.apache.unomi.common.DataTable; +import org.apache.unomi.persistence.spi.CustomObjectMapper; +import org.apache.unomi.shell.dev.services.BaseCrudCommand; +import org.apache.unomi.shell.dev.services.CrudCommand; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Reference; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.*; +import java.util.stream.Collectors; + +/** + * A command to perform CRUD operations on tenants + */ +@Component(service = CrudCommand.class, immediate = true) +public class TenantCrudCommand extends BaseCrudCommand { + + private static final Logger LOGGER = LoggerFactory.getLogger(TenantCrudCommand.class.getName()); + private static final ObjectMapper OBJECT_MAPPER = new CustomObjectMapper(); + private static final List<String> PROPERTY_NAMES = List.of( + "itemId", "name", "description", "status", "creationDate", "lastModificationDate", "resourceQuota", "properties", "restrictedEventPermissions", "authorizedIPs" + ); + + @Reference + private TenantService tenantService; + + @Override + public String getObjectType() { + return "tenant"; + } + + @Override + protected String[] getHeadersWithoutTenant() { + return new String[]{"ID", "Name", "Description", "Status", "Created", "Modified"}; + } + + /** + * Override to skip the tenant column since we're listing tenants themselves. + * The tenant ID would be the same as the ID column, making it redundant. + */ + @Override + public String[] getHeaders() { + return getHeadersWithoutTenant(); + } + + /** + * Override to skip prepending tenant ID to rows since we're listing tenants themselves. + */ + @Override + protected DataTable buildDataTable() { + PrintStream console = getConsole(); + try { + Query query = buildQuery(maxEntries); + PartialList<?> items = getItems(query); + + printPaginationWarning(items, console); + + DataTable dataTable = new DataTable(); + for (Object item : items.getList()) { + Comparable[] rowData = buildRow(item); + dataTable.addRow(rowData); + } + + return dataTable; + } catch (Exception e) { + LOGGER.error("Error building data table", e); + console.println("Error: " + e.getMessage()); + return new DataTable(); + } + } + + /** + * Override to skip prepending tenant ID to rows since we're listing tenants themselves. + */ + @Override + public void buildRows(ShellTable table, int maxEntries) { + PrintStream console = getConsole(); + try { + Query query = buildQuery(maxEntries); + PartialList<?> items = getItems(query); + + printPaginationWarning(items, console); + + for (Object item : items.getList()) { + Comparable[] rowData = buildRow(item); + table.addRow().addContent(rowData); + } + } catch (Exception e) { + console.println("Error: " + e.getMessage()); + LOGGER.error("Error building rows", e); + } + } + + @Override + protected PartialList<?> getItems(Query query) { + List<Tenant> tenants = tenantService.getAllTenants(); + // Filter out system tenant + tenants = tenants.stream() + .filter(tenant -> !TenantService.SYSTEM_TENANT.equals(tenant.getItemId())) + .collect(Collectors.toList()); + return new PartialList<>(tenants, 0, tenants.size(), tenants.size(), PartialList.Relation.EQUAL); + } + + @Override + protected Comparable[] buildRow(Object item) { + Tenant tenant = (Tenant) item; + return new Comparable[]{ + tenant.getItemId(), + tenant.getName(), + tenant.getDescription(), + tenant.getStatus() != null ? tenant.getStatus().toString() : "", + tenant.getCreationDate() != null ? tenant.getCreationDate().toString() : "", + tenant.getLastModificationDate() != null ? tenant.getLastModificationDate().toString() : "" + }; + } + + /** + * Special case for tenants: the tenant ID is the same as the item ID for tenant objects. + */ + @Override + protected String getTenantIdFromItem(Object item) { + if (item instanceof Tenant) { + Tenant tenant = (Tenant) item; + return tenant.getItemId(); + } + return super.getTenantIdFromItem(item); + } + + @Override + public Map<String, Object> read(String id) { + Tenant tenant = tenantService.getTenant(id); + if (tenant == null || TenantService.SYSTEM_TENANT.equals(tenant.getItemId())) { + return null; + } + return OBJECT_MAPPER.convertValue(tenant, Map.class); + } + + @Override + public String create(Map<String, Object> properties) { + String id = (String) properties.remove("itemId"); + if (id == null) { + return null; + } + + try { + // Create the tenant + Tenant tenant = tenantService.createTenant(id, properties); + + // Generate API keys with no expiration + ApiKey publicKey = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PUBLIC, null); + ApiKey privateKey = tenantService.generateApiKeyWithType(tenant.getItemId(), ApiKey.ApiKeyType.PRIVATE, null); + + // Save the tenant with the new API keys + tenantService.saveTenant(tenant); + + return tenant.getItemId(); + } catch (Exception e) { + return null; + } + } + + @Override + public void update(String id, Map<String, Object> properties) { + Tenant tenant = tenantService.getTenant(id); + if (tenant == null || TenantService.SYSTEM_TENANT.equals(tenant.getItemId())) { + return; + } + + try { + // Update tenant properties + if (properties.containsKey("name")) { + tenant.setName((String) properties.get("name")); + } + if (properties.containsKey("description")) { + tenant.setDescription((String) properties.get("description")); + } + if (properties.containsKey("status")) { + tenant.setStatus(Enum.valueOf(TenantStatus.class, (String) properties.get("status"))); + } + if (properties.containsKey("resourceQuota")) { + tenant.setResourceQuota(OBJECT_MAPPER.convertValue(properties.get("resourceQuota"), ResourceQuota.class)); + } + if (properties.containsKey("properties")) { + @SuppressWarnings("unchecked") + Map<String, Object> props = (Map<String, Object>) properties.get("properties"); + tenant.setProperties(props); + } + if (properties.containsKey("restrictedEventPermissions")) { + @SuppressWarnings("unchecked") + Set<String> permissions = new HashSet<>((List<String>) properties.get("restrictedEventPermissions")); + tenant.setRestrictedEventTypes(permissions); + } + if (properties.containsKey("authorizedIPs")) { + @SuppressWarnings("unchecked") + Set<String> ips = new HashSet<>((List<String>) properties.get("authorizedIPs")); + tenant.setAuthorizedIPs(ips); + } + + tenant.setLastModificationDate(new Date()); + tenantService.saveTenant(tenant); + } catch (Exception e) { + // Handle error + } Review Comment: This catch block swallows all exceptions during tenant update. Because UnomiCrudCommand prints "Updated ..." unconditionally after cmd.update(...), failures here will be reported as successful updates, making troubleshooting very difficult and potentially hiding partial writes. ########## tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/apikeys/ApiKeyCrudCommand.java: ########## @@ -0,0 +1,208 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.shell.dev.commands.apikeys; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.commons.lang3.StringUtils; +import org.apache.unomi.api.PartialList; +import org.apache.unomi.api.query.Query; +import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.ApiKey.ApiKeyType; +import org.apache.unomi.api.tenants.Tenant; +import org.apache.unomi.api.tenants.TenantService; +import org.apache.unomi.persistence.spi.CustomObjectMapper; +import org.apache.unomi.shell.dev.services.BaseCrudCommand; +import org.apache.unomi.shell.dev.services.CrudCommand; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Reference; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +@Component(service = CrudCommand.class, immediate = true) +public class ApiKeyCrudCommand extends BaseCrudCommand { + + private static final ObjectMapper OBJECT_MAPPER = new CustomObjectMapper(); + private static final List<String> PROPERTY_NAMES = List.of( + "itemId", "name", "description", "keyType", "key", "tenantId" + ); + + @Reference + private TenantService tenantService; + + @Override + public String getObjectType() { + return "apikey"; + } + + @Override + protected String[] getHeadersWithoutTenant() { + return new String[] { + "Identifier", + "Name", + "Description", + "Key Type", + "Key" + }; + } + + @Override + protected PartialList<?> getItems(Query query) { + List<ApiKey> allApiKeys = new ArrayList<>(); + for (Tenant tenant : tenantService.getAllTenants()) { + if (tenant.getApiKeys() != null) { + allApiKeys.addAll(tenant.getApiKeys()); + } + } + + // Apply query limit + Integer offset = query.getOffset(); + Integer limit = query.getLimit(); + int start = offset == null ? 0 : offset; + int size = limit == null ? allApiKeys.size() : limit; + int end = Math.min(start + size, allApiKeys.size()); + + List<ApiKey> pagedApiKeys = allApiKeys.subList(start, end); + return new PartialList<ApiKey>(pagedApiKeys, start, pagedApiKeys.size(), allApiKeys.size(), PartialList.Relation.EQUAL); + } + + @Override + protected String[] buildRow(Object item) { + ApiKey apiKey = (ApiKey) item; + return new String[] { + apiKey.getItemId(), + apiKey.getName(), + apiKey.getDescription(), + apiKey.getKeyType().toString(), + apiKey.getKey() + }; + } + + @Override + public String create(Map<String, Object> properties) { + String tenantId = (String) properties.get("tenantId"); + if (StringUtils.isBlank(tenantId)) { + throw new IllegalArgumentException("tenantId is required"); + } + + ApiKeyType keyType = ApiKeyType.valueOf((String) properties.get("keyType")); + Long validityPeriod = properties.containsKey("validityPeriod") ? + Long.valueOf((String) properties.get("validityPeriod")) : null; + + ApiKey apiKey = tenantService.generateApiKeyWithType(tenantId, keyType, validityPeriod); + if (apiKey != null) { + apiKey.setName((String) properties.get("name")); + apiKey.setDescription((String) properties.get("description")); + + // Update the tenant with the new API key metadata + Tenant tenant = tenantService.getTenant(tenantId); + tenantService.saveTenant(tenant); + } + return apiKey.getItemId(); + } Review Comment: create() sets name/description on the returned ApiKey instance, but TenantServiceImpl persists API keys inside the Tenant document. The subsequent saveTenant(tenant) saves an unmodified tenant (you never copy the new name/description into tenant.getApiKeys()), so the metadata changes won’t be persisted. ########## tools/shell-dev-commands/src/main/java/org/apache/unomi/shell/dev/commands/CacheCommands.java: ########## @@ -0,0 +1,368 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.shell.dev.commands; + +import org.apache.commons.csv.CSVFormat; +import org.apache.commons.csv.CSVPrinter; +import org.apache.karaf.shell.api.action.Command; +import org.apache.karaf.shell.api.action.Option; +import org.apache.karaf.shell.api.action.lifecycle.Reference; +import org.apache.karaf.shell.api.action.lifecycle.Service; +import org.apache.karaf.shell.support.table.ShellTable; + +import org.apache.unomi.api.services.ExecutionContextManager; +import org.apache.unomi.api.services.cache.MultiTypeCacheService; +import org.apache.unomi.api.services.cache.MultiTypeCacheService.CacheStatistics; +import org.apache.unomi.api.services.cache.MultiTypeCacheService.CacheStatistics.TypeStatistics; +import org.apache.unomi.api.tenants.TenantService; +import org.apache.unomi.shell.dev.commands.TenantContextHelper; + +import java.io.PrintStream; +import java.io.Serializable; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +@Service +@Command(scope = "unomi", name = "cache", description = "Cache management commands") +public class CacheCommands extends BaseSimpleCommand { + + @Reference + private MultiTypeCacheService cacheService; + + @Reference + private TenantService tenantService; + + @Reference + private ExecutionContextManager executionContextManager; + + @Option(name = "--stats", description = "Display cache statistics", required = false) + private boolean showStats = false; + + @Option(name = "--reset", description = "Reset statistics after displaying them", required = false) + private boolean reset = false; + + @Option(name = "--type", description = "Filter by type", required = false) + private String type; + + @Option(name = "--tenant", description = "Filter by tenant ID", required = false) + private String tenantId; + + @Option(name = "--clear", description = "Clear cache for specified tenant", required = false) + private boolean clear = false; + + @Option(name = "--inspect", description = "Inspect cache contents", required = false) + private boolean inspect = false; + + @Option(name = "--detailed", description = "Show detailed statistics", required = false) + private boolean detailed = false; + + @Option(name = "--watch", description = "Watch cache statistics (refresh interval in seconds)", required = false) + private int watchInterval = 0; + + @Option(name = "--csv", description = "Output statistics in CSV format", required = false) + private boolean csv = false; + + @Option(name = "--id", description = "Specific entry ID to view or remove", required = false) + private String entryId; + + @Option(name = "--view", description = "View a specific cache entry", required = false) + private boolean view = false; + + @Option(name = "--remove", description = "Remove a specific cache entry", required = false) + private boolean remove = false; + + @Override + public Object execute() throws Exception { + if (cacheService == null) { + println("Cache service not available"); + return null; + } + + // Initialize execution context from session + TenantContextHelper.initializeExecutionContext(session, executionContextManager); + + // Set default tenant if not specified + if (tenantId == null) { + tenantId = executionContextManager.getCurrentContext().getTenantId(); + } + + if (view && entryId != null) { + viewCacheEntry(); + return null; + } + + if (remove && entryId != null) { + removeCacheEntry(); + return null; + } + + if (clear) { + clearCache(); + return null; + } + + if (inspect) { + inspectCache(); + return null; + } + + if (watchInterval > 0) { + watchStatistics(); + return null; + } + + if (showStats || (!clear && !inspect && !view && !remove)) { + displayStatistics(); + } + + return null; + } + + private void viewCacheEntry() { + if (type == null) { + println("Please specify a type to view the entry"); + return; + } + + try { + Class<? extends Serializable> typeClass = (Class<? extends Serializable>) Class.forName(type); + Map<String, ? extends Serializable> typeCache = cacheService.getTenantCache(tenantId, typeClass); + + Serializable entry = typeCache.get(entryId); + if (entry != null) { + println("Cache entry found:"); + println(" Tenant: " + tenantId); + println(" Type: " + type); + println(" ID: " + entryId); + println(" Value: " + entry); + // Add any additional entry details you want to display + } else { + println("No cache entry found for ID: " + entryId); + } + } catch (ClassNotFoundException e) { + println("Invalid type specified: " + type); + } + } + + private void removeCacheEntry() { + if (type == null) { + println("Please specify a type to remove the entry"); + return; + } + + try { + Class<? extends Serializable> typeClass = (Class<? extends Serializable>) Class.forName(type); + + // First check if the entry exists + Map<String, ? extends Serializable> typeCache = cacheService.getTenantCache(tenantId, typeClass); + if (typeCache.containsKey(entryId)) { + cacheService.remove(type, entryId, tenantId, typeClass); + println("Successfully removed cache entry:"); Review Comment: removeCacheEntry() resolves a Class via Class.forName(type), but then calls cacheService.remove(type, ...). MultiTypeCacheService.remove expects the *itemType* string (e.g., Segment.ITEM_TYPE), not the class name/FQCN, so this call will not remove anything from the cache unless the user happens to pass the exact internal itemType. This also conflicts with --type in stats mode where `type` is treated as the statistics key (itemType), not a Java class name. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
