Skip to content

Commit

Permalink
server: Use randomized UUID for bookmarks
Browse files Browse the repository at this point in the history 
To have persistent UUID for each bookmark, we now use
randomly generated UUIDs instead of generating them based
on the bookmark's properties (e.g., name or start/end times).
The new system allows to keep the bookmarks' UUIDs consistent
even if their properties changed (i.e., through PUT endpoint)

[Changed] Bookmark's UUID generation is now randomized instead of property-based

Signed-off-by: Kaveh Shahedi <[email protected]>
  • Loading branch information
@kavehshahedi
kavehshahedi committed Nov 20, 2024
1 parent c67f20d commit 19631bb
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 115 deletions.
102 changes: 26 additions & 76 deletions ...ss/incubator/trace/server/jersey/rest/core/tests/services/BookmarkManagerServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@

import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.UUID;

import javax.ws.rs.client.Entity;
import javax.ws.rs.client.WebTarget;
Expand Down Expand Up @@ -177,30 +180,35 @@ public void testCreateBookmarkNoEndTime() {
* Test the creation of a bookmark with a repetitive data.
*/
@Test
public void testCreateBookmarkRepetitiveData() {
public void testCreateIdenticalBookmarks() {
WebTarget application = getApplicationEndpoint();
WebTarget bookmarkTarget = application.path(EXPERIMENTS)
.path(experiment.getUUID().toString())
.path(BOOKMARKS);

// Create first bookmark
Map<String, Object> parameters = new HashMap<>();
parameters.put(NAME, BOOKMARK.getName());
parameters.put(START, START_TIME);
parameters.put(END, END_TIME);

Response response = bookmarkTarget.request().post(Entity.json(new QueryParameters(parameters, Collections.emptyList())));
BookmarkModelStub firstBookmark = response.readEntity(BookmarkModelStub.class);
assertEquals("First bookmark creation should succeed", 200, response.getStatus());
assertNotNull("First bookmark should not be null", firstBookmark);
Set<UUID> uuids = new HashSet<>();
for (int i = 0; i < 3; i++) {
Map<String, Object> parameters = new HashMap<>();
parameters.put(NAME, BOOKMARK.getName());
parameters.put(START, START_TIME);
parameters.put(END, END_TIME);

Response response = bookmarkTarget.request().post(Entity.json(new QueryParameters(parameters, Collections.emptyList())));
assertEquals("Response status should be 200", 200, response.getStatus());

BookmarkModelStub expStub = response.readEntity(BookmarkModelStub.class);
assertNotNull("Response body should not be null", expStub);
assertEquals("Bookmark name should match", BOOKMARK.getName(), expStub.getName());
assertEquals("Start time should match", BOOKMARK.getStart(), expStub.getStart());
assertEquals("End time should match", BOOKMARK.getEnd(), expStub.getEnd());
assertNotNull("UUID should not be null", expStub.getUUID());

// Check if the UUID is unique
assertFalse("UUID should be unique", uuids.contains(expStub.getUUID()));
uuids.add(expStub.getUUID());
}

response = bookmarkTarget.request().post(Entity.json(new QueryParameters(parameters, Collections.emptyList())));
assertEquals("Should return conflict for duplicate data", 409, response.getStatus());

// Verify the original bookmark wasn't modified
Response getResponse = bookmarkTarget.path(firstBookmark.getUUID().toString()).request().get();
BookmarkModelStub retrievedBookmark = getResponse.readEntity(BookmarkModelStub.class);
assertEquals("Original bookmark should remain unchanged", firstBookmark, retrievedBookmark);
}

/**
Expand Down Expand Up @@ -326,71 +334,13 @@ public void testUpdateBookmark() {

assertEquals("Update should succeed", 200, response.getStatus());
BookmarkModelStub updatedBookmark = response.readEntity(BookmarkModelStub.class);

assertNotNull("Updated bookmark should not be null", updatedBookmark);
assertNotEquals("UUID should be different", originalBookmark.getUUID(), updatedBookmark.getUUID());
assertEquals("UUID should be the same", originalBookmark.getUUID(), updatedBookmark.getUUID());
assertEquals("Name should be updated", "Updated Name", updatedBookmark.getName());
assertEquals("Start time should be updated", START_TIME + 5, updatedBookmark.getStart());
assertEquals("End time should be updated", END_TIME + 5, updatedBookmark.getEnd());
}

/**
* Test updating a bookmark.
*/
@Test
public void testUpdateBookmarkRepetitiveData() {
WebTarget application = getApplicationEndpoint();
WebTarget bookmarkTarget = application.path(EXPERIMENTS)
.path(experiment.getUUID().toString())
.path(BOOKMARKS);

// Create initial bookmark
Map<String, Object> parameters = new HashMap<>();
parameters.put(NAME, BOOKMARK.getName());
parameters.put(START, START_TIME);
parameters.put(END, END_TIME);

Response response = bookmarkTarget.request().post(Entity.json(new QueryParameters(parameters, Collections.emptyList())));
BookmarkModelStub originalBookmark = response.readEntity(BookmarkModelStub.class);
assertEquals("Initial bookmark creation should succeed", 200, response.getStatus());

// Create a secondary bookmark
parameters.put(NAME, BOOKMARK.getName());
parameters.put(START, START_TIME + 1);
parameters.put(END, END_TIME + 1);

response = bookmarkTarget.request().post(Entity.json(new QueryParameters(parameters, Collections.emptyList())));
BookmarkModelStub secondaryBookmark = response.readEntity(BookmarkModelStub.class);
assertEquals("Secondary bookmark creation should succeed", 200, response.getStatus());

// Test updating non-existent bookmark
WebTarget nonExistentTarget = bookmarkTarget.path("non-existent-uuid");
Response nonExistentResponse = nonExistentTarget.request()
.put(Entity.json(new QueryParameters(parameters, Collections.emptyList())));
assertEquals("Should return 404 for non-existent bookmark", 404, nonExistentResponse.getStatus());

// Test updating with invalid parameters
parameters.put(START, END_TIME);
parameters.put(END, START_TIME);
Response invalidResponse = bookmarkTarget.path(originalBookmark.getUUID().toString())
.request()
.put(Entity.json(new QueryParameters(parameters, Collections.emptyList())));
assertEquals("Should return 400 for invalid parameters", 400, invalidResponse.getStatus());

// Test successful update
parameters.put(START, START_TIME + 1);
parameters.put(END, END_TIME + 1);

response = bookmarkTarget.path(originalBookmark.getUUID().toString())
.request()
.put(Entity.json(new QueryParameters(parameters, Collections.emptyList())));
BookmarkModelStub updatedBookmark = response.readEntity(BookmarkModelStub.class);
assertEquals("Should return conflict for duplicate name", 409, response.getStatus());
assertEquals("Name should be as before", secondaryBookmark.getName(), updatedBookmark.getName());
assertEquals("Start time should be as before", secondaryBookmark.getStart(), updatedBookmark.getStart());
assertEquals("End time should be as before", secondaryBookmark.getEnd(), updatedBookmark.getEnd());
}

/**
* Test the deletion of a bookmark with various scenarios.
*/
Expand Down
3 changes: 1 addition & 2 deletions ...e/tracecompass/incubator/trace/server/jersey/rest/core/tests/stubs/BookmarkModelStub.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
package org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core.tests.stubs;

import java.io.Serializable;
import java.nio.charset.Charset;
import java.util.Objects;
import java.util.UUID;

Expand Down Expand Up @@ -74,7 +73,7 @@ public BookmarkModelStub(String name, long start, long end) {
}

private static UUID getUUID(String name) {
return UUID.nameUUIDFromBytes(Objects.requireNonNull(name.getBytes(Charset.defaultCharset())));
return UUID.randomUUID();
}

/**
Expand Down
66 changes: 29 additions & 37 deletions ...ass/incubator/internal/trace/server/jersey/rest/core/services/BookmarkManagerService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static org.eclipse.tracecompass.incubator.internal.trace.server.jersey.rest.core.services.EndpointConstants.NO_SUCH_EXPERIMENT;
import static org.eclipse.tracecompass.incubator.internal.trace.server.jersey.rest.core.services.EndpointConstants.BKM;

import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -197,28 +196,21 @@ public Response createBookmark(
return Response.status(Status.NOT_FOUND).entity(NO_SUCH_EXPERIMENT).build();
}

String name = Objects.requireNonNull((String) parameters.get(BOOKMARK_NAME));
long start = Objects.requireNonNull((Number) parameters.get(BOOKMARK_START)).longValue();
long end = Objects.requireNonNull((Number) parameters.get(BOOKMARK_END)).longValue();

String uuidReference = name + start + end;
UUID bookmarkUUID = UUID.nameUUIDFromBytes(Objects.requireNonNull(uuidReference.getBytes(Charset.defaultCharset())));

Bookmark bookmark = new Bookmark(bookmarkUUID, name, start, end);

IFile editorFile = TmfTraceManager.getInstance().getTraceEditorFile(experiment);
if (editorFile == null) {
return Response.status(Status.NOT_FOUND).entity(EndpointConstants.BOOKMARK_NOT_FOUND).build();
}

try {
IMarker[] markers = findBookmarkMarkers(editorFile);
if (bookmarkExists(markers, bookmark.getUUID())) {
return Response.status(Status.CONFLICT).entity(bookmark).build();
}
String name = Objects.requireNonNull((String) parameters.get(BOOKMARK_NAME));
long start = Objects.requireNonNull((Number) parameters.get(BOOKMARK_START)).longValue();
long end = Objects.requireNonNull((Number) parameters.get(BOOKMARK_END)).longValue();
UUID uuid = generateUUID(editorFile);

IMarker newMarker = editorFile.createMarker(IMarker.BOOKMARK);
setMarkerAttributes(newMarker, bookmark);
Bookmark bookmark = new Bookmark(uuid, name, start, end);

IMarker marker = editorFile.createMarker(IMarker.BOOKMARK);
setMarkerAttributes(marker, bookmark);
return Response.ok(bookmark).build();
} catch (CoreException e) {
Activator.getInstance().logError("Failed to create bookmark", e); //$NON-NLS-1$
Expand Down Expand Up @@ -268,32 +260,24 @@ public Response updateBookmark(
return Response.status(Status.NOT_FOUND).entity(NO_SUCH_EXPERIMENT).build();
}

String name = Objects.requireNonNull((String) parameters.get(BOOKMARK_NAME));
long start = Objects.requireNonNull((Number) parameters.get(BOOKMARK_START)).longValue();
long end = Objects.requireNonNull((Number) parameters.get(BOOKMARK_END)).longValue();

String uuidReference = name + start + end;
String newBookmarkUUID = UUID.nameUUIDFromBytes(Objects.requireNonNull(uuidReference.getBytes(Charset.defaultCharset()))).toString();

Bookmark bookmark = new Bookmark(UUID.fromString(newBookmarkUUID), name, start, end);

IFile editorFile = TmfTraceManager.getInstance().getTraceEditorFile(experiment);
if (editorFile == null) {
return Response.status(Status.NOT_FOUND).entity(EndpointConstants.BOOKMARK_NOT_FOUND).build();
}

String name = Objects.requireNonNull((String) parameters.get(BOOKMARK_NAME));
long start = Objects.requireNonNull((Number) parameters.get(BOOKMARK_START)).longValue();
long end = Objects.requireNonNull((Number) parameters.get(BOOKMARK_END)).longValue();

Bookmark bookmark = new Bookmark(bookmarkUUID, name, start, end);

try {
IMarker[] markers = findBookmarkMarkers(editorFile);
IMarker marker = findMarkerByUUID(markers, bookmarkUUID);
if (marker == null) {
return Response.status(Status.NOT_FOUND).entity(EndpointConstants.BOOKMARK_NOT_FOUND).build();
}

IMarker duplicateMarker = findMarkerByUUID(markers, UUID.fromString(newBookmarkUUID));
if (duplicateMarker != null) {
return Response.status(Status.CONFLICT).entity(bookmark).build();
}

setMarkerAttributes(marker, bookmark);
return Response.ok(bookmark).build();
} catch (CoreException e) {
Expand Down Expand Up @@ -352,6 +336,21 @@ public Response deleteBookmark(
}
}

/**
* Generate a random UUID for a new bookmark
*/
private static UUID generateUUID(IFile editorFile) throws CoreException {
IMarker[] markers = findBookmarkMarkers(editorFile);
while (true) {
UUID uuid = UUID.randomUUID();

// Check if the UUID hasn't been used yet
if (findMarkerByUUID(markers, uuid) == null) {
return uuid;
}
}
}

/**
* Find all bookmark markers in the editor file
*/
Expand Down Expand Up @@ -450,11 +449,4 @@ private static List<Bookmark> markersToBookmarks(IMarker[] markers) {
}
return bookmarks;
}

/**
* Check if a bookmark with the same UUID already exists
*/
private static boolean bookmarkExists(IMarker[] markers, UUID bookmarkUUID) {
return findMarkerByUUID(markers, bookmarkUUID) != null;
}
}

0 comments on commit 19631bb

Please sign in to comment.