Skip to content

Capture meta endpoint refactor#168

Open
Nefarious46 wants to merge 16 commits into
teragrep:mainfrom
Nefarious46:CaptureMetaEndpointRefactor
Open

Capture meta endpoint refactor#168
Nefarious46 wants to merge 16 commits into
teragrep:mainfrom
Nefarious46:CaptureMetaEndpointRefactor

Conversation

@Nefarious46
Copy link
Copy Markdown
Contributor

@Nefarious46 Nefarious46 commented May 21, 2025

  • Void on delete
  • Procedures named accordingly
  • Controllers named accordingly
  • Tests renewed so that testData is inserted before tests.

Fixes issue #73

More info on PR #155

@Nefarious46 Nefarious46 requested a review from kortemik May 21, 2025 04:08
@Nefarious46 Nefarious46 self-assigned this May 21, 2025
@Nefarious46 Nefarious46 added the enhancement New feature or request label May 21, 2025
Comment thread src/main/java/com/teragrep/cfe18/CaptureMetaMapper.java Outdated
Comment thread src/main/java/com/teragrep/cfe18/handlers/CaptureMetaController.java Outdated
@Nefarious46 Nefarious46 force-pushed the CaptureMetaEndpointRefactor branch from ccf88e9 to 4aadefe Compare June 4, 2025 12:21
@eemhu
Copy link
Copy Markdown
Collaborator

eemhu commented Jun 5, 2025

otherwise looks ok

@Nefarious46 Nefarious46 force-pushed the CaptureMetaEndpointRefactor branch from b127696 to fcbe2e3 Compare June 23, 2025 08:39
@Nefarious46
Copy link
Copy Markdown
Contributor Author

Rebased, no changes required.

@Nefarious46 Nefarious46 requested a review from eemhu June 23, 2025 08:42
@q22u q22u removed the review label Aug 22, 2025
@Nefarious46 Nefarious46 force-pushed the CaptureMetaEndpointRefactor branch from fcbe2e3 to 8a36d15 Compare May 7, 2026 08:57
@Nefarious46
Copy link
Copy Markdown
Contributor Author

Reworked according to the new Api specification document.

@kortemik
Copy link
Copy Markdown
Member

kortemik commented May 8, 2026

please rebase

@Nefarious46 Nefarious46 force-pushed the CaptureMetaEndpointRefactor branch from 5e8e09d to f29022a Compare May 13, 2026 11:20
@Nefarious46
Copy link
Copy Markdown
Contributor Author

Endpoint and unit test renewals. Ready for review.

@Nefarious46 Nefarious46 requested a review from kortemik May 13, 2026 11:23

-- remove all capture meta from capture if key is null
IF (meta_key) IS NULL THEN
DELETE cm.*, cmk.*
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do not delete keys

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

rectified 5f04a09

WHERE cm.capture_id = capture_id;
ELSE
-- delete from capture with the given key
DELETE cm.*, cmk.*
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do not delete keys

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

rectified 5f04a09

AND cm.meta_value = capture_meta_value
AND cmk.meta_key_name = capture_meta_key) = 0) THEN
-- insert new record
INSERT INTO cfe_18.capture_meta_key(meta_key_name) VALUES (capture_meta_key);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

insert ignore or handle duplicate key before this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

rectified on 125dabb


// Creates the request
HttpPut captureMetaRequest = new HttpPut("http://localhost:" + port + "/capture/meta/");
HttpPut captureMetaRequest = new HttpPut("http://localhost:" + port + "/v2/captures/definitions/1/metadata");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should this be on the CaptureMetaControllerTest.java

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will be dealt with #248 by removing the test case altogether as it is deemed obsolete to test fetch by key in Capture Definition.

@Nefarious46 Nefarious46 requested a review from kortemik May 15, 2026 11:52
@Nefarious46 Nefarious46 force-pushed the CaptureMetaEndpointRefactor branch from 125dabb to 4ed2bd3 Compare May 15, 2026 11:53
@Nefarious46
Copy link
Copy Markdown
Contributor Author

Changes applied and rebased

@Nefarious46
Copy link
Copy Markdown
Contributor Author

Added unique constraint on capture meta key since it did not exist before. Was producing multiple key entries and always matching the latest ID instead of one ID per key.

END IF;


-- check if capture already has key value
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is it possible to insert multiple different key-value pairs with the same key? if not then this should either return error that value is already set if it does not match the inserted one or update but we do not support updating so the error path is more appropriate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

if exact same row DOES NOT exist then insert. If any of the 3 variables change then insert is always possible and will be inserted if record is new. Every relation here is N:N.

To be fair the comment is a bit misleading since we are only avoiding the constraint error from happening here and moving away from inserting the record if same record already exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants