diff --git a/trace-server/org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core.tests/src/org/eclipse/tracecompass/incubator/trace/server/jersey/rest/core/tests/services/BookmarkManagerServiceTest.java b/trace-server/org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core.tests/src/org/eclipse/tracecompass/incubator/trace/server/jersey/rest/core/tests/services/BookmarkManagerServiceTest.java index a8a71425..0feba97b 100644 --- a/trace-server/org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core.tests/src/org/eclipse/tracecompass/incubator/trace/server/jersey/rest/core/tests/services/BookmarkManagerServiceTest.java +++ b/trace-server/org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core.tests/src/org/eclipse/tracecompass/incubator/trace/server/jersey/rest/core/tests/services/BookmarkManagerServiceTest.java @@ -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; @@ -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 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 uuids = new HashSet<>(); + for (int i = 0; i < 3; i++) { + Map 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); } /** @@ -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 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. */ diff --git a/trace-server/org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core.tests/src/org/eclipse/tracecompass/incubator/trace/server/jersey/rest/core/tests/stubs/BookmarkModelStub.java b/trace-server/org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core.tests/src/org/eclipse/tracecompass/incubator/trace/server/jersey/rest/core/tests/stubs/BookmarkModelStub.java index 9c510f11..62feb604 100644 --- a/trace-server/org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core.tests/src/org/eclipse/tracecompass/incubator/trace/server/jersey/rest/core/tests/stubs/BookmarkModelStub.java +++ b/trace-server/org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core.tests/src/org/eclipse/tracecompass/incubator/trace/server/jersey/rest/core/tests/stubs/BookmarkModelStub.java @@ -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; @@ -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(); } /** diff --git a/trace-server/org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core/src/org/eclipse/tracecompass/incubator/internal/trace/server/jersey/rest/core/services/BookmarkManagerService.java b/trace-server/org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core/src/org/eclipse/tracecompass/incubator/internal/trace/server/jersey/rest/core/services/BookmarkManagerService.java index 677fc6ff..2ac2c26f 100644 --- a/trace-server/org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core/src/org/eclipse/tracecompass/incubator/internal/trace/server/jersey/rest/core/services/BookmarkManagerService.java +++ b/trace-server/org.eclipse.tracecompass.incubator.trace.server.jersey.rest.core/src/org/eclipse/tracecompass/incubator/internal/trace/server/jersey/rest/core/services/BookmarkManagerService.java @@ -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; @@ -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$ @@ -268,20 +260,17 @@ 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); @@ -289,11 +278,6 @@ public Response updateBookmark( 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) { @@ -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 */ @@ -450,11 +449,4 @@ private static List 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; - } } \ No newline at end of file