Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: embedded the contents of ipay88-my.aar #10

Merged
merged 5 commits into from
Mar 25, 2024
Merged

Conversation

vasi-twks
Copy link
Contributor

A bit of context about this PR.
Ipay88 do not host their aar on a repository manager, therefore from what I understood from Semir, we received an .aar from them, and we hosted that aar into artifactory then we loaded that dependency from artifactory.
As the goal is now to remove artifactory, we had two options.

  • host that aar in maven along with the other dependencies (but the old POM is missing a lot of entries and GPG signing)
  • embed the aar into the wrapper (actually embed its contents as we cannot embed an aar into another aar)

The second version was chosen so this PR has inlined the required resources and the classes file.
From what I can tell, nol-pay is another wrapper that has embedded the dependencies this way.

From a testing perspective, a version suffixed with "-SNAPSHOT" was published into maven local, then added a dependency for the primer-sdk-android project.
The project compiles and runs.
I attempted to do a payment flow, with both the dependency library and the embedded library, the result was failed for both of them but that seems to be related to a server workflow.

@vasi-twks vasi-twks force-pushed the release/test-maven branch from efa1818 to 8f6ce6f Compare March 12, 2024 11:04
@vasi-twks vasi-twks requested a review from semirp March 14, 2024 11:40
Copy link

gitguardian bot commented Mar 22, 2024

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@semirp semirp force-pushed the release/test-maven branch from cf4c207 to e0489ef Compare March 22, 2024 14:21
@FlaviuExtPrimer
Copy link

Fyi, don't forget about ipay88's manifest. Needs to be merged into the wrapper one (I remember there were some permissions)

@semirp semirp force-pushed the release/test-maven branch from e0489ef to 8d120b0 Compare March 22, 2024 15:06
namespace 'io.primer.ipay88'
compileSdkVersion 34
// in order not to crash their SDK, we can use their namespace to correctly find R class
namespace 'com.androids.ipay'
Copy link
Contributor

Choose a reason for hiding this comment

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

There are solutions for this:

  • unpack their code and load resources dynamically, then package it again
  • unpack their code and put it inside our package - but some things should not be pushed or added to public repo

At the end this seems as the easiest solution.

@@ -38,9 +39,15 @@ android {
}
}

tasks.configureEach { task ->
if (task.name == "generateReleaseBuildConfig") {
Copy link
Contributor

Choose a reason for hiding this comment

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

We have to disable generation of new BuildConfig file as gradle does not allow duplicates.

@semirp
Copy link
Contributor

semirp commented Mar 25, 2024

Fyi, don't forget about ipay88's manifest. Needs to be merged into the wrapper one (I remember there were some permissions)

Yeah, all of the permissions and activities were added to the new Manifest 👍

@semirp semirp merged commit 7f93256 into main Mar 25, 2024
3 checks passed
@semirp semirp deleted the release/test-maven branch March 25, 2024 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants