Skip to content

Conversation

avlo
Copy link
Collaborator

@avlo avlo commented Mar 19, 2025

@tcheeric. hi, eric. as previously discussed- below PR continuation of develop branch- comprised of project modernization, fixes & general cleanup sweep, as follows:

  1. gradle (a modern maven alternative) build tool added to project
  2. project retrofitted as spring boot multi-module, works with either maven or gradle
  3. module-info.java files/handling no longer needed/necessary, thus removed. module dependencies now auto-management by spring.
  4. 39 instances of runtime-reflection-heavy new ObjectMapper replaced by 3 instances of compile-time-byte-code generated afterburner
  5. Encoder & FEncoder interfaces refactored into single Encoder interface, as all implementers of both use exactly the same mapper
  6. pervasive null use(abuse!) refactored into Optional for performance increase and resiliency...
  7. ... declarative/procedural implementations updated/modernized for streaming & functional api
  8. all pom transitive module dependency redundancies have been cleaned up
  9. unit/integration tests moved into related modules

noteworthy:

  • either maven/gradle can now be used to build project and run tests without issue.
    (however, some IDE's will have trouble switching between maven/gradle context. easiest/common way to accommodate both is with a 2nd local copy of nostr-java repo, which can then be opened as a separate (gradle) project.)
  • maven build, test & publish commands should functional as usual/expected, no changes
  • placeholders have been created in build.gradle for your publishing specs
  • confirmed maven test count remains 160:
  util
  [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
  base
  [INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0
  event
  [INFO] Tests run: 70, Failures: 0, Errors: 0, Skipped: 0
  id
  [INFO] Tests run: 11, Failures: 0, Errors: 0, Skipped: 0
  api
  [INFO] Tests run: 68, Failures: 0, Errors: 0, Skipped: 0
gradle quickstart/reference:

after installing gradle, 1st time through gradle build will perform preliminary setup and may take some time. subsequent gradle tasks should run quickly thereafter

useful/frequent gradle commands (w/ maven equivalents)

alias gc='gradle clean'  // 'mvn clean' 
alias gb='gradle build'  // 'mvn build'
alias gcb='gradle clean build' // 'mvn clean build'

alias gbnotest='gradle build -x test'  // build w/o running tests  // 'mvn build -Dmaven.test.skip=true'
alias gcbnotest='gradle clean build -x test'  // clean build w/o running tests // 'mvn clean build -Dmaven.test.skip=true'
alias gv='gradle check' // run both integration tests & unit tests // 'mvn verify'

alias gpub='gradle publishToMavenLocal'  // 'mvn install'

note: as per convention/utility in many test frameworks (including junit & utilized by spring-test), unit test class names suffixed as *Test.java and integration tests suffixed *IT.java are automatically/cleanly contextualized, resourced and executed by the test framework (junit). thus, relevant nostr-java classes named *Test.java which are in fact integration tests- have been accordingly renamed as *IT.java.

as before, this functionality is transparent to whomever is running tests. maven/gradle build, test & publish commands should functional as usual/expected for the developer.

any questions/issues/etc, def lmk

@avlo avlo self-assigned this Mar 21, 2025
@avlo avlo added the awaiting review PR awaiting review label Mar 21, 2025
@avlo
Copy link
Collaborator Author

avlo commented Mar 21, 2025

issue #204 (ElementAttribute "nip" field) addressed with changes included as part of this PR

Copy link
Owner

@tcheeric tcheeric Mar 23, 2025

Choose a reason for hiding this comment

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

In test cases testNIP01SendTextNoteEventGeoHashTag, testNIP01SendTextNoteEventHashtagTag, testNIP01SendTextNoteEventCustomGenericTag, testFiltersListReturnSameSingularEvent, testFiltersListReturnTwoDifferentEvents, and testMultipleFiltersDifferentTypesReturnSameEvent, the published events are never purged from the relay, and the expression assertEquals(2, result.size()); will always fail if you run the test cases more than once.
To prevent this, you have to make the content unique, for example, I am introducing the index variable like this, to ensure that the variables geoHashTagTarget and genericTagTarget are unique:

    @Test
    public void testMultipleFiltersDifferentTypesReturnSameEvent() throws IOException {
        System.out.println("testMultipleFilters");

        Long index = System.currentTimeMillis();
        String geoHashTagTarget = "geohash_tag-location-DifferentTypesReturnSameEvent" + index;
        GeohashTag geohashTag = new GeohashTag(geoHashTagTarget);

        String genericTagTarget = "generic-tag-value-DifferentTypesReturnSameEvent" + index;
        GenericTag genericTag = GenericTag.create("m", genericTagTarget);

        NIP01<NIP01Event> nip01 = new NIP01<>(Identity.generateRandomIdentity());
        nip01.createTextNoteEvent(List.of(geohashTag, genericTag), "Multiple Filters").signAndSend(relays);

        Filters filters = new Filters(
                new GeohashTagFilter<>(new GeohashTag(geoHashTagTarget)),
                new GenericTagQueryFilter<>(new GenericTagQuery("#m", genericTagTarget)));

        List<String> result = nip01.sendRequest(filters, UUID.randomUUID().toString());

        assertFalse(result.isEmpty());
        assertEquals(2, result.size());
        assertTrue(result.stream().anyMatch(s -> s.contains(geoHashTagTarget)));

//        nip01.close();
    }

Thoughts?

Copy link
Collaborator Author

@avlo avlo Mar 23, 2025

Choose a reason for hiding this comment

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

ah, yes- considering some common testing-related semantics:

  • integration testing: definition/practice/expectation is for a system under test to have predefined/known state before tests are started, including any/all data. existing nostr-java (and SC) integration tests are implemented as per this definition.

  • end-to-end/functional testing: aligns with what you're describing (tests using persistent/ongoing data/states) as conventional/practice. i'm open to whatever semantics you prefer, but given the above definition- we haven't (yet) implemented a testing scenario as you've described.

if helpful/useful, i've just pushed an automated integration-test bash script into sc_spring_boot-gradle-migration branch that might help streamline/simplify your integration testing process.

prior to use, note the requisite configuring relevant nostr-java & SC directories

w/ abbreviated main logic as follows:

  1. cd to nostr-java
  2. rm m2_local
  3. build_nostr-java
  4. publish nostr-java to m2_local
  5. cd to superconductor
  6. build and test super
  7. start super_service
  8. run nostr-java tests
  9. terminate super

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

supplemental, if integration testing with SC as your relay server: as SC develop branch is currently several commits ahead of nostr-java PR branch, you'll either want to use:

or

Copy link
Owner

Choose a reason for hiding this comment

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

testNIP99CalendarContentPreRequest fails on my machine.

    assertTrue(
        JsonComparator.isEquivalentJson(
            MAPPER_AFTERBURNER.readTree(expected),
            MAPPER_AFTERBURNER.readTree(reqResponse)));

org.opentest4j.AssertionFailedError: expected: but was:
at app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at app//org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:31)
at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:183)
at app//nostr.api.integration.ApiNIP52RequestIT.testNIP99CalendarContentPreRequest(ApiNIP52RequestIT.java:142)
at java.base@21.0.6/java.lang.reflect.Method.invoke(Method.java:580)
at java.base@21.0.6/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:387)
at java.base@21.0.6/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1312)
at java.base@21.0.6/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1843)
at java.base@21.0.6/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1808)
at java.base@21.0.6/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:188)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

gotcha, can you post/send the test-command executed and full stacktrack/debug-output for that test class?

somewhat related if you haven't yet encountered- any integration test class using websockets are necessarily multi-threaded with multiple concurrent websocket connections. this can/may lead to concurrency states for both websocket connections and database state and as a result, it is not uncommon for a test method to fail. these failures are false-positives and rectifiable by activating (uncommenting) gradle's retry pass

@avlo
Copy link
Collaborator Author

avlo commented Mar 24, 2025

@tcheeric hi, eric. just a heads up if using autotest.sh maven mode, its been updated to properly terminate maven child threads during integration testing shutdown. gradle did not require any changes.

@tcheeric
Copy link
Owner

Ok, noted. I will update my SC. Thanks Nick

@tcheeric
Copy link
Owner

Nick,

Strangely, I am getting these two errors when building SC. (I am using the autotest.sh script in maven mode):
image

In both cases, the int attributes, the nip, has been removed and should not be referenced any more...

EventEntityService:
image

ElementAttributeEntity:
image

I am able to successfully build SC only after manually removing the offending int parameters.

@tcheeric tcheeric added review:approved PR review approved and removed awaiting review PR awaiting review labels Mar 27, 2025
@avlo
Copy link
Collaborator Author

avlo commented Mar 27, 2025

I am able to successfully build SC only after manually removing the offending int parameters.

good catch, yes- looks like those two files were missed when merging ElementAttribute related changes from my in-progress/develop branch. the fix changes you made are exactly correct and are now also be present in sc_spring_boot-gradle-migration commit cd04955581b2f0b30a7178247fd65ef848adb9e1, apologies!

commit cd04955581b2f0b30a7178247fd65ef848adb9e1 (HEAD -> origin/sc_spring_boot-gradle-migration)
Author: nick avlo <avlo@yahoo.com>
Date:   Wed Mar 26 18:38:05 2025 -0700
    removed leftover NIP parameters

@avlo
Copy link
Collaborator Author

avlo commented Mar 27, 2025

@tcheeric hi, eric. saw your approval, super- & prior to merging into develop, just confirming you are/were able to:

  1. successfully build and test nostr-java (against whichever/all relays you chose)
  2. continue your build/test workflow as per usual (or minimally with minor disruption/change)

if not, def lmk and i'll get to addressing either/both as necessary.

@tcheeric
Copy link
Owner

Hi Nick,

  1. The IT will only work in sc because, as I believe, you do a clean up of the SC db before running the IT?
    I'm working exclusively with SC now, but we may need a solution for other relays?

  2. Yes

Overall, the gradle integration is extremely useful.

Thanks

@avlo
Copy link
Collaborator Author

avlo commented Mar 27, 2025

Hi Nick,

  1. The IT will only work in sc because,

incorrect. autotest is a comprehensive unit-and-integration test suite for both SC and nostr- java.

you do a clean up of the SC db before running the IT?

IT db is a self-contained, transient, h2 in-memory db only. it runs separate from and independent of other external/standalone db you might have running.

files:

  • application-local_ws.properties (transient, testing)
  • application-dev_ws.properties (permanent, non-transient)
  • application-prod_ws.properties (permanent, non-transient)

are your friends.

I'm working exclusively with SC now, but we may need a solution for other relays?

  1. relays are user definable/accumulatable in relays.properties.

  2. all IT test implementations wanting relay connections do so via combination of RelayConfig and Nostr (aka, NIP<xx> classes)

  3. see the various *IT.java tests for reference implementations.

strongly advise study towards fluency with your code base. i'm here to help, lmk if any questions.

  1. Yes

ok great, will merge it 👍

@avlo avlo merged commit 0a064a3 into tcheeric:develop Mar 27, 2025
@avlo avlo deleted the spring_boot-gradle-migration branch March 27, 2025 18:25
@tcheeric
Copy link
Owner

Ok, great. I will review again tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review:approved PR review approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants